fix(approvals): tamper detection + multi-workflow tests + doc fixes (v0.3.2)

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

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

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

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",

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,
   });

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 {