Skip to content

Vacuous simulation and fuzz oracles, plus missing regression gates, weaken test soundness #169

Description

@al8n

Vacuous simulation and fuzz oracles, plus missing regression gates, weaken test soundness

Audit scope

Medium severity

1. Property test checks membership presence, not Alive-state propagation

Location

  • tests/memberlist-simulation/tests/property.rs:1-5,23-30,60-70
  • tests/memberlist-simulation/src/cluster.rs:734-745
  • memberlist-proto/src/endpoint/mod.rs:900-915

Invariant/claim

The property test claims that every Alive node eventually appears at every peer and that Alive state propagates through the cluster.

Evidence/trace

The sole final oracle at property.rs:60-70 is:

c.member(host_addr, peer_id).is_some()

Cluster::member delegates to Endpoint::member. That endpoint API returns the presence of a stored wire-format NodeState.

The adjacent API documentation explicitly states that the wire state returned by member() is fixed at insertion and that current gossip liveness must instead be read using member_liveness.

Consequently, a retained member row passes the property even when its current liveness is Suspect, Dead, or Left.

Impact

The test can remain green while the advertised Alive-propagation invariant is broken. It proves eventual row presence, not eventual liveness convergence.

Remediation/acceptance tests

  • Require member_liveness(...) == Some(State::Alive) for every expected observer/member pair.
  • Ideally also verify ID, address, and incarnation agreement, or use ConvergenceChecker if it expresses the intended invariant.
  • Add a mutation or negative test in which the row remains present but has terminal/non-Alive liveness, and prove that the property fails.

2. TCP/TLS reliable-fallback conformance tests can pass without exercising fallback

Location

  • tests/memberlist-simulation/tests/tcp_conformance.rs:118-141
  • tests/memberlist-simulation/tests/tls_conformance.rs:100-123
  • tests/memberlist-simulation/src/tcp_net/mod.rs:634-640
  • tests/memberlist-simulation/src/tls_net/mod.rs:712-718
  • memberlist-proto/src/endpoint/mod.rs:2204-2215
  • Positive control: tests/memberlist-simulation/tests/quic_conformance.rs:196-274

Invariant/claim

When UDP probing is blocked, the reliable TCP/TLS fallback should open, rescue the probe without a false suspicion, and then be reaped.

Evidence/trace

The TCP and TLS tests:

  • warm the cluster without asserting that warm-up reached Alive;
  • ignore the boolean returned by trigger_probe;
  • assert only that no suspicion was observed and that the final live_bridge_count is zero.

The harness returns the production start_probe boolean. Its documentation states that false means no eligible target existed. Therefore, the test does not establish that a probe started.

A zero bridge count also satisfies the final assertion before the loop begins. The test can pass if:

  • no probe starts;
  • the fault hook is ineffective;
  • pending work disappears;
  • reliable fallback never opens; or
  • fallback is bypassed.

The QUIC sibling is a working positive control: it asserts warm-up Alive, verifies there are no stale bridges, observes a fallback bridge open and later reap, confirms the node was never Suspected, and verifies it remains Alive.

Impact

Regressions that disable or bypass TCP/TLS reliable fallback can pass the conformance suite. The current green result does not distinguish successful recovery from failure to exercise the scenario.

Remediation/acceptance tests

Port the QUIC lifecycle oracle to TCP and TLS:

  • assert successful Alive warm-up;
  • assert trigger_probe returns true;
  • assert no old bridge exists before the probe;
  • observe the reliable bridge transition from open to reaped;
  • bound virtual time below the next periodic probe;
  • assert the target is never Suspected and remains Alive.

3. Advertised VOPR push regression gate does not run on the default branch

Location

  • .github/workflows/vopr.yml:7-12,27-34
  • .github/workflows/ci-core.yml:78-114

Invariant/claim

The VOPR workflow comments advertise rerunning the default 4096-seed × 5000-step band on pushes touching simulation/state-machine code.

Evidence/trace

on.push.branches names only:

  • refactor/sansio
  • vopr

The repository default branch is main, verified through GitHub. Merges and direct pushes to main therefore do not trigger the advertised push gate.

This is not a total absence of simulation testing:

  • ci-core.yml runs routine simulation tests and a 64-seed safety sweep on PR/push.
  • The scheduled exhaustive VOPR workflow is live.
  • Scheduled run 29390453717 executed at the pinned commit and succeeded for band [344064,348160) × 5000, with one test passing in 292.01s.
  • Recent push-triggered VOPR runs are from the older refactor/sansio branch, while scheduled main runs exist.

Impact

The expensive advertised regression band is delayed until the scheduled workflow after changes reach main. The workflow behavior and its documented push-gate policy disagree, increasing regression-detection latency.

Remediation/acceptance tests

  • Add main to on.push.branches.
  • Add a pull_request event if this band is intended to be a pre-merge gate.
  • Add workflow-event validation through an action linter or CI policy test.
  • Update workflow comments so the documented policy exactly matches the configured events.

4. Two fuzz targets do not reach the parser/decryption layers advertised by their documentation

Location

  • memberlist-proto/fuzz/README.md:3-13
  • memberlist-proto/fuzz/fuzz_targets/decode_incoming.rs:11-20
  • memberlist-proto/fuzz/fuzz_targets/unwrap_transforms.rs:13-15
  • memberlist-proto/src/codec/mod.rs:248-265
  • memberlist-proto/src/encryption/mod.rs:1065-1070

Invariant/claim

The fuzz README describes:

  • decode_incoming as exercising label stripping plus frame decoding;
  • unwrap_transforms as covering decryption.

Evidence/trace

The decode_incoming target only invokes decode_incoming. That implementation classifies or strips a label and returns the inner bytes; it does not parse the resulting message/frame.

The unwrap_transforms target always uses EncryptionOptions::new() without a keyring. For encrypted input, decode_encrypted_frame returns NoMatchingKey before AEAD or decrypted-inner processing.

Simply installing a key would still be insufficient for arbitrary fuzz bytes to pass authentication and reach post-decryption handling.

Impact

Fuzz execution can appear to cover the documented parser and encrypted-transform surfaces while remaining confined to shallow classification and early rejection paths. This is a harness reachability gap, not evidence of a production parser defect.

Remediation/acceptance tests

  • Compose label handling, transform unwrapping, and frame/message parsing for the claimed end-to-end surface.
  • Supply a keyed corpus containing valid encrypted frames and use structured mutation that preserves or recomputes authentication.
  • Alternatively, add a separate authenticated-inner target for post-decryption processing.
  • Add coverage assertions or counters proving that successful decryption and post-decryption unwrap paths are reached.
  • If these targets are intentionally layer-local, correct the README to state their actual boundaries.

Low severity

5. Isolated fuzz workspace has no automated compile or execution gate

Location

  • memberlist-proto/fuzz/Cargo.toml:56-58
  • Fuzz workflow/documentation references across the repository

Invariant/claim

Fuzz harnesses should have enough automation to detect compilation drift and periodically exercise their advertised surfaces.

Evidence/trace

The fuzz project declares its own isolated [workspace], so it is not protected by normal host-workspace builds. Its README documents manual cargo fuzz usage, and repository-wide inspection found no workflow referencing either cargo fuzz or the fuzz workspace path.

Manual validation at the pinned commit succeeded:

cargo +nightly fuzz build -D --target-dir /tmp/memberlist-scope3-fuzz-target

All four targets built successfully. The defect is therefore missing regression automation, not a current build breakage.

Impact

API or feature drift can leave fuzz targets broken until someone runs them manually, and corpus-based defect discovery depends entirely on unscheduled local execution. This is a coverage/architecture risk rather than proof that continuous fuzzing is an existing project requirement.

Remediation/acceptance tests

  • Add at least a PR compile or short smoke gate for every fuzz target.
  • Add bounded scheduled fuzz runs with corpus and crash-artifact retention.
  • Verify that CI enumerates and builds all four targets.
  • If continuous fuzzing is intentionally out of policy, document the manual-only ownership and execution expectations explicitly.

Validation performed

All validation used isolated targets under /tmp; the repository remained unchanged.

  • Property, VOPR self-test, and history masking:

    cargo +nightly test -p memberlist-simulation --features __sim-aes-gcm \
      --test property --test vopr_self_test --test history_masking
    

    Result: 33 passed (1 + 28 + 4).

  • Focused reliable_fallback_rescues_udp_blocked_probe_no_false_suspect tests:

    • QUIC: passed
    • TCP: passed
    • TLS: passed

    These green results do not disprove the TCP/TLS oracle vacuity described above.

  • Focused VOPR safety sweep:

    cargo +nightly test -p memberlist-simulation --features __sim-aes-gcm \
      --test vopr safety_seed_sweep -- --exact --nocapture
    

    Result: one passed.

  • Fuzz build:

    cargo +nightly fuzz build -D \
      --target-dir /tmp/memberlist-scope3-fuzz-target
    

    Result: succeeded; all four targets built.

  • GitHub scheduled VOPR run 29390453717 succeeded at the exact pinned commit.

  • HEAD matched aeef956bc0bdf0ed21ba386b329dab6aae64b47e.

  • git status --porcelain=v1 --untracked-files=all was clean.

Non-findings and positive controls

  • Transition-history masking protection is substantive: per-message, per-stream, and tick transitions are recorded. All four history_masking tests and the checker bite tests passed.
  • VOPR uses seeded randomness, fixed ordering, non-vacuity counters, crash/restart/partition/leave operations, calm convergence, and live-peer state sampling.
  • Routine CI includes TCP, TLS, QUIC, and AES-GCM simulation tests under __sim-aes-gcm; simulation is not absent from CI.
  • The daily exhaustive VOPR schedule is live and green at the pinned commit. The finding concerns push/pre-merge latency and a documented gate mismatch, not total absence of exhaustive execution.
  • All four fuzz targets currently compile. The findings concern layer reachability and automation, not a present build failure.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions