fix(authzen,mcp): surface parity — one engine, every surface (freeze, CPO, token horizons, MCP loop)#156
Conversation
…; MCP loop + v0.5.0 Closes the review's cross-surface drift: the AuthZEN PDP and MCP tools now answer identically to /authorize (determinism invariant). - Freeze (KILL-1): evaluateAuthZen fetches the ancestor chain once, denies WALLET_FROZEN before the redeem-write and before the ladder — no spend or grant redemption on a frozen wallet via the PDP. - CPO-1: decidePolicy/decideProvisionPolicy thread cpo; SPEND_LADDER + PROVISION_LADDER include costPerOutcomeRule (no-op without cpo), so the PDP throttles a breached ceiling like the native route. Shared readSpendState now fetches cpoContext. - /tokens: enforces the same three horizons as the gateway wall — seat daily, seat monthly, pooled subtree-daily — so MCP sanction_log_tokens can't outspend a cap it drains for gateway siblings. 402 names the horizon. - MCP: new sanction_check_authorization tool polls an escalation for its grant (spend/provision escalations were dead ends); one shared renderAuthResult surfaces request_id + real error/WALLET_FROZEN codes instead of a generic denial; log_tokens description corrected. - AARP: Idempotency-Key now required on access-request (replay guard until binding-token jti single-use lands). - Tests: cross-surface parity suite (freeze + CPO on the PDP) and /tokens horizon route tests pin the invariant so the surfaces can't drift again. - Docs: package.json 0.2.0 → 0.5.0; changelog v0.5.0 entry. Deferred to sprint 2 (queued): batch grant-atomicity refactor (move redeem to the route shell), binding-token jti single-use, AARP timeout-approve grant, PDP rate-limiting, empty-batch fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEw1RdHk5bmphKxr6N5mJY
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds Idempotency-Key enforcement with replay to the access-request route, multi-horizon (daily/monthly/subtree-daily) token budget enforcement to the tokens route, cost-per-outcome throttling and early wallet-freeze denial to AuthZEN policy evaluation, a shared renderAuthResult helper plus a new sanction_check_authorization polling tool in both MCP servers, and a version/changelog bump. ChangesIdempotency-Key Enforcement
Multi-Horizon Token Budget Enforcement
AuthZEN CPO Throttling and Wallet Freeze Denial
MCP Authorization Rendering and Escalation Polling
Changelog and Version Bump
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AccessRequestRoute
participant AuthorizationRequestDB
Client->>AccessRequestRoute: POST /access-request (Idempotency-Key?)
alt missing key
AccessRequestRoute-->>Client: 400 Idempotency-Key required
else key present
AccessRequestRoute->>AuthorizationRequestDB: lookup by agentId+idempotencyKey
alt existing request found
AuthorizationRequestDB-->>AccessRequestRoute: existing request
AccessRequestRoute-->>Client: 200 existing task response
else no existing request
AccessRequestRoute->>AuthorizationRequestDB: create new request
AccessRequestRoute-->>Client: task response
end
end
sequenceDiagram
participant evaluateAuthZen
participant WalletAncestorChain
participant readSpendState
participant decidePolicy
evaluateAuthZen->>WalletAncestorChain: fetch ancestorChain
evaluateAuthZen->>evaluateAuthZen: derive freeze state
alt wallet/ancestor frozen
evaluateAuthZen-->>evaluateAuthZen: deny WALLET_FROZEN
else not frozen
evaluateAuthZen->>readSpendState: read cpo context + spend totals
evaluateAuthZen->>decidePolicy: decide with ancestorChain + cpo
decidePolicy-->>evaluateAuthZen: PolicyDecision
end
sequenceDiagram
participant Agent
participant MCPServer
participant AuthorizeAPI
Agent->>MCPServer: sanction_authorize_tool(...)
MCPServer-->>Agent: escalated (request_id)
Agent->>MCPServer: sanction_check_authorization(request_id)
MCPServer->>AuthorizeAPI: GET /authorize/{request_id}
AuthorizeAPI-->>MCPServer: status approved/pending/denied
alt approved
MCPServer-->>Agent: grant_id (retry)
else pending/escalated
MCPServer-->>Agent: still awaiting
else denied
MCPServer-->>Agent: error with reason
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/v1/tokens/route.ts`:
- Around line 82-93: The subtree cap check in the token creation flow still
races because only the agent is serialized, so sibling agents under the same
pooled cap can both read the same total and overspend. Update the enforcement
path in the token route to also serialize by the cap wallet, using the same
lock/transaction flow around the subtree aggregate and create, or switch this
check to the shared atomic counter path before inserting the token log. Keep the
fix anchored in the existing token-cap logic and the subtree-cap branch that
returns the exceeded result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b44e20e2-9670-441d-81f9-fac3a5b39578
📒 Files selected for processing (14)
app/api/access/v1/access-request/route.tsapp/api/v1/tokens/route.tslib/authzen.tslib/changelog.tslib/decisions.tslib/rules/provision.tslib/rules/spend.tsmcp-server.tspackage.jsonpackages/sanction-mcp/mcp-server.jstests/authzen-aarp.route.test.tstests/authzen-parity.test.tstests/richDenials.route.test.tstests/tokens-horizons.route.test.ts
| for (const node of tokenCapAncestors) { | ||
| const cap = node.policy!.subtreeDailyTokenCapUsd! / 100 | ||
| const subtree = await walletSubtreeIds(tx, node.id) | ||
| const agg = await tx.tokenLog.aggregate({ | ||
| where: { agent: { walletId: { in: subtree } }, createdAt: { gte: dayStart } }, | ||
| _sum: { costUsd: true }, | ||
| }) | ||
| const subtreeSpent = agg._sum.costUsd ?? 0 | ||
| if (subtreeSpent + cost_usd > cap) { | ||
| return { exceeded: "subtree-daily" as const, spent: subtreeSpent, limit: cap, capWalletId: node.id } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any pool/wallet-level locking used alongside subtree cap enforcement.
rg -nP 'pg_advisory(_xact)?_lock' -C3
echo '--- subtree cap enforcement sites ---'
rg -nP 'subtreeDailyTokenCapUsd|walletSubtreeIds' -C3Repository: ericlovold/sanction
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file list near app/api/v1/tokens ---\n'
git ls-files 'app/api/v1/tokens/*' 'app/api/v1/*' | sed -n '1,120p'
printf '\n--- route.ts around the relevant lines ---\n'
cat -n app/api/v1/tokens/route.ts | sed -n '1,180p'
printf '\n--- advisory lock / token cap searches ---\n'
rg -n --hidden -S 'pg_advisory(_xact)?_lock|subtreeDailyTokenCapUsd|walletSubtreeIds|tokenCapAncestors|agent\.id' app api lib src . 2>/dev/null || trueRepository: ericlovold/sanction
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- route.ts around the relevant lines ---\n'
cat -n app/api/v1/tokens/route.ts | sed -n '1,200p'
printf '\n--- advisory lock / token cap searches ---\n'
rg -n --hidden -S 'pg_advisory(_xact)?_lock|subtreeDailyTokenCapUsd|walletSubtreeIds|tokenCapAncestors|agent\.id' . 2>/dev/null || trueRepository: ericlovold/sanction
Length of output: 248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file list near app/api/v1/tokens ---'
git ls-files 'app/api/v1/tokens/*' 'app/api/v1/*' | sed -n '1,120p'
echo
echo '--- route.ts around the relevant lines ---'
cat -n app/api/v1/tokens/route.ts | sed -n '1,220p'
echo
echo '--- advisory lock / token cap searches ---'
rg -n --hidden -S 'pg_advisory(_xact)?_lock|subtreeDailyTokenCapUsd|walletSubtreeIds|tokenCapAncestors|agent\.id' . 2>/dev/null || trueRepository: ericlovold/sanction
Length of output: 22087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- docs/TRACEABILITY.md relevant section ---'
sed -n '1,120p' docs/TRACEABILITY.md
echo
echo '--- concurrency/race tests mentioning advisory lock or budgets ---'
rg -n --hidden -S 'advisory|race|overshoot|subtreeDailyTokenCapUsd|token budget|cap wallet|Pool daily token cap' tests app lib docs . 2>/dev/null | sed -n '1,220p'
echo
echo '--- lib/gateway.ts relevant section ---'
cat -n lib/gateway.ts | sed -n '130,220p'Repository: ericlovold/sanction
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- lib/gateway.ts around subtree cap logic ---'
cat -n lib/gateway.ts | sed -n '140,210p'
echo
echo '--- lib/cascadeBudget.ts around atomic enforcement ---'
cat -n lib/cascadeBudget.ts | sed -n '1,220p'
echo
echo '--- concurrency.db.test.ts relevant subtree-cap test names ---'
rg -n --hidden -S 'subtree|cascade|race|cap' tests/concurrency.db.test.ts tests/* 2>/dev/null | sed -n '1,220p'Repository: ericlovold/sanction
Length of output: 36319
Serialize subtree-cap enforcement by cap wallet
In app/api/v1/tokens/route.ts, the advisory lock still only serializes a single agent, so sibling agents under the same pooled cap can both read the same subtree total and both insert, pushing the pool past its limit. Serialize this horizon on the cap wallet too, or route it through the shared atomic counter path before the aggregate/create.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/v1/tokens/route.ts` around lines 82 - 93, The subtree cap check in
the token creation flow still races because only the agent is serialized, so
sibling agents under the same pooled cap can both read the same total and
overspend. Update the enforcement path in the token route to also serialize by
the cap wallet, using the same lock/transaction flow around the subtree
aggregate and create, or switch this check to the shared atomic counter path
before inserting the token log. Keep the fix anchored in the existing token-cap
logic and the subtree-cap branch that returns the exceeded result.
Closes the cross-surface drift a code review surfaced: the AuthZEN PDP and the MCP tools were built before freeze/CPO/token-horizons landed on the native path, and had quietly fallen behind — so the same request could get a different decision depending on which surface asked. That breaks the determinism guarantee (
same request + same policy + same state ⇒ same decision) the product sells, and one gap was a real security hole. This lands the fix that was verified on an earlier branch, rebased clean onto current main.What was broken on the live surfaces
decision:truefromPOST /access/v1/evaluation, and the AARP grant-redeem path would consume a grant and ledger approved spend on a frozen wallet — while every native route hard-deniesWALLET_FROZENfirst. The one-control kill switch didn't hold on the PDP./authorizebutdecision:truefrom the PDP — an enforcement point could keep spending through a throttled channel.POST /tokens(and MCPsanction_log_tokens) enforced only the seat daily budget — not the seat monthly or the pooled subtree-daily cap — so that path could outspend a cap it still drains for gateway siblings.sanction_authorize/_provisiontold the agent to "retry with the grant_id" but never surfacedrequest_idand offered no way to poll for the grant; and config/freeze failures rendered as a generic "DENIED."The fix
evaluateAuthZen— fetches the ancestor chain once, deniesWALLET_FROZENbefore the redeem-write and before the ladder, exactly like the native routes.decidePolicy/decideProvisionPolicythread thecpocontext;SPEND_LADDER/PROVISION_LADDERincludecostPerOutcomeRule(no-op without a ceiling), so the PDP throttles identically to/authorize. SharedreadSpendStatenow fetchescpoContext./tokensthree-horizon enforcement — seat daily, seat monthly, pooled subtree-daily; the 402 names which line hit.sanction_check_authorizationtool polls an escalation for its grant; one sharedrenderAuthResultsurfacesrequest_idand the real error/WALLET_FROZENcode instead of a generic denial./authorizeagree under freeze and over-CPO, so the surfaces can't silently drift again. Version → v0.5.0 + changelog entry.Verification
npm run check: 772 unit tests green (adds the parity +/tokens-horizon suites).mainwith zero conflicts.Deferred to a follow-up (already in
docs/BACKLOG.md): batch grant-atomicity refactor, binding-token single-use (jti), PDP rate-limiting, AARP timeout-approve grant, empty-batch guard — the deeper review findings that need real design.🤖 Generated with Claude Code
https://claude.ai/code/session_01UEw1RdHk5bmphKxr6N5mJY
Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores