fix: security hardening for exec tools — concurrency, sandbox, env filtering, admin-only yolo

package.json

@@ -98,7 +98,7 @@
     "@types/node": "^22.0.0",
     "@typescript-eslint/eslint-plugin": "^8.56.0",
     "@typescript-eslint/parser": "^8.56.0",
-    "@vitest/coverage-v8": "^4.0.18",
+    "@vitest/coverage-v8": "^4.1.8",
     "audit-ci": "^7.1.0",
     "eslint": "^10.0.3",
     "husky": "^9.1.7",
@@ -110,7 +110,7 @@
     "tsup": "^8.5.1",
     "tsx": "^4.19.0",
     "typescript": "^5.9.3",
-    "vitest": "^4.0.18"
+    "vitest": "^4.1.8"
   },
   "engines": {
     "node": ">=20.0.0"

src/agent/tools/exec/__tests__/tools.test.ts

@@ -3,17 +3,18 @@ import Database from "better-sqlite3";
 import { ensureSchema } from "../../../../memory/schema.js";
 import type { ExecConfig } from "../../../../config/schema.js";
 import type { ToolContext } from "../../types.js";
-import { createExecRunExecutor, isCommandAllowed } from "../run.js";
-import { createExecInstallExecutor } from "../install.js";
-import { createExecServiceExecutor } from "../service.js";
-import { createExecStatusExecutor } from "../status.js";
 
 // Mock the runner to avoid real command execution
 vi.mock("../runner.js", () => ({
   runCommand: vi.fn(),
+  ensureSandboxDir: vi.fn(),
 }));
 
 import { runCommand } from "../runner.js";
+import { createExecRunExecutor, isCommandAllowed } from "../run.js";
+import { createExecInstallExecutor } from "../install.js";
+import { createExecServiceExecutor } from "../service.js";
+import { createExecStatusExecutor } from "../status.js";
 
 const mockRunCommand = vi.mocked(runCommand);
 
@@ -31,17 +32,25 @@ function makeExecConfig(overrides?: Partial<ExecConfig>): ExecConfig {
     command_allowlist: [],
     limits: { timeout: 120, max_output: 50000 },
     audit: { log_commands: true },
+    security: {
+      yolo_confirmation: true,
+      sandbox_dir: "/tmp/teleton-exec-sandbox",
+      env_whitelist: ["HOME", "PATH", "LANG", "TERM", "USER", "SHELL"],
+      max_concurrent: 5,
+    },
     ...overrides,
   };
 }
 
 function makeContext(overrides?: Partial<ToolContext>): ToolContext {
+  const defaultConfig = { telegram: { admin_ids: [42] } } as any;
   return {
     bridge: {} as any,
     db: new Database(":memory:"),
     chatId: "123",
     senderId: 42,
     isGroup: false,
+    config: defaultConfig,
     ...overrides,
   };
 }
@@ -74,10 +83,11 @@ describe("exec_run", () => {
       exitCode: 0,
       timedOut: false,
     });
-    expect(mockRunCommand).toHaveBeenCalledWith("echo hello", {
-      timeout: 120000,
-      maxOutput: 50000,
-    });
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "echo hello",
+      expect.objectContaining({ timeout: 120000, maxOutput: 50000 }),
+      expect.anything()
+    );
   });
 
   it("returns error when command fails", async () => {
@@ -162,7 +172,11 @@ describe("exec_install", () => {
     const executor = createExecInstallExecutor(db, makeExecConfig());
     await executor({ manager: "apt", packages: "nginx curl" }, makeContext());
 
-    expect(mockRunCommand).toHaveBeenCalledWith("apt install -y nginx curl", expect.any(Object));
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "apt install -y nginx curl",
+      expect.any(Object),
+      expect.anything()
+    );
   });
 
   it("constructs correct command for pip", async () => {
@@ -179,7 +193,11 @@ describe("exec_install", () => {
     const executor = createExecInstallExecutor(db, makeExecConfig());
     await executor({ manager: "pip", packages: "flask" }, makeContext());
 
-    expect(mockRunCommand).toHaveBeenCalledWith("pip install flask", expect.any(Object));
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "pip install flask",
+      expect.any(Object),
+      expect.anything()
+    );
   });
 
   it("constructs correct command for npm", async () => {
@@ -196,7 +214,11 @@ describe("exec_install", () => {
     const executor = createExecInstallExecutor(db, makeExecConfig());
     await executor({ manager: "npm", packages: "pm2" }, makeContext());
 
-    expect(mockRunCommand).toHaveBeenCalledWith("npm install -g pm2", expect.any(Object));
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "npm install -g pm2",
+      expect.any(Object),
+      expect.anything()
+    );
   });
 
   it("constructs correct command for docker", async () => {
@@ -213,7 +235,11 @@ describe("exec_install", () => {
     const executor = createExecInstallExecutor(db, makeExecConfig());
     await executor({ manager: "docker", packages: "nginx:latest" }, makeContext());
 
-    expect(mockRunCommand).toHaveBeenCalledWith("docker pull nginx:latest", expect.any(Object));
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "docker pull nginx:latest",
+      expect.any(Object),
+      expect.anything()
+    );
   });
 
   it("logs audit entry", async () => {
@@ -259,7 +285,11 @@ describe("exec_service", () => {
     const executor = createExecServiceExecutor(db, makeExecConfig());
     await executor({ action: "status", name: "nginx" }, makeContext());
 
-    expect(mockRunCommand).toHaveBeenCalledWith("systemctl status nginx", expect.any(Object));
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "systemctl status nginx",
+      expect.any(Object),
+      expect.anything()
+    );
   });
 
   it("logs audit entry", async () => {
@@ -300,12 +330,11 @@ describe("isCommandAllowed", () => {
   });
 
   it("does not allow prefix substring without whitespace boundary", () => {
-    // 'git' should not match 'gitconfig' without a space after it
     expect(isCommandAllowed("gitconfig --list", ["git"])).toBe(false);
   });
 
   it("trims whitespace before matching", () => {
-    expect(isCommandAllowed("  ls  /tmp", ["ls"])).toBe(true);
+    expect(isCommandAllowed(" ls /tmp", ["ls"])).toBe(true);
   });
 });
 
@@ -349,7 +378,11 @@ describe("exec_run allowlist mode", () => {
     const result = await executor({ command: "git status" }, makeContext());
 
     expect(result.success).toBe(true);
-    expect(mockRunCommand).toHaveBeenCalledWith("git status", expect.any(Object));
+    expect(mockRunCommand).toHaveBeenCalledWith(
+      "git status",
+      expect.any(Object),
+      expect.anything()
+    );
   });
 
   it("error message lists configured prefixes", async () => {
@@ -456,9 +489,7 @@ describe("exec_status", () => {
     const result = await executor({} as any, makeContext());
 
     expect(result.success).toBe(true);
-    // memory should contain the failure message
     expect(result.data.memory).toContain("failed");
-    // other keys should have data
     expect(result.data.disk).toBe("some data");
   });
 });

src/agent/tools/exec/concurrency.ts

@@ -0,0 +1,29 @@
+class ConcurrencyLimiter {
+  private running = 0;
+  private waiters: Array<{ resolve: () => void; reject: (err: Error) => void }> = [];
+
+  async acquire(maxConcurrent: number): Promise<void> {
+    if (this.running < maxConcurrent) {
+      this.running++;
+      return;
+    }
+    return new Promise((resolve, reject) => {
+      this.waiters.push({ resolve, reject });
+    });
+  }
+
+  release(): void {
+    this.running--;
+    const next = this.waiters.shift();
+    if (next) {
+      this.running++;
+      next.resolve();
+    }
+  }
+
+  get count(): number {
+    return this.running;
+  }
+}
+
+export const execConcurrency = new ConcurrencyLimiter();

src/agent/tools/exec/install.ts

@@ -1,7 +1,8 @@
 import { Type } from "@sinclair/typebox";
 import type { Tool, ToolExecutor, ToolResult } from "../types.js";
 import type { ExecConfig } from "../../../config/schema.js";
-import { runCommand } from "./runner.js";
+import { runCommand, ensureSandboxDir } from "./runner.js";
+import { execConcurrency } from "./concurrency.js";
 import { insertAuditEntry, updateAuditEntry } from "./audit.js";
 import type Database from "better-sqlite3";
 
@@ -19,8 +20,7 @@ const INSTALL_COMMANDS: Record<string, (pkgs: string) => string> = {
 
 export const execInstallTool: Tool = {
   name: "exec_install",
-  description:
-    "Install packages using a specified package manager (apt, pip, npm, or docker pull). Constructs the correct install command automatically.",
+  description: "Install packages using a specified package manager (apt, pip, npm, or docker pull). Constructs the correct install command automatically.",
   parameters: Type.Object({
     manager: Type.Union(
       [Type.Literal("apt"), Type.Literal("pip"), Type.Literal("npm"), Type.Literal("docker")],
@@ -50,6 +50,8 @@ export function createExecInstallExecutor(
 
     const command = buildCommand(packages);
 
+    await execConcurrency.acquire(execConfig.security.max_concurrent);
+
     let auditId: number | undefined;
     if (execConfig.audit.log_commands) {
       auditId = insertAuditEntry(db, {
@@ -62,42 +64,67 @@ export function createExecInstallExecutor(
       });
     }
 
-    const result = await runCommand(command, {
-      timeout: timeout * 1000,
-      maxOutput: max_output,
-    });
+    try {
+      const sandboxDir = execConfig.security.sandbox_dir;
+      if (sandboxDir) ensureSandboxDir(sandboxDir);
 
-    const status = result.timedOut ? "timeout" : result.exitCode === 0 ? "success" : "failed";
+      const security = {
+        cwd: sandboxDir || undefined,
+        env: execConfig.security.env_whitelist.length > 0
+          ? buildFilteredEnv(execConfig.security.env_whitelist)
+          : undefined,
+      };
 
-    if (auditId !== undefined) {
-      updateAuditEntry(db, auditId, {
-        status,
-        exitCode: result.exitCode ?? undefined,
-        signal: result.signal ?? undefined,
-        duration: result.duration,
-        stdout: result.stdout,
-        stderr: result.stderr,
-        truncated: result.truncated,
-      });
-    }
+      const result = await runCommand(command, {
+        timeout: timeout * 1000,
+        maxOutput: max_output,
+      }, security);
+
+      const status = result.timedOut ? "timeout" : result.exitCode === 0 ? "success" : "failed";
+
+      if (auditId !== undefined) {
+        updateAuditEntry(db, auditId, {
+          status,
+          exitCode: result.exitCode ?? undefined,
+          signal: result.signal ?? undefined,
+          duration: result.duration,
+          stdout: result.stdout,
+          stderr: result.stderr,
+          truncated: result.truncated,
+        });
+      }
 
-    return {
-      success: result.exitCode === 0 && !result.timedOut,
-      data: {
-        manager,
-        packages,
-        stdout: result.stdout,
-        stderr: result.stderr,
-        exitCode: result.exitCode,
-        duration: result.duration,
-        truncated: result.truncated,
-        timedOut: result.timedOut,
-      },
-      ...(result.timedOut
-        ? { error: `Install timed out after ${timeout}s` }
-        : result.exitCode !== 0
-          ? { error: `Install failed with exit code ${result.exitCode}` }
-          : {}),
-    };
+      return {
+        success: result.exitCode === 0 && !result.timedOut,
+        data: {
+          manager,
+          packages,
+          stdout: result.stdout,
+          stderr: result.stderr,
+          exitCode: result.exitCode,
+          duration: result.duration,
+          truncated: result.truncated,
+          timedOut: result.timedOut,
+        },
+        ...(result.timedOut
+          ? { error: `Install timed out after ${timeout}s` }
+          : result.exitCode !== 0
+            ? { error: `Install failed with exit code ${result.exitCode}` }
+            : {}),
+      };
+    } finally {
+      execConcurrency.release();
+    }
   };
 }
+
+function buildFilteredEnv(envWhitelist: string[]): NodeJS.ProcessEnv {
+  const allowed = new Set(envWhitelist);
+  const filtered: NodeJS.ProcessEnv = {};
+  for (const [key, value] of Object.entries(process.env)) {
+    if (allowed.has(key) && value !== undefined) {
+      filtered[key] = value;
+    }
+  }
+  return filtered;
+}