diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 507c58c..8e7bf41 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,39 +40,30 @@ jobs: - uses: Swatinem/rust-cache@258712b0b7b1ddf8bddc9fc3b0faca682b2736c3 # v2 with: workspaces: dpp-engine - - name: cargo clippy - working-directory: dpp-engine - run: cargo clippy --workspace --all-targets -- -D warnings - - check-features: - name: Feature-gated code compiles - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 - with: - path: dpp-engine - - uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4 # stable - with: - toolchain: stable - - uses: Swatinem/rust-cache@258712b0b7b1ddf8bddc9fc3b0faca682b2736c3 # v2 - with: - workspaces: dpp-engine - # The clippy and test-unit jobs above build default features only, so the - # `integration-tests` suites are compiled nowhere except the Docker tiers - # — which need `test-unit` to pass first. A stale import in one of them - # therefore surfaces late, or not at all when Docker is unavailable. This - # is compile-only: no test runs, no Docker, so it can sit in the fast lane. + # Linted with the `integration-tests` features ON, which folds in what a + # separate compile-only job used to do. + # + # Those suites are otherwise compiled nowhere except the Docker tiers, + # which need `test-unit` to pass first — so a stale import in one surfaced + # late, or not at all when Docker was unavailable. Clippy with the features + # on type-checks them and lints them, which a `cargo check` did not. + # + # Safe to fold rather than run twice: features are additive and the + # workspace has **zero** `cfg(not(feature = "integration-tests"))` code, so + # nothing compiled by the default build is skipped by this one. If that + # ever stops being true, the default-feature pass has to come back. # # Features are named per package rather than via `--all-features`, which # would also enable `cli/desktop` (pulls rfd's ashpd/Wayland subtree, kept # out of server-side builds on purpose — see cli/Cargo.toml) and # `dpp-plugin-host/wasm-fixture-tests` (needs the wasm32-wasip1 target and - # a nested cargo build). Neither is the gap this closes. - - name: cargo check with the integration-tests features on + # a nested cargo build). Neither is a gap this closes. + - name: cargo clippy working-directory: dpp-engine run: | - cargo check --workspace --all-targets \ - --features dpp-dal/integration-tests,dpp-vault/integration-tests,dpp-plugin-host/integration-tests,dpp-node/integration-tests + cargo clippy --workspace --all-targets \ + --features dpp-dal/integration-tests,dpp-vault/integration-tests,dpp-plugin-host/integration-tests,dpp-node/integration-tests \ + -- -D warnings test-unit: name: Unit tests diff --git a/crates/dpp-resolver/src/main.rs b/crates/dpp-resolver/src/main.rs index a7342af..7a6c24c 100644 --- a/crates/dpp-resolver/src/main.rs +++ b/crates/dpp-resolver/src/main.rs @@ -32,12 +32,29 @@ use dpp_resolver::{config::Config, infra::cache::Cache, router, state::AppState} /// keep memory bounded regardless of input. const MAX_BUCKETS: usize = 50_000; +/// How much smaller the expiry sweep's threshold is than the hard cap. +/// +/// The sweep runs first and drops only *expired* buckets; the cap is the +/// backstop for when none are. Derived rather than a second constant so the two +/// cannot drift, and so a test can shrink both together — `MAX_BUCKETS / +/// SWEEP_DIVISOR` is exactly the 10,000 that was hardcoded here. +const SWEEP_DIVISOR: usize = 5; + struct RateLimiter { max: u32, window: Duration, /// Whether to trust the `X-Forwarded-For` header (only safe behind a proxy /// that sets it and strips inbound copies). Off by default. trust_forwarded_for: bool, + /// Hard cap on bucket count. Always [`MAX_BUCKETS`] in production — + /// [`RateLimiter::new`] is the only constructor a binary calls. + /// + /// A field rather than a constant read directly so the bound can be proven + /// against a small cap. The property under test is "the map never exceeds + /// its cap", which does not depend on the cap being 50,000 — and driving + /// 55,000 iterations through a debug build to assert it cost 50 seconds, + /// making it the slowest test in the workspace by a factor of ~500. + max_buckets: usize, buckets: Mutex>, } @@ -47,21 +64,29 @@ impl RateLimiter { max, window, trust_forwarded_for, + max_buckets: MAX_BUCKETS, buckets: Mutex::new(HashMap::new()), } } + /// Shrink the bucket cap. Test-only: no binary path sets this. + #[cfg(test)] + fn with_max_buckets(mut self, max_buckets: usize) -> Self { + self.max_buckets = max_buckets; + self + } + /// Returns `true` if the request from `ip` is within the limit. fn check(&self, ip: IpAddr) -> bool { let now = Instant::now(); let mut buckets = self.buckets.lock().unwrap(); // Bound memory: drop expired buckets once the map grows large. - if buckets.len() > 10_000 { + if buckets.len() > self.max_buckets / SWEEP_DIVISOR { buckets.retain(|_, (start, _)| now.duration_since(*start) < self.window); } // Hard cap: under a flood of fresh (spoofed) IPs none are expired, so the // expiry retain above can't shrink the map. Clear it to stay bounded. - if buckets.len() > MAX_BUCKETS { + if buckets.len() > self.max_buckets { buckets.clear(); } let entry = buckets.entry(ip).or_insert((now, 0)); @@ -342,15 +367,31 @@ mod security_regression { fn bucket_map_stays_bounded_under_fresh_ip_flood() { // A flood of distinct, never-expiring IPs must not grow the map without // bound; the hard cap clears it well before it can exhaust memory. - let limiter = RateLimiter::new(1, Duration::from_secs(3600), false); - for i in 0u32..(MAX_BUCKETS as u32 + 5_000) { + // + // Driven against a small cap. The property is that the map never + // exceeds `max_buckets`, which holds at any cap — and asserting it at + // the production 50,000 meant 55,000 iterations through a debug build, + // 50 seconds, and the slowest test in the workspace by ~500x. The cap's + // production *value* is pinned separately below. + const CAP: usize = 50; + let limiter = RateLimiter::new(1, Duration::from_secs(3600), false).with_max_buckets(CAP); + for i in 0u32..(CAP as u32 + 500) { let ip = IpAddr::from((0x0a00_0000u32 + i).to_be_bytes()); limiter.check(ip); } let len = limiter.buckets.lock().unwrap().len(); - assert!( - len <= MAX_BUCKETS, - "bucket map grew to {len}, cap {MAX_BUCKETS}" - ); + assert!(len <= CAP, "bucket map grew to {len}, cap {CAP}"); + } + + /// The cap the binary actually runs with. + /// + /// Separated from the property test so shrinking the cap there cannot + /// quietly shrink it in production: that test proves the bound holds, this + /// one proves the bound is the one we intend. + #[test] + fn the_default_limiter_uses_the_production_cap() { + let limiter = RateLimiter::new(1, Duration::from_secs(60), false); + assert_eq!(limiter.max_buckets, MAX_BUCKETS); + assert_eq!(MAX_BUCKETS / SWEEP_DIVISOR, 10_000, "sweep threshold moved"); } } diff --git a/justfile b/justfile index c276b3e..9316164 100644 --- a/justfile +++ b/justfile @@ -438,3 +438,15 @@ clean: # `test-harness` feature; this is the signal that was missing. harness-check: bash scripts/harness-check.sh + +# Run only the tests a change can affect. +# +# Maps changed files to their crates and hands nextest an `rdeps()` filterset. +# Measured against the 1,041-test workspace: a dpp-resolver edit selects 67 +# tests, dpp-integrator 247, dpp-vault 471, dpp-dal 511. +# +# An iteration aid, not a gate. It reasons about crate boundaries, not +# behaviour, and falls back to the full suite whenever a change is not +# attributable to one crate. Run `just check` before pushing regardless. +test-changed BASE="origin/main": + bash scripts/test-changed.sh {{ BASE }} diff --git a/scripts/test-changed.sh b/scripts/test-changed.sh new file mode 100644 index 0000000..83833f8 --- /dev/null +++ b/scripts/test-changed.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# Run only the tests a change can affect. +# +# Maps changed files to the crates that own them, then hands nextest an +# `rdeps()` filterset — every test in those crates and in everything that +# depends on them. Measured selectivity against the 1,041-test workspace: +# a dpp-resolver edit selects 67, dpp-integrator 247, dpp-vault 471, dpp-dal 511. +# +# This is an iteration aid, never a substitute for `just check` before pushing. +# It reasons about crate boundaries, not behaviour: a change that alters a +# runtime contract without touching the dependent crate's source is invisible to +# it, and so is anything reached only through a trait object. +# +# Falls back to the full suite whenever the blast radius is not a crate — a +# manifest, a migration, CI config, or a file it cannot attribute. Erring toward +# running everything is the only safe direction for a tool that decides what to +# skip. +# +# Usage: +# just test-changed # working tree + commits, against origin/main +# just test-changed HEAD~3 # against another base +set -euo pipefail + +base="${1:-origin/main}" + +if ! git rev-parse --verify --quiet "$base" > /dev/null; then + echo "test-changed: '$base' is not a revision this repo knows; falling back to main." >&2 + base="main" +fi + +# Committed since the base, plus anything uncommitted — the point is to be +# useful mid-edit, not only after a commit. +changed=$( + { + git diff --name-only "$base"...HEAD + git diff --name-only + git diff --name-only --cached + git ls-files --others --exclude-standard + } | sort -u +) + +if [ -z "$changed" ]; then + echo "test-changed: nothing changed against $base." + exit 0 +fi + +# Paths whose blast radius is not one crate. A migration reshapes the schema +# every DB-backed suite asserts against; a manifest can move any dependency +# edge; CI and nextest config change how the whole suite runs. +full_suite_globs='^(Cargo\.toml|Cargo\.lock|\.config/|\.github/|ops/|justfile|scripts/|deny\.toml|rust-toolchain\.toml)' + +crates="" +unattributed="" +while IFS= read -r file; do + [ -z "$file" ] && continue + + if printf '%s' "$file" | grep -qE "$full_suite_globs"; then + echo "test-changed: '$file' can affect any crate — running the full suite." + exec cargo nextest run --workspace --all-features + fi + + case "$file" in + crates/*) + # crates//... -> + rest="${file#crates/}" + crates="$crates ${rest%%/*}" + ;; + cli/*) + crates="$crates dpp-cli" + ;; + api/*|docs/*|*.md) + # Prose and the API description compile nothing. + ;; + *) + unattributed="$unattributed $file" + ;; + esac +done <<< "$changed" + +if [ -n "$unattributed" ]; then + echo "test-changed: cannot attribute$unattributed to a crate — running the full suite." + exec cargo nextest run --workspace --all-features +fi + +# shellcheck disable=SC2086 +crates=$(printf '%s\n' $crates | sort -u) + +if [ -z "$crates" ]; then + echo "test-changed: only non-compiling files changed; nothing to run." + exit 0 +fi + +filter="" +while IFS= read -r c; do + [ -z "$c" ] && continue + if [ -n "$filter" ]; then + filter="$filter + rdeps($c)" + else + filter="rdeps($c)" + fi +done <<< "$crates" + +echo "test-changed: $filter" +cargo nextest run --workspace --all-features -E "$filter"