fix(sandbox): add SIGKILL enforcement, timeoutSeconds validation, and exit code 137 handling

[jlaportebot]

Fixes #3118

Changes

• Store SandboxConfig per session and retrieve in executeCode
• Use coreutils timeout with --signal=SIGKILL --kill-after=5s for in-container enforcement
• Validate timeoutSeconds: clamp to >=1, reject NaN
• Treat exit codes 124 (SIGTERM timeout) and 137 (SIGKILL) as timedOut
• Map killReason: 'timeout' for timedOut, 'signal' for killed by other signal
• Outer execFile timeout set to (timeoutSeconds + 5) * 1000 as safety net
• Added regression test for custom timeoutSeconds

Testing

• All tests pass (11/11 in sandbox.test.ts)
• Added regression test for custom timeoutSeconds behavior

AI Disclosure

This PR was developed with AI assistance.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Docs Sync

• DockerSandboxProvider in agent-governance-typescript/src/sandbox.ts -- missing docstring
• README.md -- no updates found for new timeoutSeconds validation, SIGKILL enforcement, and exit code handling changes
• CHANGELOG.md -- missing entry for changes in sandbox behavior (e.g., timeoutSeconds validation, SIGKILL enforcement, and exit code handling)

[github-actions]

🤖 AI Agent: test-generator — `agent-governance-typescript/src/sandbox.ts`

AI-generated review output. Treat it as untrusted analysis and verify before acting.

agent-governance-typescript/src/sandbox.ts

• test_invalid_timeoutSeconds_values -- Validate behavior when timeoutSeconds is set to invalid values (e.g., negative, zero, NaN, or non-numeric).
• test_SIGKILL_handling -- Ensure proper handling and reporting of SIGKILL (exit code 137) during execution.
• test_killReason_signal -- Verify killReason is correctly set to 'signal' when the process is killed by a signal other than timeout.

agent-governance-typescript/tests/sandbox.test.ts

• test_sessionConfigs_cleanup -- Confirm that sessionConfigs entries are properly deleted after session destruction.
• test_outer_execFile_timeout -- Validate that the outer execFile timeout is correctly calculated and enforced as (timeoutSeconds + 5) * 1000.

[github-actions]

🤖 AI Agent: code-reviewer — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

TL;DR: 0 blockers, 1 warning. Changes improve sandbox security but require further review for potential edge cases.

#	Sev	Issue	Where
1	Warn	Potential edge case: insufficient validation for timeoutSeconds	agent-governance-typescript/src/sandbox.ts

Action items: None (no blockers identified).

Warnings:

#	Issue	Where	Follow-up
1	timeoutSeconds validation clamps to 1 but does not reject non-integer values (e.g., 1.5). This could lead to unexpected behavior.	agent-governance-typescript/src/sandbox.ts	Fine as follow-up PR.

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

AI-generated review output. Treat it as untrusted analysis and verify before acting.

API Compatibility

Severity	Change	Impact
High	DockerSandboxProvider now enforces a minimum timeoutSeconds of 1 and rejects NaN values.	Existing code that relies on timeoutSeconds being 0 or negative for "no limit" behavior will break.
High	Exit codes 124 (SIGTERM timeout) and 137 (SIGKILL) are now treated as timedOut.	Existing code that interprets these exit codes differently may break.
High	killReason now maps timedOut for timeouts and signal for other signals.	Existing code relying on the previous killReason behavior may break.
Medium	sessionConfigs is now stored and managed per session in DockerSandboxProvider.	Potential impact on memory usage and session lifecycle management for existing integrations.

[github-actions]

🤖 AI Agent: security-scanner — Security Review

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Security Review

Severity	Finding	Fix
High	Potential sandbox escape via unvalidated code input in executeCode. Arbitrary Python code is executed within the sandbox without validation or sanitization, which could lead to malicious behavior.	Validate or restrict code input to prevent execution of unsafe or malicious commands.
High	Trust chain weakness: reliance on timeout command with SIGKILL for enforcing execution limits. If the sandboxed process can manipulate or bypass the timeout utility, it may escape restrictions.	Implement additional safeguards, such as monitoring process behavior directly or using a more robust sandboxing mechanism.
Medium	Race condition risk in sessionConfigs map management during concurrent access. Potential for inconsistent state or data corruption.	Use synchronization mechanisms (e.g., locks) to ensure thread-safe access to sessionConfigs.
Medium	Supply chain risk: reliance on python:3.11-slim Docker image without verification of its integrity or security.	Use a trusted and verified base image, and regularly update it to include security patches.

[github-actions]

🔴 Contributor Check: HIGH

Check	Result
Profile	HIGH
Credential	LOW
Overall	HIGH

Automated check by AGT Contributor Check.

[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.

[liamcrumm]

Please fix failing checks, also would prefer if you club #3198, #3201, and #3203 into one PR if possible.