fix(sandbox): honour configured timeoutSeconds in DockerSandboxProvider.executeCode

[PratikDhanave (PratikDhanave)]

Summary

DockerSandboxProvider.executeCode() always passed { timeout: 60_000 } to execFile, ignoring the timeoutSeconds a caller supplied to createSession. Someone who set e.g. timeoutSeconds: 2 to bound how long agent-generated code may run actually got 60 seconds — a silently weakened sandbox safety control. Since timeouts are part of the sandbox's safety envelope, this can cause unexpected resource usage and gives a false sense of containment.

Fixes #3118

Change

• Record each session's timeout (timeoutSeconds * 1000) in a sessionId -> ms map when the session is created.
• Use that value in executeCode instead of the hard-coded 60_000, falling back to defaultSandboxConfig().timeoutSeconds * 1000 only for sessions created before the timeout was tracked.
• Clean up the per-session entry in destroySession, alongside the existing container mapping, so the map does not leak.

Testing

Added tests/sandbox-timeout.test.ts (mocks child_process, so it runs without Docker):

• executeCode uses the session-configured timeoutSeconds (2 → 2000 ms).
• A custom timeout is not hard-coded to 60_000 (5 → 5000 ms).
• Falls back to the default (60_000 ms) when no config is provided.

Verified the tests fail on the pre-fix code (Expected: 2000 / Received: 60000) and pass with the fix. tsc build is clean, eslint is clean on the changed files, and the full jest suite passes (the one unrelated failure is the pre-existing Docker lifecycle test, which fails identically on main in an environment without a working Docker daemon and otherwise skips).

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[Copilot]

Pull request overview

Fixes the TypeScript DockerSandboxProvider so executeCode() honors the per-session timeoutSeconds configured at createSession, restoring the sandbox timeout as an effective safety control (issue #3118).

Changes:

• Track per-session exec timeout (ms) when creating a session and use it in executeCode() instead of a hard-coded 60_000.
• Clean up the per-session timeout entry in destroySession.
• Add Jest regression tests that mock child_process to validate timeout propagation without requiring Docker.

TL;DR: 2 blockers, 0 warnings. Fix #1 and #2 and this ships.

#	Sev	Issue	Where
1	Block	New test file license header format doesn’t match repo’s standard two-line MIT header	tests/sandbox-timeout.test.ts:1
2	Block	timeoutSeconds is used without validating finite/positive value; can become NaN/0 at runtime and undermine the timeout safety control	src/sandbox.ts:193

#1: Update the header to the two-line // Copyright... + // Licensed... format.
#2: Guard/fallback when timeoutSeconds isn’t a positive finite number before storing milliseconds.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
agent-governance-typescript/src/sandbox.ts	Stores per-session timeout (ms) and uses it in execFile options; cleans up timeout tracking on session destroy.
agent-governance-typescript/tests/sandbox-timeout.test.ts	Adds regression tests asserting executeCode passes the session timeout (or default) to execFile.

[MohammadHaroonAbuomar]

Minor:

• the ?? defaultSandboxConfig() fallback on the same line is dead in-process; harmless, fine to leave.

[MohammadHaroonAbuomar]

timeoutSeconds <= 0 now yields timeout: 0, which Node execFile treats as NO timeout (unlimited execution); on main the same degenerate config still got the 60s cap. Clamp to the default when <= 0 (config is operator input, but the regression is real).

[PratikDhanave (PratikDhanave)]

Thanks MohammadHaroonAbuomar and Copilot — both blockers addressed in the latest push:

1. Test license header → now the two-line // Copyright (c) Microsoft Corporation. / // Licensed under the MIT License. format matching the rest of the TS codebase.
2. timeoutSeconds validation → added resolveTimeoutMs(), which falls back to the default when the value isn't a positive, finite number, applied at the session-store site so the tracked value is always valid. This closes the timeoutSeconds <= 0 → timeout: 0 (unlimited execution) regression you both flagged — a non-positive or non-finite config can no longer disable the sandbox cap.

Added regression tests for the non-positive (0) and non-finite (NaN) cases; tsc, eslint, and jest (5/5) are green. Commits are signed off for DCO. PTAL.