v0.6.11: taint-aware safety layer (R1-R6 audit closed)

README.md

@@ -569,9 +569,11 @@ Four packages in a TypeScript monorepo:
 - [x] **Installer dedup + loopback risk fix** -- multiple Patchwork hook entries now collapse cleanly on re-install (no more duplicate attestations); curl/wget targeting localhost is medium-risk instead of critical so the dashboard's own self-introspection loop doesn't trip the policy (v0.6.8)
 - [x] **In-toto / DSSE attestations (opt-in)** -- emit each commit attestation as a [DSSE-wrapped in-toto Statement v1](docs/reference/intoto.md) alongside the bespoke format, with a stable predicate type at `https://patchwork-audit.dev/ai-agent-session/v1`. Lets Patchwork attestations slot into the SLSA / Sigstore / supply-chain world without consumers needing to know about Patchwork's own schema. Set `PATCHWORK_INTOTO=1` on your PostToolUse hook to enable; default off. Also fixes the long-standing `extractCommitInfo` regex which silently skipped attestation on root commits and detached-HEAD output (v0.6.9)
 - [x] **GPT-5.5 cross-vendor security audit** -- 28 findings on v0.6.9 reviewed across multiple rounds, 22 fixed in v0.6.10 including the in-toto signing-oracle V11→V13 closure (chain integrity, command-injection vectors, policy bypass paths, signing-oracle hardening, fail-closed audit-log API, action enum lockdown). Architectural items deferred to v0.6.11+. (v0.6.10)
-- [x] **Taint-aware policy enforcement** -- Patchwork is now a *safety layer*, not just an audit trail. Multi-kind taint engine (`prompt`/`secret`/`network_content`/`mcp`/`generated_file`) tracks untrusted content in the session; conservative shell recognizer parses Bash commands; sink classifier + dangerous-shell-combos refuse exfil and execution paths under taint. New CLIs: `patchwork approve`, `patchwork clear-taint`, `patchwork trust-repo-config`. End-to-end release-gate tests for the [canonical attack scenarios](docs/v0.6.11/threat-model.md) (A1-A8). Two GPT-5.5 audit gates (R1 + R2) before tag. See [migration guide](docs/v0.6.11/migration.md). (v0.6.11)
+- [x] **Taint-aware policy enforcement** -- Patchwork is now a *safety layer*, not just an audit trail. Multi-kind taint engine (`prompt`/`secret`/`network_content`/`mcp`/`generated_file`) tracks untrusted content in the session; conservative shell recognizer parses Bash commands; sink classifier + dangerous-shell-combos refuse exfil and execution paths under taint. New CLIs: `patchwork approve`, `patchwork clear-taint`, `patchwork trust-repo-config`. End-to-end release-gate tests for the [canonical attack scenarios](docs/v0.6.11/threat-model.md) (A1-A8). **Six GPT-5.5 adversarial audit rounds** (R1–R6) hardened the approval boundary against agent self-approval: env-override removal, system-policy regex deny, broadened regex for quoted PTY wrappers, semantic argv-based detection, `command`/`exec` modifier peeling, /proc/<X>/environ env-source coverage. Remaining same-UID residuals (alternate-name exec, variable-named exec) documented as accepted v0.6.11 limits — structurally fixed by the v0.6.12 root-owned approval daemon. 1509 tests. See [migration guide](docs/v0.6.11/migration.md) and [threat model](docs/v0.6.11/threat-model.md). (v0.6.11)
 
 **Planned:**
+- [ ] **Root-owned approval daemon (v0.6.12)** -- moves the approval boundary out of the agent's UID entirely. No more `patchwork approve` CLI for the agent to call; approval flows through a root-held channel (desktop prompt / Touch ID / phone push / etc.) the agent cannot impersonate. Structurally closes the same-UID class of bypass that audit rounds R2–R6 spent iterating on. Also signs per-session taint snapshots with a root-held HMAC key, closing the R1-001 / R1-008 snapshot-authenticity residual. See [threat model § Why v0.6.12](docs/v0.6.11/threat-model.md#why-v0612-introduces-a-root-owned-approval-daemon).
+- [ ] **URL allowlist + body-shape detection (v0.6.12)** -- first-class network policy with strict URL canonicalization; closes allowed-domain exfiltration (gist to user's own GitHub, DNS encoding, etc.).
 - [ ] **Witness endpoints** -- configure external anchoring for off-machine seal verification
 - [ ] **Diff-aware risk scoring** -- parse actual code changes, not just file paths
 - [ ] **Team mode** -- local-first with aggregated sealed bundles pushed to a team server

REVIEWS/2026-05-12-gpt55-v0.6.11-impl-audit-round5.json

@@ -0,0 +1 @@
+{"provider":"openai","model":"gpt-5.5","text":"{\n  \"overall_verdict\": \"NEEDS_REWORK\",\n  \"headline\": \"The /proc environ fix is acceptable, but the admin-CLI deny regex still misses simple shell-quoted or shell-escaped `patchwork` command words that execute normally under PTY wrappers.\",\n  \"r4_fix_assessment\": {\n    \"r4_001_fix_correct\": false,\n    \"r4_002_fix_correct\": true,\n    \"concerns\": [\n      \"R4-001 fixed the specific left-boundary miss for quotes, subshells, command substitution, pipes, semicolons, and paths, but it still assumes the executable token appears as the literal contiguous string `patchwork` immediately followed by space or tab. POSIX shell quoting/escaping can preserve the executed command name while breaking that raw-string shape.\",\n      \"Examples that likely still evade the regex but execute `patchwork approve`: `script -q -c \\\"'patchwork' approve abc\\\" /dev/null`, `script -q -c '\\\"patchwork\\\" approve abc' /dev/null`, `script -q -c \\\"p'atch'work approve abc\\\" /dev/null`, and `script -q -c 'p\\\\atchwork approve abc' /dev/null`.\",\n      \"R4-002's `^/proc/[^/]+/environ$` is a good broadening for the reported aliases and has negligible meaningful false-positive cost. A possible non-blocking follow-up is also recognizing `/proc/<pid>/task/<tid>/environ`, but the direct `/proc/<pid>/environ` and `/proc/self/environ` forms are now covered.\"\n    ]\n  },\n  \"new_findings\": [\n    {\n      \"id\": \"R5-001\",\n      \"severity\": \"CRITICAL\",\n      \"title\": \"Admin CLI deny regex still misses shell-quoted or escaped patchwork executable tokens\",\n      \"category\": \"missed_bypass\",\n      \"where\": \"docs/default-policy.yml admin-CLI deny rule\",\n      \"issue\": \"The broadened left boundary is fixed, but the rule still only matches a raw contiguous `patchwork` token followed by `[ \\\\t]+`. Shell syntax allows quotes or backslashes inside or around a command word without changing the executed command name, so the raw command string can avoid the regex while still invoking `patchwork approve`, `patchwork clear-taint`, or `patchwork trust-repo-config`.\",\n      \"exploit\": \"Concrete: `script -q -c \\\"'patchwork' approve <id>\\\" /dev/null` or `script -q -c \\\"p'atch'work approve <id>\\\" /dev/null`. The shell removes the quotes before exec, `script` provides a PTY, and the system-policy regex does not match the raw text.\",\n      \"fix\": \"Before tag, either apply the admin deny rule to a shell-normalized command representation or add a conservative pre-normalization step for this rule that removes syntactic shell quotes/backslash escapes from command words before matching. At minimum, add regression tests for quoted full command tokens, quote-split command tokens, escaped letters, and path variants such as `/usr/local/bin/'patchwork' approve <id>` and `./p'atch'work approve <id>`. Prefer semantic command-word detection over expanding a raw regex indefinitely.\",\n      \"ship_blocker\": true\n    },\n    {\n      \"id\": \"R5-002\",\n      \"severity\": \"LOW\",\n      \"title\": \"Optional proc environ recognizer follow-up for task subpaths\",\n      \"category\": \"documented_gap\",\n      \"where\": \"dangerous shell combo env-source recognizer / PROC_ENVIRON_RE\",\n      \"issue\": \"`^/proc/[^/]+/environ$` covers the aliases raised in R4 and is acceptable for v0.6.11, but Linux also exposes task-scoped proc entries such as `/proc/<pid>/task/<tid>/environ` on many systems.\",\n      \"exploit\": \"Theoretical: a command could read `/proc/self/task/<tid>/environ` and pipe it to a network sink, avoiding the single-component proc-environ regex.\",\n      \"fix\": \"Non-blocking: broaden to something like `^/proc/(?:[^/]+/environ|[^/]+/task/[^/]+/environ)$` and add one regression test if you want to close this now.\",\n      \"ship_blocker\": false\n    }\n  ],\n  \"ship_recommendation\": {\n    \"verdict\": \"NEEDS_REWORK\",\n    \"must_fix_before_tag\": [\n      \"Fix R5-001 so admin CLI denial is applied to shell-normalized command words or otherwise catches shell-quoted and shell-escaped `patchwork` executable tokens.\",\n      \"Add regression tests for `'patchwork' approve`, `\\\"patchwork\\\" approve`, `p'atch'work approve`, `p\\\\atchwork approve`, and quoted path-segment forms under `script -c`/`bash -c` wrappers.\"\n    ],\n    \"may_ship_as_v0.6.11.1_hotfix\": [\n      \"Optional broadening for `/proc/<pid>/task/<tid>/environ` if not included before tag.\"\n    ],\n    \"v0.6.12_scope\": [\n      \"Semantic command policy instead of raw deny regexes.\",\n      \"Language-level env exfil detection.\",\n      \"Out-of-band approval daemon.\",\n      \"Snapshot authenticity.\",\n      \"Formal generated_file_execute sink.\",\n      \"Broad Bash source taint.\"\n    ]\n  },\n  \"migration_step_assessment\": {\n    \"patchwork_init_upgrade_is_sufficient\": true,\n    \"missing_steps\": [\n      \"After R5-001 is fixed, ensure `patchwork init --upgrade` is idempotent and replaces all known older admin-CLI deny regex variants, not only appends a duplicate rule.\",\n      \"Document that users or fleet admins must run the upgrade with sufficient privileges on every machine where `/Library/Patchwork/policy.yml` is already installed.\",\n      \"Have the upgrade report or verify the active regex/rule version so users can confirm the system-layer policy actually changed.\",\n      \"Update the packaged default policy for fresh installs as well as the migration path for existing installs.\"\n    ]\n  },\n  \"headline_quote_for_release_notes\": \"v0.6.11 strengthens proc-environ exfiltration detection, but the admin-CLI deny rule must also handle shell-normalized command words before release.\"\n}","cost_usd":0.22207,"tokens_in":2271,"tokens_out":4984,"duration_ms":106766,"stop_reason":"end_turn"}

REVIEWS/2026-05-12-gpt55-v0.6.11-impl-audit-round5.prompt.txt

@@ -0,0 +1,205 @@
+You're being consulted by Claude (Anthropic) on the R5 VERIFICATION
+audit of Patchwork v0.6.11. R4 (your previous round) returned
+NEEDS_REWORK with 2 ship-blockers (R4-001 CRITICAL, R4-002 HIGH).
+This round verifies the fixes for those two. SHIP /
+SHIP_WITH_FOLLOWUP / NEEDS_REWORK as before.
+
+Spend so far: R1 $0.36 + R2 $0.31 + R3 $0.24 + R4 $0.27 = $1.18
+of ~$5 budget. R5 should be smaller (only re-reviewing 2 specific
+fixes) — budget ~$0.20.
+
+------------------------------------------------------------------
+R4 findings and how they were resolved
+------------------------------------------------------------------
+
+R4-001 CRITICAL — "Admin CLI deny regex missed quoted PTY wrappers"
+  Your R4 finding: the R3 regex
+    (^|[ ;&|]|/)patchwork[ \t]+(approve|clear-taint|trust-repo-config)\b
+  did not match `script -q -c 'patchwork approve <id>'` because the
+  preceding character is `'`, not in the left-boundary class. Same
+  miss for `(...)`, `$(...)`, backticks, tab, newline. You
+  recommended expanding the left boundary to all shell token
+  delimiters and adding regression tests for those forms.
+
+  Fix taken: broadened the left boundary to ANY non-identifier
+  character. The final regex shipped in docs/default-policy.yml:
+
+    (^|[^A-Za-z0-9_-])patchwork[ \t]+(approve|clear-taint|trust-repo-config)\b
+
+  Reasoning for the exclusion class:
+    - We need to allow `/usr/local/bin/patchwork approve` (absolute
+      path) to match, so `/` must NOT be excluded.
+    - We need `.` to NOT be excluded so e.g. `./patchwork approve`
+      and `~/.local/bin/patchwork approve` both match.
+    - Only word-token characters (A-Za-z0-9_) and `-` are excluded,
+      because those would mean `patchwork` is part of a longer
+      identifier (`patchwork-foo`, `xpatchwork`, etc.). The
+      trailing `[ \t]+` and `\b` already require a real word
+      boundary on the right side.
+
+  Tests: new packages/core/tests/policy/admin-cli-deny.test.ts
+  with 20 unit tests that exercise evaluatePolicy() against the
+  exact regex. They cover:
+
+    MUST DENY (15 cases):
+      - bare patchwork approve / clear-taint / trust-repo-config
+      - env-wrapped: `env X=1 patchwork approve <id>`
+      - absolute path: `/usr/local/bin/patchwork approve <id>`
+      - script single-quoted: `script -q -c 'patchwork approve <id>' /dev/null`
+      - script double-quoted: `script -q -c "patchwork approve <id>" /dev/null`
+      - subshell: `(patchwork approve <id>)`
+      - command sub: `echo $(patchwork approve <id>)`
+      - backticks: `` echo `patchwork approve <id>` ``
+      - chained: `ls; patchwork approve <id>`
+      - piped: `echo x | patchwork approve <id>`
+      - tab-separated: `exec\tpatchwork approve <id>`
+      - bash -c quoted: `bash -c 'patchwork clear-taint'`
+      - sh -c quoted: `sh -c "patchwork trust-repo-config /tmp/x"`
+
+    MUST NOT FALSE-POSITIVE (5 cases):
+      - patchwork status (allowed CLI subcommand)
+      - patchwork-foo approve (different binary)
+      - patchworkapprove (no space)
+      - bare `approve abc123` (the verb alone)
+      - `npm install @patchwork/cli` (the package name)
+
+  All 20 pass under evaluatePolicy().
+
+  Residual we acknowledge:
+    - Shell metaprogramming (`p=patchwork; $p approve <id>`,
+      `a=approve; patchwork $a <id>`, aliases, function indirection)
+      is beyond a regex's reach. Mitigation: any such command would
+      typically have non-`high` parse confidence and a tainted
+      session, so the keystone rule + sink classifier still fire.
+      A semantic command policy is v0.6.12 scope.
+    - The system-policy file MUST be redeployed on user machines
+      via `patchwork init --upgrade` before this regex takes
+      effect at the system layer. Until then, only the in-CLI TTY
+      check is active (which `script -c` defeats). Deployment
+      gating tracked separately.
+
+R4-002 HIGH — "Proc-environ detector missed obvious self-environ aliases"
+  Your R4 finding: PROC_ENVIRON_RE missed /proc/thread-self,
+  /proc/$BASHPID, /proc/${BASHPID}, /proc/$PPID. You recommended
+  broadening before tag.
+
+  Fix taken: PROC_ENVIRON_RE is now
+    /^\/proc\/[^/]+\/environ$/
+
+  Any single non-empty path component between `/proc/` and
+  `/environ` matches. This covers all of your specified aliases
+  AND any future shell-variable form. The false-positive cost is
+  essentially nil because /proc/<X>/environ is only a real file
+  for processes and threads on Linux.
+
+  3 new R4-002 tests in dangerous-shell-combos.test.ts:
+    - `cat /proc/thread-self/environ | curl ...` → DENY
+    - `cat /proc/$BASHPID/environ | curl ...` → DENY  (literal,
+      no shell expansion since the parser keeps argv as-given)
+    - `cat /proc/$PPID/environ | curl ...` → DENY
+
+  Combined with R3-003, the env-source recognizer now covers:
+    - env, printenv (bare)
+    - bare set (R3-002 narrowed)
+    - export -p
+    - declare -p / -x / -px / -xp
+    - typeset -p / -x / -px (ksh/zsh)
+    - readonly -p
+    - compgen -e
+    - any argv element OR stdin/herestring redirect to a
+      /proc/<X>/environ path
+
+  Language-level env exfil (`python -c 'import os; print(os.environ)'`,
+  Node, Ruby) remains deferred to v0.6.12 per your prior
+  guidance.
+
+------------------------------------------------------------------
+What's unchanged since R4
+------------------------------------------------------------------
+
+  - All R1, R2, R3 fixes still in place.
+  - The 5 deferred-to-v0.6.12 residuals from prior rounds (snapshot
+    authenticity, formal generated_file_execute sink, broad Bash
+    source taint, language-level env exfil, out-of-band approval
+    daemon).
+  - Commit 10 docs and commit 11 integration tests.
+  - Test count: 1460 → 1483 (+23 from R4 fixes: 20 admin-cli-deny
+    regex tests + 3 /proc alias tests).
+
+------------------------------------------------------------------
+What we want from R5
+------------------------------------------------------------------
+
+  A. Are the R4 fixes correct? Specifically:
+
+     R4-001:
+       - Is the broadened regex
+           (^|[^A-Za-z0-9_-])patchwork[ \t]+(approve|clear-taint|trust-repo-config)\b
+         tight enough now, or does it still miss obvious wrappers?
+         The 15 deny tests above are the explicit shapes we
+         considered. Anything else worth landing before tag?
+       - The 5 false-positive tests pin against accidental matches
+         on `patchwork-foo approve`, `patchworkapprove`, etc.
+         Anything that *should* be allowed that we might
+         accidentally deny?
+
+     R4-002:
+       - Is `^/proc/[^/]+/environ$` the right shape, or does it
+         over-broaden in any meaningful way?
+       - Any remaining obvious shell-level env source we should
+         land before tag?
+
+  B. New bugs introduced by these fixes?
+
+  C. Any other ship-blockers we'd regret if shipped?
+
+  D. If R5 returns SHIP or SHIP_WITH_FOLLOWUP, we plan to:
+     1. Add a `patchwork init --upgrade` migration that rewrites
+        /Library/Patchwork/policy.yml with the new admin-CLI
+        deny regex (or appends if missing). Document in
+        docs/v0.6.11/migration.md.
+     2. Cut the version bump (0.6.10 → 0.6.11).
+     3. Run a clean build, tag, npm publish, gh release.
+
+     Is that the right minimum migration step, or are we missing
+     something the user needs to know on upgrade?
+
+------------------------------------------------------------------
+OUTPUT
+------------------------------------------------------------------
+
+JSON, same shape as R4:
+
+{
+  "overall_verdict": "SHIP" | "SHIP_WITH_FOLLOWUP" | "NEEDS_REWORK",
+  "headline": "<one sentence>",
+  "r4_fix_assessment": {
+    "r4_001_fix_correct": <bool>,
+    "r4_002_fix_correct": <bool>,
+    "concerns": ["<any remaining concerns about these fixes>"]
+  },
+  "new_findings": [
+    {
+      "id": "R5-001",
+      "severity": "CRITICAL"|"HIGH"|"MEDIUM"|"LOW"|"INFO",
+      "title": "<short>",
+      "category": "regression"|"missed_bypass"|"new_bug"|"documented_gap"|"other",
+      "where": "<file or function>",
+      "issue": "<what's wrong>",
+      "exploit": "<concrete or 'theoretical'>",
+      "fix": "<recommended change>",
+      "ship_blocker": <bool>
+    }
+  ],
+  "ship_recommendation": {
+    "verdict": "<copy>",
+    "must_fix_before_tag": [],
+    "may_ship_as_v0.6.11.1_hotfix": [],
+    "v0.6.12_scope": []
+  },
+  "migration_step_assessment": {
+    "patchwork_init_upgrade_is_sufficient": <bool>,
+    "missing_steps": ["<anything else users need to do/know>"]
+  },
+  "headline_quote_for_release_notes": "<sentence>"
+}