Skip to content

Bound the workflow-artifact rate limiter's per-run map - #530

Merged
TheGreatAxios merged 2 commits into
mainfrom
cl-7243-bound-artifact-rate-limiter
Aug 31, 2026
Merged

TheGreatAxios merged 2 commits into
mainfrom
cl-7243-bound-artifact-rate-limiter

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Contributor

Summary

createRunCreateRateLimiter (in packages/artifacts-hub/src/workflow-routes.ts) backs the sliding-window rate limit on POST / and POST /binary in the workflow-artifacts routes. It closed over a plain Map<string, number[]> keyed by workflow run id, created once per hub process, with no eviction — every finished run's entry stayed resident for the rest of the process's uptime (CL-7243).

  • Routes the map through @corbits/collections' createExpiringMap (CL-7233), with ttlMs: RATE_WINDOW_MS (the rate limiter's own 60s window).
  • Why this TTL is safe: every allow() call sets the entry, refreshing its expiry, so an entry only lapses after a full window with no calls for that run. By the time an entry could expire, every timestamp it held is already older than RATE_WINDOW_MS and would already have been filtered out of recent by the existing cutoff check — the two boundaries coincide exactly, so eviction never removes a timestamp the sliding window would otherwise still be counting. A shorter TTL could drop a still-in-window timestamp early and reset a caller's quota; this one can't.
  • Exported createRunCreateRateLimiter, RATE_WINDOW_MS, and MAX_CREATES_PER_RUN_PER_MINUTE for direct unit testing, plus an injectable now() (mirroring createExpiringMap's own hook) and a trackedRunCount getter so a test can assert on map size without a real clock.

Test plan

  • New test: many distinct run ids age out after RATE_WINDOW_MS of inactivity, map size returns to a small bound.
  • New test: a single run cannot exceed its per-window quota across the exact TTL boundary (eviction can't be exploited to reset the count early).
  • New test: one continuously-active run stays tracked while a burst of one-shot idle runs around it get reclaimed.
  • bun run typecheck
  • HUB_DATA_DIR=$(mktemp -d) bun test packages/artifacts-hub — 69 pass, 0 fail
  • bun run check:structural — all checks pass except check:report-error, which is already red on main and unrelated to this change
  • bunx prettier --check . — clean

Cover idle-run eviction and the sliding window's exact-boundary edge
so a caller can't exploit eviction timing to exceed the rate.
createRunCreateRateLimiter closed over a plain Map<string, number[]>
keyed by workflow run id, created once per hub process and never
pruned — every finished run's entry stayed resident for the rest of
the process's uptime. Route it through @corbits/collections'
createExpiringMap (CL-7233) with a TTL equal to RATE_WINDOW_MS, so an
idle run's entry is reclaimed instead of accumulating forever.

The TTL can't let a caller exceed the rate: every allow() call
re-sets the entry, refreshing its expiry, so an entry only lapses
after a full window with no calls for that run — by which point every
timestamp it held has already aged out of the sliding window's own
cutoff filter. A shorter TTL could evict an entry (and its
still-in-window timestamps) before the window's own filter would have
dropped them, resetting a caller's quota early; RATE_WINDOW_MS can't.
@TheGreatAxios
TheGreatAxios force-pushed the cl-7243-bound-artifact-rate-limiter branch from e2f78bc to 68fca77 Compare August 31, 2026 03:47
@TheGreatAxios
TheGreatAxios merged commit 7a4a5e7 into main Aug 31, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant