feat(notary): add per-principal machine evaluation quota#276
Conversation
Registry Notary had per-request size caps and source limits, but no budget on sustained evaluate/batch_evaluate volume from a single machine principal. Add evidence.machine_quota: a fixed one-minute window, counted in subjects (a single evaluate costs 1, a batch costs items.len()), keyed by principal_id, and enforced only for principals that are not classified as self-attestation. A request whose cost would exceed the remaining budget is rejected whole, so a denied batch never partially consumes the window. Exhaustion returns 429 with the stable code evaluation.quota_exceeded and a Retry-After header. The quota is disabled by default (subjects_per_minute: 6000 when enabled), so existing deployments are unaffected until an operator opts in. Part of #78 (carries former #60 scope). Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 222696a96e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let batch_cost = u32::try_from(request.items.len()).unwrap_or(u32::MAX); | ||
| if let Err(error) = state | ||
| .machine_quota_limiter | ||
| .check_and_consume(&principal.principal_id, batch_cost) |
There was a problem hiding this comment.
When machine quota is enabled, this check runs before runtime.batch_evaluate reaches its existing idempotency-key lookup (store.idempotent_batch). If a client successfully submits a batch that uses the remaining budget but loses the response, retrying the same Idempotency-Key and body is rejected with 429 (or consumes more quota) instead of returning the cached response, so the batch endpoint is no longer safely retryable under the quota.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. Fixed in 5c0d46d: batch idempotency lookup now happens before machine quota debit, and idempotent replay returns the cached response without spending more quota. Added idempotent_batch_replay_does_not_consume_machine_quota; the machine quota HTTP suite passes.
| let batch_cost = u32::try_from(request.items.len()).unwrap_or(u32::MAX); | ||
| if let Err(error) = state | ||
| .machine_quota_limiter | ||
| .check_and_consume(&principal.principal_id, batch_cost) |
There was a problem hiding this comment.
With machine quota enabled, this consumes items.len() before runtime.batch_evaluate enforces the per-claim max_subjects cap. An over-limit batch can therefore either spend quota and then return 413, or return 429 instead of the existing batch.too_large response when its size exceeds the remaining quota, even though the request is invalid under the independent batch-size cap.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. Fixed in 5c0d46d: batch max-subject validation now runs before quota debit, so oversized batches keep returning 413 batch.too_large and do not consume machine quota. Added oversized_batch_does_not_consume_machine_quota; the machine quota HTTP suite passes.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c0d46d55d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `evaluation.quota_exceeded` and a `Retry-After` header giving the number of | ||
| seconds until the window rolls over. The quota is disabled by default; enable | ||
| it and size `subjects_per_minute` to the traffic pattern of your machine | ||
| callers before relying on it in production. |
There was a problem hiding this comment.
In multi-replica deployments this quota is only enforced per process: the limiter stores counters in an in-memory Mutex<HashMap<...>> and each RegistryNotaryApiState constructs its own limiter, so a client balanced across N Notary replicas can spend up to N× subjects_per_minute. Advising operators to size this before relying on it in production is misleading unless the docs call it per-instance or the limiter uses shared storage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed. Fixed in 47f608a: the operator config reference now states the quota is enforced per Notary instance, that N replicas give a caller up to N x subjects_per_minute, and points operators at replica-count sizing or a shared limiter.
The machine evaluation quota limiter keeps its counters in an in-memory map and each RegistryNotaryApiState builds its own limiter, so the quota is enforced per Notary process, not cluster-wide. Document that N replicas give a caller up to N x subjects_per_minute, and point operators at replica-count sizing or a shared limiter. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
|
Superseded by the integration meta-PR #292 — this branch is merged there unchanged (with cross-PR conflicts resolved once). Branch retained. |
1 participant
Summary
Adds
evidence.machine_quota: a per-principal budget for machineevaluate/batch_evaluatetraffic that accounts for request rate andbatch size together, instead of controlling them independently.
principal_id: a single/v1/evaluationscall costs 1, a/v1/batch-evaluationscall costsitems.len().(no partial consumption), so retrying with a smaller batch or waiting out
the window is safe.
429with the stable error codeevaluation.quota_exceededand aRetry-Afterheader (seconds until thewindow rolls over).
enabled: false,subjects_per_minute: 6000onceenabled), so existing deployments are unaffected until an operator opts in.
Part of #78 (carries former #60 scope: "machine batch quota").
Design rationale
crates/registry-notary-server/src/machine_quota.rs: afixed-window counter keyed by
principal_id, mirroring the structure ofthe existing
SelfAttestationRateLimiterbut with cost-aware (rather thanone-per-call) consumption and a bounded map with oldest-window eviction.
evaluate/batch_evaluatehandlers, in thenon-self-attestation branch, after self-attestation classification and
before the runtime is invoked.
EvidenceError::MachineQuotaExceeded { retry_after_seconds }is a new#[non_exhaustive]variant mapping to429/evaluation.quota_exceeded.per-request
max_subjectsbatch-size cap, source limits, or theself-attestation public rate limiters.
Review notes (security-sensitive)
This adds a new denial surface on the evaluation path, so flagging for
explicit review:
gated on
EvidencePrincipal::is_self_attestation()being false; adedicated integration test (
self_attestation_evaluate_succeeds_while_machine_quota_is_exhausted_for_same_principal_id)authenticates a self-attestation citizen JWT with the same
subas analready-exhausted machine principal and confirms the self-attestation call
still succeeds.
principal_id, which is operator-configured only underauth.mode: api_key(the key'sid). Underauth.mode: oidc,principal_idis the JWTsub(or whichever claimprincipal_claimnames) — i.e. caller-supplied, not a static operator-configured id. This
is the same identity Notary already uses for audit principal identity, but
it's worth calling out explicitly since it's now also a rate-limit key.
principal_idtoo long for the bounded key type istreated as over-quota rather than silently bypassing the limiter.
redaction logic.
Verification
From the worktree root:
cargo fmt --check— cleancargo check --locked --workspace --all-targets— cleancargo clippy --locked -p registry-notary-core -p registry-notary-server --all-targets -- -D warnings— cleancargo test --locked -p registry-notary-core -p registry-notary-server— all green:registry-notary-corelib: 322 passedregistry-notary-serverlib: 326 passed (includes 8 newmachine_quotaunit tests: exact boundary, window expiry, cost-exceeds-remaining rejects whole batch without partial consumption, map bound/eviction, disabled bypass, per-principal isolation, oversized-key fail-closed, zero-cost no-op)machine_quota_http_testintegration file: 5/5 passed (disabled-by-default allows repeated max-size batches; enabled quota returns 429 deterministically with stable code +Retry-After; two machine credentials have independent budgets; single-evaluate and batch-evaluate share one budget; self-attestation bypasses an exhausted machine quota for a collidingprincipal_id)registry-notary-serverintegration test files (standalone_http,concurrency_http_test,target_matching_contract_test, etc.) unchanged and passing429response on/v1/evaluationsand/v1/batch-evaluationsalready existed (from self-attestation rate limiting); updated its description to also mention the machine quota and regenerated the committed baseline viacargo run -q -p registry-notary -- openapi > products/notary/openapi/registry-notary.openapi.json.just openapi-check(fromproducts/notary) passes.just exposure-check(fromproducts/notary) currently fails onmainfor an unrelated, pre-existing reason (referenced enforcement test file does not exist: crates/registry-notary-server/src/standalone.rs, which is a path-resolution issue in the check script, not a missing file); confirmed viagit stashthat this failure reproduces identically without this branch's changes.products/notary/docs/operator-config-reference.mdand anUnreleased/Addedentry toproducts/notary/CHANGELOG.md.No deviations from the original design spec.