Repository navigation
Enforce single-occupancy slot pinning with human-approved moves - #16
josephschorr wants to merge 9 commits into
Conversation
AuthzSlot gains occupancy (single|multi, default single) and rebind (approval|never, default approval). The installer completes both before SSA apply, the same way membership is completed, so a byte-identical re-install stays a no-op. Class admission rejects a single-occupancy slot declaring more than one default. AgentSession status gains a display-only slotPins mirror type; SpiceDB remains the only enforcement source of truth.
The schema composer adds a non-expiring slot_pin relation to every slot resource definition. GrantSlots — the one funnel every slot-grant write passes through — gates single-occupancy bindings: the first bind writes the pin under a MUST_NOT_MATCH precondition, grant writes carry a MUST_MATCH guard so a bind can never race an approved move, a different instance is refused with a message routed by the slot's rebind policy, and a single call naming two instances of one single-occupancy type binds nothing for that type while other types still land. Grants keep their mandatory expiry; the pin does not expire. Tuple validation refuses tool-authored writes to slot_pin and slot_grant relations, and a guard test over pkg/, internal/ and cmd/ asserts no production write bypasses the gate. Pinned grant writes and moves advance the read-your-writes freshness floor.
…d amendment Every producer that reaches GrantSlots — defaults, extracted and observed promotion, thread seeding, trigger binding, tool approval and the plan gate — copies the slot's occupancy and rebind onto its bindings. A plan amendment that names a different instance for a filled single-occupancy slot records the displaced instance on the approval card at build time; the approved move executes atomically (the pin repoints under a MUST_MATCH guard and the displaced instance's actual grants are revoked in the same request), and the card shows the move as a move: current instance, proposed instance, and the revocation. A move whose recorded starting point no longer matches the live pin fails the approval loudly rather than moving something the approver never saw.
The runner mirrors pins onto AgentSession.status.slotPins — derived from SpiceDB reads after a successful bind, never from candidate lists, so the mirror cannot claim a pin that was never written — and oap session show renders them. Pin refusals from the promotion paths are recorded per resource type (newest wins, cleared when that type later binds or moves) and annotate the next denied tool call for that type, so the model is told the pinned instance and the route out rather than reading a bare denial as a system fault. A pin failure after a plan-gate approval tells the approver whether the slot had drifted or was already committed elsewhere, with accurate claims about what was granted.
channelsd pre-stamps the session finalizer at its create site and sweeps stale slot tuples itself before trigger and thread-seed binding, so the operator's admission sweep can never race the mint and silently delete legitimate authority; a sweep failure fails the session start rather than proceeding past a possible leak. Mint-time pin refusals record a SlotBindRefused session notice with the refusal text, with the suggested next step routed by the slot's rebind policy. A tool approval that lands on a pinned slot in a class with no plan gate is rejected naming the pinned instance and the new-session route.
Fork and restart copy grant and pin tuples verbatim — a lifecycle clone is not a new arrival and never re-enters the gate — while session teardown deletes pins alongside grants (pins never expire, so GC is the only thing that removes them). A new session's first reconcile sweeps stale slot tuples left under its name by a dead predecessor, except for fork children, whose tuples are the parent's legitimate copy and whose generated names make name reuse moot. The status webhook waives slotPins for the runner: it is a display-only mirror no authorization reads.
A scenario suite drives the full arc on the in-process harness: the first bind pins, a second instance is refused and never written, an approved amendment moves the pin and revokes the displaced instance's grants, rebind: never admits no move, and two concurrent first binds resolve to exactly one winner through the precondition rather than process ordering. A scenario bundle proves the move card on a real published approval; the existing re-point bundle keeps its re-ask coverage, and two fixtures that bind a constant workspace instance alongside the chosen repository under one resource type now declare that slot multi-occupancy, which is what they are.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The security model's slot section now describes what the code enforces: the commitment is a non-expiring pinned relationship written under an atomic precondition; a filled single-occupancy slot cannot be silently reopened — a different instance needs a human-approved plan amendment that moves the pin and revokes the old instance's access, and rebind: never admits no move. The stated limits are documented plainly: pins are per resource type, pin identity is derived-ID string equality, moving a pin requires the plan gate, and a slot binding a constant instance alongside the chosen one needs multi occupancy or split types.
3ac96c9 to
97e6c39
Compare
Review: slot pinning (verified against the branch head)Each finding below was checked against the code, not just flagged. Fixes are in progress locally. Should fix before merge
Needs a decision
Minor
🤖 Generated with Claude Code |
Addresses the verified findings from the PR #16 review: - rebind: never is enforced on the approval path. The plan gate no longer stamps a move for a never slot (new PlanGateDeps.SlotRebind), and moveApprovedPins refuses one as a backstop. - First-approval and whole-plan cards show the move. The plan's own slots now carry MovedFrom, the whole-plan renderer prints the move and the revocation, and CardLine.MovedFrom becomes a separate revocation line on the wire. - The AgentSession admission sweep runs before the finalizer is added, so a failed sweep is retried on requeue instead of skipped forever. - moveApprovedPins refuses deterministic conflicts (a never slot, or a second instance of the moving type in the same approval) before any MovePin. A grant failure after a committed move returns ErrSlotMoveCommitted, and a MUST_MATCH failure returns ErrSlotMoveDrifted, so the approver notice says what actually changed. - GrantSlots collects EnsurePin/WriteGrantsPinned errors instead of returning early, so later types and the multi-occupancy write land. - The slot occupancy lookup logs the error it used to drop. - The occupancy: single default is documented as an upgrade break (CRD UPGRADE NOTE, security model), with CRDs, install.yaml and CRD docs regenerated. mage test:unit passes. mage test:integration and mage test:e2e were not run: Docker was unavailable, so every SpiceDB-backed test failed to start. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fixes for the review findings (1db0c42)Pushed fixes for the findings in the review above.
Not changed: scope-first ordering in TestsNew regression tests:
🤖 Generated with Claude Code |
This branch was successfully deployed
2 participants
What this changes
The security model has long claimed that a slot, once filled, "cannot be reopened" and that the slot write "checks that the slot is still empty as a precondition." Neither was enforced: slot grants were plain TOUCH writes, and a session bound to resource A could silently bind resource B of the same type through any bind path. This PR makes the claim true by construction, with one honest amendment: a slot cannot be silently reopened — a different instance needs a human's approval.
How it works
Declaration.
AuthzSlotgains two fields:occupancy: single | multi(defaultsingle) andrebind: approval | never(defaultapproval). The installer completes both before apply so re-installs stay SSA no-ops, and class admission rejects a single-occupancy slot declaring multiple defaults.Enforcement. The schema composes a non-expiring
slot_pinrelation onto every slot resource type. The first bind of a single-occupancy slot writes the pin under an atomicMUST_NOT_MATCHprecondition atauthz.GrantSlots— the single funnel every production slot-grant write already passes through — and grant writes for pinned slots carry aMUST_MATCHguard so a bind can never race a move. A different instance is refused with a message that names the pinned instance and the route out, routed by the slot'srebindpolicy. Grants keep their mandatory expiry and revocability; the pin persists for the session's life, so expiry never silently reopens the slot. Tool-authored writes toslot_pin/slot_grant_*relations are refused at tuple validation, and a guard test asserts no production write bypasses the gate.The approved move. A plan amendment naming a different instance records the displaced instance on the approval card at build time. Approval executes the move atomically: the pin repoints under a
MUST_MATCHguard and the displaced instance's actual grants are revoked in the same request. The card shows the move as a move — current instance, proposed instance, and the revocation — and a move whose starting point no longer matches the live pin fails the approval loudly instead of moving something the approver never saw.Lifecycle. Fork/restart copies grant and pin tuples verbatim (a clone is not a new arrival); teardown deletes pins alongside grants; a new session's first reconcile sweeps stale tuples left under a reused name, with channelsd sweeping before its own mint-time binds so the two can never race.
Visibility. Pins mirror onto
AgentSession.status.slotPins(display-only; SpiceDB stays the only enforcement input) and render inoap session show. Pin refusals annotate the model's next denial for that type with the pinned instance and the route out, and mint-time refusals record a session notice.Documented limits
occupancy: multi(or split types).Migration
Existing classes pick up the defaults on their next apply. Multi-instance classes must declare
occupancy: multi. Pre-existing sessions have no pins; the first bind after upgrade writes one.Verification
All three suites green at the tip (
mage test:unit,mage test:integration,mage test:e2e, including all 82 scenario bundles). New coverage includes a real-SpiceDB end-to-end arc (pin → refusal → approved move → revocation), a concurrent first-bind race resolved by the precondition rather than process ordering, a red-first regression test for the fork/sweep interaction, and a scenario bundle asserting the move card on a real published approval.