test(harness): destructive suites refuse to FLUSHALL a server they do not own (#292)

[fcostaoliveira]

Closes #292.

The defect

Four integration suites share one flush_db() shape — drop every index FT._LIST
reports, then FLUSHALL — and each defaults to a port from
tests/docker-compose.test.yml that several sessions share. The REDIS_TEST_PORT
style override existed, but the default was the shared instance, so the
protection only helped whoever remembered to set it. Two incidents followed:

1. A port override spelled as a sed against const TEST_PORT: u16 = 6399;
   matched nothing (the constant had become fn test_port()), exited 0, and the
   run proceeded against the shared server. No error, no output difference.
2. A docker run -p 6399:6379 failed because the shared container already held
   the port, so the writes landed in the shared server.

The common shape is an override that looks applied but isn't, on a shared
instance whose teardown path is destructive.

What this PR does

1 — refuse to run against an instance the harness has no claim for
(tests/common/mod.rs: claim_verdict / claim_resp_instance). On first use a
destructive suite probes the server. Unless it can positively establish the
server is safe, every test in the suite fails with an explanatory panic.

The claim lives inside fn test_port(), not inside flush_db(), because
test_port() is the one place every path to the server resolves through: the
direct redis::Client helpers and the REDIS_PORT-style value handed to
spawned benchmark binaries.

The probe reads INFO persistence (loading), INFO keyspace (keys across all
databases) and FT._LIST. Every way it can fail — unreachable, still
loading, a denied or unsupported command, an unparseable reply — produces
Probe::Inconclusive, which claim_verdict refuses on. Probe::Reachable is
constructed at exactly one place: the last statement of probe_server, after
both INFO keyspace and FT._LIST returned Ok.

Identity is run_id from INFO server, falling back to master_replid from
INFO replication (dragonfly df-v1.40.1 reports no run_id). The claim is
recorded in a file under the test binary's target directory
(<target>/vdbb-test-claims/<suite>-<host>-<port>.claim), not as a key on the
server — integration_valkey asserts exact DBSIZE values, so a server-side
marker key would have broken it. Escape hatch: VDBB_TEST_ALLOW_DIRTY=1, which
warns on every run.

2 — single-source the port, enforced by a test (tests/harness_invariants.rs,
a new pure test target wired into the container-free CI job next to
overhead_invariants). Seven invariants: one source of truth per port (P1); every server-wiping suite's
test_port() matches the one allowed shape, claim call and arguments included
(P2); the wipe scan's suite set is pinned (P3); the decision fails closed, and so
does the probe that feeds it (P4); no shared helper under tests/common/, at any
depth, may wipe a server (P5); each suite's default port equals the port
docker-compose.test.yml publishes for it (P6); and the set of test targets is
pinned so a new one must be classified (P7).

Not chosen: a randomised default port (CI and docker-compose.test.yml could no
longer name the port, and it does nothing about a stale override spelling), and a
marker key on the server (breaks valkey's DBSIZE assertions).

The single clearest demonstration

A Redis holding 20 000 000 keys, restarted so it replays its RDB, with the
suite fired during the replay. At 556914d (the first version of this guard):

=== RUN 1 at 556914d, fired during the RDB replay ===
running 1 test
test test_redis_upload_and_retrieve ... FAILED
thread 'test_redis_upload_and_retrieve' panicked at tests/integration_redis.rs:83:13:
Redis not available on port 7626 after 10s
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 43 filtered out; finished in 10.01s
claim minted DURING the load: integration_redis-127.0.0.1-7626.claim
claim content: 4ea63d80ebb97465068e192e22fa9080cc0c2c1d

Run 1 fails — but at wait_for_redis(), not at the guard. DBSIZE had
errored with -LOADING, .unwrap_or(0) turned that into "0 keys", the verdict was
Fresh, and a claim was minted for that server. Once the load finished:

load finished, dbsize=20000000
=== RUN 2 at 556914d, via the claim run 1 minted ===
running 1 test
test test_redis_upload_and_retrieve ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 43 filtered out; finished in 20.36s
dbsize after: 0

Twenty million foreign keys destroyed on the second run, reported as ok. On this
branch, same container, same sequence — refused twice, nothing minted, nothing
lost:

=== RUN 1 on b5c8b32, during the replay ===
test test_redis_upload_and_retrieve ... FAILED
    127.0.0.1:7626 is still loading its dataset (INFO persistence reports loading:1), so its keyspace cannot be trusted yet
claim minted during load: ls: cannot access '.../vdbb-test-claims/': No such file or directory
load finished, dbsize=20000000
=== RUN 2 on b5c8b32 ===
test test_redis_upload_and_retrieve ... FAILED
REFUSING TO RUN `integration_redis` AGAINST 127.0.0.1:7626
That server holds 20000000 key(s) across all databases and 0 search index(es)
This target directory has NO claim recorded for 127.0.0.1:7626.
dbsize after: 20000000

Review round 2 — three bypasses INSIDE the guard

All shared one cause: the probe was narrower than the destruction, and every way
it could fail was coerced to "empty". Each reproduced live, before and after.

B1 — skipped while the server was starting. The guard returned early when it
could not connect ("an unreachable server cannot be damaged"). False: the suite's
own wait_for_*() then waits 10-30 s, the server comes up, and the OnceLock
means the claim is never retried. RED at 556914d — seeded container, docker start fired 3 s in:

running 1 test
test test_redis_upload_and_retrieve ... ok
test result: ok. 1 passed; ... finished in 4.25s
after: keyspace=# Keyspace  FT._LIST=[ ]
claim files: ls: cannot access '.../vdbb-test-claims/': No such file or directory

GREEN, same race: REFUSING …, keyspace=db0:keys=3, FT._LIST=[prod_idx ]. The
probe now retries for 30 s (>= the longest wait_for_*() deadline in the guarded
suites) and refuses if the server never becomes reachable.

B2 — probe errors swallowed. RED at 556914d with ACL SETUSER default -dbsize (FLUSHALL still permitted): test … ok, 3 keys destroyed. GREEN twice:
with -dbsize the probe no longer uses it and refuses on INFO keyspace; with
-info denied it refuses with
`INFO keyspace` failed … (NoPerm: … 'info' command), so the number of keys at risk is unknown.

B3 — DBSIZE measures one database, FLUSHALL destroys all. RED at
556914d, 3 keys in db 1 and an empty db 0: test … ok, after: db0=0 db1=0.
GREEN: refuses, db0=0 db1=3. The probe sums INFO keyspace over every dbN:
line, parsing keys= by name because the layouts differ — all captured live
and pinned, including both empty-server shapes (redis/valkey/kividb omit the db
line; dragonfly still emits db0:keys=0).

Also in that round: INV-P3's scan was widened from one call spelling to the quoted
command name; INV-P2 gained an argument check; INV-P1 learned
.unwrap_or_else(|| N); the waiver gained a warning written straight to
std::io::stderr() (libtest only replays eprintln! for failing tests); and the
refusal message stopped asserting "this harness did not create" — false when your
own container is restarted — in favour of "has no claim for", plus the claim path
and a missing-vs-stale distinction.

Review round 3 — three more, each reproduced against 7ad5e51

SF1 — the wipe scan was case-sensitive. Round 2 widened the matcher from one
call spelling to "the quoted command name" and never mutated case: the same
guards-one-notch-weak pattern, inside the fix for the previous instance of it.
Redis command names are case-insensitive on the wire, so cmd("flushall") wipes
just as hard.

SF2 — the scan never read tests/common/mod.rs. Moving the FT._LIST drop
loop + FLUSHALL into a shared pub fn wipe_server() is the likely refactor,
since this guard's premise is that four suites share one flush_db() shape.

RED at 7ad5e51, with a synthetic tests/integration_synthwipe.rs whose
flush_db is cmd("flushall") and which has no claim call at all, plus that
wipe_server() appended to tests/common/mod.rs:

=== RED at 7ad5e51: same two evasions ===
running 23 tests
test result: ok. 23 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

GREEN on this branch, identical mutations:

running 27 tests
test inv_p5_shared_helpers_must_not_wipe_a_server ... FAILED
test inv_p3_wiping_suite_set_is_pinned ... FAILED
test inv_p2_wiping_suites_claim_their_instance_in_test_port ... FAILED
(the set diff below is INV-P3's output)
  left: {"integration_dragonfly", "integration_kividb", "integration_redis", "integration_synthwipe", "integration_valkey"}
 right: {"integration_dragonfly", "integration_kividb", "integration_redis", "integration_valkey"}
test result: FAILED. 24 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s

Both synthetic mutations were reverted afterwards and the pushed tree
grep-verified clean (wipe_server → 0 hits, no synthwipe file). The matcher now
uppercases the source and accepts either quote style, so "flushall",
"FlushAll", "flushdb" and a Lua EVAL "redis.call('flushall')" are all
positive controls, with a prose-mention negative control that keeps
integration_vectorsets out of the pinned set.

SF3 — the 30 s probe re-ran for every test. OnceLock::get_or_init does not
memoize a panicking initializer, so after a refusal each later test paid the full
reachability loop again — and the PR body claimed "runs once per process", which
was false on exactly the refusal path. The memoized value is now the verdict
(None = allowed, Some(msg) = refuse) and the panic happens outside the
initializer. Measured against a dead port (7999), same tests, same command:

	7ad5e51	b5c8b32
2 tests (test_redis_knn)	finished in 60.21s	finished in 30.07s
full suite (44 tests)	not run to completion	running 44 tests … finished in 30.17s

At 7ad5e51 the 44-test suite would have taken ~22 minutes to report "server not
available"; it now takes 30.17 s in total.

NITs also closed. sum_keyspace_keys returns Option<i64> so it can say "I
cannot tell" instead of 0 — an empty reply, a missing # Keyspace header, a
dbN: line with no usable keys=, a negative count and an overflow are each a
None (which becomes Inconclusive) rather than Fresh; that was this guard's
own bug class one notch in. INV-P1 now sees through Rust digit separators
(6_399) while leaving foo_6399 alone. The INFO persistence loading check is
documented as best-effort and inert on kividb v1.0.2-full, which reports no
loading field (measured). Doc typo in integration_dragonfly.rs fixed.

Review rounds 4-5 — the audits found the guard's own claims, then the claim itself

Round 4 (claims audit at b5c8b32). Every refusal said "Editing the port
literal in tests/.rs is rejected by tests/harness_invariants.rs". It was
not: INV-P1 rejects a second literal, and default_port() re-reads whatever
is on the .unwrap_or(..) line, so editing the single one straight through is
unpoliced — the exact bypass incident 1 used. Worse, the test policing the
message asserted only that the string "harness_invariants" was present, never
that it was true.

RED at b5c8b32, .unwrap_or(6399) → .unwrap_or(7911), nothing else touched:

running 27 tests
test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

GREEN here, same one-line edit:

running 30 tests
test inv_p6_suite_defaults_match_the_compose_mapping ... FAILED
assertion `left == right` failed: integration_redis's default port (7911) must equal the host port tests/docker-compose.test.yml publishes for `redis` (6399). To run against your own server use the suite's env var — editing this literal moves the default for everyone.

Fixed by making the sentence true, not by weakening it: INV-P6 pins each
wiping suite's default to the host port compose publishes, parsed from the
compose file so the two cannot drift apart in either direction. Also in round 4:

• The remedy was unrunnable on kividb. The footer hardcoded
  docker run -p <your-port>:6379, but kividb listens on 6380 (compose maps
  6386:6380), so an operator following its refusal got a container the suite
  cannot reach — and then a second refusal from this same guard. The container
  port is now per-suite data, checked against compose. Live:
  docker run -d --rm -p <your-port>:6380 … / KIVIDB_PORT=<your-port> … for
  kividb, :6379 for redis.
• is_standalone_number treated _ as a boundary, so INV-P1 flagged
  let foo_6399 = 1; — a line with no literal at all — and the comment claiming
  otherwise was false. Identifier characters now disqualify, with a negative
  control that also proves a real literal on the same shape of line is still
  caught.
• CLAIM_OUTCOME's doc cited "60.14 s instead of 20.13 s". 20.13 s is not
  reproducible and is structurally impossible (the wait is 30 s either way).
  Replaced with the measured pair, 60.21 s → 30.07 s.
• The header said "Four invariants" while shipping five; the CI comment said
  "INV-P1/P2/P3". Both corrected.
• integration_valkey.rs and integration_kividb.rs still said "state this
  harness did not create" — the phrasing the message test forbids in the message.

Round 5 (mutation campaign at b5c8b32). Sixteen invariant mutants were
caught; two were not.

The claim could simply be commented out. INV-P2 matched
squeeze(body).contains(..) against raw source, while INV-P1's bare_port_hits
had skipped // lines since round 1 — same file, one dimension short. Live A/B
on a redis:8.8.0 seeded with 1 key in db0, 1 key in db1 and one FT index. RED at
b5c8b32 with a single / prepended to the claim line:

running 27 tests
test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.05s
running 1 test
test test_redis_upload_and_retrieve ... ok
server after: keyspace=[# Keyspace ] idx=[ ]

INV-P2 now compares the whole test_port() body, comments stripped, against
the single shape it may have. Equality also closes if false { .. } and
stringify!(..), pins the arguments and container port, and makes squeeze's
rustfmt tolerance load-bearing. The comment stripper leaves string literals
alone so redis:// survives, with a control.

INV-P5 read one directory level. Round 3's headline fix, re-evaded in its own
idiom: a wipe helper at tests/common/wipe/mod.rs was invisible. The walk now
recurses and the file set is pinned rather than asserted > 0.

GREEN here with both mutations applied at once:

running 38 tests
test inv_p5_shared_helpers_must_not_wipe_a_server ... FAILED
test inv_p2_wiping_suites_claim_their_instance_in_test_port ... FAILED
test result: FAILED. 36 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

Both mutations were reverted and the pushed tree grep-verified (common/wipe → 0
files, // common::claim → 0 hits).

Round 5 also closed four gaps where the guard could be made inert with every
suite still passing — a permissive mutation makes integration tests pass harder,
so only a source pin can catch them:

• INV-P4 exercised claim_verdict, the consumer, only. A probe_server
  returning Reachable { keys: 0, index_count: 0 } for an unreachable server —
  the exact regression INV-P4's own docstring cites — passed everything.
  probe_server is now pub and asserted Inconclusive against a closed
  ephemeral port at zero wait, plus source pins that every early return is
  Inconclusive, that Probe::Reachable is constructed exactly once and last,
  that claim_resp_instance panics on a refusal, that the waiver defaults to
  off, and that CLAIM_REACHABLE_WAIT >= the longest wait_for_*() deadline.
• Fresh whenever keys == 0 was unpinned even though flush_db() drops
  indexes and the refusal counts them: 0 keys + 1 index must now refuse.
• \"FLUSHALL\" — the natural Rust spelling of a Lua payload — was not matched,
  which made the shipped "either quote style" claim false. \ is now a quote
  boundary, with a positive control.
• New INV-P7 pins the set of test targets, so a wiping suite named outside
  integration_*.rs cannot slip past the INV-P3 scan unclassified.

Engine survey

Server-wide destruction — guarded here, all four live-validated:

suite	default port	destructive helper
integration_redis	6399	flush_db() → FT.DROPINDEX <each> DD + FLUSHALL
integration_valkey	6380	flush_db() → FT.DROPINDEX <each> + FLUSHALL
integration_dragonfly	6385	flush_db() → FT.DROPINDEX <each> + FLUSHALL
integration_kividb	6386	flush_db() → FT.DROPINDEX <each> + FLUSHALL

Narrower destruction, NOT guarded here. No other suite has an
enumerate-and-delete loop, a DROP DATABASE/SCHEMA, a dropDatabase, a
DELETE /_all or a wildcard index delete. Two are worth naming rather than filing
under "named targets only":

• integration_pgvector (5433) — DROP TABLE IF EXISTS items CASCADE at
  :54, :60, :485. The in-process connection is built from
  const PG_PORT: u16 = 5433 / PG_DB = "postgres" with no env read at all
  (PGVECTOR_PORT only reaches the spawned child), so the target is unmovable,
  the table name is maximally generic, and CASCADE takes dependents with it.
  This is the natural integration_redis FLUSHALLs whatever is on REDIS_TEST_PORT's default 6399, and the old port-override spelling silently no-ops #292 follow-up.
• integration_qdrant (6334/6335) — :2030 and :2127 take the collection
  from std::env::var("QDRANT_COLLECTION_NAME").unwrap_or("benchmark") and those
  two tests' child spawns do not pin the var, so an inherited value steers the
  drop/recreate.
• pgvector's other tables, integration_mongodb's drop_test_collection(),
  ES/OS's DELETE /<index>, and milvus/weaviate/chroma's named collection/class
  deletes are all fixed-name targets.
• integration_vectorsets (6398) issues no wipe; since VectorSets uses a hardcoded idx key — it never got the #151-4 per-config namespacing, so concurrent runs clobber each other #236 each config owns
  idx:<config-name>. Covered by INV-P1, not by the claim.

Gates

Baselines derived on e99d2b5 in a dedicated target dir, after
touch src/lib.rs src/bin/vector_db_benchmark/main.rs:

gate	e99d2b5	this branch
cargo fmt --check	—	clean
cargo clippy --all-targets -- -D warnings	—	exit code 0, zero warning/error lines
cargo test --lib --bins	129 + 0 + 0 + 0 + 2 + 806 = 937	937, all passed
cargo test --test overhead_invariants	running 9 tests, 9 passed	running 9 tests, 9 passed
cargo test --test harness_invariants	target does not exist	running 38 tests, 38 passed
integration_redis -- --test-threads=1	running 44 tests	running 44 tests, 44 passed
integration_valkey -- --test-threads=1	—	running 29 tests, 29 passed
integration_dragonfly -- --test-threads=1	—	running 15 tests, 15 passed
integration_kividb -- --test-threads=1	—	running 21 tests, 21 passed (622.48s)
integration_vectorsets (untouched)	—	running 15 tests, 15 passed

Refusal checks with foreign data present, all on my own containers: redis 7621,
valkey 7624, dragonfly 7622 — each REFUSING …, holds 2 key(s) across all databases, keys intact: 2. Ports 7601-7999 only; 6399/6398/6380/27018 were
never touched, and tests-redis-1/tests-valkey-1 confirmed still running.

One filtered run of mine printed running 0 tests / ok because the filter
matched no test name. I caught it, found the real name, and re-ran — every count
above is from a line I read.

Stated limits

• False positive: docker restart of your own container. Both run_id and
  master_replid are regenerated on restart while an RDB-persisted dataset
  survives, so the claim goes stale and the suite refuses data it created itself.
  Safe direction; the refusal message names this case and the one-line fix.
• False positive: a lost claim directory. cargo clean, a fresh
  CARGO_TARGET_DIR, a new worktree, a CI cache miss, or --target <triple> all
  drop the claim. This only bites after a run killed mid-suite: a clean full run
  leaves the server empty, so ordinary re-runs take the Fresh path.
• A search-less but provably empty Redis is refused, because FT._LIST
  failing is treated as inconclusive. Correct for these four suites — they all
  call FT.DROPINDEX — but it does mean the guard is stricter than "has data".
• The loading check is best-effort. An engine whose INFO persistence errors
  or omits loading gets no loading check; kividb v1.0.2-full is such an engine
  (measured). A server so locked down that INFO fails entirely is still caught by
  INFO keyspace.
• Concurrent runs are only partly detected. A rival session's run is refused
  once it has written anything, but in the window right after its flush_db() the
  server is empty and this guard will claim it.
• master_replid identifies a replication GROUP, not an instance. A Dragonfly
  replica reports the same value as its primary, so a claim recorded against a
  primary would also authorise a replica of it that later took the same
  host:port. run_id (redis/valkey/kividb) does not have this property.
• An unreachable server costs 30 s before the refusal, once per process.
• The wipe scan is a text scan. It catches a quoted command name in any case,
  in either quote style, and escaped (\"FLUSHALL\"), but not a KEYS * + DEL
  loop, a SCAN+UNLINK sweep, or a command assembled from fragments. All three
  were confirmed missed, exactly as stated.
• INV-P2 pins the SHAPE of test_port(), not its runtime effect. Any
  refactor of that function fails the invariant until the expected body is
  updated — deliberate, because it is the choke point, but it is friction.
• The comment stripper does not understand raw strings (r#"..."#). No suite
  uses one inside test_port(), and INV-P2's equality would fail loudly rather
  than silently if one appeared.
• INV-P1 covers only suites with a fn test_port() -> u16 fallback — the five
  RESP suites. integration_opensearch, _milvus, _elasticsearch and
  _pgvector still pass bare port literals to spawned binaries; not fixed here,
  and none of them can wipe a server.
• Two literals live outside every scan, noted rather than fixed:
  src/redisearch/configure.rs:53 FLUSHALLs as a Memorystore fallback (the scan
  reads only tests/), and src/bin/vector_db_benchmark/engine/dragonfly.rs:101
  has .unwrap_or(6385) in production code.
• No production code changed. Everything is under tests/, plus one CI step.

🤖 Generated with Claude Code