Skip to content

feat: policy model allowlist, Claude Opus 5.5, and policy-scoped pool status - #267

Merged
steipete merged 2 commits into
mainfrom
feat/policy-models-pool-status
Oct 12, 2026
Merged

steipete merged 2 commits into
mainfrom
feat/policy-models-pool-status

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #266 so a subscription pool can expose exactly the models its operator wants and so clients can show pool status without admin access.

  • Per-policy model allowlist. AccessPolicy.models (null = every model of the allowed providers, unchanged default). Enforced at the single proxy preparation owner for native Messages/count_tokens, manifest routes, OpenAI-compatible routes, Responses, WebSocket creates, and Fusion; discovery (/v1/catalog, /v1/models) lists only allowed models; Access-session policy selection skips policies that do not allow the requested model. Requests whose model cannot be resolved are rejected for restricted policies (403 model_not_allowed) instead of passing through an opaque gateway.
  • Claude Opus 5.5 (anthropic/claude-opus-5-5): $4 / $20 per MTok, $0.20 cache read, $5 / $8 cache writes, 1M context, 128K output.
  • GET /v1/pool, opt-in per policy (poolStatus: "visible", default hidden → 404 pool_status_hidden). Returns the caller's own policy routing, allowed models, per-grant account facts, quota windows as % free with resets, extra-usage spend, per-model eligibility, and 7-day per-grant/lane usage. Shares eligibility logic with the admin Subscriptions page. No tokens or other policies' data; cache-control: private, no-store. Visible pools intentionally expose account emails to the policy's key holders.
  • Policy editor gains model allowlist and pool-visibility controls.

Verification

  • pnpm provider:compile, pnpm check (1,222 Worker / 224 admin / 337 script tests), pnpm worker:e2e, admin build, 80 focused browser tests
  • Dashboard/Fusion screenshot baselines change because the policy editor gained fields; refreshed from CI renders.

Deployment

After merge: Deploy Cloudflare workflow, then set peter_claude_pool to models: [anthropic/claude-opus-5-5, anthropic/claude-fable-5-1], poolStatus: visible.

@clawsweeper

clawsweeper Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Oct 12, 2026
@clawsweeper

clawsweeper Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge.

What this changes

This PR adds policy model allowlists, Claude Opus 5.5 metadata, and opt-in pool status for policy holders.

Example: An Anthropic policy allows only anthropic/claude-opus-5-5.

  • Before: Provider access also permits anthropic/claude-sonnet-5.
  • After: Sonnet inference returns 403 model_not_allowed before grant selection or budget reservation.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) Useful owner-directed work with passing checks, but the attribution defect and unresolved authorization and upgrade evidence prevent a merge-ready verdict.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Product

Kind: Feature · Worth it: Yes · Fix scope: Complete
User problem: Operators cannot restrict a pool to selected models or let its users inspect capacity without admin access.
Reason: The recorded owner direction provides concrete pool-management value and explicitly accepts opt-in account-email disclosure.

Merge readiness

⛔ Blocked before merge - 5 items remain

Keep open: this owner-directed feature remains absent from main, and the prior grant-identifier collision is still unfixed.

Priority: P2
Reviewed head: fe148d3acbb6b0178e73f0ba09b749720a82e6a2

Before merge

  • Add real behavior proof - Authority-chain proof required: the body and discussion provide tests and CI claims, without a production-path trace showing permitted versus unheld-policy pool access, forbidden model rejection before provider I/O, or invalidation after revocation, reassignment, and policy narrowing on retained authority. Supply redacted terminal output or logs, omitting credentials, IPs, and non-public endpoints. Persisted policy fields also lack an upgrade run from v0.4.0 showing existing policies, credentials, grants, budgets, and access intact with null-model and hidden-pool defaults. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Preserve scope in public grant identifiers (P2) - With oauth/pool/claude and oauth/tenants/tenant-pool/claude configured, the endpoint returns two inventory entries with key "claude". The usage projection at line 54 likewise turns their separately aggregated keys into "claude", so clients cannot associate traffic with the correct account. Both scopes are supported and explicitly scanned here. Project a distinct non-secret identifier consistently into inventory and usage, and cover equal token references across the two scopes. This repeats the still-unfixed prior finding.
  • Resolve security review attention item - No concrete authorization bypass was established, but the new permissions and account disclosure still require final-effect evidence.
  • Resolve merge risk - Before merge, establish that revoked or reassigned authority cannot disclose pool accounts or dispatch newly forbidden models through retained sessions or WebSocket connections.
  • Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.

Findings

  • [P2] Preserve scope in public grant identifiers — worker/pool.ts:42

Tests

  • Missing end-to-end proof: Supply redacted production-path output showing allowed and forbidden pool access and model dispatch, including revocation or policy narrowing on reused authority.
Agent review details

How this fits together

The Worker authenticates callers, resolves policies, selects upstream grants, reserves budgets, and dispatches inference; the new pool endpoint returns scoped account and usage information.

flowchart LR
Caller --> Authentication
Authentication --> Policy
Policy --> ModelAdmission
ModelAdmission --> GrantSelection
GrantSelection --> Provider
Policy --> PoolProjection
PoolProjection --> Caller
Loading

Technical review

Best possible solution:

Keep centralized model admission and shared quota rules, while preserving a distinct non-secret grant identifier consistently across pool inventory and usage.

Do we have a high-confidence way to reproduce the issue?

The identifier collision follows directly from scanning both supported grant scopes and projecting their distinct keys to the same token reference; no target code was executed.

Is this the best way to solve the issue?

Centralized admission and shared eligibility are appropriate, but the public representation must retain enough identity to associate each account with its own usage.

Full review comments:

  • [P2] Preserve scope in public grant identifiers — worker/pool.ts:42
    With oauth/pool/claude and oauth/tenants/tenant-pool/claude configured, the endpoint returns two inventory entries with key "claude". The usage projection at line 54 likewise turns their separately aggregated keys into "claude", so clients cannot associate traffic with the correct account. Both scopes are supported and explicitly scanned here. Project a distinct non-secret identifier consistently into inventory and usage, and cover equal token references across the two scopes. This repeats the still-unfixed prior finding.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 61b3a8623746.

Merge-risk options

Maintainer options:

  1. Hold for authorization evidence (recommended)
    Keep this PR open until allowed and nearest-forbidden callers are demonstrated at pool disclosure and provider I/O, including authority revocation and policy narrowing.

Provenance checked

  • Shared quota predicates and subscription presentation keeps the original intent (61b3a86: Make subscription selection model-aware and prevent unwanted paid overage; this PR moves dispatch predicates into shared code.)
  • Discovery policy selection keeps the original intent (86566bb: Choose scoped executable operations before observing affordability; model grouping preserves that ordering.)
  • Canonical policy normalization keeps the original intent (e33fdac: Keep canonical policy state in the authority with bounded legacy migration; absent new fields retain unrestricted models and hidden pool status.)

Testing

Proof path: in-process harness.

Security

Needs attention: No concrete authorization bypass was established, but the new permissions and account disclosure still require final-effect evidence.

Evidence

What I checked:

  • Feature remains absent from main: The default branch remains at the pinned base; the introduced diff adds model admission and the pool route. (worker/proxy.ts:413, 61b3a8623746)
  • Prior finding remains unfixed: Inventory projects both policy and tenant grants to tokenRef, and usage strips their full keys to the final segment; worker/pool.ts is unchanged since the earlier reviewed head. (worker/pool.ts:42, fe148d3acbb6)
  • Routing preserves distinct grant scopes: Grant resolution retains separate oauth/<policy>/<reference> and oauth/tenants/<tenant>/<reference> identities; usage aggregation also groups by full grant key. (worker/grant-selection.ts:162, fe148d3acbb6)
  • Authorization coverage is test-based: Added tests exercise handler-based denials and held-policy pool access with mocked upstream or ledger I/O; WebSocket model denial calls prepareSelected directly, without exercising a retained connection after revocation. (worker/test/subscription-pool.test.mjs:300, fe148d3acbb6)
  • Existing policy defaults are preserved in source: Canonical normalization supplies null models and hidden pool status when fields are absent, while admin updates preserve existing values when omitted. (worker/authority.ts:888, fe148d3acbb6)
  • Intentional account disclosure: The owner-directed PR explicitly accepts account-email visibility for policy holders after administrator opt-in; that accepted disclosure is not an accidental-leak finding. (worker/pool.ts:44, fe148d3acbb6)

Likely related people:

  • Peter Steinberger: Raw commit 61b3a86 adds admin/src/subscriptions-data.ts:37 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 61b3a8623746; files: admin/src/subscriptions-data.ts)
  • Vincent Koc: Raw commit 86566bb adds worker/discovery.ts:273 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 86566bb1e2bb; files: worker/discovery.ts)

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This useful operator feature has a bounded grant-attribution defect requiring correction before release.
  • merge-risk: 🚨 security-boundary: Persisted model permissions and newly exposed account data require final-effect authorization evidence under revoked or reassigned authority.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add final-effect proof for a caller lacking the selected policy and prove revocation or reassignment prevents pool disclosure and forbidden model I/O.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-12T00:19:44.558Z sha e047d2f :: needs real behavior proof before merge. :: [P2] Preserve scope in public grant identifiers

Reviewed October 11, 2026, 8:34 PM ET / October 12, 2026, 00:34 UTC (Revision 2).

@steipete
steipete merged commit 6e26587 into main Oct 12, 2026
18 checks passed
@steipete
steipete deleted the feat/policy-models-pool-status branch October 12, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant