From ef2a25b5fb465932b8c549a19491366e9fad30cb Mon Sep 17 00:00:00 2001 From: Josh Rzemien Date: Mon, 23 Feb 2026 14:21:22 -0500 Subject: [PATCH] Add AST-based security middleware and enforcement wiring --- .env.example | 10 + AGENTS.md | 9 + README.md | 35 ++- docs/orchestration-engine.md | 11 +- docs/pipeline-policies.md | 1 + docs/security-middleware.md | 42 +++ package-lock.json | 256 +++++++++++++++++- package.json | 4 +- src/agents/manifest.ts | 21 +- src/agents/orchestration.ts | 67 ++++- src/agents/persona-registry.ts | 21 +- src/agents/pipeline.ts | 42 +++ src/config.ts | 105 +++++++ src/mcp.ts | 3 + src/mcp/handlers.ts | 83 +++++- src/security/audit-log.ts | 14 + src/security/errors.ts | 18 ++ src/security/executor.ts | 176 ++++++++++++ src/security/index.ts | 33 +++ src/security/rules-engine.ts | 421 +++++++++++++++++++++++++++++ src/security/schemas.ts | 95 +++++++ src/security/shell-parser.ts | 180 ++++++++++++ src/types/bash-parser.d.ts | 3 + tests/config.test.ts | 8 + tests/mcp-registry.test.ts | 31 +++ tests/orchestration-engine.test.ts | 157 +++++++++++ tests/security-middleware.test.ts | 125 +++++++++ tsconfig.json | 2 +- 28 files changed, 1936 insertions(+), 37 deletions(-) create mode 100644 docs/security-middleware.md create mode 100644 src/security/audit-log.ts create mode 100644 src/security/errors.ts create mode 100644 src/security/executor.ts create mode 100644 src/security/index.ts create mode 100644 src/security/rules-engine.ts create mode 100644 src/security/schemas.ts create mode 100644 src/security/shell-parser.ts create mode 100644 src/types/bash-parser.d.ts create mode 100644 tests/security-middleware.test.ts diff --git a/.env.example b/.env.example index f646b77..f55f722 100644 --- a/.env.example +++ b/.env.example @@ -31,3 +31,13 @@ AGENT_PORT_BLOCK_COUNT=512 AGENT_PORT_PRIMARY_OFFSET=0 AGENT_PORT_LOCK_DIR=.ai_ops/locks/ports AGENT_DISCOVERY_FILE_RELATIVE_PATH=.agent-context/resources.json + +# Security middleware +AGENT_SECURITY_VIOLATION_MODE=hard_abort +AGENT_SECURITY_ALLOWED_BINARIES=git,npm,node,cat,ls,pwd,echo,bash,sh +AGENT_SECURITY_COMMAND_TIMEOUT_MS=120000 +AGENT_SECURITY_AUDIT_LOG_PATH=.ai_ops/security/command-audit.ndjson +AGENT_SECURITY_ENV_INHERIT=PATH,HOME,TMPDIR,TMP,TEMP,LANG,LC_ALL +AGENT_SECURITY_ENV_SCRUB= +AGENT_SECURITY_DROP_UID= +AGENT_SECURITY_DROP_GID= diff --git a/AGENTS.md b/AGENTS.md index 32142fa..2283ebe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,15 @@ - `AGENT_PORT_PRIMARY_OFFSET` - `AGENT_PORT_LOCK_DIR` - `AGENT_DISCOVERY_FILE_RELATIVE_PATH` +- Security middleware controls: + - `AGENT_SECURITY_VIOLATION_MODE` + - `AGENT_SECURITY_ALLOWED_BINARIES` + - `AGENT_SECURITY_COMMAND_TIMEOUT_MS` + - `AGENT_SECURITY_AUDIT_LOG_PATH` + - `AGENT_SECURITY_ENV_INHERIT` + - `AGENT_SECURITY_ENV_SCRUB` + - `AGENT_SECURITY_DROP_UID` + - `AGENT_SECURITY_DROP_GID` ## Documentation Standards - Update `README.md` for user-facing behavior. diff --git a/README.md b/README.md index 4676063..ea1f536 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ TypeScript runtime for deterministic multi-agent execution with: - Typed domain events for edge-triggered routing - Resource provisioning (git worktrees + deterministic port ranges) - MCP configuration layer with handler policy hooks +- Security middleware for shell/tool policy enforcement ## Architecture Summary @@ -37,6 +38,7 @@ TypeScript runtime for deterministic multi-agent execution with: - `runtime.ts`: env-driven defaults/singletons - `provisioning.ts`: resource provisioning and child suballocation helpers - `src/mcp`: MCP config types/conversion/handlers +- `src/security`: shell AST parsing, rules engine, secure executor, and audit sinks - `src/examples`: provider entrypoints (`codex.ts`, `claude.ts`) - `src/config.ts`: centralized env parsing/validation/defaulting - `tests`: manager, manifest, pipeline/orchestration, state, provisioning, MCP @@ -69,6 +71,7 @@ npm run dev -- claude "List potential improvements." - supported topologies (`sequential`, `parallel`, `hierarchical`, `retry-unrolled`) - persona definitions and tool-clearance metadata +- persona definitions and tool-clearance policy (validated by shared Zod schema) - relationship DAG and unknown persona references - strict pipeline DAG - topology constraints (`maxDepth`, `maxRetries`) @@ -98,6 +101,26 @@ Actors can emit events in `ActorExecutionResult.events`. Pipeline status also em - session closure aborts child recursive work - run summaries expose aggregate `status`: success requires successful terminal executed DAG nodes and no critical-path failure +## Security Middleware + +- Shell command parsing uses `bash-parser` AST traversal and extracts `Command`/`Word` nodes. +- Rules are validated with strict Zod schemas (`src/security/schemas.ts`) before execution. +- `SecurityRulesEngine` enforces: + - binary allowlists + - cwd/worktree boundary checks + - path traversal blocking (`../`) + - protected path blocking (state root + project context path) + - unified tool allowlist/banlist checks for shell binaries and MCP tool lists +- `SecureCommandExecutor` runs commands via `child_process.spawn` with: + - explicit env scrub/inject policy (no implicit full env inheritance) + - timeout enforcement + - optional uid/gid drop + - stdout/stderr streaming hooks for audit +- Every actor execution input now includes `security` helpers (`rulesEngine`, `createCommandExecutor(...)`) so executors can enforce shell/tool policy at the execution boundary. +- Pipeline behavior on `SecurityViolationError` is configurable: + - `hard_abort` (default) + - `validation_fail` (retry-unrolled remediation) + ## Environment Variables ### Provider/Auth @@ -136,6 +159,17 @@ Actors can emit events in `ActorExecutionResult.events`. Pipeline status also em - `AGENT_PORT_LOCK_DIR` - `AGENT_DISCOVERY_FILE_RELATIVE_PATH` +### Security Middleware + +- `AGENT_SECURITY_VIOLATION_MODE` +- `AGENT_SECURITY_ALLOWED_BINARIES` +- `AGENT_SECURITY_COMMAND_TIMEOUT_MS` +- `AGENT_SECURITY_AUDIT_LOG_PATH` +- `AGENT_SECURITY_ENV_INHERIT` +- `AGENT_SECURITY_ENV_SCRUB` +- `AGENT_SECURITY_DROP_UID` +- `AGENT_SECURITY_DROP_GID` + Defaults are documented in `.env.example`. ## Quality Gate @@ -155,5 +189,4 @@ npm run build ## Notes -- Tool clearance allowlist/banlist is currently metadata only; hard enforcement must happen at the tool execution boundary. - `AgentManager.runRecursiveAgent(...)` remains available for low-level testing, but pipeline execution should use `SchemaDrivenExecutionEngine.runSession(...)`. diff --git a/docs/orchestration-engine.md b/docs/orchestration-engine.md index 3188f9f..f80ef0d 100644 --- a/docs/orchestration-engine.md +++ b/docs/orchestration-engine.md @@ -45,4 +45,13 @@ Node payloads are persisted under the state root. Nodes do not inherit in-memory ## Security note -Tool clearance allowlists/banlists are currently data-model stubs. Enforcement must be implemented in the tool execution boundary before relying on these policies for hard guarantees. +Security enforcement now lives in `src/security`: + +- `bash-parser` AST parsing for shell command tokenization (`Command`/`Word` nodes). +- Zod-validated shell/tool policy schemas. +- `SecurityRulesEngine` for binary allowlists, path traversal checks, worktree boundaries, and tool clearance checks. +- `SecureCommandExecutor` for controlled `child_process` execution with timeout + explicit env policy. + +`PipelineExecutor` treats `SecurityViolationError` via configurable policy: +- `hard_abort` (default): immediate pipeline termination. +- `validation_fail`: maps to retry-unrolled remediation. diff --git a/docs/pipeline-policies.md b/docs/pipeline-policies.md index f443cc1..64157be 100644 --- a/docs/pipeline-policies.md +++ b/docs/pipeline-policies.md @@ -14,6 +14,7 @@ - `PipelineExecutor` (`src/agents/pipeline.ts`) - Coordinates DAG traversal and retry behavior. - Computes aggregate run status from executed terminal nodes plus critical-path failures. + - Applies dedicated `SecurityViolationError` handling policy (`hard_abort` or `validation_fail` mapping). ## Aggregate status semantics diff --git a/docs/security-middleware.md b/docs/security-middleware.md new file mode 100644 index 0000000..9dc8eac --- /dev/null +++ b/docs/security-middleware.md @@ -0,0 +1,42 @@ +# Security Middleware + +## Scope + +This middleware provides a first-pass hardening layer for agent-executed shell commands and MCP tool usage. + +## Components + +- `src/security/shell-parser.ts` + - Uses `bash-parser` to parse shell scripts and extract command-level data from `Command` and `Word` nodes. + - Traverses nested constructs (logical operators, pipelines, subshells, command expansions) so chained commands are fully visible to policy checks. +- `src/security/schemas.ts` + - Zod schemas for shell policies, tool clearance policies, execution env policy, and security violation handling mode. +- `src/security/rules-engine.ts` + - Enforces: + - binary allowlist + - cwd boundary (`AGENT_WORKTREE_ROOT`) + - argument path boundary and `../` traversal rejection + - protected path blocking (`AGENT_STATE_ROOT`, `AGENT_PROJECT_CONTEXT_PATH`) + - unified tool allowlist/banlist checks +- `src/security/executor.ts` + - Runs commands via `child_process.spawn` using explicit env construction (no implicit full parent env inheritance). + - Supports timeout enforcement, optional uid/gid drop, and stdout/stderr stream callbacks. +- `src/security/audit-log.ts` + - File-backed audit sink (`ndjson`) for profiling emitted shell commands and tool access decisions. + +## Pipeline behavior + +`SecurityViolationError` is treated as a first-class error type by `PipelineExecutor`: + +- `hard_abort` (default): fail fast and stop the pipeline. +- `validation_fail`: map violation to retry-unrolled behavior so the actor can attempt a compliant alternative. + +## MCP integration + +`McpRegistry.resolveServerWithHandler(...)` now accepts optional tool clearance and applies it to resolved Codex MCP tool lists (`enabled_tools`, `disabled_tools`). + +## Known limits and TODOs + +- AST validation is token-based and does not yet model full shell evaluation semantics (e.g. runtime-generated paths from env expansion). +- Audit output is line-oriented file logging; move to a centralized telemetry pipeline for long-term profiling. +- Deno sandbox mode is not enforced yet. A future executor mode can wrap shell runs via `deno run` with strict `--allow-read/--allow-run` flags. diff --git a/package-lock.json b/package-lock.json index b6b6cc1..571950d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,9 @@ "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.50", "@openai/codex-sdk": "^0.104.0", - "dotenv": "^17.3.1" + "bash-parser": "^0.5.0", + "dotenv": "^17.3.1", + "zod": "^4.3.6" }, "devDependencies": { "@types/node": "^25.3.0", @@ -932,6 +934,85 @@ "undici-types": "~7.18.0" } }, + "node_modules/arity-n": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/arity-n/-/arity-n-1.0.4.tgz", + "integrity": "sha512-fExL2kFDC1Q2DUOx3whE/9KoN66IzkY4b4zUHUBFM1ojEYjZZYDcUW3bek/ufGionX9giIKDC5redH2IlGqcQQ==", + "license": "MIT" + }, + "node_modules/array-last": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/array-last/-/array-last-1.3.0.tgz", + "integrity": "sha512-eOCut5rXlI6aCOS7Z7kCplKRKyiFQ6dHFBem4PwlwKeNFk2/XxTrhRh5T9PyaEWGy/NHTZWbY+nsZlNFJu9rYg==", + "license": "MIT", + "dependencies": { + "is-number": "^4.0.0" + }, + "engines": { + "node": ">=0.10.0" + } + }, + "node_modules/babylon": { + "version": "6.18.0", + "resolved": "https://registry.npmjs.org/babylon/-/babylon-6.18.0.tgz", + "integrity": "sha512-q/UEjfGJ2Cm3oKV71DJz9d25TPnq5rhBVL2Q4fA5wcC3jcrdn7+SssEybFIxwAvvP+YCsCYNKughoF33GxgycQ==", + "license": "MIT", + "bin": { + "babylon": "bin/babylon.js" + } + }, + "node_modules/bash-parser": { + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/bash-parser/-/bash-parser-0.5.0.tgz", + "integrity": "sha512-AQR43o4W4sj4Jf+oy4cFtGgyBps4B+MYnJg6Xds8VVC7yomFtQekhOORQNHfQ8D6YJ0XENykr3TpxMn3rUtgeg==", + "license": "MIT", + "dependencies": { + "array-last": "^1.1.1", + "babylon": "^6.9.1", + "compose-function": "^3.0.3", + "curry": "^1.2.0", + "deep-freeze": "0.0.1", + "filter-iterator": "0.0.1", + "filter-obj": "^1.1.0", + "has-own-property": "^0.1.0", + "identity-function": "^1.0.0", + "iterable-lookahead": "^1.0.0", + "iterable-transform-replace": "^1.1.1", + "magic-string": "^0.16.0", + "map-iterable": "^1.0.1", + "map-obj": "^2.0.0", + "object-pairs": "^0.1.0", + "object-values": "^1.0.0", + "reverse-arguments": "^1.0.0", + "shell-quote-word": "^1.0.1", + "to-pascal-case": "^1.0.0", + "transform-spread-iterable": "^1.1.0", + "unescape-js": "^1.0.5" + }, + "engines": { + "node": ">=4" + } + }, + "node_modules/compose-function": { + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/compose-function/-/compose-function-3.0.3.tgz", + "integrity": "sha512-xzhzTJ5eC+gmIzvZq+C3kCJHsp9os6tJkrigDRZclyGtOKINbZtE8n1Tzmeh32jW+BUDPbvZpibwvJHBLGMVwg==", + "license": "MIT", + "dependencies": { + "arity-n": "^1.0.4" + } + }, + "node_modules/curry": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/curry/-/curry-1.2.0.tgz", + "integrity": "sha512-PAdmqPH2DUYTCc/aknv6RxRxmqdRHclvbz+wP8t1Xpg2Nu13qg+oLb6/5iFoDmf4dbmC9loYoy9PwwGbFt/AqA==" + }, + "node_modules/deep-freeze": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/deep-freeze/-/deep-freeze-0.0.1.tgz", + "integrity": "sha512-Z+z8HiAvsGwmjqlphnHW5oz6yWlOwu6EQfFTjmeTWlDeda3FS2yv3jhq35TX/ewmsnqB+RX2IdsIOyjJCQN5tg==", + "license": "public domain" + }, "node_modules/dotenv": { "version": "17.3.1", "resolved": "https://registry.npmjs.org/dotenv/-/dotenv-17.3.1.tgz", @@ -986,6 +1067,20 @@ "@esbuild/win32-x64": "0.27.3" } }, + "node_modules/filter-iterator": { + "version": "0.0.1", + "resolved": "https://registry.npmjs.org/filter-iterator/-/filter-iterator-0.0.1.tgz", + "integrity": "sha512-v4lhL7Qa8XpbW3LN46CEnmhGk3eHZwxfNl5at20aEkreesht4YKb/Ba3BUIbnPhAC/r3dmu7ABaGk6MAvh2alA==" + }, + "node_modules/filter-obj": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/filter-obj/-/filter-obj-1.1.0.tgz", + "integrity": "sha512-8rXg1ZnX7xzy2NGDVkBVaAy+lSlPNwad13BtgSlLuxfIslyt5Vg64U7tFcCt4WS1R0hvtnQybT/IyCkGZ3DpXQ==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/fsevents": { "version": "2.3.3", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz", @@ -1014,6 +1109,100 @@ "url": "https://github.com/privatenumber/get-tsconfig?sponsor=1" } }, + "node_modules/has-own-property": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/has-own-property/-/has-own-property-0.1.0.tgz", + "integrity": "sha512-14qdBKoonU99XDhWcFKZTShK+QV47qU97u8zzoVo9cL5TZ3BmBHXogItSt9qJjR0KUMFRhcCW8uGIGl8nkl7Aw==", + "license": "MIT" + }, + "node_modules/identity-function": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/identity-function/-/identity-function-1.0.0.tgz", + "integrity": "sha512-kNrgUK0qI+9qLTBidsH85HjDLpZfrrS0ElquKKe/fJFdB3D7VeKdXXEvOPDUHSHOzdZKCAAaQIWWyp0l2yq6pw==", + "license": "public domain" + }, + "node_modules/is-iterable": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/is-iterable/-/is-iterable-1.1.1.tgz", + "integrity": "sha512-EdOZCr0NsGE00Pot+x1ZFx9MJK3C6wy91geZpXwvwexDLJvA4nzYyZf7r+EIwSeVsOLDdBz7ATg9NqKTzuNYuQ==", + "license": "MIT", + "engines": { + "node": ">= 4" + } + }, + "node_modules/is-number": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/is-number/-/is-number-4.0.0.tgz", + "integrity": "sha512-rSklcAIlf1OmFdyAqbnWTLVelsQ58uvZ66S/ZyawjWqIviTWCjg2PzVGw8WUA+nNuPTqb4wgA+NszrJ+08LlgQ==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, + "node_modules/iterable-lookahead": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/iterable-lookahead/-/iterable-lookahead-1.0.0.tgz", + "integrity": "sha512-hJnEP2Xk4+44DDwJqUQGdXal5VbyeWLaPyDl2AQc242Zr7iqz4DgpQOrEzglWVMGHMDCkguLHEKxd1+rOsmgSQ==", + "license": "MIT", + "engines": { + "node": ">=4" + } + }, + "node_modules/iterable-transform-replace": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/iterable-transform-replace/-/iterable-transform-replace-1.2.0.tgz", + "integrity": "sha512-AVCCj7CTUifWQ0ubraDgx5/e6tOWaL5qh/C8BDTjH0GuhNyFMCSsSmDtYpa4Y3ReAAQNSjUWfQ+ojhmjX10pdQ==", + "license": "MIT", + "dependencies": { + "curry": "^1.2.0" + } + }, + "node_modules/magic-string": { + "version": "0.16.0", + "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.16.0.tgz", + "integrity": "sha512-c4BEos3y6G2qO0B9X7K0FVLOPT9uGrjYwYRLFmDqyl5YMboUviyecnXWp94fJTSMwPw2/sf+CEYt5AGpmklkkQ==", + "license": "MIT", + "dependencies": { + "vlq": "^0.2.1" + } + }, + "node_modules/map-iterable": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/map-iterable/-/map-iterable-1.0.1.tgz", + "integrity": "sha512-siKFftph+ka2jWt8faiOWFzKP+eEuXrHuhYBitssJ5zJm209FCw5JBnaNLDiaCCb/CYZmxprdM6P7p16nA6YRA==", + "license": "MIT", + "dependencies": { + "curry": "^1.2.0", + "is-iterable": "^1.1.0" + }, + "engines": { + "node": ">=4" + } + }, + "node_modules/map-obj": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/map-obj/-/map-obj-2.0.0.tgz", + "integrity": "sha512-TzQSV2DiMYgoF5RycneKVUzIa9bQsj/B3tTgsE3dOGqlzHnGIDaC7XBE7grnA+8kZPnfqSGFe95VHc2oc0VFUQ==", + "license": "MIT", + "engines": { + "node": ">=4" + } + }, + "node_modules/object-pairs": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/object-pairs/-/object-pairs-0.1.0.tgz", + "integrity": "sha512-3ECr6K831I4xX/Mduxr9UC+HPOz/d6WKKYj9p4cmC8Lg8p7g8gitzsxNX5IWlSIgFWN/a4JgrJaoAMKn20oKwA==", + "license": "MIT" + }, + "node_modules/object-values": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/object-values/-/object-values-1.0.0.tgz", + "integrity": "sha512-+8hwcz/JnQ9EpLIXzN0Rs7DLsBpJNT/xYehtB/jU93tHYr5BFEO8E+JGQNOSqE7opVzz5cGksKFHt7uUJVLSjQ==", + "license": "MIT", + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/resolve-pkg-maps": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/resolve-pkg-maps/-/resolve-pkg-maps-1.0.0.tgz", @@ -1024,6 +1213,56 @@ "url": "https://github.com/privatenumber/resolve-pkg-maps?sponsor=1" } }, + "node_modules/reverse-arguments": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/reverse-arguments/-/reverse-arguments-1.0.0.tgz", + "integrity": "sha512-/x8uIPdTafBqakK0TmPNJzgkLP+3H+yxpUJhCQHsLBg1rYEVNR2D8BRYNWQhVBjyOd7oo1dZRVzIkwMY2oqfYQ==", + "license": "MIT" + }, + "node_modules/shell-quote-word": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/shell-quote-word/-/shell-quote-word-1.0.1.tgz", + "integrity": "sha512-lT297f1WLAdq0A4O+AknIFRP6kkiI3s8C913eJ0XqBxJbZPGWUNkRQk2u8zk4bEAjUJ5i+fSLwB6z1HzeT+DEg==", + "license": "MIT" + }, + "node_modules/string.fromcodepoint": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/string.fromcodepoint/-/string.fromcodepoint-0.2.1.tgz", + "integrity": "sha512-n69H31OnxSGSZyZbgBlvYIXlrMhJQ0dQAX1js1QDhpaUH6zmU3QYlj07bCwCNlPOu3oRXIubGPl2gDGnHsiCqg==" + }, + "node_modules/to-no-case": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/to-no-case/-/to-no-case-1.0.2.tgz", + "integrity": "sha512-Z3g735FxuZY8rodxV4gH7LxClE4H0hTIyHNIHdk+vpQxjLm0cwnKXq/OFVZ76SOQmto7txVcwSCwkU5kqp+FKg==", + "license": "MIT" + }, + "node_modules/to-pascal-case": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/to-pascal-case/-/to-pascal-case-1.0.0.tgz", + "integrity": "sha512-QGMWHqM6xPrcQW57S23c5/3BbYb0Tbe9p+ur98ckRnGDwD4wbbtDiYI38CfmMKNB5Iv0REjs5SNDntTwvDxzZA==", + "license": "MIT", + "dependencies": { + "to-space-case": "^1.0.0" + } + }, + "node_modules/to-space-case": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/to-space-case/-/to-space-case-1.0.0.tgz", + "integrity": "sha512-rLdvwXZ39VOn1IxGL3V6ZstoTbwLRckQmn/U8ZDLuWwIXNpuZDhQ3AiRUlhTbOXFVE9C+dR51wM0CBDhk31VcA==", + "license": "MIT", + "dependencies": { + "to-no-case": "^1.0.0" + } + }, + "node_modules/transform-spread-iterable": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/transform-spread-iterable/-/transform-spread-iterable-1.4.1.tgz", + "integrity": "sha512-/GnF26X3zC8wfWyRzvuXX/Vb31TrU3Rwipmr4MC5hTi6X/yOXxXUSw4+pcHmKJ2+0KRrcS21YWZw77ukhVJBdQ==", + "license": "MIT", + "dependencies": { + "curry": "^1.2.0" + } + }, "node_modules/tsx": { "version": "4.21.0", "resolved": "https://registry.npmjs.org/tsx/-/tsx-4.21.0.tgz", @@ -1065,6 +1304,21 @@ "dev": true, "license": "MIT" }, + "node_modules/unescape-js": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/unescape-js/-/unescape-js-1.1.4.tgz", + "integrity": "sha512-42SD8NOQEhdYntEiUQdYq/1V/YHwr1HLwlHuTJB5InVVdOSbgI6xu8jK5q65yIzuFCfczzyDF/7hbGzVbyCw0g==", + "license": "MIT", + "dependencies": { + "string.fromcodepoint": "^0.2.1" + } + }, + "node_modules/vlq": { + "version": "0.2.3", + "resolved": "https://registry.npmjs.org/vlq/-/vlq-0.2.3.tgz", + "integrity": "sha512-DRibZL6DsNhIgYQ+wNdWDL2SL3bKPlVrRiBqV5yuMm++op8W4kGFtaQfCs4KEJn0wBZcHVHJ3eoywX8983k1ow==", + "license": "MIT" + }, "node_modules/zod": { "version": "4.3.6", "resolved": "https://registry.npmjs.org/zod/-/zod-4.3.6.tgz", diff --git a/package.json b/package.json index 6ec4358..33ba28d 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,9 @@ "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.50", "@openai/codex-sdk": "^0.104.0", - "dotenv": "^17.3.1" + "bash-parser": "^0.5.0", + "dotenv": "^17.3.1", + "zod": "^4.3.6" }, "devDependencies": { "@types/node": "^25.3.0", diff --git a/src/agents/manifest.ts b/src/agents/manifest.ts index 4beec7a..30bc48c 100644 --- a/src/agents/manifest.ts +++ b/src/agents/manifest.ts @@ -1,10 +1,11 @@ import { isRecord } from "./types.js"; import { isDomainEventType, type DomainEventType } from "./domain-events.js"; +import { + parseToolClearancePolicy, + type ToolClearancePolicy as SecurityToolClearancePolicy, +} from "../security/schemas.js"; -export type ToolClearancePolicy = { - allowlist: string[]; - banlist: string[]; -}; +export type ToolClearancePolicy = SecurityToolClearancePolicy; export type ManifestPersona = { id: string; @@ -139,14 +140,12 @@ function readStringArray(record: Record, key: string): string[] } function parseToolClearance(value: unknown): ToolClearancePolicy { - if (!isRecord(value)) { - throw new Error("Manifest persona toolClearance must be an object."); + try { + return parseToolClearancePolicy(value); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + throw new Error(`Manifest persona toolClearance is invalid: ${detail}`); } - - return { - allowlist: readStringArray(value, "allowlist"), - banlist: readStringArray(value, "banlist"), - }; } function parsePersona(value: unknown): ManifestPersona { diff --git a/src/agents/orchestration.ts b/src/agents/orchestration.ts index 1fb549f..cedae23 100644 --- a/src/agents/orchestration.ts +++ b/src/agents/orchestration.ts @@ -8,10 +8,20 @@ import { type PersonaBehaviorEvent, type PersonaBehaviorHandler, } from "./persona-registry.js"; -import { PipelineExecutor, type ActorExecutor, type PipelineRunSummary } from "./pipeline.js"; +import { + PipelineExecutor, + type ActorExecutionSecurityContext, + type ActorExecutor, + type PipelineRunSummary, +} from "./pipeline.js"; import { FileSystemProjectContextStore } from "./project-context.js"; import { FileSystemStateContextManager, type StoredSessionState } from "./state-context.js"; import type { JsonObject } from "./types.js"; +import { + SecureCommandExecutor, + SecurityRulesEngine, + createFileSecurityAuditSink, +} from "../security/index.js"; export type OrchestrationSettings = { workspaceRoot: string; @@ -20,6 +30,7 @@ export type OrchestrationSettings = { maxDepth: number; maxRetries: number; maxChildren: number; + securityViolationHandling: "hard_abort" | "validation_fail"; runtimeContext: Record; }; @@ -37,6 +48,7 @@ export function loadOrchestrationSettingsFromEnv( maxDepth: config.orchestration.maxDepth, maxRetries: config.orchestration.maxRetries, maxChildren: config.orchestration.maxChildren, + securityViolationHandling: config.security.violationHandling, }; } @@ -64,6 +76,50 @@ function getChildrenByParent(manifest: AgentManifest): Map; + settings: OrchestrationSettings; +}): ActorExecutionSecurityContext { + const auditSink = createFileSecurityAuditSink( + resolve(input.settings.workspaceRoot, input.config.security.auditLogPath), + ); + const rulesEngine = new SecurityRulesEngine( + { + allowedBinaries: input.config.security.shellAllowedBinaries, + worktreeRoot: resolve( + input.settings.workspaceRoot, + input.config.provisioning.gitWorktree.rootDirectory, + ), + protectedPaths: [input.settings.stateRoot, input.settings.projectContextPath], + requireCwdWithinWorktree: true, + rejectRelativePathTraversal: true, + enforcePathBoundaryOnArguments: true, + allowedEnvAssignments: [], + blockedEnvAssignments: ["AGENT_STATE_ROOT", "AGENT_PROJECT_CONTEXT_PATH"], + }, + auditSink, + ); + + return { + rulesEngine, + createCommandExecutor: (overrides) => + new SecureCommandExecutor({ + rulesEngine, + timeoutMs: overrides?.timeoutMs ?? input.config.security.commandTimeoutMs, + envPolicy: + overrides?.envPolicy ?? + { + inherit: [...input.config.security.inheritedEnvVars], + scrub: [...input.config.security.scrubbedEnvVars], + inject: {}, + }, + shellPath: overrides?.shellPath, + uid: overrides?.uid ?? input.config.security.dropUid, + gid: overrides?.gid ?? input.config.security.dropGid, + }), + }; +} + export class SchemaDrivenExecutionEngine { private readonly manifest: AgentManifest; private readonly personaRegistry = new PersonaRegistry(); @@ -74,6 +130,7 @@ export class SchemaDrivenExecutionEngine { private readonly childrenByParent: Map; private readonly manager: AgentManager; private readonly mcpRegistry: McpRegistry; + private readonly securityContext: ActorExecutionSecurityContext; constructor(input: { manifest: AgentManifest | unknown; @@ -99,6 +156,8 @@ export class SchemaDrivenExecutionEngine { maxDepth: input.settings?.maxDepth ?? config.orchestration.maxDepth, maxRetries: input.settings?.maxRetries ?? config.orchestration.maxRetries, maxChildren: input.settings?.maxChildren ?? config.orchestration.maxChildren, + securityViolationHandling: + input.settings?.securityViolationHandling ?? config.security.violationHandling, runtimeContext: { ...(input.settings?.runtimeContext ?? {}), }, @@ -120,6 +179,10 @@ export class SchemaDrivenExecutionEngine { maxRecursiveDepth: config.agentManager.maxRecursiveDepth, }); this.mcpRegistry = input.mcpRegistry ?? createDefaultMcpRegistry(); + this.securityContext = createActorSecurityContext({ + config, + settings: this.settings, + }); for (const persona of this.manifest.personas) { this.personaRegistry.register({ @@ -198,6 +261,8 @@ export class SchemaDrivenExecutionEngine { managerSessionId, projectContextStore: this.projectContextStore, mcpRegistry: this.mcpRegistry, + securityViolationHandling: this.settings.securityViolationHandling, + securityContext: this.securityContext, }, ); try { diff --git a/src/agents/persona-registry.ts b/src/agents/persona-registry.ts index 3341bf3..1f31e64 100644 --- a/src/agents/persona-registry.ts +++ b/src/agents/persona-registry.ts @@ -1,5 +1,6 @@ import type { JsonObject } from "./types.js"; import type { ManifestPersona, ToolClearancePolicy } from "./manifest.js"; +import { parseToolClearancePolicy } from "../security/schemas.js"; export type PersonaBehaviorEvent = "onTaskComplete" | "onValidationFail"; @@ -28,18 +29,6 @@ function renderTemplate( }); } -function uniqueStrings(values: string[]): string[] { - const output: string[] = []; - const seen = new Set(); - for (const value of values) { - if (!seen.has(value)) { - output.push(value); - seen.add(value); - } - } - return output; -} - export class PersonaRegistry { private readonly personas = new Map(); @@ -54,12 +43,10 @@ export class PersonaRegistry { throw new Error(`Persona \"${persona.id}\" is already registered.`); } + const toolClearance = parseToolClearancePolicy(persona.toolClearance); this.personas.set(persona.id, { ...persona, - toolClearance: { - allowlist: uniqueStrings(persona.toolClearance.allowlist), - banlist: uniqueStrings(persona.toolClearance.banlist), - }, + toolClearance, }); } @@ -81,8 +68,6 @@ export class PersonaRegistry { getToolClearance(personaId: string): ToolClearancePolicy { const persona = this.getById(personaId); - - // TODO(security): enforce allowlist/banlist in the tool execution boundary. return { allowlist: [...persona.toolClearance.allowlist], banlist: [...persona.toolClearance.banlist], diff --git a/src/agents/pipeline.ts b/src/agents/pipeline.ts index 2144dc8..fdf5d89 100644 --- a/src/agents/pipeline.ts +++ b/src/agents/pipeline.ts @@ -25,6 +25,13 @@ import { type StoredSessionState, } from "./state-context.js"; import type { JsonObject } from "./types.js"; +import { + SecureCommandExecutor, + SecurityRulesEngine, + SecurityViolationError, + type ExecutionEnvPolicy, + type SecurityViolationHandling, +} from "../security/index.js"; export type ActorResultStatus = "success" | "validation_fail" | "failure"; export type ActorFailureKind = "soft" | "hard"; @@ -50,6 +57,7 @@ export type ActorExecutionInput = { allowlist: string[]; banlist: string[]; }; + security?: ActorExecutionSecurityContext; }; export type ActorExecutor = (input: ActorExecutionInput) => Promise; @@ -84,6 +92,19 @@ export type PipelineExecutorOptions = { failurePolicy?: FailurePolicy; lifecycleObserver?: PipelineLifecycleObserver; hardFailureThreshold?: number; + securityViolationHandling?: SecurityViolationHandling; + securityContext?: ActorExecutionSecurityContext; +}; + +export type ActorExecutionSecurityContext = { + rulesEngine: SecurityRulesEngine; + createCommandExecutor: (input?: { + timeoutMs?: number; + envPolicy?: ExecutionEnvPolicy; + shellPath?: string; + uid?: number; + gid?: number; + }) => SecureCommandExecutor; }; type QueueItem = { @@ -283,6 +304,8 @@ export class PipelineExecutor { private readonly failurePolicy: FailurePolicy; private readonly lifecycleObserver: PipelineLifecycleObserver; private readonly hardFailureThreshold: number; + private readonly securityViolationHandling: SecurityViolationHandling; + private readonly securityContext?: ActorExecutionSecurityContext; private managerRunCounter = 0; constructor( @@ -294,6 +317,8 @@ export class PipelineExecutor { ) { this.failurePolicy = options.failurePolicy ?? new FailurePolicy(); this.hardFailureThreshold = options.hardFailureThreshold ?? 2; + this.securityViolationHandling = options.securityViolationHandling ?? "hard_abort"; + this.securityContext = options.securityContext; this.lifecycleObserver = options.lifecycleObserver ?? new PersistenceLifecycleObserver({ @@ -719,12 +744,29 @@ export class PipelineExecutor { context: input.context, signal: input.signal, toolClearance: this.personaRegistry.getToolClearance(input.node.personaId), + security: this.securityContext, }); } catch (error) { if (input.signal.aborted) { throw toAbortError(input.signal); } + if (error instanceof SecurityViolationError) { + if (this.securityViolationHandling === "hard_abort") { + throw error; + } + + return { + status: "validation_fail", + payload: { + error: error.message, + security_violation: true, + }, + failureCode: error.code, + failureKind: "soft", + }; + } + const classified = this.failurePolicy.classifyFailureFromError(error); return { diff --git a/src/config.ts b/src/config.ts index 8f6f14c..a9f28aa 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,5 +1,6 @@ import type { AgentManagerLimits } from "./agents/manager.js"; import type { BuiltInProvisioningConfig } from "./agents/provisioning.js"; +import { parseSecurityViolationHandling, type SecurityViolationHandling } from "./security/index.js"; export type ProviderRuntimeConfig = { codexApiKey?: string; @@ -27,6 +28,17 @@ export type DiscoveryRuntimeConfig = { fileRelativePath: string; }; +export type SecurityRuntimeConfig = { + violationHandling: SecurityViolationHandling; + shellAllowedBinaries: string[]; + commandTimeoutMs: number; + auditLogPath: string; + inheritedEnvVars: string[]; + scrubbedEnvVars: string[]; + dropUid?: number; + dropGid?: number; +}; + export type AppConfig = { provider: ProviderRuntimeConfig; mcp: McpRuntimeConfig; @@ -34,6 +46,7 @@ export type AppConfig = { orchestration: OrchestrationRuntimeConfig; provisioning: BuiltInProvisioningConfig; discovery: DiscoveryRuntimeConfig; + security: SecurityRuntimeConfig; }; const DEFAULT_AGENT_MANAGER: AgentManagerLimits = { @@ -68,6 +81,15 @@ const DEFAULT_DISCOVERY: DiscoveryRuntimeConfig = { fileRelativePath: ".agent-context/resources.json", }; +const DEFAULT_SECURITY: SecurityRuntimeConfig = { + violationHandling: "hard_abort", + shellAllowedBinaries: ["git", "npm", "node", "cat", "ls", "pwd", "echo", "bash", "sh"], + commandTimeoutMs: 120_000, + auditLogPath: ".ai_ops/security/command-audit.ndjson", + inheritedEnvVars: ["PATH", "HOME", "TMPDIR", "TMP", "TEMP", "LANG", "LC_ALL"], + scrubbedEnvVars: [], +}; + function readOptionalString( env: NodeJS.ProcessEnv, key: string, @@ -87,6 +109,31 @@ function readStringWithFallback( return readOptionalString(env, key) ?? fallback; } +function readCsvStringArrayWithFallback( + env: NodeJS.ProcessEnv, + key: string, + fallback: readonly string[], + options: { + allowEmpty?: boolean; + } = {}, +): string[] { + const raw = env[key]?.trim(); + if (!raw) { + return [...fallback]; + } + + const parsed = raw + .split(",") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); + + if (parsed.length === 0 && options.allowEmpty !== true) { + throw new Error(`Environment variable ${key} must include at least one comma-delimited value.`); + } + + return parsed; +} + function readIntegerWithBounds( env: NodeJS.ProcessEnv, key: string, @@ -108,6 +155,26 @@ function readIntegerWithBounds( return parsed; } +function readOptionalIntegerWithBounds( + env: NodeJS.ProcessEnv, + key: string, + bounds: { + min: number; + }, +): number | undefined { + const raw = env[key]?.trim(); + if (!raw) { + return undefined; + } + + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed < bounds.min) { + throw new Error(`Environment variable ${key} must be an integer >= ${String(bounds.min)}.`); + } + + return parsed; +} + function readBooleanWithFallback( env: NodeJS.ProcessEnv, key: string, @@ -142,6 +209,12 @@ function deepFreeze(value: T): Readonly { } export function loadConfig(env: NodeJS.ProcessEnv = process.env): Readonly { + const rawViolationHandling = readStringWithFallback( + env, + "AGENT_SECURITY_VIOLATION_MODE", + DEFAULT_SECURITY.violationHandling, + ); + const config: AppConfig = { provider: { codexApiKey: readOptionalString(env, "CODEX_API_KEY"), @@ -257,6 +330,38 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): Readonly { return typeof value === "object" && value !== null && !Array.isArray(value); @@ -62,6 +63,7 @@ export function loadMcpConfigFromEnv( options?: { config?: Readonly; registry?: McpRegistry; + toolClearance?: ToolClearancePolicy; }, ): LoadedMcpConfig { const runtimeConfig = options?.config ?? getConfig(); @@ -82,6 +84,7 @@ export function loadMcpConfigFromEnv( server, context, fullConfig: config, + toolClearance: options?.toolClearance, }); resolvedHandlers[serverName] = resolved.handlerId; diff --git a/src/mcp/handlers.ts b/src/mcp/handlers.ts index 40e941c..eec678c 100644 --- a/src/mcp/handlers.ts +++ b/src/mcp/handlers.ts @@ -10,6 +10,10 @@ import type { SharedMcpConfigFile, SharedMcpServer, } from "./types.js"; +import { + parseToolClearancePolicy, + type ToolClearancePolicy, +} from "../security/schemas.js"; export type McpHandlerUtils = { inferTransport: typeof inferTransport; @@ -114,6 +118,77 @@ function readBooleanConfigValue( return typeof value === "boolean" ? value : undefined; } +function readStringArray(value: unknown): string[] | undefined { + if (!Array.isArray(value)) { + return undefined; + } + + const output: string[] = []; + for (const item of value) { + if (typeof item !== "string") { + continue; + } + + const normalized = item.trim(); + if (!normalized) { + continue; + } + + output.push(normalized); + } + + return output; +} + +function dedupe(values: string[]): string[] { + const output: string[] = []; + const seen = new Set(); + for (const value of values) { + if (seen.has(value)) { + continue; + } + seen.add(value); + output.push(value); + } + return output; +} + +function applyToolClearanceToResult( + result: McpHandlerResult, + toolClearance: ToolClearancePolicy, +): McpHandlerResult { + if (!result.codex) { + return result; + } + + const codexConfig = result.codex; + const existingEnabled = readStringArray(codexConfig.enabled_tools); + const existingDisabled = readStringArray(codexConfig.disabled_tools) ?? []; + const banlist = new Set(toolClearance.banlist); + + let enabledTools = existingEnabled; + if (toolClearance.allowlist.length > 0) { + enabledTools = (enabledTools ?? toolClearance.allowlist).filter((tool) => + toolClearance.allowlist.includes(tool), + ); + } + + if (enabledTools) { + enabledTools = enabledTools.filter((tool) => !banlist.has(tool)); + } + + const disabledTools = dedupe([...existingDisabled, ...toolClearance.banlist]); + + return { + ...result, + codex: { + ...codexConfig, + ...(enabledTools ? { enabled_tools: enabledTools } : {}), + ...(disabledTools.length > 0 ? { disabled_tools: disabledTools } : {}), + }, + }; +} + function applyEnabledByDefault(input: McpHandlerBusinessLogicInput): McpHandlerResult { if (input.server.enabled !== undefined) { return input.baseResult; @@ -187,8 +262,9 @@ export class McpRegistry { server: SharedMcpServer; context: McpLoadContext; fullConfig: SharedMcpConfigFile; + toolClearance?: ToolClearancePolicy; }): McpHandlerResult & { handlerId: string } { - const { serverName, server, context, fullConfig } = input; + const { serverName, server, context, fullConfig, toolClearance } = input; const handler = this.resolveHandler(serverName, server); const handlerConfig = { ...(fullConfig.handlerSettings?.[handler.id] ?? {}), @@ -203,9 +279,12 @@ export class McpRegistry { fullConfig, utils, }); + const securedResult = toolClearance + ? applyToolClearanceToResult(result, parseToolClearancePolicy(toolClearance)) + : result; return { - ...result, + ...securedResult, handlerId: handler.id, }; } diff --git a/src/security/audit-log.ts b/src/security/audit-log.ts new file mode 100644 index 0000000..6d06319 --- /dev/null +++ b/src/security/audit-log.ts @@ -0,0 +1,14 @@ +import { mkdir, appendFile } from "node:fs/promises"; +import { dirname, resolve } from "node:path"; +import type { SecurityAuditEvent, SecurityAuditSink } from "./rules-engine.js"; + +export function createFileSecurityAuditSink(filePath: string): SecurityAuditSink { + const resolvedPath = resolve(filePath); + + return (event: SecurityAuditEvent): void => { + void (async () => { + await mkdir(dirname(resolvedPath), { recursive: true }); + await appendFile(resolvedPath, `${JSON.stringify(event)}\n`, "utf8"); + })(); + }; +} diff --git a/src/security/errors.ts b/src/security/errors.ts new file mode 100644 index 0000000..d6383a2 --- /dev/null +++ b/src/security/errors.ts @@ -0,0 +1,18 @@ +export class SecurityViolationError extends Error { + readonly code: string; + readonly details?: Record; + + constructor( + message: string, + input: { + code?: string; + details?: Record; + cause?: unknown; + } = {}, + ) { + super(message, input.cause !== undefined ? { cause: input.cause } : undefined); + this.name = "SecurityViolationError"; + this.code = input.code ?? "SECURITY_VIOLATION"; + this.details = input.details; + } +} diff --git a/src/security/executor.ts b/src/security/executor.ts new file mode 100644 index 0000000..0de733c --- /dev/null +++ b/src/security/executor.ts @@ -0,0 +1,176 @@ +import { spawn } from "node:child_process"; +import type { ParsedShellScript } from "./shell-parser.js"; +import { + parseExecutionEnvPolicy, + type ExecutionEnvPolicy, + type ToolClearancePolicy, +} from "./schemas.js"; +import { SecurityRulesEngine } from "./rules-engine.js"; + +export type SecureCommandExecutionResult = { + exitCode: number | null; + signal: NodeJS.Signals | null; + stdout: string; + stderr: string; + timedOut: boolean; + durationMs: number; + parsed: ParsedShellScript; +}; + +export type SecureCommandExecutorOptions = { + rulesEngine: SecurityRulesEngine; + envPolicy?: ExecutionEnvPolicy; + timeoutMs?: number; + shellPath?: string; + uid?: number; + gid?: number; +}; + +export class SecureCommandExecutor { + private readonly rulesEngine: SecurityRulesEngine; + private readonly envPolicy: ExecutionEnvPolicy; + private readonly timeoutMs: number; + private readonly shellPath: string; + private readonly uid?: number; + private readonly gid?: number; + + constructor(options: SecureCommandExecutorOptions) { + this.rulesEngine = options.rulesEngine; + this.envPolicy = parseExecutionEnvPolicy(options.envPolicy ?? {}); + this.timeoutMs = options.timeoutMs ?? 120_000; + this.shellPath = options.shellPath ?? "/bin/bash"; + this.uid = options.uid; + this.gid = options.gid; + } + + async execute(input: { + command: string; + cwd: string; + baseEnv?: Record; + injectedEnv?: Record; + toolClearance?: ToolClearancePolicy; + signal?: AbortSignal; + onStdoutChunk?: (chunk: string) => void; + onStderrChunk?: (chunk: string) => void; + }): Promise { + const validated = this.rulesEngine.validateShellCommand({ + command: input.command, + cwd: input.cwd, + toolClearance: input.toolClearance, + }); + + const startedAt = Date.now(); + const env = this.buildExecutionEnv(input.baseEnv, input.injectedEnv); + + return new Promise((resolvePromise, rejectPromise) => { + let stdout = ""; + let stderr = ""; + let settled = false; + let timedOut = false; + + const settleResolve = (result: SecureCommandExecutionResult): void => { + if (settled) { + return; + } + settled = true; + resolvePromise(result); + }; + + const settleReject = (error: unknown): void => { + if (settled) { + return; + } + settled = true; + rejectPromise(error); + }; + + const child = spawn(this.shellPath, ["-c", input.command], { + cwd: validated.cwd, + env, + ...(this.uid !== undefined ? { uid: this.uid } : {}), + ...(this.gid !== undefined ? { gid: this.gid } : {}), + ...(input.signal ? { signal: input.signal } : {}), + stdio: ["ignore", "pipe", "pipe"], + }); + + const timeoutHandle = + this.timeoutMs > 0 + ? setTimeout(() => { + timedOut = true; + child.kill("SIGKILL"); + }, this.timeoutMs) + : undefined; + + child.stdout.on("data", (chunk) => { + const text = String(chunk); + stdout += text; + input.onStdoutChunk?.(text); + }); + + child.stderr.on("data", (chunk) => { + const text = String(chunk); + stderr += text; + input.onStderrChunk?.(text); + }); + + child.on("error", (error) => { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } + settleReject(error); + }); + + child.on("close", (exitCode, signal) => { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } + + if (timedOut) { + settleReject( + new Error( + `Command timed out after ${String(this.timeoutMs)}ms: ${input.command}`, + ), + ); + return; + } + + settleResolve({ + exitCode, + signal, + stdout, + stderr, + timedOut, + durationMs: Date.now() - startedAt, + parsed: validated.parsed, + }); + }); + }); + } + + private buildExecutionEnv( + baseEnv: Record | undefined, + injectedEnv: Record | undefined, + ): Record { + const out: Record = {}; + const source = baseEnv ?? process.env; + + for (const key of this.envPolicy.inherit) { + const value = source[key]; + if (typeof value === "string") { + out[key] = value; + } + } + + for (const key of this.envPolicy.scrub) { + delete out[key]; + } + + Object.assign(out, this.envPolicy.inject); + + if (injectedEnv) { + Object.assign(out, injectedEnv); + } + + return out; + } +} diff --git a/src/security/index.ts b/src/security/index.ts new file mode 100644 index 0000000..38f6155 --- /dev/null +++ b/src/security/index.ts @@ -0,0 +1,33 @@ +export { SecurityViolationError } from "./errors.js"; +export { + parseExecutionEnvPolicy, + parseSecurityViolationHandling, + parseShellValidationPolicy, + parseToolClearancePolicy, + securityViolationHandlingSchema, + shellValidationPolicySchema, + toolClearancePolicySchema, + executionEnvPolicySchema, + type ExecutionEnvPolicy, + type SecurityViolationHandling, + type ShellValidationPolicy, + type ToolClearancePolicy, +} from "./schemas.js"; +export { + parseShellScript, + type ParsedShellAssignment, + type ParsedShellCommand, + type ParsedShellScript, +} from "./shell-parser.js"; +export { + SecurityRulesEngine, + type SecurityAuditEvent, + type SecurityAuditSink, + type ValidatedShellCommand, +} from "./rules-engine.js"; +export { + SecureCommandExecutor, + type SecureCommandExecutionResult, + type SecureCommandExecutorOptions, +} from "./executor.js"; +export { createFileSecurityAuditSink } from "./audit-log.js"; diff --git a/src/security/rules-engine.ts b/src/security/rules-engine.ts new file mode 100644 index 0000000..b27b18c --- /dev/null +++ b/src/security/rules-engine.ts @@ -0,0 +1,421 @@ +import { basename, isAbsolute, relative, resolve, sep } from "node:path"; +import { SecurityViolationError } from "./errors.js"; +import { + parseShellScript, + type ParsedShellCommand, + type ParsedShellScript, +} from "./shell-parser.js"; +import { + parseShellValidationPolicy, + parseToolClearancePolicy, + type ShellValidationPolicy, + type ToolClearancePolicy, +} from "./schemas.js"; + +export type SecurityAuditEvent = + | { + type: "shell.command_profiled"; + timestamp: string; + command: string; + cwd: string; + parsed: ParsedShellScript; + } + | { + type: "shell.command_allowed"; + timestamp: string; + command: string; + cwd: string; + commandCount: number; + } + | { + type: "shell.command_blocked"; + timestamp: string; + command: string; + cwd: string; + reason: string; + code: string; + details?: Record; + } + | { + type: "tool.invocation_allowed"; + timestamp: string; + tool: string; + } + | { + type: "tool.invocation_blocked"; + timestamp: string; + tool: string; + reason: string; + code: string; + }; + +export type SecurityAuditSink = (event: SecurityAuditEvent) => void; + +export type ValidatedShellCommand = { + cwd: string; + parsed: ParsedShellScript; +}; + +function normalizeToken(value: string): string { + return value.trim(); +} + +function hasPathTraversalSegment(token: string): boolean { + const normalized = token.replaceAll("\\", "/"); + if (normalized === ".." || normalized.startsWith("../") || normalized.endsWith("/..")) { + return true; + } + return normalized.includes("/../"); +} + +function isPathLikeToken(token: string): boolean { + if (!token || token === ".") { + return false; + } + + if (token.startsWith("/") || token.startsWith("./") || token.startsWith("../")) { + return true; + } + + return token.includes("/") || token.includes("\\"); +} + +function isWithinPath(root: string, candidate: string): boolean { + const rel = relative(root, candidate); + return rel === "" || (!rel.startsWith(`..${sep}`) && rel !== ".." && !isAbsolute(rel)); +} + +function isPathBlocked(candidatePath: string, protectedPath: string): boolean { + const rel = relative(protectedPath, candidatePath); + return rel === "" || (!rel.startsWith(`..${sep}`) && rel !== ".." && !isAbsolute(rel)); +} + +function toToolSet(values: readonly string[]): Set { + const out = new Set(); + for (const value of values) { + out.add(value); + } + return out; +} + +function toNow(): string { + return new Date().toISOString(); +} + +export class SecurityRulesEngine { + private readonly policy: ShellValidationPolicy; + private readonly allowedBinaries: Set; + private readonly allowedEnvAssignments: Set; + private readonly blockedEnvAssignments: Set; + private readonly worktreeRoot: string; + private readonly protectedPaths: string[]; + + constructor( + policy: ShellValidationPolicy, + private readonly auditSink?: SecurityAuditSink, + ) { + this.policy = parseShellValidationPolicy(policy); + this.allowedBinaries = toToolSet(this.policy.allowedBinaries); + this.allowedEnvAssignments = toToolSet(this.policy.allowedEnvAssignments); + this.blockedEnvAssignments = toToolSet(this.policy.blockedEnvAssignments); + this.worktreeRoot = resolve(this.policy.worktreeRoot); + this.protectedPaths = this.policy.protectedPaths.map((path) => resolve(path)); + } + + getPolicy(): ShellValidationPolicy { + return { + ...this.policy, + allowedBinaries: [...this.policy.allowedBinaries], + protectedPaths: [...this.policy.protectedPaths], + allowedEnvAssignments: [...this.policy.allowedEnvAssignments], + blockedEnvAssignments: [...this.policy.blockedEnvAssignments], + }; + } + + validateShellCommand(input: { + command: string; + cwd: string; + toolClearance?: ToolClearancePolicy; + }): ValidatedShellCommand { + const resolvedCwd = resolve(input.cwd); + + try { + this.assertCwdBoundary(resolvedCwd); + const parsed = parseShellScript(input.command); + const toolClearance = input.toolClearance + ? parseToolClearancePolicy(input.toolClearance) + : undefined; + + this.emit({ + type: "shell.command_profiled", + timestamp: toNow(), + command: input.command, + cwd: resolvedCwd, + parsed, + }); + + for (const command of parsed.commands) { + this.assertBinaryAllowed(command, toolClearance); + this.assertAssignmentsAllowed(command); + this.assertArgumentPaths(command, resolvedCwd); + } + + this.emit({ + type: "shell.command_allowed", + timestamp: toNow(), + command: input.command, + cwd: resolvedCwd, + commandCount: parsed.commandCount, + }); + + return { + cwd: resolvedCwd, + parsed, + }; + } catch (error) { + if (error instanceof SecurityViolationError) { + this.emit({ + type: "shell.command_blocked", + timestamp: toNow(), + command: input.command, + cwd: resolvedCwd, + reason: error.message, + code: error.code, + details: error.details, + }); + throw error; + } + + throw new SecurityViolationError("Unexpected error while validating shell command.", { + code: "SECURITY_VALIDATION_FAILED", + cause: error, + }); + } + } + + assertToolInvocationAllowed(input: { + tool: string; + toolClearance: ToolClearancePolicy; + }): void { + const policy = parseToolClearancePolicy(input.toolClearance); + + if (policy.banlist.includes(input.tool)) { + this.emit({ + type: "tool.invocation_blocked", + timestamp: toNow(), + tool: input.tool, + reason: `Tool "${input.tool}" is explicitly banned by policy.`, + code: "TOOL_BANNED", + }); + throw new SecurityViolationError( + `Tool "${input.tool}" is explicitly banned by policy.`, + { + code: "TOOL_BANNED", + details: { + tool: input.tool, + }, + }, + ); + } + + if (policy.allowlist.length > 0 && !policy.allowlist.includes(input.tool)) { + this.emit({ + type: "tool.invocation_blocked", + timestamp: toNow(), + tool: input.tool, + reason: `Tool "${input.tool}" is not present in allowlist.`, + code: "TOOL_NOT_ALLOWED", + }); + throw new SecurityViolationError( + `Tool "${input.tool}" is not present in allowlist.`, + { + code: "TOOL_NOT_ALLOWED", + details: { + tool: input.tool, + }, + }, + ); + } + + this.emit({ + type: "tool.invocation_allowed", + timestamp: toNow(), + tool: input.tool, + }); + } + + filterAllowedTools(tools: string[], toolClearance: ToolClearancePolicy): string[] { + const policy = parseToolClearancePolicy(toolClearance); + + const allowedByAllowlist = + policy.allowlist.length === 0 + ? tools + : tools.filter((tool) => policy.allowlist.includes(tool)); + + return allowedByAllowlist.filter((tool) => !policy.banlist.includes(tool)); + } + + private assertCwdBoundary(cwd: string): void { + if (this.policy.requireCwdWithinWorktree && !isWithinPath(this.worktreeRoot, cwd)) { + throw new SecurityViolationError( + `cwd "${cwd}" is outside configured worktree root "${this.worktreeRoot}".`, + { + code: "CWD_OUTSIDE_WORKTREE", + details: { + cwd, + worktreeRoot: this.worktreeRoot, + }, + }, + ); + } + + for (const blockedPath of this.protectedPaths) { + if (!isPathBlocked(cwd, blockedPath)) { + continue; + } + + throw new SecurityViolationError( + `cwd "${cwd}" is inside protected path "${blockedPath}".`, + { + code: "CWD_INSIDE_PROTECTED_PATH", + details: { + cwd, + protectedPath: blockedPath, + }, + }, + ); + } + } + + private assertBinaryAllowed( + command: ParsedShellCommand, + toolClearance?: ToolClearancePolicy, + ): void { + const binaryToken = normalizeToken(command.binary); + const binaryName = basename(binaryToken); + + if (!this.allowedBinaries.has(binaryToken) && !this.allowedBinaries.has(binaryName)) { + throw new SecurityViolationError( + `Binary "${command.binary}" is not in the shell allowlist.`, + { + code: "BINARY_NOT_ALLOWED", + details: { + binary: command.binary, + }, + }, + ); + } + + if (!toolClearance) { + return; + } + + this.assertToolInvocationAllowed({ + tool: binaryName, + toolClearance, + }); + } + + private assertAssignmentsAllowed(command: ParsedShellCommand): void { + for (const assignment of command.assignments) { + if (this.blockedEnvAssignments.has(assignment.key)) { + throw new SecurityViolationError( + `Environment assignment "${assignment.key}" is blocked by policy.`, + { + code: "ENV_ASSIGNMENT_BLOCKED", + details: { + key: assignment.key, + }, + }, + ); + } + + if ( + this.allowedEnvAssignments.size > 0 && + !this.allowedEnvAssignments.has(assignment.key) + ) { + throw new SecurityViolationError( + `Environment assignment "${assignment.key}" is not in the assignment allowlist.`, + { + code: "ENV_ASSIGNMENT_NOT_ALLOWED", + details: { + key: assignment.key, + }, + }, + ); + } + } + } + + private assertArgumentPaths(command: ParsedShellCommand, cwd: string): void { + if (!this.policy.enforcePathBoundaryOnArguments) { + return; + } + + for (const token of [...command.args, ...command.redirects]) { + if (!isPathLikeToken(token)) { + continue; + } + + if (this.policy.rejectRelativePathTraversal && hasPathTraversalSegment(token)) { + throw new SecurityViolationError(`Path traversal token "${token}" is blocked.`, { + code: "PATH_TRAVERSAL_BLOCKED", + details: { + token, + binary: command.binary, + }, + }); + } + + if (token.startsWith("~")) { + throw new SecurityViolationError( + `Home-expansion path token "${token}" is blocked.`, + { + code: "HOME_EXPANSION_BLOCKED", + details: { + token, + binary: command.binary, + }, + }, + ); + } + + const resolvedPath = isAbsolute(token) ? resolve(token) : resolve(cwd, token); + if (!isWithinPath(this.worktreeRoot, resolvedPath)) { + throw new SecurityViolationError( + `Path token "${token}" resolves outside worktree root.`, + { + code: "PATH_OUTSIDE_WORKTREE", + details: { + token, + resolvedPath, + worktreeRoot: this.worktreeRoot, + }, + }, + ); + } + + for (const protectedPath of this.protectedPaths) { + if (!isPathBlocked(resolvedPath, protectedPath)) { + continue; + } + + throw new SecurityViolationError( + `Path token "${token}" resolves inside protected path.`, + { + code: "PATH_INSIDE_PROTECTED_PATH", + details: { + token, + resolvedPath, + protectedPath, + }, + }, + ); + } + } + } + + private emit(event: SecurityAuditEvent): void { + this.auditSink?.(event); + } +} diff --git a/src/security/schemas.ts b/src/security/schemas.ts new file mode 100644 index 0000000..6629c43 --- /dev/null +++ b/src/security/schemas.ts @@ -0,0 +1,95 @@ +import { z } from "zod"; + +function dedupe(values: readonly string[]): string[] { + const out: string[] = []; + const seen = new Set(); + + for (const value of values) { + if (seen.has(value)) { + continue; + } + seen.add(value); + out.push(value); + } + + return out; +} + +const stringTokenSchema = z + .string() + .trim() + .min(1) + .refine((value) => !value.includes("\0"), "String token cannot include null bytes."); + +export const toolClearancePolicySchema = z + .object({ + allowlist: z.array(stringTokenSchema).default([]), + banlist: z.array(stringTokenSchema).default([]), + }) + .strict(); + +export type ToolClearancePolicy = z.infer; + +export function parseToolClearancePolicy(input: unknown): ToolClearancePolicy { + const parsed = toolClearancePolicySchema.parse(input); + return { + allowlist: dedupe(parsed.allowlist), + banlist: dedupe(parsed.banlist), + }; +} + +export const shellValidationPolicySchema = z + .object({ + allowedBinaries: z.array(stringTokenSchema).min(1), + worktreeRoot: stringTokenSchema, + protectedPaths: z.array(stringTokenSchema).default([]), + requireCwdWithinWorktree: z.boolean().default(true), + rejectRelativePathTraversal: z.boolean().default(true), + enforcePathBoundaryOnArguments: z.boolean().default(true), + allowedEnvAssignments: z.array(stringTokenSchema).default([]), + blockedEnvAssignments: z.array(stringTokenSchema).default([]), + }) + .strict(); + +export type ShellValidationPolicy = z.infer; + +export function parseShellValidationPolicy(input: unknown): ShellValidationPolicy { + const parsed = shellValidationPolicySchema.parse(input); + return { + ...parsed, + allowedBinaries: dedupe(parsed.allowedBinaries), + protectedPaths: dedupe(parsed.protectedPaths), + allowedEnvAssignments: dedupe(parsed.allowedEnvAssignments), + blockedEnvAssignments: dedupe(parsed.blockedEnvAssignments), + }; +} + +export const executionEnvPolicySchema = z + .object({ + inherit: z.array(stringTokenSchema).default(["PATH", "HOME", "TMPDIR", "TMP", "TEMP", "LANG", "LC_ALL"]), + scrub: z.array(stringTokenSchema).default([]), + inject: z.record(z.string(), z.string()).default({}), + }) + .strict(); + +export type ExecutionEnvPolicy = z.infer; + +export function parseExecutionEnvPolicy(input: unknown): ExecutionEnvPolicy { + const parsed = executionEnvPolicySchema.parse(input); + return { + inherit: dedupe(parsed.inherit), + scrub: dedupe(parsed.scrub), + inject: { ...parsed.inject }, + }; +} + +export type SecurityViolationHandling = "hard_abort" | "validation_fail"; + +export const securityViolationHandlingSchema = z.union([ + z.literal("hard_abort"), + z.literal("validation_fail"), +]); + +export function parseSecurityViolationHandling(input: unknown): SecurityViolationHandling { + return securityViolationHandlingSchema.parse(input); +} diff --git a/src/security/shell-parser.ts b/src/security/shell-parser.ts new file mode 100644 index 0000000..ebd39eb --- /dev/null +++ b/src/security/shell-parser.ts @@ -0,0 +1,180 @@ +import parseBash from "bash-parser"; +import { SecurityViolationError } from "./errors.js"; + +type UnknownRecord = Record; + +export type ParsedShellAssignment = { + raw: string; + key: string; + value: string; +}; + +export type ParsedShellCommand = { + binary: string; + args: string[]; + flags: string[]; + assignments: ParsedShellAssignment[]; + redirects: string[]; + words: string[]; +}; + +export type ParsedShellScript = { + commandCount: number; + commands: ParsedShellCommand[]; +}; + +function isRecord(value: unknown): value is UnknownRecord { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function readString(value: unknown): string | undefined { + return typeof value === "string" ? value : undefined; +} + +function readTextNode(node: unknown): string | undefined { + if (!isRecord(node)) { + return undefined; + } + + const text = readString(node.text); + if (!text) { + return undefined; + } + + return text; +} + +function parseAssignment(raw: string): ParsedShellAssignment | undefined { + const separatorIndex = raw.indexOf("="); + if (separatorIndex <= 0) { + return undefined; + } + + const key = raw.slice(0, separatorIndex); + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(key)) { + return undefined; + } + + return { + raw, + key, + value: raw.slice(separatorIndex + 1), + }; +} + +function toParsedCommand(node: UnknownRecord): ParsedShellCommand | undefined { + if (node.type !== "Command") { + return undefined; + } + + const binary = readTextNode(node.name)?.trim(); + if (!binary) { + return undefined; + } + + const args: string[] = []; + const assignments: ParsedShellAssignment[] = []; + const redirects: string[] = []; + + const prefix = Array.isArray(node.prefix) ? node.prefix : []; + for (const entry of prefix) { + if (!isRecord(entry) || entry.type !== "AssignmentWord") { + continue; + } + + const raw = readString(entry.text); + if (!raw) { + continue; + } + + const assignment = parseAssignment(raw); + if (assignment) { + assignments.push(assignment); + } + } + + const suffix = Array.isArray(node.suffix) ? node.suffix : []; + for (const entry of suffix) { + if (!isRecord(entry)) { + continue; + } + + if (entry.type === "Word") { + const text = readString(entry.text); + if (text) { + args.push(text); + } + continue; + } + + if (entry.type !== "Redirect") { + continue; + } + + const fileWord = readTextNode(entry.file); + if (fileWord) { + redirects.push(fileWord); + } + } + + return { + binary, + args, + flags: args.filter((arg) => arg.startsWith("-")), + assignments, + redirects, + words: [binary, ...args, ...redirects], + }; +} + +export function parseShellScript(script: string): ParsedShellScript { + let ast: unknown; + try { + ast = parseBash(script); + } catch (error) { + throw new SecurityViolationError("Shell command failed AST parsing.", { + code: "SHELL_PARSE_FAILED", + cause: error, + details: { + script, + }, + }); + } + + const commands: ParsedShellCommand[] = []; + const seen = new WeakSet(); + + const visit = (node: unknown): void => { + if (!isRecord(node)) { + return; + } + + if (seen.has(node)) { + return; + } + seen.add(node); + + const parsedCommand = toParsedCommand(node); + if (parsedCommand) { + commands.push(parsedCommand); + } + + for (const value of Object.values(node)) { + if (Array.isArray(value)) { + for (const item of value) { + visit(item); + } + continue; + } + + visit(value); + } + }; + + visit(ast); + + return { + commandCount: commands.length, + commands, + }; +} diff --git a/src/types/bash-parser.d.ts b/src/types/bash-parser.d.ts new file mode 100644 index 0000000..2e82ef7 --- /dev/null +++ b/src/types/bash-parser.d.ts @@ -0,0 +1,3 @@ +declare module "bash-parser" { + export default function parseBash(script: string): unknown; +} diff --git a/tests/config.test.ts b/tests/config.test.ts index bea5122..04dcdd2 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -9,6 +9,8 @@ test("loads defaults and freezes config", () => { assert.equal(config.orchestration.maxDepth, 4); assert.equal(config.provisioning.portRange.basePort, 36000); assert.equal(config.discovery.fileRelativePath, ".agent-context/resources.json"); + assert.equal(config.security.violationHandling, "hard_abort"); + assert.equal(config.security.commandTimeoutMs, 120000); assert.equal(Object.isFrozen(config), true); assert.equal(Object.isFrozen(config.orchestration), true); }); @@ -20,3 +22,9 @@ test("validates boolean env values", () => { ); }); +test("validates security violation mode", () => { + assert.throws( + () => loadConfig({ AGENT_SECURITY_VIOLATION_MODE: "retry_forever" }), + /invalid_union|Invalid input/i, + ); +}); diff --git a/tests/mcp-registry.test.ts b/tests/mcp-registry.test.ts index 83ee9ff..f08f339 100644 --- a/tests/mcp-registry.test.ts +++ b/tests/mcp-registry.test.ts @@ -63,3 +63,34 @@ test("mcp registry rejects unknown explicit handlers", () => { /Unknown MCP handler/, ); }); + +test("mcp registry enforces tool clearance on resolved codex tool lists", () => { + const registry = createDefaultMcpRegistry(); + + const resolved = registry.resolveServerWithHandler({ + serverName: "sandbox-tools", + server: { + type: "stdio", + command: "node", + args: ["server.js"], + enabled_tools: ["read_file", "write_file", "search"], + disabled_tools: ["legacy_tool"], + }, + context: {}, + fullConfig: { + servers: {}, + }, + toolClearance: { + allowlist: ["read_file", "search"], + banlist: ["search", "write_file"], + }, + }); + + assert.ok(resolved.codex); + assert.deepEqual(resolved.codex.enabled_tools, ["read_file"]); + assert.deepEqual(resolved.codex.disabled_tools, [ + "legacy_tool", + "search", + "write_file", + ]); +}); diff --git a/tests/orchestration-engine.test.ts b/tests/orchestration-engine.test.ts index 3d3cf7b..3de3d8c 100644 --- a/tests/orchestration-engine.test.ts +++ b/tests/orchestration-engine.test.ts @@ -5,6 +5,7 @@ import { tmpdir } from "node:os"; import { resolve } from "node:path"; import { SchemaDrivenExecutionEngine } from "../src/agents/orchestration.js"; import type { ActorExecutionResult } from "../src/agents/pipeline.js"; +import { SecurityViolationError } from "../src/security/index.js"; function createManifest(): unknown { return { @@ -191,6 +192,7 @@ test("runs DAG pipeline with state-dependent routing and retry behavior", async coder: async (input): Promise => { assert.match(input.prompt, /AIOPS-123/); assert.deepEqual(input.toolClearance.allowlist, ["read_file", "write_file"]); + assert.ok(input.security); coderAttempts += 1; if (coderAttempts === 1) { return { @@ -759,3 +761,158 @@ test("propagates abort signal into actor execution and stops the run", async () await assert.rejects(() => runPromise, /(AbortError|manual-abort|aborted)/i); assert.equal(observedAbort, true); }); + +test("hard-aborts pipeline on security violations by default", async () => { + const workspaceRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-workspace-")); + const stateRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-session-state-")); + const projectContextPath = resolve(stateRoot, "project-context.json"); + + const manifest = { + schemaVersion: "1", + topologies: ["retry-unrolled", "sequential"], + personas: [ + { + id: "coder", + displayName: "Coder", + systemPromptTemplate: "Coder", + toolClearance: { + allowlist: ["git"], + banlist: [], + }, + }, + ], + relationships: [], + topologyConstraints: { + maxDepth: 3, + maxRetries: 2, + }, + pipeline: { + entryNodeId: "secure-node", + nodes: [ + { + id: "secure-node", + actorId: "secure_actor", + personaId: "coder", + }, + ], + edges: [], + }, + } as const; + + const engine = new SchemaDrivenExecutionEngine({ + manifest, + settings: { + workspaceRoot, + stateRoot, + projectContextPath, + maxDepth: 3, + maxRetries: 2, + maxChildren: 2, + runtimeContext: {}, + }, + actorExecutors: { + secure_actor: async () => { + throw new SecurityViolationError("blocked by policy", { + code: "TOOL_NOT_ALLOWED", + }); + }, + }, + }); + + await assert.rejects( + () => + engine.runSession({ + sessionId: "session-security-hard-abort", + initialPayload: { + task: "Security hard abort", + }, + }), + /blocked by policy/, + ); +}); + +test("can map security violations to validation_fail for retry-unrolled remediation", async () => { + const workspaceRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-workspace-")); + const stateRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-session-state-")); + const projectContextPath = resolve(stateRoot, "project-context.json"); + + const manifest = { + schemaVersion: "1", + topologies: ["retry-unrolled", "sequential"], + personas: [ + { + id: "coder", + displayName: "Coder", + systemPromptTemplate: "Coder", + toolClearance: { + allowlist: ["git"], + banlist: [], + }, + }, + ], + relationships: [], + topologyConstraints: { + maxDepth: 3, + maxRetries: 2, + }, + pipeline: { + entryNodeId: "secure-node", + nodes: [ + { + id: "secure-node", + actorId: "secure_actor", + personaId: "coder", + constraints: { + maxRetries: 1, + }, + }, + ], + edges: [], + }, + } as const; + + let attempts = 0; + const engine = new SchemaDrivenExecutionEngine({ + manifest, + settings: { + workspaceRoot, + stateRoot, + projectContextPath, + maxDepth: 3, + maxRetries: 2, + maxChildren: 2, + securityViolationHandling: "validation_fail", + runtimeContext: {}, + }, + actorExecutors: { + secure_actor: async () => { + attempts += 1; + if (attempts === 1) { + throw new SecurityViolationError("first attempt blocked", { + code: "PATH_TRAVERSAL_BLOCKED", + }); + } + + return { + status: "success", + payload: { + fixed: true, + }, + }; + }, + }, + }); + + const result = await engine.runSession({ + sessionId: "session-security-validation-retry", + initialPayload: { + task: "Security retry path", + }, + }); + + assert.equal(result.status, "success"); + assert.deepEqual( + result.records.map((record) => `${record.nodeId}:${record.status}:${String(record.attempt)}`), + ["secure-node:validation_fail:1", "secure-node:success:2"], + ); +}); diff --git a/tests/security-middleware.test.ts b/tests/security-middleware.test.ts new file mode 100644 index 0000000..5372512 --- /dev/null +++ b/tests/security-middleware.test.ts @@ -0,0 +1,125 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { resolve } from "node:path"; +import { + SecurityRulesEngine, + SecureCommandExecutor, + SecurityViolationError, + parseShellScript, +} from "../src/security/index.js"; + +test("shell parser extracts Command and Word nodes across chained expressions", () => { + const parsed = parseShellScript("FOO=bar git status && npm test | cat > logs/output.txt"); + + assert.equal(parsed.commandCount, 3); + assert.deepEqual( + parsed.commands.map((command) => command.binary), + ["git", "npm", "cat"], + ); + + const gitCommand = parsed.commands[0]; + assert.ok(gitCommand); + assert.equal(gitCommand.assignments[0]?.key, "FOO"); + assert.deepEqual(gitCommand.args, ["status"]); + + const catCommand = parsed.commands[2]; + assert.ok(catCommand); + assert.deepEqual(catCommand.redirects, ["logs/output.txt"]); +}); + +test("rules engine enforces binary allowlist, tool policy, and path boundaries", async () => { + const worktreeRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-security-worktree-")); + const stateRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-security-state-")); + const projectContextPath = resolve(stateRoot, "project-context.json"); + + const rules = new SecurityRulesEngine({ + allowedBinaries: ["git", "npm", "cat"], + worktreeRoot, + protectedPaths: [stateRoot, projectContextPath], + requireCwdWithinWorktree: true, + rejectRelativePathTraversal: true, + enforcePathBoundaryOnArguments: true, + allowedEnvAssignments: [], + blockedEnvAssignments: [], + }); + + const allowed = rules.validateShellCommand({ + command: "git status && npm test | cat > logs/output.txt", + cwd: worktreeRoot, + toolClearance: { + allowlist: ["git", "npm", "cat"], + banlist: [], + }, + }); + + assert.equal(allowed.parsed.commandCount, 3); + + assert.throws( + () => + rules.validateShellCommand({ + command: "cat ../secrets.txt", + cwd: worktreeRoot, + }), + (error) => + error instanceof SecurityViolationError && + error.code === "PATH_TRAVERSAL_BLOCKED", + ); + + assert.throws( + () => + rules.validateShellCommand({ + command: "git status", + cwd: stateRoot, + }), + (error) => + error instanceof SecurityViolationError && + error.code === "CWD_OUTSIDE_WORKTREE", + ); +}); + +test("secure executor runs with explicit env policy", async () => { + const worktreeRoot = await mkdtemp(resolve(tmpdir(), "ai-ops-security-exec-")); + + const rules = new SecurityRulesEngine({ + allowedBinaries: ["echo"], + worktreeRoot, + protectedPaths: [], + requireCwdWithinWorktree: true, + rejectRelativePathTraversal: true, + enforcePathBoundaryOnArguments: true, + allowedEnvAssignments: [], + blockedEnvAssignments: [], + }); + + const executor = new SecureCommandExecutor({ + rulesEngine: rules, + timeoutMs: 2000, + envPolicy: { + inherit: ["PATH", "HOME"], + scrub: ["SECRET_VALUE"], + inject: { + SAFE_VALUE: "ok", + }, + }, + }); + + let streamedStdout = ""; + const result = await executor.execute({ + command: "echo \"$SAFE_VALUE|$SECRET_VALUE\"", + cwd: worktreeRoot, + baseEnv: { + PATH: process.env.PATH, + HOME: process.env.HOME, + SECRET_VALUE: "hidden", + }, + onStdoutChunk: (chunk) => { + streamedStdout += chunk; + }, + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, "ok|\n"); + assert.equal(streamedStdout, result.stdout); +}); diff --git a/tsconfig.json b/tsconfig.json index abce301..59c4be8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -12,5 +12,5 @@ "forceConsistentCasingInFileNames": true, "skipLibCheck": true }, - "include": ["src/**/*.ts"] + "include": ["src/**/*.ts", "src/**/*.d.ts"] }