Repository navigation
feat(trust): TRUST phase — content-keyed trust store, package identity, pmcp trust verbs (see #230) - #240
Merged
Conversation
ViperJuice
added a commit
that referenced
this pull request
Sep 10, 2026
Six advisories were published against the pinned httpx2/httpcore2 2.9.1 in the last two days, so `main` started failing its own audit gate without anyone changing code — which is exactly what that gate (added in #228) exists to do. Every open PR inherits the red X, including the TRUST phase in #240. httpx2 2.9.1 PYSEC-2026-3845/3846/3847/3848/3849 fixes 2.10.0/2.11.0/2.12.0 httpcore2 2.9.1 PYSEC-2026-3844 fix 2.10.0 Lockfile only. `pyproject.toml:86` already declares `httpx2>=2.5.0,<3.0.0` and 2.12.0 sits inside it, so no constraint moves. Scoped with `--upgrade-package` for each flagged package rather than a bare `uv lock --upgrade`, which over-reached in this repo before and dragged ruff forward, breaking ~40 unrelated files under scripts/probes/. Exactly three lock entries changed: httpcore2 2.9.1 -> 2.12.0 httpx2 2.9.1 -> 2.12.0 httpx2-jsfetch (new) 1.0 The new transitive was checked rather than waved through — a novel package appearing under an HTTP library is the shape this repo's current programme is about. It is declared by httpx2 2.12.0 itself, comes from PyPI with sdist and wheel hashes, and is gated `sys_platform == 'emscripten'`, so it never installs off Pyodide. Verified empirically: not importable after `uv sync --locked`. Verification: - `pip-audit --strict` on the resolved environment: "No known vulnerabilities found" (was six). - Full suite, 3456 collected — identical population to main, no new tests. 3215 passed + 107 failed + 106 errors = 3428, exactly main's passed count. - The failures are the known worktree-location artifact: `node_modules` at the volume root trips npm's local-prefix rule and the resolver refuses by design (#195). Every failure reason traces to that refusal or its downstream UpdateServerOutput cascade, and the output contains zero occurrences of httpx, httpcore, ConnectError, ReadError or TransportError — so the transport bump introduced none of them. A checkout outside /mnt runs clean, as CI does. Claude-Session: https://claude.ai/code/session_018FncHYooSXexECJ2DYcnu6 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…230) Publishes IF-0-TRUST-1, the contract CONSENT and PKGID are planned against. Wires no caller -- that is the point of the freeze phase. `is_approved(path, content)` hashes the bytes the caller is ABOUT TO APPLY and never reads the file, so swapping the file between the check and the use cannot buy an approval. Every I/O, parse, and configuration failure answers False: a caller that read an exception as permission would be granted trust by a corrupt store. The other verbs raise instead, because they are operator-facing and a silent failure there hides a broken store. The store lives beside the credential store at ~/.config/pmcp/trust.json, mode 0600, one record per absolute path (re-approving replaces, it never appends a history), and REFUSES to resolve inside the current checkout -- symlinks resolved first, so a planted ~/.config/pmcp -> ./vendor is caught too. That rule binds where the store FILE lives, never which paths may be recorded; record() takes the path being approved, which is normally inside a checkout. `decision` is the closed vocabulary {approved, denied}: an unknown value is rejected at record() time rather than stored, and a "denied" record is a recorded no, not an absence. Falsification: the module does not exist on main, so a bare run there proves only ImportError. Each of the eight tests was instead falsified against a naive negative-control implementation -- path-keyed with no digest, appending history, fail-open on an unknown path, exceptions propagating from a corrupt store, no residency check -- and the symlink test additionally against a residency check that compares unresolved paths, which it alone catches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HURFR7uz5DzzxoApYm45Ev
…e (see #230) Adds the `pmcp trust` subcommand SL-3 owns: `approve <path>`, `list`, and `revoke <path>` over the SL-1 trust store. The verb spelling is a contract -- CONSENT and PKGID refusal messages print the runnable string `pmcp trust approve <absolute path>`, so a spelling of our own choosing would send operators to a command that does not exist. Decisions worth stating, because the plan left them open: * `scope` is hardcoded "user". The store is user-scoped by construction, so a --scope flag would name a project store that does not exist. * `revoke` of a path with no record exits 1 rather than reporting a success that did nothing (the `git branch -d` reading). * No `deny` verb: the lane names exactly three verbs. A "denied" record is therefore reachable only from a downstream caller, not the CLI. `run_trust` dispatches through a verb->handler map so PKGID can add its sibling `approve-package` / `list-packages` / `revoke-package` beside it. Tests enter through `parse_args` + `async_main`, not by hand-building a Namespace: async_main's fallthrough is "run the gateway server", so a test that skipped the parser would stay green with the dispatch branch missing. They repoint $HOME at tmp_path and assert the redirect took before any body runs -- a test run cannot record an approval in the operator's real store. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HURFR7uz5DzzxoApYm45Ev
…g it (see #230) TRUST lane SL-2, IF-0-TRUST-2. `register_discovered_server` accepts any agent-chosen npm package and `provision` runs it with `npx -y`, while policy checks only the agent-chosen SERVER NAME -- so allowlisting `internal-approved-tool` executes `totally-arbitrary-evil-package` (2026-09-01 review, finding S-01). This adds the identity a later phase gates on; TRUST deliberately wires no caller. `resolve_package_identity(spec)` reads the registry's own metadata document and returns `PackageIdentity(registry, name, resolved_version, integrity)`, or `None`. A resolved version is the point: `npx -y pkg` re-resolves `latest` at every spawn, so approving an unpinned spec approves code that does not exist yet. A range pins nothing and is therefore not an identity. Resolution never spawns a process and never runs the package -- `npx`'s own resolution runs the thing it is resolving, which is too late. The #195 `npm_resolver` is not called: this module consumes its output TYPE (a spec string) and copies its conventions (frozen result, refuse rather than guess, a module-level seam tests patch). Everything fails closed to `None`, never an exception: a caller gating provisioning would otherwise need a `try` around every call, and one bare `except Exception` there reads as consent. Falsified before implementing, behaviourally rather than on ImportError: a `npm view` strawman failed the offline and no-subprocess tests on the blocked spawn, and a seam-having strawman raised `KeyError` on every range and minted an unrequested `1.4.2` for the spec `example-mcp@`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HURFR7uz5DzzxoApYm45Ev
…dments (see #230) SL-docs, the terminal lane of the v13 TRUST phase. Three files, no source. **Catalog (SL-docs.1).** The lane brief expected the rescan helper to be absent; it is not. `_shared/scaffold_docs_catalog.py` was found at `~/.claude/skills/_shared/` and run with `--rescan` from the worktree root, so this is a machine rescan, not a manual audit. It picked up the five files added since 2026-08-24 (the three v13 phase plans, the v13 roadmap, and the 2026-09-01 codebase review) and preserved every existing `touched_by_phases` history. It also **dropped `SPEC_COMPLIANCE.md`**, together with its `["P3B"]` history. The helper's docstring promises history is always preserved, but that only holds for files it re-finds: `SPEC_COMPLIANCE.md` is at the repo root and is not in the helper's `KNOWN_FILES` list, so a rescan silently deletes the entry. It is restored by hand here, in its previous position. Anyone running `--rescan` in this repo must re-check that entry afterwards. `TRUST` was then added to the three files this phase actually changed: `CHANGELOG.md`, `specs/phase-plans-v13.md` and `plans/phase-plan-v13-TRUST.md`. **SECURITY.md is deliberately NOT updated (SL-docs.2).** The roadmap assigns the trust-model write-up to SEAL, once, rather than accepting four partial descriptions of one model from four phases. Its catalog entry is unchanged. `README.md` is also untouched: it gains no `pmcp trust` section here, because PKGID's docs lane owns that file and its own operator surface lands in the same prose. Nothing in README is made *false* by this phase — it documents commands in prose and enumerates none exhaustively — only incomplete. **CHANGELOG.** A new `### Added` block under `[Unreleased]`, placed first to match 2.7.0 and 2.6.0. It describes the three `pmcp trust` verbs, the store at `~/.config/pmcp/trust.json`, the bytes-not-paths predicate, the outside-the-checkout residency rule and the fail-closed read, and states plainly that **nothing consults the store yet** — an operator who reads only that a trust store shipped would otherwise assume approving is now required, or that approving something has any effect today. It does not. **Three post-execution amendments (SL-docs.3).** Both IF-0-TRUST gates shipped with the signatures they declared and no exit criterion was weakened, so nothing below reopens a contract. What each gate left implicit is what costs the phase downstream of it: 1. IF-0-TRUST-1 froze `decision` as `{"approved", "denied"}` with `is_approved` distinguishing a denied record from an absent one — and EC-TRUST-4 froze a CLI of exactly approve/list/revoke, so no shipped command writes `"denied"`. Not a security hole (absence already refuses; the reachable behaviour is a strict subset of the frozen one), but a reserved value with no writer, which a CONSENT or PKGID lane planning an explicit-deny path would find missing. 2. IF-0-TRUST-2 froze a synchronous `resolve_package_identity` over blocking `urllib` I/O with a 10 s timeout, while every sibling registry lookup in `version_checker.py` is async/aiohttp — and PKGID SL-1 resolves at registration, inside `async def register_discovered_server`. Calling it directly there stalls the gateway's event loop. The signature stays frozen; the fix is caller-side and belongs to PKGID. A pointer to the amendment is added to the PKGID phase's Scope notes, because that is the section its executor reads. 3. A plan defect: SL-1.1 says "exactly these names" and lists eight test identifiers, while SL-2.1's identical column carries behaviour descriptions. The asymmetry is defensible but undocumented, and it misled a lane brief. **The roadmap changed, so all three `roadmap_sha256:` pins are now stale.** New digest: 9b77ee0a3e9c1ff6d71f65ef6c36ccdc2714b506b4f62f74b746da5299891aa9. This lane does not own `plans/phase-plan-v13-{TRUST,CONSENT,PKGID}.md` and has deliberately not repinned them; the orchestrator must. Note that `scripts/check_plan_consistency.py` does **not** check the pins — it reports 0 blocking with them stale — so the mismatch is caught only by the explicit digest comparison. Verified: `phase-loop validate-roadmap specs/phase-plans-v13.md` → OK, 5 phases; `check_plan_consistency.py plans/phase-plan-v13-*.md` → 0 blocking, warning set byte-identical to the pre-edit baseline; `uv run ruff format --check src/ tests/ scripts/` → 138 files already formatted; `uv run ruff check` → all checks passed; catalog parses as JSON. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HURFR7uz5DzzxoApYm45Ev
…gate pins in CI script Follow-up to the SL-docs lane, which correctly edited the roadmap and then STOPPED rather than repinning plans it does not own. Three fixes, all of defects in my own earlier work. - Repinned all three v13 plans to 9b77ee0a…, the post-amendment roadmap digest. SL-docs appended TRUST's Post-execution amendments and a pointer into PKGID's Scope notes, which changed the digest and left every pin stale. - scripts/check_plan_consistency.py now verifies roadmap_sha256 against the roadmap each plan names. The script was added two days ago specifically to catch drift between a plan's registries, and it reported "0 blocking" while all three pins were stale — it checked lane tables against acceptance criteria but never the trust contract in the frontmatter. Verified both ways: a deliberately stale pin now exits 1 naming the file and both digests, and the tree is clean again after restore. - Repaired plans/phase-plan-v13-PKGID.md SL-1.1, where an earlier sed of mine left a double comma and orphaned the "because the acceptance criteria address them individually" clause from the names it introduces. Also carried TRUST SL-2's finding into PKGID's SL-1 lane, where its executor will read it: resolve_package_identity is synchronous and does blocking network I/O with a 10 s timeout, and PKGID calls it from inside async def register_discovered_server, so a direct call blocks the event loop for the whole gateway. TRUST's signature is frozen, so the fix is caller-side — anyio.to_thread.run_sync plus a handler-level timeout, since a socket timeout is not a request bound. See #230. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HURFR7uz5DzzxoApYm45Ev
…-write Advisor board on the assembled phase: grok AGREE, gemini AGREE, codex DISAGREE with two blocking defects. Both reproduce; both are fixed here. The two AGREE seats each noted the non-atomic write and classed it non-blocking, so codex was alone in escalating it — and alone in finding the dist-tag hole at all. 1. A dist-tag's target bypassed concrete-version validation (package_identity.py). The user-supplied half of a spec was refused when it named a range; the registry-supplied half was not. Reproduced: a packument whose `dist-tags.latest` is `^1.0.0`, with a matching `versions` entry, returned PackageIdentity(resolved_version='^1.0.0'). A consumer pinning argv to `pkg@^1.0.0` then re-resolves at every spawn — the check-then-use hole this primitive exists to close, reached through registry data rather than through the caller. Every resolution path now ends at a concrete SemVer or returns None. 2. A concurrent approve could silently resurrect a revoked approval (trust_store.py). `record` and `revoke` each read every record, change one and write the set back, with no locking anywhere in the module. Interleaved: record reads a snapshot containing A, revoke removes A, record writes its stale snapshot back, and A is approved again — an operator's withdrawal of trust reversed by an unrelated approve. Both now hold an exclusive advisory lock for the whole read-modify-write (POSIX; degrades to no serialization where fcntl is absent rather than failing the operation). The store is also written atomically now — temp file, fsync, os.replace — so an interrupted write leaves the previous store intact instead of truncating it. Both AGREE seats raised this independently as a future enhancement. Regression tests added for both, as the reviewer asked, and mutation-proven: reverting the dist-tag check fails test_a_dist_tag_pointing_at_a_range_resolves_ to_nothing; removing both lock sites fails test_concurrent_record_and_revoke_do_ not_resurrect_an_approval, which reproduces the race deterministically within 40 iterations. Files restored byte-identical after each mutation. 45 passed. Neither fix changes a frozen interface, so no plan amendment is required and the roadmap pins are untouched. See #230. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HURFR7uz5DzzxoApYm45Ev
The lock added in 624f93e introduced a second `mkdir` for the store directory that runs before `_write_store`'s. `_write_store` decides whether to tighten the directory with `parent_created = not parent.exists()`, so once the lock had already created it that check was False, the `chmod 0o700` was skipped, and a fresh install got the umask default instead. Measured on a clean HOME: the store directory came out 0o775 — group-writable, so another account in the group can rename or replace trust.json wholesale even though the file itself is 0o600 and its contents stay unreadable. Replacing the store is enough to forge an approval, so this is an integrity hole rather than a confidentiality one. The cause was two independent answers to one question. `_store_lock` and `_write_store` each decided for themselves whether PMCP had created the directory, and whichever ran first made the other's answer wrong. Both now call one `_ensure_store_dir` helper that creates and tightens in the same step, and still never tightens a directory the user already had. Removing the lock's mkdir is not an alternative: the lockfile cannot be created in a directory that does not exist, which is why it was there. Verified on a clean HOME: directory 0o700, store 0o600, lockfile 0o600. The regression test is mutation-proven — giving the two functions independent decisions again fails it on the directory mode, and the file is byte-identical after restore. 55 passed. Found by the advisor while the board was reviewing the previous commit; the board's bundle still contains the regression, so its verdict is on the code as of 624f93e. See #230. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VydToCyPeDKACFYyzaUdiT
The adversarial seat raised this as a non-blocking leftover. It is not a nit for this component: `os.replace` is atomic against readers, but the rename is only durable once the directory entry is synced. A crash immediately after a revoke can therefore leave the pre-revoke store on disk and resurrect the approval the operator just withdrew — the same invariant the lock protects against a race, reached through power loss instead. Leaving a known revoke-can-resurrect path in a trust store because two seats called it minor is the wrong call. `_write_store` now fsyncs the directory after the replace, alongside the existing fsync of the file contents. Not covered by a unit test: reproducing it requires an untimely power loss, and a test that mocked `os.fsync` would assert the implementation rather than the behaviour. Stated plainly rather than papered over with a test that proves nothing. 55 passed. Post-board change: the board reviewed 624f93e and has not seen this or the directory-permissions fix in 78b3bc3. See #230. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VydToCyPeDKACFYyzaUdiT
ViperJuice
force-pushed
the
phase/v13-trust
branch
from
September 10, 2026 20:16
4770984 to
e4c8578
Compare
…mpleted write Board on the two previously unreviewed commits: grok AGREE, codex DISAGREE, gemini DISAGREE. All three findings reproduced before being acted on; two are fixed here and one is deliberately not. 1. A durability optimisation was turning successful writes into errors (gemini). The directory fsync runs AFTER os.replace, so when fsync fails the write has already landed -- but the exception propagated. Measured with a directory fsync that raises EINVAL, the shape macOS returns and NFS/overlay mounts can: revoke() raised OSError while is_approved() already read False. The operator is told their withdrawal of trust did not take, when it did. For this tool that is the worst direction to be wrong in, and it would have fired on every record and revoke on macOS, on Windows (os.open of a directory fails), and on affected network filesystems. Both the open and the fsync are now best effort; durability is lost only where the platform cannot provide it. 2. No directory is created group- or world-accessible, even transiently (codex). Two measured facts: `mkdir(parents=True, mode=0o700)` ignores `mode` for the intermediates, so a freshly created ~/.config lands 0o775 under umask 002 while only the leaf gets tightened; and creating loosely then tightening leaves a window in which another account in the user's group can insert a forged trust.json that a later record() reads and carries forward. The store file being 0o600 does not help -- the attack replaces the file rather than reading it. Every missing component is now created with mkdir(mode=0o700), which the kernel applies at creation; umask can only remove bits, never add them. NOT done, and recorded rather than silently skipped: codex also asked that the store refuse paths whose writable ancestors permit substitution. That is a policy change, not a bug fix -- it would refuse to operate on a machine whose $HOME is group-writable -- and it belongs to SEAL with its own decision. The same applies to repairing a directory the user already had; grok noted the existing "never tighten what the user created" rule is a deliberate choice. Both fixes are mutation-proven: restoring the loose mkdir fails the new intermediate-permissions assertion, and dropping the except OSError fails the new fsync test with the same OSError(22) measured above. trust_store.py is byte-identical after each mutation. 12 passed. See #230. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018FncHYooSXexECJ2DYcnu6
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
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.
Executes Phase 1 (TRUST) of
specs/phase-plans-v13.md— the trust-boundary programme opened by the 2026-09-01 codebase review (S-01, S-03, S-11, S-04). Four lanes, each executed in an isolated worktree by a separate agent, merged in dependency order.Nothing consults the store yet and no existing behaviour changes. This ships the primitives that CONSENT, PKGID, EGRESS and SEAL gate on. S-01 stays open until PKGID — that is this phase's stated non-goal, not an oversight.
What ships
src/pmcp/trust_store.py— user-scoped store at~/.config/pmcp/trust.json, keyed on content, not pathssrc/pmcp/manifest/package_identity.py— resolves an npm spec to a pinned identity without running itpmcp trust approve <path>/list/revoke <path>Two design points worth reading:
is_approved(path, content: bytes)hashes the bytes the caller is about to apply and never re-reads the file, closing the check-then-use window. Edit one byte and the approval is gone.False, never a raise a caller might read as permission. Operator verbs, by contrast, fail loudly with a non-zero exit.Review history
Three advisor-board rounds on the plans, then one on the implementation, then one on the fixes. The implementation board returned two blocking defects, both from the red-team seat while the other two returned AGREE — the same split seen throughout this programme.
Both reproduced before being fixed:
dist-tags.latest = "^1.0.0"yieldedPackageIdentity(resolved_version='^1.0.0'). PKGID would then pin argv tonpx -y pkg@^1.0.0, which re-resolves at every spawn — the exact check-then-use hole this programme exists to close, reached through registry data rather than through the caller.recordandrevokeeach read all records, change one and write the set back, with no locking anywhere in the module.The re-board of those fixes then found a regression the fix itself introduced: the new lock's
mkdirran before_write_store'sparent_createdcheck, so thechmod 0o700never fired and a fresh install got the umask default — measured at 0o775. The file stays0o600, but a group-writable directory lets another account replacetrust.jsonwholesale, and replacing the store is enough to forge an approval. Fixed by giving both call sites one_ensure_store_dirdecision instead of two independent ones.Also fixed, raised as a nit but not treated as one:
_write_storenow fsyncs the directory afteros.replace, so a crash right after a revoke cannot leave the pre-revoke store on disk and resurrect the withdrawn approval.Verification
Every security property is mutation-proven — a test that passes against a broken implementation is worthless:
is_approvedignores the content hashTrue(fail open)For SL-2, "fails on unchanged
main" can only beImportErrorfor a new module, so falsification used two strawmen: annpm viewsubprocess version failed all 35 tests, while a seam-having naive version passed the two spawn tests and failed 15/15 on versions — which is what proves the first set of failures was behavioural rather than structural.No regression:
maincollects 3456 items, this branch 3508 — a delta of exactly 52, the new tests (8 + 9 + 35). The 107 failures seen in a/mnt/workspaceworktree are a location artifact:node_modulesat the volume root trips npm's local-prefix rule and the resolver refuses by design (#195). A checkout outside/mntruns clean, and CI uses one.Known follow-ups, recorded not dropped
denyverb. The vocabulary is{"approved","denied"}andis_approveddistinguishes denied from absent, but no CLI verb writes"denied". Absence already denies, so this is a reserved value with no writer, not a hole. CONSENT/PKGID are told to treat it as reserved.resolve_package_identityis synchronous and does blocking network I/O (10 s), while every existing registry lookup inversion_checker.pyis async. PKGID calls it from insideasync def register_discovered_server, so it needsanyio.to_thread.run_syncplus a handler-level timeout — a socket timeout is not a request bound. Carried into PKGID's SL-1 lane, not just the roadmap.78b3bc3,4770984) are post-board and no seat has reviewed them.See #230.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VydToCyPeDKACFYyzaUdiT
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.