fix(vault): write/admin scope split — schema mutations, pack-apply, and MCP vault-info require vault:admin (#134 A.1–A.2) - #235
Merged
unforced merged 3 commits intoAug 11, 2026
Conversation
…fo description require vault:admin (cloud#134 A.1–A.2) Cloud's REST dispatch gated every mutating method on the coarse AuthResult.permission summary, which collapses vault:write and vault:admin into "full" — so a limited write token could rename/merge/delete/update tag schemas over REST, the exact gap vault #580 closed on the self-hosted door. - auth.ts: isTagSchemaMutation — the EXPLICIT admin-only REST enumeration (PUT/DELETE /tags/:name, POST /tags/merge, POST /tags/:name/rename), ported verbatim from parachute-vault routing.ts; permission field doc now warns it must never authorize admin. - vault-do.ts: the REST scope gate now computes requiredVerb (admin for the enumerated ops, else verbForMethod) and checks hasScopeForVault against the real scope list — write-tier behavior byte-identical (permission "full" ⟺ hasScopeForVault(write)), admin ops 403 insufficient_scope with required_scope vault:admin (bun error-shape parity). - mcp.ts: overrideVaultInfo's description-write bumped write → admin (bun mcp-tools.ts parity, message text identical); stale write-scope comments fixed. MCP tag tools were already admin via core's manifest + the visibleTools scope filter. - tests: each of the four REST admin ops pinned refused-for-write (and not persisted) + allowed-for-admin; write tier pinned unbroken (notes CRUD, tag reads, conformance preview); MCP tools/list + tools/call write/admin split pinned; vault-info description refusal pinned for both read AND write tokens. Out of scope (rest of ParachuteComputer#134): doctor route, aggregate, vault-info/vault map, link-resolution parity, conformance-POST read carve-out.
… vault-info drift (cloud#134 review) Adversarial review of 389381a (the A.1-A.2 write/admin scope split) confirmed the chokepoint design but found two defects. Defect 1 (fixed): POST /api/packs/:name dispatched at the generic write tier, but handleApplyPack reaches core's applySeedPack -> upsertTagRecord for every tag a pack declares - the exact mutation PUT /api/tags/:name requires vault:admin for (isTagSchemaMutation). A write-tier token could POST /api/packs/starter-ontology and silently overwrite an owner's curated tag schema (upsertTagRecord replaces `fields` wholesale, no merge). Adds isPackApply() (auth.ts) alongside isTagSchemaMutation() in the vault-do.ts REST gate so pack-apply now requires vault:admin too. The console's own "Add the Surface Starter guide" button mints its POST /api/packs/:name call through workers/identity/src/console.ts's postVaultApi, which minted a plain vault:<name>:write token - closing the gate without touching this seam would have 403'd the console's only legitimate caller. Re-minted at vault:admin instead; updated the pinned mint-shape assertion in console.test.ts. New regression coverage in conformance.test.ts: a write token is refused (403 insufficient_scope / required_scope vault:admin) on POST /api/packs/starter-ontology, with an operator read-back proving a pre-existing curated tag schema (fields + description) survives untouched; an admin token still succeeds. packs.test.ts's write-token case is split into an admin-succeeds case and a write-refused case. Defect 2 (annotated, not fixed): PATCH /api/vault still writes `description` (and audio_retention/auto_transcribe) at the generic write tier via REST (rest/vault.ts), while the MCP vault-info door was already tightened to vault:admin for the same mutation (mcp.ts). The upstream bun vault has the identical write-tier REST gap, so re-tiering cloud's REST door alone would fork the wire contract. Added KNOWN GAP comments at both sites: the MCP admin gate is advisory until both doors move together. Refs cloud#134 (A.1/A.2) and its adversarial review.
…eview hardening)
The pack button's 60s admin token was stamped FIRST_PARTY_CLIENT_ID, which
is one half of internalForbidden's platform gate — admin verb made it both
halves, a latently platform-tier credential. New PACK_APPLY_CLIENT_ID
("parachute-packs") satisfies the packs scope gate (client_id never enters
that check) while internalForbidden refuses it on /api/internal/*, pinned
in workers/vault/test/internal-config.test.ts. postVaultApi renamed
postVaultPackApply: it hardcodes admin+this id for its one caller, and the
generic name was a trap for the next console write call-site. Stale
write-verb module doc corrected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
unforcedagi
force-pushed
the
fix/write-admin-scope-split
branch
from
August 11, 2026 19:30
7c365be to
e395373
Compare
unforcedagi
added a commit
to unforcedagi/parachute-cloud
that referenced
this pull request
Aug 11, 2026
…loud#186) A failed plan-entitlement push (vault-call.ts mint seam) at create-time or plan-change previously had no automatic recovery — a paying customer could silently miss caps/voice forever, short of an operator running scripts/backfill-plans.ts. Closes both fix directions from the issue: - pushVaultCap now retries up to CAP_PUSH_MAX_ATTEMPTS (3) with a short fixed backoff before giving up, absorbing the common transient window cheaply (event=plan_cap_push_retry, then plan_cap_push_failed only after all attempts are exhausted). - The existing daily USAGE_CRON rollup (usage.ts) now also reconciles: each vault's internal-config GET already carries the DO's resolved caps/frozen/ transcription entitlement (vault-do.ts `shape()` already returned these fields), so the rollup compares that against the owner's CURRENT plan (mirroring applyPlanToVaults's resolution, not a separate source of truth) and re-pushes on any mismatch, logging event=entitlement_reconciled. This is bounded by construction: it only ever pushes exactly what entitlementPlanFor(plan, pending_plan) resolves to, so it can shrink an over-entitled DO (e.g. a stale push surviving a downgrade) as readily as it repairs an under-entitled one — never a grant beyond the current paid plan. Tests (workers/identity/test/usage.test.ts, plans.test.ts): - RETRY: a transient cap-push failure heals inside one pushVaultCap call. - reconcile after a failed push restores the entitlement, then a second tick is a clean no-op (idempotent, single reconciled=1 then reconciled=0). - reconcile is bounded by the current plan: a DO holding a richer stale entitlement (power) than the owner's actual plan (entry) is pulled DOWN to exactly entry's caps/voice, never left over-provisioned. Overlap note for the eventual rebase onto main: PR ParachuteComputer#235 (fix/write-admin- scope-split) also touched workers/identity/src/vault-call.ts, inserting PACK_APPLY_CLIENT_ID near FIRST_PARTY_CLIENT_ID and a doc comment on the `verb` field. This branch's edits are further down (CAP_PUSH_* constants after INTERNAL_MINT_TTL_SECONDS, readVaultUsage, CapPushResult, pushVaultCap) and touch none of the same lines/symbols — expected to merge cleanly.
unforced
pushed a commit
that referenced
this pull request
Aug 11, 2026
…loud#186) (#239) * fix(identity): entitlement push retry + daily self-heal reconciler (cloud#186) A failed plan-entitlement push (vault-call.ts mint seam) at create-time or plan-change previously had no automatic recovery — a paying customer could silently miss caps/voice forever, short of an operator running scripts/backfill-plans.ts. Closes both fix directions from the issue: - pushVaultCap now retries up to CAP_PUSH_MAX_ATTEMPTS (3) with a short fixed backoff before giving up, absorbing the common transient window cheaply (event=plan_cap_push_retry, then plan_cap_push_failed only after all attempts are exhausted). - The existing daily USAGE_CRON rollup (usage.ts) now also reconciles: each vault's internal-config GET already carries the DO's resolved caps/frozen/ transcription entitlement (vault-do.ts `shape()` already returned these fields), so the rollup compares that against the owner's CURRENT plan (mirroring applyPlanToVaults's resolution, not a separate source of truth) and re-pushes on any mismatch, logging event=entitlement_reconciled. This is bounded by construction: it only ever pushes exactly what entitlementPlanFor(plan, pending_plan) resolves to, so it can shrink an over-entitled DO (e.g. a stale push surviving a downgrade) as readily as it repairs an under-entitled one — never a grant beyond the current paid plan. Tests (workers/identity/test/usage.test.ts, plans.test.ts): - RETRY: a transient cap-push failure heals inside one pushVaultCap call. - reconcile after a failed push restores the entitlement, then a second tick is a clean no-op (idempotent, single reconciled=1 then reconciled=0). - reconcile is bounded by the current plan: a DO holding a richer stale entitlement (power) than the owner's actual plan (entry) is pulled DOWN to exactly entry's caps/voice, never left over-provisioned. Overlap note for the eventual rebase onto main: PR #235 (fix/write-admin- scope-split) also touched workers/identity/src/vault-call.ts, inserting PACK_APPLY_CLIENT_ID near FIRST_PARTY_CLIENT_ID and a doc comment on the `verb` field. This branch's edits are further down (CAP_PUSH_* constants after INTERNAL_MINT_TTL_SECONDS, readVaultUsage, CapPushResult, pushVaultCap) and touch none of the same lines/symbols — expected to merge cleanly. * fix(identity): harden the entitlement reconciler against review findings D1-D4 (cloud#186) Follow-up to 9efdb47, closing four correctness gaps found in review of the retry + daily-reconciler work. Each is a case where the first pass could have made a billing state WORSE rather than repairing it. D1 — snapshot/plan race. `runUsageRollup`'s batch enumeration snapshots u.plan/u.pending_plan once for the whole run, so a vault reached late in a long run can be judged against a stale plan. A checkout completing mid-run would then be clobbered straight back down — precisely the under-entitlement this issue exists to prevent, only now on an automatic daily timer. Reconciliation now pays for one fresh `getUserById` ONLY on a detected mismatch (rare at steady state, so the "no extra D1 round-trip per vault" property still holds for the common case) and pushes the FRESH entitlement, never the snapshot. A DO that already reflects newer truth logs `entitlement_reconcile_skipped_race` and is left alone. D2 — an unrecognized plan must not actuate. `coercePlanId` degrades an unknown value to the 'expired' floor; combined with an actuating push that meant a hand-edited row, a raw restore, or migration 0018's documented DEFAULT 'free' would silently PUSH frozen:true onto a live, possibly-paying vault with no human in the loop. The reconcile path now guards with `isPlanId` BEFORE computing any entitlement and does nothing at all, logging `entitlement_reconcile_skipped_unknown_plan`. An actuating path fails safe by acting, never by freezing. D3 — a vault-worker rollback must not zero out usage recording. Requiring the resolved-entitlement fields inside `readVaultUsage` meant a rollback to a pre-entitlement vault build would throw for EVERY vault, turning a reconciliation-only outage into a fleet-wide loss of the usage rollup itself. The entitlement is now an optional `VaultUsageReading.entitlement` (null = fields absent entirely, distinct from the legitimate unpushed `caps: null`); usage still records, and only that vault's reconciliation is skipped. D4 — retry only what can succeed. The loop retried every non-2xx, so a deterministic 400/403 burned the full budget plus backoff for a guaranteed-repeat failure, delaying the operator-visible failure event. `isRetryableStatus` limits retries to 5xx/429; transport errors (no status at all) still always retry. Tests: D1 race (fresh re-read, zero pushes, DO left on the newer plan), D2 unknown plan (zero pushes — the old code would have pushed a freeze), D3 rollback (usage still recorded, reconcile skipped, distinctly logged), D4 fail-fast 400 (1 attempt, never retried) and 5xx exhaustion (3 attempts, clean result). The pre-existing push-failure test now `.persist()`s its interceptor: single-shot, attempts 2-3 hit "mock dispatch not matched" — a TRANSPORT error, not a second 500 — so it passed without ever exercising a real 5xx retry-then-exhaust. Known-deferred (D5, TODO in vault-call.ts): no per-attempt AbortSignal timeout is threaded, so a HUNG (not fast-erroring) vault can burn the retry budget at the platform's timeout rather than this module's. * test(identity): end-to-end regression for the cloud#186 scenario The two halves of the fix were tested separately — push-time retry (plans.test) and the daily reconciler (usage.test, via a hand-placed mismatch). Neither proved they COMPOSE, which is the actual claim the issue makes. This drives the issue's headline narrative through the real seams: a paying customer upgrades, `applyPlanToVaults` exhausts its full retry budget against a 503ing vault, and the assertion is made that the DO is genuinely left on the trial caps WITHOUT the voice entitlement the customer is now billed for. The window then closes, the daily rollup wakes, and the vault is repaired to exactly standard's entitlement with no operator and no backfill-plans.ts run — then the next tick pushes nothing at all. Pins that the state a totally-failed push leaves behind is exactly the state the reconciler detects, so a future change to either half can't silently decouple them. * chore: bump rc.131 → rc.132 (cloud#186 entitlement reconciler) * fix(identity): pin the D1 fix, split reconcile error attribution, pin the cross-worker contract (cloud#186) Four merge-gate items from fresh-eyes review. 1. TEST PINNING THE D1 CORE FIX. usage.ts pushes `freshExpected`, but NO test could tell: in every reconcile test the snapshot and the fresh re-read resolve to the same plan, and the D1 race test returns at the already-converged check before reaching the push. Flipping the push to `snapshotExpected` passed the entire suite. Added the case that distinguishes them — snapshot says standard, the DO holds trial (so a real repair is due), and the owner flips to a THIRD value (power) mid-run — and verified by mutation that it is the ONLY test that fails when the push carries the snapshot. 2. ERROR ATTRIBUTION. The reconcile call sat inside the try that owns the usage-recording accounting, but it makes calls the usage read never made (a D1 getUserById, a vault PUT). A throw there counted the vault as BOTH recorded and failed — breaking the recorded + failed <= vaults invariant the summary is read with — and logged event=usage_fetch_failed for something that never touched the usage fetch. Reconciliation now has its own try/catch logging event=entitlement_reconcile_failed; the usage row stands and reconciliation self-heals next tick. Pinned with a test that breaks only getUserById's query. 3. CROSS-WORKER PIN. The reconciler detects staleness by reading four field NAMES off the vault worker's GET /api/internal/config, and that check is deliberately non-fatal — a rename on the vault side would make identity read every vault as "pre-entitlement build" and SILENTLY skip reconciliation fleet-wide, at console.log level, with both suites green. Nothing on either side caught that. workers/vault/test/internal-config.test.ts now asserts all four fields in both the unpushed (caps:null, fields present) and post-push round-trip shapes; verified by mutation that renaming transcription_enabled fails it. 4. TODO CORRECTION. The D5 TODO proposed an AbortSignal as the fix for subrequest amplification, but a timeout caps wall-clock, not COUNT: change a PLAN_SPECS byte and every vault mismatches, so USAGE_CRON issues up to 2 x USAGE_RUN_CAP = 1000 subrequests from fast, healthy responses with nothing hanging. Rewritten to state that correctly and to reference the filed follow-up cloud#238 (AbortSignal for hangs, RECONCILE_RUN_CAP for count, TOCTOU post-push re-read, parseResolvedCaps degrade). --------- Co-authored-by: unforcedagi <unforcedagi@users.noreply.github.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
2 participants
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.
Closes the first slice (A.1–A.2) of #134: on hosted, a limited
vault:writetoken could perform admin-only operations.What changes
vault:admin(PUT/DELETE/rename/merge/prune on/api/tags/*): the REST gate now checks granted scopes viahasScopeForVault, not the collapsed permission, at the single pre-dispatch chokepoint every/api/*route funnels through.POST /api/packs/:namerequiresvault:admin— found during review, not in the original fix: pack-apply reaches the sameupsertTagRecordmutation the tags door now gates, so a write token could overwrite an owner's curated tag schemas wholesale. The gate predicate is character-identical to the dispatch predicate, so no routing shape reaches the handler ungated (11 adversarial shapes probed, all refused).write. It now mints a 60s admin token stamped with a new non-platformclient_id(parachute-packs) — admin verb + the first-party id would have satisfied both halves of the vault's platform gate (internalForbidden), making the token latently platform-tier. The new id passes the packs scope gate but is refused on/api/internal/*, pinned by test.PATCH /api/vaultdoor intentionally keeps description at write tier for wire-contract parity with the upstream vault (same asymmetry exists there) — documented as KNOWN GAP comments at both sites. So the honest claim is: MCP door tightened; REST parity gap tracked, to be closed in lockstep with upstream.Review trail
Independent adversarial review demonstrated both the pack-apply escalation and the REST parity gap empirically before the fixes; a second delta review verified the fixes (verdict: ship) and requested the client-id hardening, applied here.
Tests
insufficient_scope+required_scope: vault:admin, then byte-for-byte proof the owner's curated schema survived (conformance.test.ts, packs.test.ts).🤖 Generated with Claude Code