Uh oh!
There was an error while loading. Please reload this page.
test(security): probe the trash/restore door for package-declared permission sets — pin the unreachability, measure the residual - #12021
Merged
os-sam merged 2 commits intoAug 25, 2026
Conversation
…he packaged-set lock does not cover Probe deliverable, not a fix. The 2026-08-24 "lock the base, clone to customize" ruling is enforced on the write-through's insert and update legs; its author named the restore leg as the one write point it does not guard, and that leg re-authors a restored record's body into metadata with no provenance check and catches rather than throws. Four cases, in the order they answer the question: a CONTROL that identifies the gate answering an update (the lock, by its own message — ADR-0005's tier gate answers the same code for the same row), the MEASUREMENT of the restore leg on an identical fixture, the FENCE explaining why it is not a live defect (the delete leg resets an artifact-backed definition and never reaches the driver, so no restorable row exists), and the BLIND SPOT in the outer two-doors gate that would otherwise pre-empt it (it keys on `managed_by`, which the lock module records as measurably the wrong fact). The measurement case pins the RESIDUAL, deliberately: extending the lock to this leg must invert it in the same PR, which is what makes the extension visible instead of silent. Not re-pinned here (one spelling): that `restore` is not an engine middleware dispatch verb (objectql's #7809 vocabulary weld) and that it never derives as an API operation (spec's api-derivation suite). Those two are what make the leg unreachable today. Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ate dispatch The double's `update()` now opens with `assertEngineUpdateDispatch(data, options)` and its `find()` REFUSES the operator shapes it does not implement, so a fixture that drifts to a call shape `ObjectQL` would reject fails loudly instead of quietly matching nothing. `tryUpdate` in the subject swallows a throw, so a double looser than the engine would have been invisible twice over. `scripts/engine-double-contract.pinned.json` learns the new pinned coverage (additive; the shrink-only baseline is untouched). Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-sam
marked this pull request as ready for review
August 25, 2026 05:23
Uh oh!
There was an error while loading. Please reload this page.
os-sam
deleted the
claude/issue-11725-packaged-permission-set-trash-probe
branch
August 25, 2026 05:42
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 freeto 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.
Fixes#11725
A probe, delivered as a pin. No behaviour changes — one new test file and the additive pinned-ledger row it earns.
The two questions, and what was measured
Leg 1 — can a package-declared
sys_permission_setenter trash at all? No, and for a wider reason than the card assumed. The card expected the answer to rest on the delete path "resetting instead of trashing" for packaged sets. That is true and is pinned here, but it is the second reason. The first is that nothing enters trash, on any object:restoreis not a member of the engine's middleware dispatch vocabulary (OperationContext['operation']is a 7-member union, welded to source and driven through a real engine by objectql's #7809 suite), and it never derives as an API operation —API_METHOD_DERIVATION.restore.flaghas been permanentlyfalsesinceenable.trashwas retired in #2377 / ADR-0049, with a real recycle bin parked at #3146.Leg 2 — what does
restorethen do? It re-authors, with no lock consulted. This is the half the card called the one that matters, and it is measured rather than read: on an identical fixture — same engine double, same artifact registry, same protocol posture, same row — theupdateleg refuses and therestoreleg re-authors the package-declared body into the environment overlay store without raising anything. The leg catches rather than throws by design (it runs after the engine has already un-trashed the row), which is exactly why a refusal added there is a design question and not a mechanical edit.Which gate answered, and how I know. The card's cautionary case (
docs/qa/platform-checklist/areas/access-security.json:1245, where an earlier gate answered and a probe would have scored a false pass) is guarded against explicitly.NOT_OVERRIDABLE/403 alone does not identify a gate here — ADR-0005's tier gate insidesaveMetaItemanswers with the same envelope for the same row. The CONTROL case therefore asserts the envelope and the lock's own message (it names the clone path; the tier gate's message instead says the type has not opted into per-org overlay writes), and that nothing reached the metadata write. Ablation B confirms the control is falsifiable: removing the lock from the update leg turns that case, and only that case, red.What lands
packages/plugins/plugin-security/src/packaged-permission-set-restore-leg.test.ts— four cases, in the order they answer the question:updaterestoreis re-authored, unrefuseddeleteon a packaged set resets and never reaches the driver, so no restorable row existsmanaged_by, which the lock module's own header records as the wrong fact⛔ Deliberately not re-pinned here (one spelling, Prime Directive #8): the engine dispatch vocabulary (objectql #7809) and the API-operation derivation (spec's
api-derivationsuite). A copy in this package would be a claim about this file rather than about the producer. Those two are cited in the header as what makes the leg unreachable today.Sibling card #11843 asks the metadata-door direction and is untouched here.
Verification
Gate union re-run after the final commit, at
f127c6bc6e— all green:@objectstack/plugin-security— 81 test files, 1527 tests passed;typecheckclean.check:engine-double-contract,check:where-matcher,check:cross-package-test-inputs(both thelint.ymlandci.ymlinvocations),check:query-options-erasure,check:type-check-coverage,check:test-source-alias,check:type-source-resolution,check:slot-lookup,check:published-files,check:agent-test-spelling,check:entry-guard,check:parse-guard,check:pnpm-filter-targets,check:i18n,check:nul-bytes,check-ci-filter-parity,check-plugin-teardown-shape.scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackafter the ledger row was added — thescripts/**path pulled in four families the first derivation did not name, and all four were then run.Reverse verification — two ablations, each in the predicted direction, each restored against
HEAD(never the index) under atrap … EXIT INT TERM, with the mutation confirmed on disk by anchored greps before the run:1 failed | 3 passed, the failure being MEASURED RESIDUAL. The pin is a real tripwire.1 failed | 3 passed, the failure being CONTROL. The control is not vacuous.Both restores verified disk == index == HEAD at blob
3bd4fb2628cbd4d2b1c293c4df1f290e3b200092with an emptygit status --porcelain, per the hazard recorded on #11725 itself. No rebuild was needed for either leg: the subject is reached through a relative source import, not through a packageexportsedge, andplugin-security's vitest config carries no alias that would redirect it.pnpm lintwas narrowed to the changed paths rather than run repo-wide; the narrowing is declared with its three readings in the probe report. ESLint reported on 2 files, 0 errors, with the ledger JSON outside its configured population — and this repo's singleeslint.config.mjsenables type-aware linting for no file (recorded with a positive control ateslint.config.mjs:327), so this diff cannot move the verdict on any file it does not touch.check:type-check-debt --re-measureneeds the whole workspace closure built and is left to CI.Changeset
None — tests plus a CI ledger row publish nothing. Labelled
skip-changeset.Generated by Claude Code