test(analyzers): negative regression tests + ARP eviction cosmetic refactor (PR #365 LOW follow-ups) by Zious11 · Pull Request #366 · Zious11/wirerust

Zious11 · 2026-07-06T17:40:48Z

Silent-limit LOW follow-ups: negative regression tests + ARP eviction cosmetic refactor

Source: Non-blocking LOW findings from PR #365 review (feat(analyzers): surface silently-dropped/evicted state via observability counters)
Severity: LOW (non-blocking)
Type: test + cosmetic refactor (no behavior change)

This PR addresses the 3 non-blocking LOW follow-up items raised during the PR #365 review cycle.
No user-facing behavior changes; all changes are test additions and an internal cosmetic refactor.

What Changed

Commit 1 — `30a26b6` test(analyzers): negative regression tests (HTTP-AC008-NEG-TEST-001 + EVICTION-NO-FINDING-NEG-TEST-001)

Two negative-path regression tests added to tests/bc_silent_resource_caps_tests.rs guarding
already-correct shipped behavior:

HTTP-AC008-NEG-TEST-001 (BC-2.06.024 AC-008 negative)

Asserts that repeated requests reusing existing Host/User-Agent keys do not increment
HttpAnalyzer::dropped_map_entries. Only refused NEW keys at the MAX_MAP_ENTRIES=50_000
cap may touch that counter.
Guards BC-2.06.024 AC-008: map-hit (existing key) must not be treated as a drop.

EVICTION-NO-FINDING-NEG-TEST-001 (BC-2.16.006 Inv3 / BC-2.16.008 Inv5 / BC-2.16.010 Inv7)

Part A (live, fast): after saturating the 256-slot Modbus pending table, the overflow
request increments dropped_transactions but produces zero Finding entries — drop events
are COUNTER-ONLY (no finding emitted).
Part B (#[ignore], ARP, ~8 s): fills MAX_ARP_BINDINGS=65_536 distinct IPs then inserts
one more; asserts the eviction call returns 0 findings and increments bindings_evicted.
Run with: cargo test ...arp_eviction_emits_no_finding -- --ignored

Commit 2 — `0c520da` refactor(arp): ARP-BINDINGS-EVICT-PRECHECK-COSMETIC-001

Pure cosmetic refactor: fold eviction detection into insert_binding_lru (returns bool),
deduplicating the two identical if self.bindings.len() >= MAX_ARP_BINDINGS call sites.

Before: two callers each checked length before calling insert_binding_lru (void).
After: insert_binding_lru returns true when an LRU eviction occurred; callers increment
bindings_evicted on true. Behavior is identical — verified by the --ignored increment tests.

Spec Traceability

flowchart LR
    BC1["BC-2.06.024<br/>HTTP map cap"] --> AC1["AC-008<br/>existing-key no-drop"]
    BC2["BC-2.16.006 Inv3<br/>Modbus drop=counter-only"] --> T2["EVICTION-NO-FINDING<br/>Part A (Modbus)"]
    BC3["BC-2.16.008 Inv5<br/>ARP eviction no-finding"] --> T3["EVICTION-NO-FINDING<br/>Part B (ARP, #[ignore])"]
    BC4["BC-2.16.010 Inv7<br/>counter invariant"] --> T3
    AC1 --> T1["HTTP-AC008-NEG-TEST-001"]
    T1 --> S1["tests/bc_silent_resource_caps_tests.rs"]
    T2 --> S1
    T3 --> S1
    S1 -.->|cosmetic refactor| S2["src/analyzer/arp.rs"]

Test Evidence

Suite	Result	Notes
`cargo test --all-targets`	All pass (0 failed)	Full suite including new tests
`cargo test -- --ignored` (5 tests)	All 5 pass	Includes both `#[ignore]` ARP eviction tests
`cargo clippy --all-targets -- -D warnings`	Clean	No warnings
`cargo fmt --check`	Clean	No formatting issues

New tests added:

bc_silent_resource_caps::http_ac008_neg_test_001_existing_key_no_drop_increment
bc_silent_resource_caps::eviction_no_finding_neg_test_001_modbus_drop_counter_only (live)
bc_silent_resource_caps::eviction_no_finding_neg_test_001_arp_eviction_emits_no_finding (#[ignore])

Demo Evidence

N/A — tests and cosmetic refactor, no user-facing behavior change.

Security Review

N/A — no security-relevant changes. Refactor is internal to src/analyzer/arp.rs; tests are
in tests/. No new public API surface, no input handling changes, no auth/authz changes.

Story Dependencies

graph LR
    PR365["PR #365<br/>merged develop cc2a87c"] --> ThisPR["test/silent-limit-followups<br/>this PR"]
    style PR365 fill:#90EE90
    style ThisPR fill:#FFD700

Depends on: PR #365 (merged, develop cc2a87c) — provides the observability counters under test.

Risk Assessment

Blast radius: Minimal. One test file added, one internal function signature changed (returns bool instead of void).
Behavior change: None. The refactor is semantically equivalent; verified by existing + new tests.
Performance impact: None.

AI Pipeline Metadata

Pipeline mode: feature (follow-up / fix-PR)
Model: claude-sonnet-4-6
No stubs/Red-Gate/wave-gate (fix-PR flow)
No demo recording (no behavior change)

Pre-Merge Checklist

…vability counters (PR #365 follow-ups) HTTP-AC008-NEG-TEST-001 (BC-2.06.024 AC-008 negative): Asserts that repeated requests using existing Host/User-Agent keys do NOT increment HttpAnalyzer::dropped_map_entries. Only refused NEW keys at the MAX_MAP_ENTRIES=50_000 cap must touch that counter. EVICTION-NO-FINDING-NEG-TEST-001 (BC-2.16.006 Inv3 / BC-2.16.008 Inv5 / BC-2.16.010 Inv7 / BC-2.14.012 v1.1 negative): Part A (live, fast): after saturating the 256-slot Modbus pending table, the overflow request increments dropped_transactions but must produce zero Finding entries — drop events are COUNTER-ONLY. Part B (#[ignore], ARP): fills MAX_ARP_BINDINGS=65_536 distinct IPs then inserts one more; asserts the eviction-triggering call returns 0 findings and increments bindings_evicted. Marked #[ignore] — ~8 s to fill 65_537 entries. Run with: cargo test ...arp_eviction_emits_no_finding -- --ignored.

…BINDINGS-EVICT-PRECHECK-COSMETIC-001 Eliminates the duplicated `if self.bindings.len() >= MAX_ARP_BINDINGS { bindings_evicted.saturating_add(1); }` pre-check that preceded each of the two `insert_binding_lru` call sites (GARP no-conflict path and non-GARP new-IP path). Change: `insert_binding_lru` now returns `bool` — `true` when an LRU eviction occurred (new IP at cap), `false` for update-in-place or insert below cap. Each call site increments `bindings_evicted` iff the return value is `true`, making the eviction detection single-source. Observable behavior is unchanged: - `bindings_evicted` still increments by exactly 1 per actual LRU eviction at the same moments (proven by the ignored regression tests). - No change to what gets inserted/evicted, no new/removed Findings, no change to `summarize()` output. - `storm_counters_evicted` path (`insert_storm_counter_lru`) untouched. Tests confirming correctness after refactor: - `test_BC_2_16_008_bindings_evicted_increments_at_cap` (--ignored) PASS - `test_EVICTION_NO_FINDING_NEG_TEST_001_arp_eviction_emits_no_finding` (--ignored) PASS - Full `cargo test --all-targets` suite: all pass, 0 failures.

…D-386) v0.11.4 RELEASED 2026-07-06: - PR #366 (squash to develop): 3 silent-limit follow-up items RESOLVED: HTTP-AC008-NEG-TEST-001 (negative regression test, no dropped_map_entries on hit) EVICTION-NO-FINDING-NEG-TEST-001 (Modbus + ARP eviction emit no Finding) ARP-BINDINGS-EVICT-PRECHECK-COSMETIC-001 (insert_binding_lru returns bool; dedup) - PR #367 release/0.11.4 → main (merge f0f2136); tag v0.11.4 (obj e6ee614) - PR #368 back-merge main → develop (squash f7460b4); Cargo.toml 0.11.4 - develop has NO unreleased commits; pipeline IDLE Informational note added (REBIND-COUNT-SATURATING-001): rebind_count in src/analyzer/arp.rs uses plain += — pre-existing LOW/informational, same class as cleared SEC-004+SEC-007; DF-VALIDATION-001-gated before any issue. STATE.md updates: frontmatter (released_version, HEADs, cargo_version, timestamp, current_step), EXACT RESUME POINT, Project Metadata table, Phase Progress new row, Current Phase Steps rotation, Decisions Log D-386, Backlog (3 RESOLVED + 1 new LOW), Session Resume Checkpoint (develop f7460b4 / main f0f2136, NO unreleased commits).

Human-requested pause (D-387, 2026-07-06) after v0.11.4 release. Changes: - STATE.md: pipeline IDLE→PAUSED; timestamp refreshed; current_step updated to session-wrap note; D-387 added to Decisions Log; D-381 Current Phase Steps row rotated to burst-log (last-5 rule); Session Resume Checkpoint replaced (prior IDLE checkpoint archived to cycles/feature-protocol-coverage/session-checkpoints.md). - code-delivery/silent-limit-followups/pr-description.md: untracked PR #366 description artifact included to clean working tree. - cycles/feature-protocol-coverage/burst-log.md: archived D-381 Current Phase Steps row. - cycles/feature-protocol-coverage/session-checkpoints.md: archived prior IDLE checkpoint (superseded by D-387). Ground truth at pause: develop=f7460b4 (Cargo.toml 0.11.4, NO unreleased commits), main=f0f2136 (v0.11.4, tag obj e6ee614).

…#370) **Maintenance run:** maint-2026-07-06 FIX-B **Mode:** maintenance / fix **Convergence:** N/A — evaluated at wave gate ![Tests](https://img.shields.io/badge/tests-9%2F9%20new%20pass%2C%20453%20pre--existing%20green-brightgreen) ![Coverage](https://img.shields.io/badge/coverage-additive%20only-brightgreen) ![Holdout](https://img.shields.io/badge/holdout-pre--checked%20no%20scenario%20asserts%20exact%20DNP3%20key%20counts-blue) This PR closes three genuine observability gaps identified by the maint-2026-07-06 pattern-consistency sweep (FIX-B). Three new monotonic counters surface DNP3 analyzer state that was previously silently dropped or evicted at hard resource caps. No behavior changes: detection logic, Finding emission, and all invariants are unchanged. Counters are purely additive — new keys appear in `summarize()` JSON output only. **Direct precedent:** PR #365 / PR #366 (same counter pattern for ARP `bindings_evicted`/`storm_counters_evicted`, Modbus `dropped_transactions`, HTTP/TLS `dropped_map_entries`). **Summary — new DNP3 counter keys after this PR:** | Counter | Cap constant | Semantics | |---------|-------------|-----------| | `dropped_findings` | `MAX_FINDINGS = 10_000` | Incremented at each of 11 cap-check sites when a finding is suppressed; `&mut u64` threaded through 10 detection functions | | `master_addrs_dropped` | `MAX_MASTER_ADDRS = 64` | Incremented only for new-unique master addresses silently ignored because the cap is full; existing-address hits do NOT increment (BC-2.15.016 v2.1 PC-6) | | `pending_requests_evicted` | `MAX_PENDING_REQUESTS = 256` | Incremented when `insert_pending_request` LRU-evicts an entry; function now returns `bool` — consistent with the `insert_binding_lru`-returns-bool pattern from PR #366 (ARP) | --- ## Architecture Changes ```mermaid graph TD Dnp3Analyzer["Dnp3Analyzer\n(src/analyzer/dnp3.rs)"] -->|"dropped_findings\nmaster_addrs_dropped\npending_requests_evicted"| Summarize["summarize() → JSON"] style Dnp3Analyzer fill:#90EE90 ``` <details> <summary><strong>Architecture Decision</strong></summary> ### ADR: Additive observability counters, no behavior change **Context:** Three DNP3 resource caps silently drop or evict state without any observable signal. Operators could not distinguish "no cap pressure" from "high cap pressure" on `pending_requests`, `master_addrs_seen`, or the findings list. **Decision:** Add one monotonic saturating counter per silent-drop/eviction site; expose each counter as a new key in `Dnp3Analyzer::summarize()` JSON detail map. Counters are always present (value=0 when no events occurred). **Rationale:** Purely additive — zero risk of breaking existing consumers. No Finding is emitted on any eviction/drop event (observability-only). Consistent with v0.11.4 counter pattern established in PR #365. **Implementation note — `dropped_findings` vs. `insert_pending_request`:** `dropped_findings` is threaded as `&mut u64` through the 10 detection functions that hold cap-check guards, mirroring the existing `&mut Vec<Finding>` pattern. `pending_requests_evicted` uses the `insert_pending_request`-returns-`bool` pattern (bool = eviction occurred), consistent with PR #366's `insert_binding_lru`-returns-bool for ARP. `master_addrs_dropped` increments directly in the frame-walk `else` branch (no helper function involved). **Alternatives Considered:** 1. Emit a `Finding` on each eviction — rejected: violates BC-2.15.022 Invariant 1 (DoS cap), would inflate finding counts. 2. Log to stderr on eviction — rejected: not machine-readable. </details> --- ## DF-VALIDATION-001 Evidence **File:** `.factory/research/df-validation-pc019-pc020-2026-07-06.md` **Policy enforced:** `DF-VALIDATION-001` (`.factory/policies.yaml`) All three pattern-consistency findings were independently verified before this PR was filed: | Canonical ID | Claim | Verdict | |---|---|---| | PC-016 (team-lead: PC-019) | `master_addrs_seen` silent-ignore has no counter | **CONFIRMED** (observability gap); T1692.001-masking mechanism **REFUTED** — the cap gates the push, not the membership check, so rogue-source detection still fires for any address not in the (full) set | | PC-017 (team-lead: PC-020) | `pending_requests` LRU eviction has no counter | **CONFIRMED** — in-code doc comment at `dnp3.rs:1797` explicitly states the eviction produces no T1691.001 event; T1691.001 correlation degradation path confirmed | | PC-003 | `dropped_findings` counter missing on 11 MAX_FINDINGS cap sites | **CONFIRMED** — Modbus and ENIP both carry equivalent counters; DNP3 was the sole omission | **Important:** `master_addrs_dropped` counter is added for **observability parity** with the v0.11.4 pattern (ARP, Modbus, HTTP, TLS). The PR does NOT claim this counter restores T1692.001 detection capability — T1692.001 detection is not gated by the cap (validated per research file above). --- ## Spec Traceability **BC-INDEX version: v2.19 Amendment 5.** ```mermaid flowchart LR BC016["BC-2.15.016 v2.1\nPC-6: master_addrs cap\nPC-10: pending_requests LRU"] --> AC_MA["master_addrs_dropped\nkey in summarize()"] BC016 --> AC_PE["pending_requests_evicted\nkey in summarize()"] BC022["BC-2.15.022\nMAX_FINDINGS=10,000 cap"] --> AC_DF["dropped_findings\nkey in summarize()"] BC020["BC-2.15.020\nDNP3 observability counters"] --> AC_MA BC020 --> AC_PE BC020 --> AC_DF AC_MA --> T["tests/bc_2_15_020_dnp3_observability_counters_tests.rs"] AC_PE --> T AC_DF --> T T --> S["src/analyzer/dnp3.rs"] ``` | Behavioral Contract | Amendment | Counter | Status | |--------------------|-----------|---------|--------| | BC-2.15.016 v2.1 PC-6 | master_addrs cap | `master_addrs_dropped` | PASS | | BC-2.15.016 v2.1 PC-10 | pending_requests LRU | `pending_requests_evicted` | PASS | | BC-2.15.020 | DNP3 observability counters | all three | PASS | | BC-2.15.022 | MAX_FINDINGS DoS cap | `dropped_findings` | PASS | --- ## Test Evidence (TDD — Red Gate First) | Step | Commit | Details | |------|--------|---------| | Red gate | `4defc7b` | 9 new tests committed first; all 9 fail (keys absent from `summarize()`); all 453 pre-existing tests pass | | Green gate | `636c0d6` | Implementation committed; 9/9 new tests pass; 453 pre-existing tests still green | **Test file:** `tests/bc_2_15_020_dnp3_observability_counters_tests.rs` (722 lines, new file) **Test coverage:** - `dropped_findings`: verifies counter increments at MAX_FINDINGS boundary; verifies eviction counter does NOT fire on `scan_block_timeouts` age-out or on normal finding completion (negative tests) - `master_addrs_dropped`: verifies counter increments only for new-unique addresses at cap; existing addresses do NOT increment counter - `pending_requests_evicted`: verifies counter increments on LRU eviction; verifies counter does NOT fire on normal completion or `scan_block_timeouts` age-out - Counters are observability-only: no `Finding` is emitted for any of the three counter increment paths **Holdout pre-check:** No holdout scenario asserts exact DNP3 summary key counts — additive counter keys cannot break existing scenarios. --- ## Risk Assessment - **Blast radius:** DNP3 `summarize()` JSON output only — new keys, zero behavior change - **Risk level:** LOW (additive, no behavioral change, saturating arithmetic) - **Memory delta:** +24 bytes per `Dnp3Analyzer` instance (3 × `u64`) - **Throughput delta:** negligible (one add per rare eviction event) --- ## Pre-Merge Checklist - [ ] All CI status checks passing - [x] TDD Red gate committed first (4defc7b) - [x] 9/9 new tests pass; 453 pre-existing tests green - [x] DF-VALIDATION-001 evidence on file before PR filed - [x] PC-016 T1692.001-masking claim explicitly NOT asserted in PR body - [x] Precedent pattern consistent (insert_pending_request returns bool, per PR #366) - [x] BC-INDEX v2.19 Amendment 5 traced to tests - [x] Holdout pre-checked — no impact on existing scenarios - [ ] Code review, security review, fresh-eyes review dispatched post-creation

Zious11 added 2 commits July 6, 2026 11:32

Zious11 merged commit acfcd33 into develop Jul 6, 2026
11 checks passed

Zious11 deleted the test/silent-limit-followups branch July 6, 2026 17:45

Zious11 mentioned this pull request Jul 6, 2026

chore: release v0.11.4 #367

Merged

5 tasks

Zious11 mentioned this pull request Jul 6, 2026

feat(dnp3): surface silently-dropped state via observability counters #370

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(analyzers): negative regression tests + ARP eviction cosmetic refactor (PR #365 LOW follow-ups)#366

test(analyzers): negative regression tests + ARP eviction cosmetic refactor (PR #365 LOW follow-ups)#366
Zious11 merged 2 commits into
developfrom
test/silent-limit-followups

Zious11 commented Jul 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Zious11 commented Jul 6, 2026

Silent-limit LOW follow-ups: negative regression tests + ARP eviction cosmetic refactor

What Changed

Commit 1 — 30a26b6 test(analyzers): negative regression tests (HTTP-AC008-NEG-TEST-001 + EVICTION-NO-FINDING-NEG-TEST-001)

Commit 2 — 0c520da refactor(arp): ARP-BINDINGS-EVICT-PRECHECK-COSMETIC-001

Spec Traceability

Test Evidence

Demo Evidence

Security Review

Story Dependencies

Risk Assessment

AI Pipeline Metadata

Pre-Merge Checklist

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Commit 1 — `30a26b6` test(analyzers): negative regression tests (HTTP-AC008-NEG-TEST-001 + EVICTION-NO-FINDING-NEG-TEST-001)

Commit 2 — `0c520da` refactor(arp): ARP-BINDINGS-EVICT-PRECHECK-COSMETIC-001