feat(api): rate-limit legacy admin and public Agent View routes (AP-14)#85
Conversation
AP-14 Security: Rate-limit legacy admin-token and public bearer read routes
ContextLegacy admin-token routes and public bearer read surfaces currently lack explicit rate limits in places, especially Source docs
ScopeInventory affected routes, add explicit route-contract rate-limit classes/bindings where needed, and cover admin-token plus public bearer read surfaces. Out of scopeDo not redesign rate-limit policy or billing-tier rate limits. DependenciesNone. Implementation notesUse existing ADR 0072 route registrar and worker-runtime rate-limit primitives. Keep error codes stable and include AcceptanceRoutes identified in the status docs have explicit limits or a documented reason for no limit. Tests cover allowed and limited requests. VerificationRun focused worker-runtime/API/content tests and Remote Cursor handoffStart by reading |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR enforces actor-scoped rate limiting for legacy admin_token principals and artifact-scoped limiting for public Agent View tokens. It adds an ARTIFACT_RATE_LIMIT env binding and Wrangler entries, exports and uses applyRateLimit in the API (including adminWhoami and registrar wiring), updates route contracts and OpenAPI (ArtifactRateLimitErrorEnvelope / artifact 429), adds unit/integration tests, and updates ops/docs. Sequence Diagram(s)sequenceDiagram
participant AdminClient as Admin Client (admin_token)
participant AdminWhoami as adminWhoami endpoint
participant ApplyRate as applyRateLimit
participant ActorBucket as Actor Bucket (platform:admin:id)
participant Response as Response Handler
AdminClient->>AdminWhoami: GET /admin/whoami
AdminWhoami->>ApplyRate: check actor limit
ApplyRate->>ActorBucket: increment/check
alt Rate limit exceeded
ActorBucket-->>ApplyRate: blocked
ApplyRate-->>Response: {ok:false, code:"rate_limited_actor"}
Response-->>AdminClient: 429 + Retry-After: 60
else Within limit
ActorBucket-->>ApplyRate: allowed
ApplyRate-->>AdminWhoami: {ok:true}
AdminWhoami-->>Response: 200 OK with actor
Response-->>AdminClient: 200 OK
end
sequenceDiagram
participant PublicClient as Public Client
participant AgentViewAPI as GET /v1/public/agent-view/{token}
participant ApplyRate as applyRateLimit
participant ArtifactBucket as Artifact Bucket (artifact_id)
participant DBLoad as Load Agent View
participant Response as Response Handler
PublicClient->>AgentViewAPI: GET /v1/public/agent-view/{token}
AgentViewAPI->>ApplyRate: check artifact limit
ApplyRate->>ArtifactBucket: increment/check
alt Rate limit exceeded
ArtifactBucket-->>ApplyRate: blocked
ApplyRate-->>Response: {ok:false, code:"rate_limited_artifact"}
Response-->>PublicClient: 429 + Retry-After: 60
else Within limit
ArtifactBucket-->>ApplyRate: allowed
ApplyRate->>DBLoad: load public Agent View
DBLoad-->>Response: 200 OK + AgentView
Response-->>PublicClient: 200 OK
end
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-85.isaac-a46.workers.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/ops/project-status.md`:
- Around line 22-23: Update the stale bullet that claims "rate limits still
needed for legacy admin-token/public bearer reads" (the later bullet in the same
project-status section) to reflect that the debt has been addressed—either
remove the sentence or change it to indicate the rate-limit requirement is
resolved; ensure the remaining text matches the new status change that removed
the "rate limits still needed" note earlier in the document so the file is
internally consistent (search for the bullet containing "legacy
admin-token/public bearer" or "rate limits still needed" to locate the line to
edit).
In `@docs/ops/status/hosted-ops.md`:
- Around line 68-70: The Open Ops Items list still marks adding rate limiting as
pending while the new bullet claims legacy admin-token routes and public Agent
View now use ARTIFACT_RATE_LIMIT / ACTOR_RATE_LIMIT bindings on api; update the
Open Ops Items entry (the pending item at the top-level Ops list) to reflect
completion by either removing the pending item or rewording it to a follow-up
task (e.g., "verify and audit ARTIFACT_RATE_LIMIT / ACTOR_RATE_LIMIT deployment
for legacy admin-token routes and public Agent View") so the ledger no longer
contradicts the new bullet referencing ARTIFACT_RATE_LIMIT and ACTOR_RATE_LIMIT.
In `@packages/worker-runtime/src/rate-limit.test.ts`:
- Around line 45-55: The test for "rate-limits legacy admin-token principals by
actor only" currently doesn't assert that the workspace contract isn't invoked;
add a mock for the workspace rate-limit function (e.g., workspace =
vi.fn().mockResolvedValue(...)) and pass it into applyRateLimit's contract map
alongside actorContract, then add an assertion that workspace was not called
(expect(workspace).not.toHaveBeenCalled()) to explicitly verify "actor only"
behavior for applyRateLimit and the actorContract.
In `@packages/worker-runtime/src/rate-limit.ts`:
- Around line 41-45: The branch handling principal.kind === "admin_token"
currently returns { ok: true } when adminIdForPrincipal(principal) yields falsy,
which incorrectly bypasses actor throttling; change this to fail closed by
returning the same not_authenticated result used for missing/invalid actors
(i.e., return the not_authenticated error shape instead of ok true) so
admin-token paths without a usable admin id are rejected; update the branch in
rate-limit.ts where adminIdForPrincipal(principal) is checked (the admin_token
handling block) to return the not_authenticated result consistent with
actor-scoped behavior.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b086173-766e-4916-947b-beeae331994c
📒 Files selected for processing (14)
apps/api/src/index.test.tsapps/api/src/index.tsapps/api/src/worker-configuration.d.tsapps/api/wrangler.jsoncdocs/ops/project-status.mddocs/ops/status/hosted-ops.mddocs/ops/status/phase-backlog.mdpackages/contracts/openapi/api.jsonpackages/contracts/src/mvp-contracts.test.tspackages/contracts/src/openapi/api.tspackages/contracts/src/routes.tspackages/worker-runtime/src/index.tspackages/worker-runtime/src/rate-limit.test.tspackages/worker-runtime/src/rate-limit.ts
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-85.isaac-a46.workers.dev |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-85.isaac-a46.workers.dev |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-85.isaac-a46.workers.dev |
Apply actor-scoped limits to all contract-mounted /admin/* routes and /admin/whoami, and artifact-scoped limits to public Agent View reads. Share the content worker ARTIFACT_RATE_LIMIT namespace on api so per-artifact budgets apply across read surfaces. Closes AP-14. Co-authored-by: Isaac Suttell <isaac@isaacsuttell.com>
Co-authored-by: Isaac Suttell <isaac@isaacsuttell.com>
Co-authored-by: Isaac Suttell <isaac@isaacsuttell.com>
Co-authored-by: Isaac Suttell <isaac@isaacsuttell.com>
Co-authored-by: Isaac Suttell <isaac@isaacsuttell.com>
- Fail closed when admin_token principal lacks a usable admin id - Assert workspace bucket is not used for legacy admin rate limits - Remove stale pending rate-limit ops bullets from status ledgers Co-authored-by: Isaac Suttell <isaac@isaacsuttell.com>
cursor
Bot
force-pushed
the
cursor/rate-limit-admin-public-bearer-f4ae
branch
from
1f07faa to
ae7639d
Compare
cursor
Bot
changed the title
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-85.isaac-a46.workers.dev |
|
agent-paste PR preview resources were cleaned up. The pr-preview-${context.issue.number} environment is left in place; remove it from the GitHub UI if desired. |
2 participants
Summary
Closes AP-14 by adding explicit rate limits to legacy admin-token routes and the public Agent View bearer read surface, using the existing route-contract registrar and worker-runtime rate-limit primitives (ADR 0072 / ADR 0064).
Legacy admin (
/admin/*)rateLimit: "actor"./admin/whoamiapplies the same actor limit after auth.platform:admin:{admin_id}onACTOR_RATE_LIMIT.429withrate_limited_actorandRetry-After: 60.not_authenticated(no workspace bucket).Public Agent View (
GET /v1/public/agent-view/{token})rateLimit: "artifact"withrate_limited_artifactin errors.apimountsARTIFACT_RATE_LIMITusing the same namespace IDs ascontent, so per-artifact budgets are shared across read surfaces.429withrate_limited_artifactandRetry-After: 60.Documented exception
GET /v1/usage-policyremainsrateLimit: "none"(static policy; no separate abuse surface).Files changed
packages/contracts/src/routes.ts— rate-limit classes and error codes on admin/public routespackages/worker-runtime/src/rate-limit.ts— admin-token and signed-agent-view-token artifact keysapps/api/src/index.ts,apps/api/wrangler.jsonc—ARTIFACT_RATE_LIMITbinding on API workerapps/api/src/index.test.ts,packages/worker-runtime/src/rate-limit.test.ts,packages/contracts/src/mvp-contracts.test.ts— allowed + limited request coveragedocs/ops/project-status.md,docs/ops/status/hosted-ops.md,docs/ops/status/phase-backlog.mdVerification
pnpm --filter @agent-paste/worker-runtime test(22 passed)pnpm --filter @agent-paste/api test(107 passed)pnpm verify(72 tasks passed)Hosted verification
Not run (no production credentials in this environment).
Linear Issue: AP-14
Summary by CodeRabbit
New Features
API Contract
Tests
Documentation