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

- 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

Signed-off-by: jlaportebot <jlaportebot@gmail.com>

Author: jlaportebot

agent-governance-typescript/src/sandbox.ts

@@ -131,6 +131,7 @@ function validateResourceName(value: string, label: string): void {
 export class DockerSandboxProvider implements SandboxProvider {
   private readonly image: string;
   private readonly containers = new Map<string, string>(); // sessionId -> containerId
+  private readonly sessionConfigs = new Map<string, SandboxConfig>(); // sessionId -> config
 
   constructor(image: string = 'python:3.11-slim') {
     this.image = image;
@@ -189,6 +190,7 @@ export class DockerSandboxProvider implements SandboxProvider {
         .trim();
 
       this.containers.set(sessionId, containerId);
+      this.sessionConfigs.set(sessionId, cfg);
 
       return {
         agentId,
@@ -213,17 +215,31 @@ export class DockerSandboxProvider implements SandboxProvider {
       throw new Error(`No active session '${sessionId}' for agent '${agentId}'`);
     }
 
+    const cfg = this.sessionConfigs.get(sessionId) ?? defaultSandboxConfig();
+    // Validate timeoutSeconds: must be positive. Zero/negative would mean "no limit"
+    // in coreutils timeout, but our outer guard would still enforce a limit.
+    // Clamp to 1 second minimum to avoid confusion.
+    const timeoutSeconds = Number.isFinite(cfg.timeoutSeconds) && cfg.timeoutSeconds >= 1
+      ? Math.floor(cfg.timeoutSeconds)
+      : 1;
+
     const executionId = randomUUID();
     const startTime = Date.now();
 
     return new Promise<ExecutionHandle>((resolve) => {
       const encoded = Buffer.from(code).toString('base64');
+      // Use 'timeout' command with SIGKILL and kill-after to enforce execution time limit.
+      // The default signal (SIGTERM) can be caught/ignored by sandboxed code,
+      // allowing it to bypass the timeout. SIGKILL cannot be caught.
+      // --kill-after=5s sends SIGKILL 5 seconds after the initial signal if the process
+      // hasn't exited, providing a hard backstop.
       const execArgs = [
-        'exec', containerId, 'python3', '-c',
+        'exec', containerId, 'timeout', '--signal=SIGKILL', '--kill-after=5s', String(timeoutSeconds),
+        'python3', '-c',
         `import base64; exec(base64.b64decode('${encoded}').decode())`,
       ];
 
-      execFile('docker', execArgs, { timeout: 60_000 }, (error, stdout, stderr) => {
+      execFile('docker', execArgs, { timeout: (timeoutSeconds + 5) * 1000 }, (error, stdout, stderr) => {
         const durationSeconds = (Date.now() - startTime) / 1000.0;
         // Node's ExecException.code can be: a numeric exit code (child exited
         // non-zero), `null` (child killed by a signal — `error.signal` is set
@@ -239,14 +255,19 @@ export class DockerSandboxProvider implements SandboxProvider {
           : 0;
         const killed = error !== null && 'killed' in error && (error as { killed: boolean }).killed;
 
+        // timeout command exits with 124 when the command times out (SIGTERM sent).
+        // With --signal=SIGKILL, the child gets SIGKILL and exits with 137 (128 + 9).
+        // Treat both 124 and 137 as timeout.
+        const timedOut = exitCode === 124 || exitCode === 137;
+
         const result: SandboxResult = {
           success: exitCode === 0,
           exitCode,
           stdout: stdout ?? '',
           stderr: stderr ?? '',
           durationSeconds,
-          killed,
-          killReason: killed ? 'timeout' : '',
+          killed: timedOut || killed,
+          killReason: timedOut ? 'timeout' : (killed ? 'signal' : ''),
         };
 
         resolve({
@@ -275,6 +296,7 @@ export class DockerSandboxProvider implements SandboxProvider {
       });
     } finally {
       this.containers.delete(sessionId);
+      this.sessionConfigs.delete(sessionId);
     }
   }
 }

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

@@ -133,4 +133,34 @@ describe('DockerSandboxProvider lifecycle', () => {
       provider.executeCode('testAgent', 'nonexistent-id', 'print("hi")'),
     ).rejects.toThrow(/No active session/);
   });
+
+  it('uses custom timeoutSeconds from session config', async () => {
+    if (!dockerAvailable) {
+      console.log('Skipping timeout test — Docker not available');
+      return;
+    }
+
+    // Create session with 2 second timeout
+    const session = await provider.createSession('testAgent', {
+      ...defaultSandboxConfig(),
+      timeoutSeconds: 2,
+    });
+    expect(session.status).toBe(SessionStatus.Ready);
+
+    try {
+      // Execute code that sleeps for 3 seconds — should be killed by timeout
+      const handle = await provider.executeCode(
+        'testAgent',
+        session.sessionId,
+        'import time; time.sleep(3); print("done")',
+      );
+      expect(handle.status).toBe(ExecutionStatus.Failed);
+      expect(handle.result).toBeDefined();
+      expect(handle.result!.killed).toBe(true);
+      expect(handle.result!.killReason).toBe('timeout');
+      expect(handle.result!.durationSeconds).toBeLessThan(2.5); // Should timeout around 2s
+    } finally {
+      await provider.destroySession('testAgent', session.sessionId);
+    }
+  }, 15_000);
 });