diff --git a/CHANGELOG.md b/CHANGELOG.md index 69bd925..73d9ad6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 0.3.2 (2026-04-21) + +- fix: `verifyApprovalSignature` now performs a tamper check against the stored `signature.signed_payload`. Previously the canonical payload was rebuilt from the current grant but then discarded; post-sign edits to `approver`, `reason`, `expires_at`, or `task_hash` fields in `approvals.ndjson` would not be detected (the ssh provider only checks signature-against-signed_payload). The rebuilt payload is now compared to `signature.signed_payload` and divergence returns `verified: false` with reason "grant fields do not match signed payload (possible tampering)" +- tests: new tamper-detection test (ssh-signed grant, then mutate `approver`/`reason`/`expires_at` in the stored record and assert each edit fails verification) and new multi-workflow disambiguation tests (grantApproval throws without `--workflow` on multi-workflow manifests, throws on unknown workflow id, and correctly scopes grants per workflow with distinct task hashes) +- docs: clarify that `contract.max_cost_usd` is enforced by runtimes that track cost-attributed operations; declarative-only for shell-target `agentcli exec` (no cost signal to enforce against) +- docs: add recommendation that production-grade isolation on Linux / Windows should run `agentcli exec` inside a container until native OS sandbox adapters ship; manifest declaration remains valid metadata + ## 0.3.1 (2026-04-21) - fix: concurrent `agentcli exec` calls on the same gated task no longer double-consume a single approval. A new `claimApproval` primitive performs find-then-consume atomically under an fs-lockfile at `~/.agentcli/state/approvals.ndjson.lock` (openSync `wx` + `Atomics.wait` backoff); stale locks (older than 30s) are broken; `approval_lock_timeout` is raised after 5s of contention diff --git a/docs/field-reference.md b/docs/field-reference.md index 075d33f..b5deff8 100644 --- a/docs/field-reference.md +++ b/docs/field-reference.md @@ -560,7 +560,7 @@ Workflow-level `contract` acts as a default for tasks. Task-level overrides key | `sandbox` | string | No | `none`, `permissive`, `strict` | Sandbox enforcement level. | | `allowed_paths` | array of strings | No | -- | Filesystem paths the execution may access. Each element must be a non-empty string. | | `network` | string | No | `unrestricted`, `restricted`, `none` | Network access level. | -| `max_cost_usd` | number | No | >= 0 | Maximum cost in USD. | +| `max_cost_usd` | number | No | >= 0 | Maximum cost in USD. Enforced by runtimes that track cost-attributed operations (e.g. `openclaw-scheduler` for LLM tasks that report token usage). Declarative-only for shell-target tasks executed via `agentcli exec`, which has no cost signal to enforce against. | | `audit` | string | No | `none`, `on-failure`, `always` | Audit trail strategy. Default: `always` for exec. | | `required_trust_level` | string | No | `untrusted`, `restricted`, `supervised`, `autonomous` | Minimum trust level for execution (v0.2). Must not exceed the resolved identity's `trust.constraints.max_autonomy`. | | `trust_enforcement` | string | No | `none`, `advisory`, `strict` | How trust level mismatches are handled (v0.2). Default: `none`. | diff --git a/docs/guide-identity.md b/docs/guide-identity.md index 49c4dd4..8f32bff 100644 --- a/docs/guide-identity.md +++ b/docs/guide-identity.md @@ -1191,6 +1191,8 @@ Use this rule of thumb: - other OSes: keep the contract declaration, but rely on a backend or environment that can enforce it until an OS-specific adapter is available - if you want no warning during local runs on an unsupported machine, use `sandbox: "none"` and `network: "unrestricted"` +For production-grade isolation on Linux or Windows, the recommended path today is **run `agentcli exec` inside a container** and let the container's namespace / AppContainer boundary provide the sandbox. The manifest declaration remains valid metadata; the OS-level boundary comes from the execution environment rather than an in-process sandbox adapter. Native Linux (bubblewrap / seccomp-bpf) and Windows (AppContainer / Job Objects) adapters are on the roadmap but not in the current release. + ### Example: require supervised trust for production tasks ```json diff --git a/package.json b/package.json index 9ea170b..1d67386 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@amittell/agentcli", - "version": "0.3.1", + "version": "0.3.2", "description": "Agent-native workflow manifest CLI, schema, and protocol surface for standalone planning and backend compilation.", "type": "module", "main": "./src/index.js", diff --git a/src/approvals.js b/src/approvals.js index f5cb268..17f1c5f 100644 --- a/src/approvals.js +++ b/src/approvals.js @@ -278,6 +278,21 @@ export function verifyApprovalSignature(grant, { env = process.env } = {}) { if (!grant.signature) return { verified: null, reason: 'unsigned' }; const provider = getProvider(grant.signature.method?.replace(/-signature$/, '') || 'ssh'); if (!provider) return { verified: false, reason: `unknown signer "${grant.signature.method}"` }; + + // Tamper check: rebuild the canonical payload from the current grant fields + // and compare against the payload that was signed at grant time. An attacker + // who edits approver/reason/expires_at/task_hash in the ndjson after signing + // would leave signature.signed_payload untouched; the divergence catches it. + // The ssh provider only re-verifies that signature matches signed_payload, + // so without this check, post-sign field edits would go undetected. + const expectedPayload = buildApprovalSignaturePayload(grant); + if (grant.signature.signed_payload !== expectedPayload) { + return { + verified: false, + reason: 'grant fields do not match signed payload (possible tampering)', + }; + } + const paths = getAgentcliPaths({ env }); let allowedSigners = resolveAllowedSigners({ env, statePath: paths.allowed_signers }); if (!allowedSigners && grant.signature.method === 'ssh-signature') { @@ -294,9 +309,7 @@ export function verifyApprovalSignature(grant, { env = process.env } = {}) { }; } } - const payload = buildApprovalSignaturePayload(grant); - const attestation = { ...grant.signature, payload }; - return provider.verify(attestation, { + return provider.verify(grant.signature, { allowedSignersPath: allowedSigners, principal: grant.approver, }); diff --git a/test/approvals.test.js b/test/approvals.test.js index 12d6726..25011d1 100644 --- a/test/approvals.test.js +++ b/test/approvals.test.js @@ -659,6 +659,139 @@ test('concurrency: stale lock is broken and claim proceeds', () => { } }); +test('tamper: edit to approver/reason/expires_at in ndjson fails verification', async (t) => { + const { existsSync: fsExists } = await import('node:fs'); + const { homedir } = await import('node:os'); + const sshCandidates = ['id_ed25519', 'id_ecdsa', 'id_rsa'] + .map(k => join(homedir(), '.ssh', k)) + .filter(p => fsExists(p) && fsExists(`${p}.pub`)); + if (sshCandidates.length === 0) { + t.skip('no local SSH key pair found; skipping signed tamper test'); + return; + } + + const home = mkdtempSync(join(tmpdir(), 'agentcli-approval-tamper-')); + const env = { ...process.env, AGENTCLI_HOME: home }; + delete env.AGENTCLI_SIGNER; + try { + const m = makeManifest({ approval: { policy: 'manual', risk_level: 'high' } }); + const rec = grantApproval({ + manifest: m, + taskId: 'echo-task', + approver: 'alice', + reason: 'original reason', + signer: 'ssh', + env, + }); + assert.ok(rec.signature, 'grant should be signed'); + + const paths = getAgentcliPaths({ env }); + const raw = readFileSync(paths.approvals, 'utf8').trim().split('\n'); + const grantEvent = JSON.parse(raw[0]); + + // 1. Unmodified grant: verifies cleanly (also exercises bootstrap) + const clean = verifyApprovalSignature(grantEvent, { env }); + assert.equal(clean.verified, true, `clean grant should verify, got: ${clean.reason}`); + + // 2. Tamper with approver field: should fail with tamper reason + const tampered = { ...grantEvent, approver: 'mallory' }; + const bad = verifyApprovalSignature(tampered, { env }); + assert.equal(bad.verified, false); + assert.match(bad.reason || '', /tamper|signed payload/i); + + // 3. Tamper with reason field: should also fail + const tamperedReason = { ...grantEvent, reason: 'escalated privileges' }; + const bad2 = verifyApprovalSignature(tamperedReason, { env }); + assert.equal(bad2.verified, false); + + // 4. Tamper with expires_at: should fail (could be used to extend a grant) + const tamperedExpiry = { + ...grantEvent, + expires_at: new Date(Date.now() + 10 * 365 * 24 * 3600 * 1000).toISOString(), + }; + const bad3 = verifyApprovalSignature(tamperedExpiry, { env }); + assert.equal(bad3.verified, false); + } finally { + rmSync(home, { recursive: true, force: true }); + } +}); + +test('multi-workflow manifest: grantApproval requires --workflow disambiguation', () => { + const { env, cleanup } = isolatedEnv(); + try { + const multi = { + version: '0.2', + workflows: [ + { + id: 'wf-a', + name: 'Workflow A', + contract: { sandbox: 'permissive', network: 'unrestricted', audit: 'always' }, + tasks: [ + { + id: 'task-x', + name: 'Task X', + shell: { program: 'printf', args: ['a'] }, + target: { session_target: 'shell' }, + output: { format: 'text' }, + schedule: { cron: '0 * * * *' }, + approval: { policy: 'manual', risk_level: 'medium' }, + }, + ], + }, + { + id: 'wf-b', + name: 'Workflow B', + contract: { sandbox: 'permissive', network: 'unrestricted', audit: 'always' }, + tasks: [ + { + id: 'task-x', + name: 'Task X in B', + shell: { program: 'printf', args: ['b'] }, + target: { session_target: 'shell' }, + output: { format: 'text' }, + schedule: { cron: '0 * * * *' }, + approval: { policy: 'manual', risk_level: 'medium' }, + }, + ], + }, + ], + }; + + // No --workflow with multiple workflows → throws + assert.throws( + () => grantApproval({ manifest: multi, taskId: 'task-x', approver: 'alice', env }), + /multiple workflows|--workflow/ + ); + + // --workflow=nonexistent → throws + assert.throws( + () => grantApproval({ manifest: multi, workflowId: 'wf-ghost', taskId: 'task-x', approver: 'alice', env }), + /not found/ + ); + + // --workflow=wf-a → works, grant scoped to wf-a + const recA = grantApproval({ manifest: multi, workflowId: 'wf-a', taskId: 'task-x', approver: 'alice', env }); + assert.equal(recA.workflow_id, 'wf-a'); + + // --workflow=wf-b → works, distinct grant for same task-id in different workflow + const recB = grantApproval({ manifest: multi, workflowId: 'wf-b', taskId: 'task-x', approver: 'alice', env }); + assert.equal(recB.workflow_id, 'wf-b'); + assert.notEqual(recA.approval_id, recB.approval_id); + assert.notEqual(recA.task_hash, recB.task_hash, 'different workflows → different task hashes'); + + // Grant for wf-a does not satisfy wf-b: findValidApproval on wf-b sees only recB + const foundB = findValidApproval({ + workflowId: 'wf-b', + taskId: 'task-x', + taskHash: recB.task_hash, + env, + }); + assert.equal(foundB.approval_id, recB.approval_id); + } finally { + cleanup(); + } +}); + test('concurrency: lock held past timeout throws approval_lock_timeout', () => { const { env, cleanup } = isolatedEnv(); try {