Repository navigation
feat(id): shared abuse evaluator core — buckets, structured refusal, counter registry (#3881) - #3891
Conversation
…3881) P0-A core of #2244 (design #2033, defaults #2032). New std-only module pkgs/id/src/abuse.rs; nothing is wired into existing paths and no existing limiter changes behaviour (Packet B integrates). - AbusePolicy defaults: authorized endpoint 60 frames/s burst 180 and 512 KiB/s burst 4 MiB; pre-authorization 10 frames/s burst 30; max 4 concurrent connections per peer. - AbuseEvaluator: per-endpoint frame/byte token buckets (integer milli-token arithmetic, injected now_ms) shared across all of an endpoint's connections — more streams do not multiply allowance. - Refusal: the one structured refusal contract — reason (rate_limited | budget_exhausted | blocked_by_operator | unauthorized) + retry_after_ms clamped to 1s..60s for temporary refusals, absent for permanent operator blocks. - CounterRegistry: per-(local node, remote endpoint) frame/byte counters with a deterministic snapshot API. Unit tests (8/8 via standalone rustc --test; cargo/CI verification on the PR): defaults, frame/byte drain+refill, shared allowance across connections, per-peer connection cap, pre-auth bucket separation, retry bounds, counter increments + snapshot.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d20bddd90
ℹ️ 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".
| self.endpoints | ||
| .entry(endpoint) | ||
| .or_insert_with(|| EndpointState::new(&policy, now_ms)) |
There was a problem hiding this comment.
For a long-lived evaluator handling successive distinct endpoint keys, every open_connection or traffic check permanently inserts an EndpointState. close_connection only removes the connection ID, and neither the endpoint map nor the counter registry has an eviction or pruning path. Consequently, even peers that send one pre-authorization frame and disconnect retain allocations indefinitely; the per-peer connection cap does not bound this growth. A standalone test with 10,000 such peers confirmed that all entries remain after advancing time by a day. Add bounded retention or idle-expiry handling for endpoint state and associated counters, while preserving depleted buckets long enough to prevent reconnects from resetting allowances.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 162b374. Retained state is now bounded by pruning on both stores:
AbuseEvaluator::prune_idle_endpoints(now_ms)evicts endpoint states with no open connections whose four buckets have all fully refilled. A fully-refilled, connection-less state is indistinguishable from a never-seen endpoint, so eviction changes no admission decision (covered by a test: an evicted endpoint gets its full fresh burst; endpoints with open connections or partially drained buckets are kept).- The counter registry now stamps each row with its last-recorded-traffic time, and
CounterRegistry::prune(now_ms, retention_ms)(exposed asAbuseEvaluator::prune_counters) drops rows idle past the caller-chosen retention. The module docs direct callers to run both periodically and to snapshot before pruning if lifetime totals are needed.
New tests: prune_evicts_only_idle_fully_refilled_endpoints, prune_counters_drops_rows_idle_past_retention.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
P0-A core of #2244 (design #2033 in PR #3834; defaults from #2032). New module only — nothing is wired into frames/HTTP/world chat (that is Packet B), and no existing path changes behavior.
What
New
pkgs/id/src/abuse.rs(std-only, time injected asnow_msfor determinism):AbusePolicywith the id: investigate: comms abuse controls — blocks, rate limits, and per-counterparty budgets #2032/id: design: abuse-control primitives v1 for ID comms #2033 defaults: authorized endpoint 60 frames/s burst 180 and 512 KiB/s burst 4 MiB; pre-authorization 10 frames/s burst 30; max 4 concurrent connections per peer. (No separate pre-auth byte default exists in id: investigate: comms abuse controls — blocks, rate limits, and per-counterparty budgets #2032/id: design: abuse-control primitives v1 for ID comms #2033; the pre-auth byte bucket matches the authorized surface until a later phase sets one — noted in the module docs.)AbuseEvaluator— per-endpoint frame and byte token buckets (integer milli-token arithmetic) keyed by endpoint alone, so the allowance is shared across all of a peer's connections; plus the per-peer concurrent-connection cap (open_connection/close_connection).Refusal— the one structured refusal contract:reason(rate_limited|budget_exhausted|blocked_by_operator|unauthorized) +retry_after_msclamped to the 1 s floor / 60 s cap for temporary refusals, absent for permanent operator blocks. The localRefusalenums inevent_store/ingest.rsanddirectory.rsare untouched — this is the shared contract Packet B will adapt them to.CounterRegistry— per-(local node, remote endpoint) frame/byte counters, recorded by the evaluator for admitted traffic, with a deterministic (sorted) snapshot API for the observability surface.Tests
Unit tests cover: defaults vs the design, frame bucket drain/refill, byte bucket drain/refill, endpoint allowance shared across connections, per-peer connection cap (incl. close freeing a slot and a different peer's separate cap), pre-auth bucket separation, refusal retry bounds (floor, cap, permanent = none), and counter increments + snapshot.
Verified standalone on the laptop with
rustc --edition 2024 --test(the module is std-only): 8/8 pass. Full cargo test + clippy for the crate run in CI on this PR — the laptop's cold target dir for a fresh worktree could not finish a cargo build inside the shift budget, so the task is parked for the CI result.Queue: task 5845 (repo-health-code-pilot), lane A.
Closes #3881
Need help on this PR? Tag
@codesmith-botwith what you need.