Repository navigation
v0.6.12: close R6-002 admin-CLI self-approval bypass + honest security model - #5
Merged
Merged
Conversation
Closes the alternate-name self-approval bypass flagged as a ship-blocker
in the v0.6.11 round-6 implementation audit. `isAdminCliInvocation`
relied on `basenameOf(peeled[0]) === "patchwork"`, so an agent could
self-approve through a symlinked alternate name:
ln -s "$(command -v patchwork)" ./pw
script -q -c './pw approve <pending-id>' /dev/null
`isPatchworkExecutable` now runs the basename match first (cheap, and
unchanged behaviour for direct invocations), then falls back to realpath
identity against the running CLI's own canonical path. That catches
`./pw`-style symlinks because realpath canonicalises both sides. The
canonical realpath defaults to `process.argv[1]` and can be overridden
with `PATCHWORK_CANONICAL_REALPATH` for tests and unusual install
layouts.
Honest residual, documented in the code and in the test corpus: literal
binary copies (`cp $(which patchwork) /tmp/pw`) and wrapper scripts are
NOT caught, because their realpath is themselves rather than the
canonical CLI. Those stay residual until a root-owned approval authority
replaces argv-based identity entirely in Protected Mode (v0.7). The
corpus includes one explicit DOCUMENTED-RESIDUAL test asserting current
behaviour, so that future change has a clean inversion point.
Tests added under "F1 / R6-002: alternate-name (symlink) execution" in
packages/agents/tests/claude-code/dangerous-shell-combos.test.ts:
- denies `<symlinkName> approve <id>` via realpath identity
- denies alternate-name + clear-taint under taint
- denies a symlink wrapped in `exec`
- does not match a non-patchwork executable with a similar name
- does not crash on a non-existent path (fail-safe)
- documented residual: literal binary copies still bypass
The new describe block uses top-level ESM imports rather than inline
require("node:*") calls, which is the biome-clean form and the standard
vitest pattern in this file.
Validated 2026-10-06: clean build, full suite 1515 passing,
dangerous-shell-combos.test.ts 58 passing.
Refs: REVIEWS/2026-05-12-gpt55-v0.6.11-impl-audit-round6.json
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
….6.12
Per the release strategy, v0.6.12 ships the R6-002 self-approval fix
together with an honest security-model disclosure, so the fix lands in
the same release as the admission that the prior model is bypassable.
- README: replaces the "Tamper-resistant ... impossible for non-admin
users to remove" overclaim with an accurate "Tamper-evident" line plus
a prominent "Security model -- read this" callout. v0.6.x is Advisory
and tamper-evident; hook enforcement runs at the agent's own
privilege; a same-UID process can route around it; this is not an OS
sandbox; true tamper-resistance is Protected Mode in v0.7. Points at
SECURITY.md for vulnerability reporting, not public issues.
- README roadmap: the "root-owned approval daemon (v0.6.12)" bullet
became "Protected Mode (v0.7)". A daemon fed by a same-privilege hook
is tamper-evident logging, not an enforcement boundary; the boundary
is OS-level containment with the daemon as its control plane. The
v0.6.11 bullet's forward reference and the URL-allowlist bullet were
corrected to match.
- docs/v0.6.11/threat-model.md: dated banner marking the forward-looking
v0.6.12-daemon claims as superseded. Statements about v0.6.11's actual
behaviour and residuals are unchanged.
- docs/v0.6.11/{migration,index}.md: `patchwork init --upgrade` never
existed. The guide told upgraders to run it to install the admin-CLI
deny rule, so anyone who followed it never got the rule and has the
in-CLI TTY check as their only gate, which a PTY wrapper passes. Both
pages now give the manual policy edit and say to verify with
`patchwork policy show`.
- .github/workflows/ci.yml: `pnpm/action-setup@v4.1` is not a real tag,
so every CI run since 2026-05-12 failed at "Set up job". Pinned to
@v4 (which publish.yml already used, hence releases still worked) and
moved `pnpm lint` into its own non-blocking job so the pre-existing
biome backlog stops masking real build and test failures.
- README test counts refreshed to 1515.
- .changeset: patch bump across the fixed group.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The v0.6.12 README callout said Patchwork is Advisory and tamper-evident,
while the documentation site still told visitors the opposite. Shipping
both would have had the release note publicly contradicted by the
project's own homepage.
Corrected, 25 edits across 16 files:
- docs/index.md: the homepage feature card claimed a 5-layer architecture
'makes it impossible for the AI to disable its own monitoring'. It does
not, and that was the single most visible overclaim in the project.
- docs/security/threat-model.md: the AI-agent-tampering section asserted
'the agent cannot access or modify the hook configuration, the audit
log, or the policy files'. The hooks run at the agent's own UID, so it
can modify its own user-level hook config, and it can act entirely
outside the hooks' view via grandchild processes, package lifecycle
scripts or 'python -c'. Rewritten to state what is actually protected
(root-owned system policy, relay copy, chain detection) and what is
not, plus the same Advisory banner the README now carries.
- docs/concepts/tamper-proof-layers.md: retitled 'Tamper-Evident Layers'.
The URL is deliberately unchanged so inbound links keep working, with a
note explaining the rename.
- docs/concepts/compliance.md: the EU AI Act Art. 15(1) row claimed
'tamper-proof logging'. A compliance mapping should be the most
conservative claim in the repo, not the loosest.
- docs/team-mode-architecture.md: 'root-owned and tamper-proof, a
malicious user cannot modify it' now also states the two things it does
not cover: omitted events, and fabricated events submitted over the
socket by a same-UID process.
- Remaining prose, headings, nav label and link text moved from
tamper-proof to tamper-evident across guide, guides/team-mode,
security/architecture, concepts/how-it-works, concepts/seals-and-
witnesses, getting-started/{installation,quickstart,configuration},
.vitepress/config.mts and README.
- packages/cli/src/commands/setup.ts: four user-facing strings, including
the install prompt, described the relay as tamper-proof.
Site builds clean. Full suite still 1515 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ran the repo's own verification gate (`pnpm test:log:write`) over the v0.6.12 branch. Overall PASS, 1404/1404 across 299 suites. Previous entry was 2026-04-28, so the log had not been refreshed across the whole v0.6.11 cycle. Note on the two test counts, since they look contradictory: `test:log` covers core + agents + cli only (1404), while `pnpm test` adds team (99) and web (12) for 1515. The README quotes the all-packages figure, which is consistent with the 1509 it quoted at v0.6.11, plus the six new R6-002 cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ak docs build Two problems found by actually walking the v0.6.11 upgrade path on a real hardened install rather than reading it. 1. The documented remediation was impossible. The system-level install flags /Library/Patchwork/policy.yml immutable (chflags schg on macOS, chattr +i on Linux, attrib +R on Windows), so writing it fails as root with 'Operation not permitted' — not a permissions message, and nothing in the docs mentioned the flag despite the README advertising it as a feature. So between the non-existent 'init --upgrade' flag and this, there was NO working documented way to install the R6 admin-CLI deny rule on a hardened install. The migration guide now gives the three-step sequence, insists the flag is restored afterwards (leaving it cleared silently downgrades the install), and notes that 'policy validate' reports on the file you point it at, so a failed write followed by a successful validate is not evidence the edit landed. An installation.md warning cross-references it. It also records that a policy change needs no relay restart, since the hook loads policy per invocation — worth saying, because an unnecessary restart drops any event in flight. 2. docs/TEST_LOG.md broke the docs build, which I introduced earlier today by regenerating it. VitePress compiles every .md under docs/ through the Vue compiler, and the new R6-002 test names contain angle-bracket placeholders that read as unclosed HTML. The build then fails with 'Element is missing end tag' pointing at an unrelated line 67 characters long, which is why it was not obvious. TEST_LOG is a generated record and is not linked from the nav or sidebar, so it is now srcExcluded. That also stops any future 'test:log:write' from breaking the docs deploy when the corpus gains such a name. This would have failed the Deploy Docs workflow on push to main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ands The migration guide now tells people to install this rule, so it should also tell them its one sharp edge. The rule is a regex over the raw command string and cannot distinguish an invocation from the phrase appearing in text, so it also denies writing documentation about the admin CLI, commit messages describing it, greps searching for it, and test fixtures containing it. Found the hard way: within minutes of installing the rule on a real machine, its first block was a command writing a code comment that merely quoted the verb phrase as a placeholder example. It fails closed, which is the safe direction, and the in-process isAdminCliInvocation check is argv-aware and unaffected. But it is a genuine nuisance, worst for anyone working on Patchwork itself, so it belongs in the guide rather than being discovered. The note gives the workaround and says a proper fix will evaluate policy against parsed argv rather than text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v0.6.12: admin-CLI identity hardening + honest security model
Closes R6-002, the alternate-name self-approval bypass that shipped as a documented accepted residual in v0.6.11.
isAdminCliInvocationrelied on a basename-only check, so an agent could self-approve a denied action through a symlinked alternate name:isPatchworkExecutablenow falls back to realpath identity against the running CLI's canonical path. Literal binary copies and wrapper scripts remain a documented residual, with an explicit test asserting current behaviour so Protected Mode has a clean inversion point.Per the release strategy this ships with the honesty correction rather than after it, so users cannot infer that same-UID enforcement is now handled. It isn't.
Also in this PR
Security-model correction across the README, the docs site and the CLI. The README claimed "tamper-resistant ... impossible for non-admin users to remove", the docs homepage claimed the architecture "makes it impossible for the AI to disable its own monitoring", and the site threat model asserted "the agent cannot access or modify the hook configuration, the audit log, or the policy files". The hooks run at the agent's own UID, so the last one is false. All corrected to tamper-evident, with the Advisory versus Protected distinction stated plainly. The EU AI Act Art. 15(1) compliance row was also softened, since a regulatory mapping should be the most conservative claim in the repo.
concepts/tamper-proof-layers.mdis retitled but its URL is deliberately unchanged so inbound links keep working.Roadmap correction. The README promised a "root-owned approval daemon (v0.6.12)". A daemon fed by a same-privilege hook is tamper-evident logging, not an enforcement boundary, because an agent can route around any same-UID hook via child processes, alternate interpreters or direct sockets. The enforcement boundary is OS-level containment with the daemon as its control plane, and that is Protected Mode (v0.7).
patchwork init --upgradenever existed. The v0.6.11 migration guide told upgraders to run it to install the admin-CLI deny rule. Anyone who followed the guide therefore never got the rule, leaving the in-CLI TTY check as the only self-approval gate, which a PTY wrapper passes. Both pages now give the manual policy edit and say to verify withpatchwork policy show.CI has been failing since 2026-05-12.
ci.ymlpinnedpnpm/action-setup@v4.1, which is not a real tag, so both matrix jobs died at "Set up job" in 6 seconds.publish.ymlused@v4, which is why releases kept working and nobody noticed. Pinned to@v4, andpnpm lintmoved to its own non-blocking job so the pre-existing biome backlog stops masking real build and test failures.Verification
dangerous-shell-combos.test.ts58 passing, including six new R6-002 casesCloses #3 on release, since
pnpm run versionregenerates the CLI version string that has read 0.6.9 since v0.6.10.