Skip to content

fix(rate-limit): admins bypass storyboard eval rate limits#2729

Merged
bokelley merged 4 commits into
mainfrom
EmmaLouise2018/admin-rate-limit
Apr 26, 2026
Merged

fix(rate-limit): admins bypass storyboard eval rate limits#2729
bokelley merged 4 commits into
mainfrom
EmmaLouise2018/admin-rate-limit

Conversation

@EmmaLouise2018

Copy link
Copy Markdown
Contributor

Summary

  • Admins hit the 5/hour storyboard evaluation cap (and 30/hour step cap) while curating and debugging. The limiter is one shared bucket per user across all storyboards (keyed only by user ID at server/src/middleware/rate-limit.ts:12-29), so a handful of clicks across different agents/storyboards exhausts it.
  • Adds a skipForAdmins helper mirroring requireAdmin semantics (pre-resolved user.isAdminADMIN_EMAILS env var → async isWebUserAAOAdmin(userId) with its built-in cache) and wires it into both storyboardEvalRateLimiter and storyboardStepRateLimiter via express-rate-limit's async skip option.
  • Non-admin users are unaffected — same 5/hour and 30/hour limits apply.

Test plan

  • As a non-admin user, confirm storyboard /run still returns 429 after 5 evaluations within an hour.
  • As an AAO admin (via working group membership), confirm 6+ /run calls within an hour all succeed (modulo normal failures).
  • As an admin via ADMIN_EMAILS env var override (no working group row), confirm same bypass behavior.
  • Confirm /storyboard/:id/compare and /applicable-storyboards also bypass for admins.
  • Dev mode: dev user with isAdmin: true bypasses without hitting the DB admin check.

Admins hit the 5/hour storyboard evaluation cap (and 30/hour step cap)
while curating and debugging — the limit is one shared bucket per user
across all storyboards, so even browsing-heavy admin work exhausts it.

Add a `skipForAdmins` helper mirroring `requireAdmin` semantics (user
`isAdmin` flag → ADMIN_EMAILS → `isWebUserAAOAdmin`) and wire it into
both `storyboardEvalRateLimiter` and `storyboardStepRateLimiter` via
express-rate-limit's async `skip` option. Non-admins are unaffected.
@EmmaLouise2018 EmmaLouise2018 requested a review from bokelley April 21, 2026 21:35
@bokelley

Copy link
Copy Markdown
Contributor

deeper question - is 5/hour reasonable? doesn't seem like it?

@bokelley bokelley added admin-tool Internal tools for AAO staff bug Something isn't working labels Apr 22, 2026
@bokelley bokelley force-pushed the EmmaLouise2018/admin-rate-limit branch from 436663e to 481a7ca Compare April 26, 2026 14:46
@bokelley bokelley merged commit 663bb21 into main Apr 26, 2026
12 checks passed
@bokelley bokelley deleted the EmmaLouise2018/admin-rate-limit branch April 26, 2026 15:41
bokelley pushed a commit that referenced this pull request Apr 26, 2026
…tests

The top-level `import { invalidateMemberContextCache }` on member-tools.ts
forced member-context.ts (and transitively auth.ts) to load at module init,
triggering the eager WorkOS constructor before vi.mock could intercept it.

Removed the static import; both call sites in the fire-and-forget .then()
callbacks now use dynamic `await import('../member-context.js')` — same
pattern as line 2195 and as the fix applied in #2729.

https://claude.ai/code/session_018QWw9FNTxNgKLejx54HYs2
bokelley pushed a commit that referenced this pull request Apr 26, 2026
…tests

The top-level `import { invalidateMemberContextCache }` on member-tools.ts
forced member-context.ts (and transitively auth.ts) to load at module init,
triggering the eager WorkOS constructor before vi.mock could intercept it.

Removed the static import; both call sites in the fire-and-forget .then()
callbacks now use dynamic `await import('../member-context.js')` — same
pattern as line 2195 and as the fix applied in #2729.

https://claude.ai/code/session_018QWw9FNTxNgKLejx54HYs2
bokelley added a commit that referenced this pull request Apr 27, 2026
…3258)

* feat(addie): instrument agent test runs for staleness-aware prompts (#3254)

Adds `agent_test_runs` table and hydrates `agent_testing` on `MemberContext`
so #2299 Stage 2 can fire a "Time for a fresh agent test" suggested prompt
to builder personas who haven't run a test recently.

https://claude.ai/code/session_018QWw9FNTxNgKLejx54HYs2

* fix(addie): lazy-import invalidateMemberContextCache to unblock unit tests

The top-level `import { invalidateMemberContextCache }` on member-tools.ts
forced member-context.ts (and transitively auth.ts) to load at module init,
triggering the eager WorkOS constructor before vi.mock could intercept it.

Removed the static import; both call sites in the fire-and-forget .then()
callbacks now use dynamic `await import('../member-context.js')` — same
pattern as line 2195 and as the fix applied in #2729.

https://claude.ai/code/session_018QWw9FNTxNgKLejx54HYs2

* fix(addie): reconcile agent_testing type and renumber migration 444

Two post-rebase conflicts against main:

1. Remove fetchAgentTesting stub (main's narrow type) — keep this PR's
   richer AgentTestingContext shape (total_tests_30d + 4-state outcome)
   which the staleness-prompt rule actually depends on.

2. Renumber 442_agent_test_runs.sql → 444_agent_test_runs.sql; main
   claimed 442 (addie_prompt_telemetry_clicks) and 443
   (agent_test_history_user_id_index) while this branch was in review.

https://claude.ai/code/session_01W4Djuhqn6SJJk5qBSPSdTt

---------

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

Labels

admin-tool Internal tools for AAO staff bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants