Skip to content

Guard seam: one decision function for every privileged action (guarded-actions phase 2)#2986

Open
moshe-nanoco wants to merge 9 commits into
mainfrom
feat/guard-seam
Open

Guard seam: one decision function for every privileged action (guarded-actions phase 2)#2986
moshe-nanoco wants to merge 9 commits into
mainfrom
feat/guard-seam

Conversation

@moshe-nanoco

@moshe-nanoco moshe-nanoco commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Every privileged action crossing the container or channel boundary now passes one decision functionguard() in the new src/guard/ leaf — before it executes: allow | hold | deny.

The problem this solves

Gating was voluntary. Each call site decided inline whether to check anything — create-agent.ts's cli_scope === 'global' branch, dispatch's per-command access constants, router gates that default allow-all when the permissions module isn't loaded. Nothing guaranteed a privileged action passed any gate, and a new feature could ship ungated silently.

The fix is structural, not behavioral: today's checks stay the decision, verbatim — they move to one seam that the privileged entry points provably pass. This PR deliberately does not make policy data-driven; that is a separate problem, deferred (a generalized rules table can arrive later).

What ships

  • src/guard/ domain-free leafguard(action, input) runs the action's decide (today's structural checks: allow | hold | deny) → grant check, fail-closed on throw. defineGuardedAction returns a branded value and consult sites carry the value, so a typo'd action or dropped module-edge import is a compile error, not a silent allow; a forged value denies at runtime.
  • Module-edge decisionsguard.ts adapters define the domain entries: agents.create, a2a.send (destination ACL + agent_message_policies hold inside the decide), self_mod.*, senders.admit, channels.register.
  • Keyed registries demand a guard at registration — every ncl command derives its guard from its own CommandDef inside register() (dotted action names stamped by registerResource); a delivery action registers with a guard spec or an explicit unguarded(<reason>) declaration — omission doesn't compile, grep "unguarded(" is the justified inventory, and re-registering a guarded action without a spec throws instead of silently disarming the guard. The consult pipeline (runGuarded, DeliveryGuardSpec) lives in src/delivery-guard.ts so delivery.ts keeps main's shape.
  • Broadcast hooks stay plain registrations — response handlers and message interceptors are unchanged from main; the privileged click path (the channel-registration card handler) consults the guard inline, like the a2a route and the unknown-sender gate. The free-text name reply is deliberately not re-authorized — the click arms the capture and stands as the auth, exactly as on main.
  • Grant-carrying replay — approved continuations re-enter their entry point with the verified approval row as the grant (ApprovalHandlerContext.approval); a grant satisfies a hold, never a deny (live-row + grantActionName + grantCoversRequest checks). The forgeable approved: true boolean is deleted — it was the same voluntary-gating hole on the replay path: any in-process caller passing it executed ungated.
  • Conformance as a CI testsrc/guard/conformance.test.ts walks the real registries: every holding action pairs with a registered approval handler, every mutating ncl command holds via cli_command, duplicate definitions throw. (No boot-time enforcement: at runtime a missing continuation already resolves loudly — the requester is told no handler is installed.)

Behavior deltas (deliberate, inherent to a non-vacuous gate on replays — also in CHANGELOG)

  • a2a approve-then-revoke no longer delivers — the structural checks re-run live on replay. A grant that short-circuited the decision would be a second door.
  • Forged / already-consumed / mismatched grants refuse instead of executing (previously any in-process caller passing approved: true executed).

Everything else is main's behavior verbatim: host trusted-caller, cli_scope enforcement, create_agent's scope branch, a2a check order, unconditional self-mod hold, unknown_sender_policy, channel click-auth (anchor-group approver included), click-armed name capture. Sender/channel holds stay on their own tables; holds map onto the existing requestApproval (approverUserId).

Deliberately NOT here

  • The approval-contract work (approver rules/mayResolve, migration, sender fold) — separate PR.
  • The D1/D2/D4 click-auth fixes from the defect inventory — a separate PR owns them (D2's ?? { caller: 'host' } replay fallback is preserved verbatim).
  • Policy-as-data (a runtime rules table + ncl guard surface).
  • Flagged fast-follows: commands.admin (chat slash-command gate in command-gate.ts), guard declarations on the router's single-slot gate setters (setAccessGate / setSenderScopeGate / setChannelRequestGate); credentials.use stays on the OneCLI gateway.
  • Zero changes under container/.

Verification

pnpm test 773/773 (79 files) · pnpm run build clean — at 5a114b5 (current main merged in; the channel-defaults suites pass against the guard-consulting paths).

🤖 Generated with Claude Code

moshe-nanoco and others added 4 commits July 9, 2026 14:22
…rrying replay, boot conformance

Every privileged action crossing the container or channel boundary now
passes one decision function — guard() in the new src/guard/ leaf —
before it executes: allow | hold | deny (hub:
engineering/requirements/guarded-actions +
engineering/discovery/guarded-actions-decisions, phase 2).

- Registration-derived catalog: registry.register() derives one entry
  per ncl command (dotted action names stamped by registerResource);
  module-edge guard.ts adapters register the domain baselines
  (agents.create, a2a.send incl. the agent_message_policies hold,
  self_mod.*, senders.admit, channels.register). The baseline is the
  decision — policy-as-data (tighten-only rules) is deferred to
  phase 3.
- All four handler registries wrap at registration: dispatch consults
  the guard; guarded delivery actions (create_agent, install_packages,
  add_mcp_server) store only the precheck → guard → handler wrapper
  (the raw handler is never stored; spec-less re-registration throws);
  response handlers + message interceptors take guard specs (channel
  registration's click + free-text name capture).
- Grant-carrying replay: approved continuations re-enter their entry
  point with the verified approval row as the grant
  (ApprovalHandlerContext.approval). A grant satisfies a hold, never a
  deny — live-row + approvalAction + grantMatches checks; the forgeable
  approved:true boolean is deleted.
- Boot conformance: the registry walk (src/guard-conformance.ts) runs
  in CI and at boot — an unmapped privileged registration stops the
  host with a banner (skill-installed code never runs this repo's CI).

Baselines are main's behavior verbatim (host trusted-caller, cli_scope
allowlist, create_agent scope branch, a2a ACL order, unconditional
self-mod hold, unknown_sender_policy, channel click auth incl. the
anchor-group approver). Sender/channel holds stay on their own tables;
guard holds map onto the existing requestApproval options
(approverUserId).

Deliberate outcome changes inherent to the replay semantics (called out
in CHANGELOG too): (a) a2a approve-then-revoke no longer delivers — the
structural baseline re-runs live on replay; (b) forged, already-consumed,
or mismatched grants refuse instead of executing; (c) the channel-name
free-text reply re-checks approver eligibility at reply time. The
D1/D2/D4 click-auth fixes are deliberately NOT here — they belong to the
approval-contract PR (D2's host fallback at dispatch replay is preserved
verbatim).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
File + internal map (catalog → guardedActions). The concept stays "the
action catalog" in prose (the term the requirements/decisions docs and
the conformance banner use); exported symbols (registerGuardedAction,
GuardedActionSpec, …) unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… registry walk

The guard's weak point was the wiring: catalog entries registered by
side-effect import, consult sites naming them by string, a map miss
resolving to ALLOW, and a boot walk that could only see 2 of the 4
registries (response handlers and interceptors erased their specs into
closures). Dropping one unreferenced import silently disarmed
channel-registration click-auth.

Make the broken wiring unconstructible instead of detected:

- defineGuardedAction returns a branded GuardedAction value; guard(action,
  input) takes the value, not a name. A dropped module-edge import or a
  typo'd action is now a compile error; there is no lookup and no
  fail-open branch. A forged value (outside TS) is denied at runtime.
- Every registry (delivery actions, response handlers, interceptors)
  requires a guard spec or an explicit unguarded(<reason>) declaration at
  the registration site — unguarded-by-omission is not representable, and
  the justification travels with the registration (grep "unguarded(" for
  the complete inventory). CLI commands derive their guard inside
  register(), so a command cannot exist without one.
- guardConformanceViolations() and EXEMPT_DELIVERY_ACTIONS are gone. The
  boot check shrinks to the one cross-registry invariant the compiler
  can't see: every holding action has a registered approve continuation
  (grantContinuationGaps), same fail-closed refuse-to-start posture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The unguarded brand becomes a real (module-private) symbol and
isUnguarded() its exported type guard, replacing three hand-rolled
`'reason' in guardDecl` checks (delivery, router, response-registry)
plus the one inside isUnguardedEntry. Only unguarded() can mint the
brand now, so a look-alike { reason } object — or a guard spec that
someday grows a `reason` field — can't pass as an unguarded
declaration at runtime.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
moshe-nanoco and others added 4 commits July 11, 2026 22:16
…and interceptors consult inline

registerResponseHandler and registerMessageInterceptor return to main's
single-argument shape: the guard-spec/unguarded declaration machinery
(claims, onDeny, the registry wrappers) is deleted. The declaration
requirement stays where registration is keyed — every ncl command derives
its guard inside register(), and registerDeliveryAction demands a spec or
unguarded(<reason>) — while broadcast hooks gate inline at the point of
privilege, like the a2a route and the unknown-sender gate:

- handleChannelApprovalResponse consults guard(channelsRegister) inline
  where main had the click-auth if — same decision, same timing.
- The free-text name capture returns to main verbatim: the click arms it,
  the reply is not re-authorized (the body's own pending-row re-fetch
  still refuses a vanished registration). Behavior delta (c) is withdrawn
  — the deliberate deltas are back to (a) approve-then-revoke and
  (b) grant refusals.

Also gone with the wrappers: the claims/body predicate duplication (the
same early-exits existed twice and could drift apart) and the repeated
extractAndUpsertUser upsert per captured event.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…xtract runGuarded

Three trims to the seam, no behavior change (713/713 green):

- Boot-time conformance check deleted (src/guard-conformance.ts + its
  main() call). The invariant — every holding action has a registered
  approve continuation — is already covered in-tree by the conformance
  test, and at runtime response-handler.ts handles a missing continuation
  loudly (warn + "approved, but no handler is installed" notify + row
  cleanup). Refusing to boot bought no safety over that and could brick
  the host on a mis-installed skill.

- GuardedActionSpec vocabulary renamed to read as what each field does:
  baseline → decide (the decision function — allow | hold | deny),
  approvalAction → grantActionName (the pending_approvals.action string a
  grant's row must carry; "Name" because at consult sites `action` means
  the branded GuardedAction value), grantMatches → grantCoversRequest.
  Comment prose follows ("structural baseline" → the checks / the
  decision).

- runGuarded + DeliveryGuardSpec extracted to src/delivery-guard.ts.
  delivery.ts is a high-traffic file for forks: the registry itself
  (tagged entries, spec-or-unguarded registration overloads, disarm-throw,
  one-door getDeliveryAction, grant-carrying reenter) stays there, evolved
  in place — only the consult pipeline (precheck → guard → deny/hold/allow)
  and the spec type move out; runGuarded takes spec + handler explicitly so
  the new file has no import back into delivery.ts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The public tree shouldn't point at internal artifacts: phase numbering
("guarded-actions phase 2/3") and the team-hub decisions-doc pointer are
gone from the CHANGELOG bullet, the CLAUDE.md guard row, and the guard
file headers. The facts stay ("policy-as-data is deliberately deferred —
a generalized rules table can arrive later"); only the internal roadmap
vocabulary goes. Scope: lines this branch introduced only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The channelsRegister docstring and the CLAUDE.md guard row still described
the withdrawn reply-time re-check (behavior delta (c), removed in fe235c7):
the docstring claimed the decision is consulted by the name-capture
interceptor "so a privilege revoked mid-flow is re-checked at each step",
and the CLAUDE.md row said message interceptors consult the guard inline.
Neither is true — channels.register is consulted only by the card-click
handler, and the free-text reply deliberately authorizes off the click
(main's behavior). Both docs now state that explicitly, including the
consequence: a privilege revoked between click and reply still completes
the flow.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core-team PR opened by a core team member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant