Repository navigation
Bound the workflow-artifact rate limiter's per-run map - #530
Merged
Merged
Conversation
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
force-pushed
the
cl-7243-bound-artifact-rate-limiter
branch
from
August 31, 2026 03:47
e2f78bc to
68fca77
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
createRunCreateRateLimiter(inpackages/artifacts-hub/src/workflow-routes.ts) backs the sliding-window rate limit onPOST /andPOST /binaryin the workflow-artifacts routes. It closed over a plainMap<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).@corbits/collections'createExpiringMap(CL-7233), withttlMs: RATE_WINDOW_MS(the rate limiter's own 60s window).allow()callsets 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 thanRATE_WINDOW_MSand would already have been filtered out ofrecentby the existingcutoffcheck — 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.createRunCreateRateLimiter,RATE_WINDOW_MS, andMAX_CREATES_PER_RUN_PER_MINUTEfor direct unit testing, plus an injectablenow()(mirroringcreateExpiringMap's own hook) and atrackedRunCountgetter so a test can assert on map size without a real clock.Test plan
RATE_WINDOW_MSof inactivity, map size returns to a small bound.bun run typecheckHUB_DATA_DIR=$(mktemp -d) bun test packages/artifacts-hub— 69 pass, 0 failbun run check:structural— all checks pass exceptcheck:report-error, which is already red onmainand unrelated to this changebunx prettier --check .— clean