feat(cryptify): move the upload limits and rolling window into config, and serve the default tier's on GET /limits - #392
Merged
Conversation
The four byte limits and the rolling window were consts in `store.rs`, and `GET /usage` hardcoded the API-key tier's numbers a third time, independently of the two enforcement branches. Nothing served the default tier's limits at all: `/usage` is API-key gated, so the website kept its own copy of them. `per_upload_limit`, `rolling_limit`, `api_key_per_upload_limit`, `api_key_rolling_limit` and `rolling_window_days` are now `CryptifyConfig` keys, defaulting to the values the consts had, so no deployment changes behaviour. Every reader goes through an accessor, including `/usage`. The window is configured in days and reaches the store as a parameter on `Store::with_idle_ttl` and `prune_records`: `store.rs` takes the value, it does not fetch it. `GET /limits` is new and unauthenticated. It serves the default tier's two limits and the window, takes no credential guard, and carries nothing per-tenant, so its body is the same bytes for every caller. Closes #386 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g them `conf/config.toml` gained no entry for `per_upload_limit`, `rolling_limit`, `api_key_per_upload_limit`, `api_key_rolling_limit` or `rolling_window_days`, so an operator reading the shipped production config had no way to discover that the numbers moved out of the consts at all. #383 set the precedent one commit ago by documenting `attributed_email` there; follow it. The `rolling_window_days` block carries the caveat the field's doc comment does, since the config file is where an operator edits it. `api-description.yaml` stated the old constants as fixed facts in six descriptions: `/usage`'s summary ("the last 14 days"), its `limit_bytes` and `per_upload_limit_bytes` ("100 GB"), `/fileupload/init`'s Authorization ("100 GB", "5 GB") and both 413s. A consumer generating client copy from the spec would show those regardless of what the deployment is configured with. The `example:` values stay -- an example is not a guarantee. Description-only, so oasdiff sees no breaking change, and no `mod api_gate_tests` anchor covers these lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`StateDb` stored `rolling_window_secs` and `SharedState` stored the same number again. `StateDb::record_usage` is the field's only reader and its only caller, `Store::record_upload`, already holds `self.shared.rolling_window_secs` -- it uses it two lines later for the in-memory prune. So take the window as a parameter the way `prune_records` now does, and let `StateDb::open` go back to taking only a path. The field's own doc comment claimed "there is one authority for the window and it is not here" while being a second stored authority, and the two could be constructed independently: `StateDb::open` is called directly in tests with a hardcoded window and no `Store`. Nothing diverged, both being set from the same argument, but "two places hold this number" is the shape #386 set out to remove one layer up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…quota Both ends of `rolling_window_days` failed open, and only silently. `0` was accepted with no validation: `prune_records`' cutoff becomes `now`, so every recorded upload falls outside the window at once and the rolling quota stops enforcing -- on a path whose init and chunk PUT take no credential. It now logs a startup warning next to the `metrics_token` one. A warning rather than a rejection: 0 is a coherent thing for an operator to ask for, it just must not be silent. While the value was a const this state was unreachable. `rolling_window_secs()` multiplied `days as i64` by 86_400, which wraps silently in release for an absurd input, and a *negative* window makes `prune_records` drop every record it sees -- the same failure, reached from the other end. It saturates now, so a misconfiguration lands on "window far too long" instead of "quota off", with a test over `u64::MAX` and `i64::MAX`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rubenhensen
approved these changes
Aug 31, 2026
Open
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.
Closes #386.
Five config keys
per_upload_limit,rolling_limit,api_key_per_upload_limit,api_key_rolling_limitandrolling_window_daysare optional onRawCryptifyConfigand resolved onCryptifyConfig, followingchunk_size.They default to 5 GB / 5 GB / 100 GB / 100 GB / 14 days, which is what the five
consts in
store.rsheld, so no deployment changes behaviour. Those consts aregone.
The window is configured in days.
CryptifyConfigcarries bothrolling_window_days()androlling_window_secs(), and the doc comment on thefield records what the ticket asked it to: changing the window reinterprets
usage already recorded, which changing a byte limit never does.
Every reader goes through an accessor
The per-upload branch, the rolling branch, the
{}-dayin the rolling 413'smessage, and
GET /usage, which held its own copy of the API-key tier's numbersindependently of both branches. That third authority is most of why the ticket
exists;
usagenow takes&State<CryptifyConfig>like everything else.The window reaches the store through
Store::with_idle_ttl, which grows arolling_window_secs: i64parameter, and on toStateDbandprune_records.store.rstakes the value and never fetches it: no config is read there.GET /limits
Unauthenticated, mounted in
api_routes()and added toapi-description.yaml.{"per_upload_limit_bytes":5000000000,"rolling_limit_bytes":5000000000,"window_days":14}Default tier only. No guard, no
Option<ValidatedApiKey>, nothing per-tenant,so there is no path by which the body could vary with
Authorization. Itinherits the global CORS fairing and needs no preflight.
rolling_limit_bytesis deliberately not
/usage'slimit_bytes, since the two report differenttiers.
Tests
served_per_upload_limit_is_the_enforced_oneandserved_rolling_limit_is_the_enforced_oneread/limitsunauthenticated, drivean upload past each cap, and assert the 413's
limit_bytesequals what was justserved. The two numbers are compared against each other, not against a literal.
limits_are_byte_identical_for_every_callercompares the anonymous body againstone sent with an API key the mock pg-pkg really validates, checked by a 200 from
/usagewith the same key first.limits_serve_the_default_tier_onlyconfigures all four byte limits to distinctvalues and pins both which pair is served and that the body has exactly three
keys.
a_configured_rolling_window_reaches_limits_and_the_storesets the window to 3days and asserts
/limitsreports it andget_usage'soldest_expires_atmoves with it.
Five config tests, one per key: the default when the key is absent, the
configured value when it is set.
Test 3 shown red
As the acceptance check asks. Giving
limitsanApiKeyguard and returningthe API-key tier when a tenant resolves:
Restored afterwards. The committed tree is the one that passes.
Where the deleted consts' test readers get their number
Per the amendment on the issue. The store tests pass the window in as the new
parameter and compute their expectations from that same binding; the
main.rstest that asserted against
ROLLING_LIMITreads it off theCryptifyConfigitsfigment already built. No test-local const. The tests that never mention the
window pass it at the call site rather than through a shared name.
One anchor in
mod api_gate_testshad to movenarrow_a_response_property_typeanchored onwhich
/limitsnow also matches, sooncefailed on a non-unique anchor: themodule's own trap firing as designed. The anchor is now
USAGE_WINDOW_DAYS,which takes in the
per_upload_limit_bytesline that follows it in/usage.Same mutation, same verdict.
Scope
cryptify/CLAUDE.mdgained a line about where the numbers live now, since theold one said the store prunes on a 14-day window and the store no longer knows.
Nothing else moved: not
metrics.rs, notCOMPATIBILITY.md, nothing under.github/workflows/, not the website. The companionwebsite ticket in
postguard-jsis unblocked by this one.Verification
cargo test -p cryptify(219 passed),cargo clippy -p cryptify --all-targets -- -D warnings,cargo fmt --checkandcargo test --workspaceare all green.oasdiffis not installed in this container, so the gate's mutation testskipped locally; its anchor assertions ran and pass.
🤖 Generated with Claude Code
Reviewed by dobby: the
code-commentsrule over the full diff (no breach — config.rs, store.rs and main.rs all comment at their files' measured rate), plus a correctness/design pass; 4 findings, all 4 fixed in cbad854, d6b1434 and 643249d —cargo test -p cryptify(220 passed),cargo test --workspace,cargo clippy -p cryptify --all-targets -- -D warningsandcargo fmt --checkall green — approve.Note: the Scope section above no longer says
conf/config.tomlis untouched; the five keys are documented there now, which is one of the four fixes.