mirror of
https://github.com/jmcorgan/fips.git
synced 2026-07-30 19:46:15 +00:00
cbbdf2c13c9c9e0250c4431d772f13d408ce9136
599
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Johnathan Corgan
|
cbbdf2c13c |
Stop the firewall and ACL suites rebuilding an image their caller supplied
Both suites ran `docker compose up -d --build` unconditionally, and their compose files carry both a build context and an image name, so the flag rebuilt and retagged the shared test image from the shared build context on every run. Their --skip-build flag did not cover it: it guarded only the Rust build above. Under a harness that has already built the image and handed it over, that rebuild can only replace the binaries under test with whatever the shared context currently holds. The nat scripts had the mirror-image problem. Their image guard built a replacement whenever the image was missing, which is right by hand and wrong under a harness: when the caller names an image it built, a miss means something upstream is broken, and manufacturing a substitute hides that. They now fail loudly in that case and keep building only on the hand path. The mesh-lab guard reads the same variable and its build hint moves to the script that still produces the shared tag. |
||
|
Johnathan Corgan
|
24bf4d46d4 |
Widen the churn-mixed baseline bounds to sit outside the scenario's own spread
The root-count ceiling of 4 was set from six runs whose observed maximum was 3. Fourteen runs that recorded a baseline verdict put the real distribution at 1 to 5 roots, so the ceiling sat at roughly the 93rd percentile of the scenario's own variance: one sample in fourteen exceeded it and four sat at or above it. It therefore reddened a share of runs whatever the daemon did, which is a calibration fault rather than a convergence one. Every sample ran the file's fixed seed, so the spread is container timing and not a differing chaos schedule. The ceiling is now 6, one step beyond the observed maximum, and the parented floor moves to its complement at 4. That still fails a mesh where seven or more of ten nodes are islanded, which is the collapse case the assertion exists to catch; driving evaluate_baseline directly confirms all fourteen samples pass and 7, 8 and 10 roots still fail. min_sessions is left at 10 against an observed minimum of 12. It has never fired, but the comment now records the margin as thin so a future failure there is read as calibration first. |
||
|
Johnathan Corgan
|
cf129cde77 |
Retire the three ignored Ethernet tests, which covered nothing unique
All three required root or CAP_NET_RAW, neither runner passes --ignored, and so none had ever executed in CI. Before deleting them I checked what each covered and whether anything else covered it: - The two-node handshake is covered by the ethernet-only chaos scenario, whose baseline assertion cannot pass unless handshakes complete over AF_PACKET. - Tree convergence and root election by smallest NodeAddr are covered by ethernet-only's single-root assertion and by the in-process spanning-tree tests respectively. - The mixed-transport coexistence property is covered twice over by test_tcp_mixed_transport_coexistence and test_ble_mixed_transport, which call the same verify_tree_convergence_components helper with the same two-component shape and both run today. So the only thing running them would have added is an AF_PACKET variant of a property already proven on two other transports. The counts confirm nothing running was lost: 1392 tests pass before and after, and the ignored count falls from 7 to 4. The ethernet-only comment block cited these tests as the reason its assertion was the only Ethernet coverage in CI. It now records the sharper fact found while reading them: that coverage is control plane only. Both Ethernet scenarios disable traffic, so no datagram crosses an Ethernet link anywhere, which leaves the frame length field that trims NIC minimum frame padding ahead of AEAD verification unexercised. Deleting these tests does not widen that gap, because the test named for data exchange did not exchange any. |
||
|
Johnathan Corgan
|
4315328f2b |
Stop the log analyzer double-counting outbound peer promotions
The shared analyzer matched two strings for one event: "Peer promoted to active", emitted at info, and "Outbound handshake completed", emitted at debug. Both come from handle_msg2 on the same call path for the same outbound promotion, so any run at debug level counted every outbound promotion twice. Measured rather than argued. On the archived debug-level churn-mixed run of 2026-07-25, the node logs carry 20 of each string; the old matcher reported 40 promotions and the new one reports 20. The neighbouring counters are unmoved at 13 sessions and 33 parent switches. Info-level runs are unaffected, which was checked rather than assumed: on the tcp-mesh run of the same date the debug string occurs zero times, and the old and new matchers both report 7 against 7 raw occurrences. Three scenarios run at debug, one of them gating, so this corrects the promotion figure in their artifacts. No assertion reads the counter, so no suite's verdict changes. The earlier fix that introduced this added the live string alongside the dead one where it needed to replace it. |
||
|
Johnathan Corgan
|
44e04eb2e2 |
Retire the tcp-chain static test topology
The tcp-chain profile has never been runnable. ping-test.sh dispatches on
chain and mesh only, and the string tcp-chain has never appeared in that
script in its history, so invoking the profile has always fallen through
to the unknown-profile branch: silently asserting nothing before that
branch was made to fail, and exiting 2 since. Neither runner referenced
it either.
What it would have covered is already covered. The chaos tcp-mesh
scenario runs in local CI and gives TCP a discriminating gate: n04's only
edges are TCP, so if the transport were broken n04 could not parent and
the scenario's baseline assertion would fail. It also carries a pure-TCP
two-hop path, n01 to n04 to n05, under netem and link flaps. The only
residual tcp-chain would have added is a mesh with no UDP present
anywhere, which is not worth a fixture that has never run.
Remove the topology, its three compose services, and the documentation
rows. Every profile the static compose file still defines is now
exercised by a suite.
Also correct the chaos README, which has listed tcp-only and tcp-chain in
its transport table since those two scenarios were deleted in
|
||
|
Johnathan Corgan
|
fa49dc1210 |
Retire the mesh-public static test topology
The mesh-public profile ran in neither runner: ci-local's static suite list carries only static-mesh and static-chain, and the GitHub matrix has only mesh and chain. It also added no coverage over static-mesh. ping-test.sh and iperf-test.sh branched mesh and mesh-public together and exercised the same 20 directed pairs among node-a through node-e, and no script referenced the external node at all. The convergence waits even used mesh's peer counts rather than mesh-public's, so the extra link to the public node was never counted, let alone asserted. Running it would therefore have added a dependency on a live internet host (test-us01.fips.network) in exchange for zero additional assertions. Remove the topology, its five compose services, the script branches that aliased it to mesh, and the documentation rows. An invocation using the old profile name now fails on ping-test.sh's unknown-profile guard instead of silently behaving as mesh. The config generator's external-node support (external_ip, is_external_node) stays. It has no consumer now, but it is woven into the config path every remaining topology uses, so removing it would put the gating suites at risk for no present gain. |
||
|
Johnathan Corgan
|
0eea7dae13 |
Make a zero-peer floor unreachable in the shared convergence wait
wait_for_peers reads the connected-peer count through a pipeline ending in `|| echo 0`, so a container that never answers contributes 0. That is safe against a floor of 1 or more, where 0 reads as "not converged yet" and the wait eventually times out, and it is unsafe against a floor of 0, where the first read from a dead container satisfies the wait and the caller proceeds as though convergence had been observed. No caller passes 0 today; every one passes 1 or more, and the single variable minimum is advisory. Rather than harden the reader, reject the input: a floor below 1 is now an error. That makes the shape unreachable instead of repairing instances of it, and it costs a future caller nothing except a clear message saying that asserting "exactly zero peers" needs a reader that distinguishes no answer from zero, which this floor is not. wait_for_links goes with it. It had no caller anywhere in the tree on any branch, and it carried the identical fallback, so the only thing it could do was hand the hazard to whoever called it first. An uncalled helper cannot be wrong today, which is exactly why it was the risk worth removing rather than the one worth keeping for symmetry. The hermetic gate suite gains a case covering both directions, with docker stubbed so it stays container-free: a floor of 0 is refused with a message naming why, and the same unreachable container with a floor of 1 still polls its full budget and times out. Verified by removing the guard and confirming exactly the three zero-floor assertions red while both floor-of-one controls stay green. |
||
|
Johnathan Corgan
|
1077fd6a7d |
Stop the firewall peer reader turning a silent container into a count of zero
wait_for_peers_exact read the connected-peer count through a pipeline ending in `|| echo 0`, so a container that never answered and a daemon that answered zero produced the same value. That is only harmless while every caller expects a non-zero count, which is true here today and is the reason this copy was left alone when the acl suite's copy was fixed. It leaves the trap armed for whoever adds the first caller expecting zero: the check would be satisfied on its first iteration without the property it exists to verify ever being observed. The read moves into its own function that returns the empty string when the container does not answer, and the caller treats empty as "no answer" rather than as a count. A run that never gets an answer now fails saying so, distinctly from one that answered the wrong count, and the diagnostic peer dump runs only on the latter, since dumping from a container that cannot answer prints a docker error rather than evidence. This is the shape the acl-allowlist suite already uses. The two copies had diverged on whether a silent container counts as an answer, which is the kind of disagreement that decides a security assertion in whichever file is read last. Checked by construction rather than by a green run: driving the old form against an absent container with an expected count of zero returns success on the first iteration, and the new form fails; with an expected count of one, the shape both real callers use, both forms still fail, and a genuine zero from a live daemon is still reported as zero. |
||
|
Johnathan Corgan
|
0e42c789be |
Let the firewall suite float its docker subnet
The compose pinned 172.32.0.0/24 with a per-container ipv4_address, so two concurrent runs asked docker for the same address space and the second failed with a pool-overlap error. Request no subnet and let docker assign one from the daemon pool. Peers address each other by the docker hostname the compose already sets (host-a, host-b) rather than by literal IP; docker's embedded DNS is per-network, so the same hostname in two runs resolves inside each run's own subnet. Nothing this suite asserts on moves as a result: its checks run over the fips0 overlay, whose addresses are derived from node npubs and are independent of docker addressing, and the packaged nftables ruleset matches on interface rather than on any address. The generated config directory moves under the run suffix for the same reason it did in the acl suite, and the run teardown gains the matching removal so a run no longer leaves its directory behind. Case (b) also wrote curl's output to a fixed path under /tmp. Two concurrent runs shared that one file, and either run's cleanup landing between the other's write and read left an empty read, failing the http_code check for a reason having nothing to do with the firewall. It uses mktemp now. As in the acl suite, the floating subnet removes one of the two obstacles to concurrent runs. The compose project name is still fixed; the local CI runner scopes it externally, a bare hand run does not, and the comment at the site says so rather than claiming the file is self-sufficient. |
||
|
Johnathan Corgan
|
611f045d33 |
Let the acl-allowlist suite float its docker subnet
The compose pinned 172.31.0.0/24 with a per-container ipv4_address, so two concurrent runs of this suite asked docker for the same address space and the second failed with a pool-overlap error. Request no subnet instead and let docker assign one from the daemon pool, which leaves nothing for two runs to contend for. Peers now address each other by the docker hostname the compose already sets (host-a through host-f) rather than by literal IP, and docker's embedded DNS is per-network, so the same hostname in two runs resolves inside each run's own subnet. The generated config directory moves under the run suffix in the same change, because it has to: the generator does rm -rf on it, so a shared directory means one run deletes the fixtures another is still using, which fails silently rather than loudly at bring-up. Scoping it requires the generator output path, all five bind-mount sources per service, and the gitignore entry together; scoping only the generator leaves the compose reading the unscoped path. The floating subnet removes one of the two obstacles to running this suite twice at once, not both: the compose project name is still fixed, and nothing scopes it for this suite. Recorded at the site so the comment does not claim more than the change delivers. |
||
|
Johnathan Corgan
|
0cb5574077 |
Reject rekey settings that fire the trigger immediately and forever
Config validation said nothing about the rekey block, so two settings that disable rekey in appearance and hammer it in practice were accepted silently. after_messages of zero makes the message-count arm true on every poll, since the trigger tests the counter against it with a greater-or-equal. It reads like a way to switch the arm off and does the opposite. after_secs at or below the per-session jitter is the same trap on the timer arm. Each session offsets the interval by a random value drawn from plus or minus the jitter bound, so a smaller interval saturates to zero on a negative draw and rekeys on sight, for roughly half of sessions and not the other half. The rule is expressed against the jitter constant rather than its current value, so it tracks if the bound ever moves. Both are checked whether or not rekey is enabled, so turning it on later cannot surface a configuration error at a surprising moment. Neither has an upper bound: a very large value is the established way to disable one arm of the trigger and stays legal. |
||
|
Johnathan Corgan
|
989ae65fc5 |
Align the datagram hop-limit helpers with the forwarder's semantics
The forwarding path was corrected to full IP semantics: local delivery is not hop-limit gated, and forwarding decrements first and drops at zero. Two helpers on SessionDatagram were left implementing the old rule. decrement_ttl still checked before decrementing, and can_forward still answered true at a hop limit of one, where the decrement leaves zero and the datagram is dropped. Neither is called anywhere today, which is precisely why they are worth correcting rather than leaving as an invitation to reintroduce the defect. The TtlExhausted reject doc described behaviour that no longer exists. The reject is charged for a transit arrival at one as well as at zero, and is never charged for a datagram addressed to this node, whose delivery is decided ahead of the test. Both helpers are pinned by tests at the boundary, which fail against the previous implementations. |
||
|
Johnathan Corgan
|
1eabe08575 |
Correct what the address-to-link map claims to be for
The doc said the map "enables dispatching incoming packets to the right connection before authentication completes." It does not: find_link_by_addr has no callers outside its own tests on any branch, and encrypted frames dispatch by session index. Its live readers are the msg1 admission fast path and the duplicate-inbound-handshake check. The comment mattered because the map has a trap that invites exactly the misuse it was advertising. An outbound dial registers the literal configured address string, which may be a hostname, while an inbound packet carries the resolved form; TransportAddr compares byte-wise, so the two never match and a lookup keyed on an inbound address returns "no such link" for every hostname-configured peer rather than failing. The entry is also single-valued per key, so an inbound handshake overwrites an outbound dial's entry for the same address. Both current readers survive this because each compares a key written in the same form it reads. That is a property of those two call sites and not of the map, so the comment now says so, and says not to key a peer-identity question on it. Documentation only; no behaviour changes. |
||
|
Johnathan Corgan
|
9846e85705 |
Retire the admission-cap Docker suite; the inbound gate is proven in-process
The inbound max_peers early-gate in handle_msg1 (silent-drop a Msg1 from a net-new identity at saturation, send no Msg2, admit no peer) is unit-tested over a real UDP socket by handle_msg1_silent_drops_at_cap_for_new_peer in src/node/tests/unit.rs: it saturates a node, sends a Msg1 from a fresh identity, and polls the sender socket to assert no Msg2 comes back — the same wire-observable discriminator the Docker suite's tcpdump used — with a sibling test for the existing-peer bypass. That is a deterministic superset of the Docker packet-capture assertion. Drop it from both runners in lockstep (its *_SUITES array, run function, default-flow loop, --only dispatch arm, and the GitHub matrix leg with its steps) so the parity guard stays green, and record the retirement in the deliberately-not-run block with a pointer to the standalone runner. The admission-cap-test.sh script stays on disk and runs by hand. |
||
|
Johnathan Corgan
|
41ce64ba82 |
Retire the acl-allowlist Docker suite; the decision is proven in-process
The ACL admission decision is exhaustively unit-tested per npub over real loaded allow/deny files (src/node/acl.rs test module: allow-match-wins, allowlist-miss falls through, deny-only, deny-all, allow_all override, deny-after-allowlist-miss), and the inbound and outbound handshake- admission paths are covered in-process over the loopback transport (src/node/tests/acl.rs: inbound msg1 denial, outbound denial, reload). The Docker suite's only unique coverage was admission over a real UDP transport, which every other real-transport suite already exercises, and its operator-facing log assertion. Drop it from both runners in lockstep (its *_SUITES array, run function, default-flow call, --only dispatch arm, and the GitHub matrix leg with its steps) so the parity guard stays green, and record the retirement in the deliberately-not-run block with a pointer to the standalone runner. The testing/acl-allowlist/ suite stays on disk and runs by hand. |
||
|
Johnathan Corgan
|
7cbe1d3d4e |
Retire the smoke-10 chaos scenario; convergence is covered in-process
smoke-10 was a no-stressor 10-node tree-convergence sanity check (netem off, no ping). Its subject, spanning-tree convergence and root election, is now covered in-process, faster and deterministically, by the loopback spanning-tree harness (src/node/tests/spanning_tree.rs: ring, star, chain, 100-node and disconnected-component convergence) plus end-to-end datagram delivery (src/node/tests/forwarding.rs). Real-UDP convergence smoke still runs via static-mesh and the other scenarios' baseline assertions, so no Docker coverage is lost. Drop it from both runners in lockstep (the CHAOS_SUITES list and the GitHub chaos matrix) so the parity guard stays green, delete the scenario YAML, and update the chaos README. |
||
|
Johnathan Corgan
|
08a226fb63 |
Test cost-based parent selection and kernel-drop detection as unit tests
The cost-selection chaos scenarios (cost-reeval, cost-avoidance, cost-stability, depth-vs-cost, mixed-technology, bottleneck-parent) tested TreeState::evaluate_parent's decision logic through a Docker mesh that could not exercise it reliably: the tree roots at whichever node holds the smallest NodeAddr, MMP link costs take several measurement windows to settle, and the parent hold-down plus hysteresis timing all confound the outcome. A deterministic link-cost flap still produced zero periodic parent switches in a full run. Replace those six scenarios with deterministic unit tests in src/tree/tests.rs that drive evaluate_parent directly: cheaper-link selection at equal depth, switch-on-cost-change, hysteresis suppressing a marginal change while allowing a significant one, and the depth-versus-cost effective-depth tradeoff. Each is constructed so that breaking the cost or hysteresis logic makes it fail. The congestion kernel-drop signal (SO_RXQ_OVFL) cannot be provoked deterministically in Docker: a fresh daemon reader keeps up with container-speed traffic, so the socket receive queue never overflows (an unshaped run with a 4 KB buffer and heavy traffic recorded zero drops on every node). Extract the drop-detection edge -- read the cumulative counter, fire an event only on the transition into a new drop burst -- into TransportDropState::observe_drops and unit-test it directly. congestion-stress keeps its ECN and MMP congestion-signal assertions, which do need the real shaped bottleneck queue. Remove the retired scenarios from both CI runners and update the chaos README. |
||
|
Johnathan Corgan
|
be5deee814 |
Make the gateway integration suite safe for concurrent CI runs
gateway-lan pinned 172.20.1.0/24 and fd02::/64, so two concurrent local CI runs collided on "Pool overlaps" at network creation. The IPv4 subnet has no consumer -- the whole gateway LAN path is IPv6 -- so drop it and let docker auto-assign. The IPv6 side cannot float, because the LAN clients' resolv.conf pins the gateway's address as their nameserver and that must be a literal known before they start, so claim a free /64 per run and thread the prefix through the compose address pins, a generated resolv.conf, and the test via a single exported variable. The claim and the external network ship as a harness-only compose overlay applied by run_gateway; the base compose keeps a normal gateway-lan network, so the GitHub matrix and any standalone bring-up are unaffected. With no claim every address renders exactly as before. |
||
|
Johnathan Corgan
|
bb9bca4d83 |
Accept a degraded systemd as a booted one in the deb-install boot wait
The boot check piped `systemctl is-system-running --wait` into `grep -qE 'running|degraded'`, but the script runs `set -o pipefail` and is-system-running exits non-zero for `degraded`. So the pipeline failed on a degraded system even though grep matched, and the wait looped to its timeout. The older distros reach `running` (exit 0) and passed; debian:trixie and ubuntu:26.04 reach `degraded` because a unit that cannot run in a container (systemd-modules-load) fails, so they timed out at every ceiling -- which is why the earlier timeout increase did nothing. Capture the state string and test it directly instead of trusting the pipeline exit, so a degraded-but-booted system is accepted as intended. Validated: all five distros pass, including the two that previously failed. |
||
|
Johnathan Corgan
|
2bc0345174 |
Make the cost-based chaos assertions reliable under parallel CI load
cost-avoidance and mixed-technology decide a parent by measured link cost (etx * (1 + srtt_ms/100)). Under the local CI's 4-way-parallel chaos the fast fiber link's measured srtt spikes from host scheduling and can exceed the Bluetooth link's, flipping the choice and reddening a run that proves nothing about the daemon. The old margin was ~0.4 cost units, which a ~24ms one-sided scheduling blip closes. Widen the Bluetooth delay to 150-250ms so the margin dwarfs any plausible one-sided spike. The link still establishes well within the handshake budget (a ~500ms RTT against a 30s stale-handshake window), so the losing candidate is still a real, established peer rather than an absent one. Add a netem mutation exclude_edges option and use it in mixed-technology to pin the two links n08 chooses between, so a random degradation can't flip the asserted comparison either. An unknown excluded edge is a hard error, since a silent no-op would reintroduce the flakiness it exists to remove. Validated by running both scenarios repeatedly under heavier-than-CI concurrent load: the parent choice holds every time. |
||
|
Johnathan Corgan
|
9b7a27f219 |
Claim the chaos network range instead of deriving it, ending the collision
Two concurrent ci-local runs could not both run chaos. Each child's subnet came from its position in the suite list, so both runs walked 10.30.0 through 10.30.12 and requested identical ranges; whichever reached the daemon first won and the other died with a pool overlap. A run index or hashed offset would only make that unlikely, and it is precisely the failure being removed. The simulator now claims its range: attempt-create on a candidate, advance on docker's own overlap error, fail loudly on anything else. Docker's address pool becomes the arbiter, so an overlap is impossible rather than improbable. The claim lives in the sim rather than in ci-local because only the process that creates the network can advance on conflict, and because it fixes the bare chaos.sh path too, which a ci-local-only fix would have left broken. The generated compose now declares the network external over the claimed one, and teardown releases the range from both paths, including the setup-failed path where a run that fell over after claiming still holds one. Ordering matters here and is not obvious: node IPs derive from the subnet during topology generation, and traffic shaping keys its filters on those addresses, so the claim happens before the topology exists. Claiming later would give a network on one range and filters on another, which does not fail at bring-up and instead leaves the shaping matching nothing. --subnet survives as an explicit pin for when a known range is wanted, and now fails loudly if that range is taken rather than silently overlapping. ci-local no longer passes it. Validated by running two instances of the same scenario deliberately concurrently: they claimed 10.30.1.0/24 and 10.30.0.0/24, both exited 0 with their assertions passing, and both released their networks. The negative control holds too: with a squatter on a pinned range the run aborts with a message naming the range rather than proceeding. |
||
|
Johnathan Corgan
|
f478f51afe |
Assert mixed-technology's sound criterion, and correct the one that never was
With the root pinned, n08's test subject is assertable for the first time: its two candidates sit at equal depth with fiber-grade default netem on one side and Bluetooth on the other, which is exactly the preference this scenario exists to test, and it takes the fiber parent in four of four runs. Encoded as written rather than calibrated from the runs, since "should pick n03" is the spec and the runs merely confirm it is met. n06 is deliberately absent, and its documented criterion is corrected rather than left standing. The header called n03 a fiber parent for n06; the netem policies in the same file say that link is WiFi and the other candidate is Bluetooth, so n06 chooses between two impaired links. Its only fiber-grade link reaches a node at depth 3 that is never a good parent. There was never a reason to expect it to prefer n03, and the two-of-four split observed is correct behaviour: in a run where it took the Bluetooth peer, the fiber-labelled candidate measured a link cost of 3 against the other's 1, so the daemon chose the cheaper effective depth as it should. Validated by replaying the assertion against all four archived runs, then breaking it three ways to confirm it can fail, then one live run for the wiring. One of the breaks expects n06 to take n03 and reports that it chose n02, which is the direct demonstration of why n06 is not encoded. cost-reeval records that its subject now fires in two runs of three, against zero of five when it was structurally impossible, and why that rate is still not one an assertion can rest on. |
||
|
Johnathan Corgan
|
9d92cfeab4 |
Correct three scenario headers that documented conclusions since disproved
congestion-stress said the transport drop-detection path "is exercised by nothing". Widening the scan beyond this one scenario finds it firing in six others, so the path is live and what is dead is this scenario's ability to provoke it. Records the arithmetic that explains why: at the 1 Mbps cap the receive queue needs tens of milliseconds of reader stall to overflow, and the ingress policer discards the same traffic a layer below the socket. mixed-technology said the implementation does not do what its criteria say. It does. The criteria were conditioned on a tree rooted at n01 while the mesh rooted at n09, under which n08's choice was settled by depth before link technology could matter. The archived parent distributions are relabelled as describing the old root so nobody calibrates against them. cost-reeval now records why it had no assertion about its own subject: its designated subject was the root, so the parent switch it exists to observe could not occur. The historical logs show the switch on n04 in 13 of 14 runs when the root still wandered, and only on n01 in the five runs before the fix. Each header now names what remains to be done and where it is tracked, rather than leaving a disproved conclusion in place for the next reader to inherit. |
||
|
Johnathan Corgan
|
6319c7f577 |
Describe the trailing command accurately in the gate's finding message
For a value-returning helper the trailing echo is the return mechanism, not a log call. The hazard is the same either way -- it fixes the exit status at zero -- but calling it a log call misdescribes four of the sites the gate now finds. |
||
|
Johnathan Corgan
|
d4a2504f99 |
Teach the trailing-log gate to see command-substitution call sites
Found while fixing count_log_pattern: the gate reported that function clean even though it ends in echo and both callers consume its status. Its call-site patterns only matched direct forms -- if fn, while fn, fn ||, fn && -- so a status consumed through command substitution was invisible, because the line begins with the variable rather than the function name. That is not an exotic form. It is how a shell function returns a value, and it is precisely the shape of the swallowed-failure family this gate exists to catch, so the gate was blind to a large part of its own stated class. Three patterns added for the assignment, if-guarded and test-expression forms. The extension finds four real instances, all value-returning helpers whose trailing echo made their exit status unconditionally zero, and each now carries an explicit return 0. Also corrects the finding message, which described the trailing command as a log call; for these it is the return mechanism, and the hazard is that it fixes the status either way. Validated by breaking what it guards: a probe reintroducing the defect shape behind a command substitution is reported and exits 1, where before the extension it would have passed. |
||
|
Johnathan Corgan
|
bf173d8d98 |
Pin the chaos mesh root to n01 so scenario diagrams describe the real tree
Every chaos scenario draws n01 at the top of its topology, and until now that held in three of thirteen. The mesh roots itself at the numerically smallest NodeAddr, which is a hash of the node's public key and bears no relation to the node numbering, so which node ended up as root was effectively arbitrary. The consequences were not cosmetic. cost-reeval rooted at n04, its own designated test subject, so that node had no parent and the periodic parent switch the scenario exists to observe could not occur at all. mixed-technology rooted at n09, which put its two documented parent criteria out of reach and made correct cost-based selection look like a defect. Identities are still derived from the mesh name exactly as before and are still deterministic. What changes is which node id holds which one: they are now assigned in NodeAddr order, so n01 holds the smallest and is the root. Verified against a model of the daemon's own derivation that reproduces the previously observed root for every scenario and n01's address byte for byte; all twelve pinned scenarios now root at n01. smoke-10 deliberately opts out via pin_root: false so that root election from an arbitrary key distribution stays exercised somewhere. Its assertion is a convergence floor and is root-agnostic, which is why it is the cheapest home for that. The new key is rejected when non-boolean, and a near-miss spelling is rejected as unknown; both checked. Not yet established: the trees themselves change, so the parent-dependent assertions in bottleneck-parent, cost-avoidance and cost-stability need re-deriving against live runs. Those are held until the in-flight CI finishes, because a chaos run rebuilds the shared fips-test image that run is using. |
||
|
Johnathan Corgan
|
34a2561af7 |
Close the remaining "caught" and "produces red" harness holes
Four sites where a check could report a verdict it had not established, or detect a failure and then not turn it red. assert_no_panic in both NAT suites read `docker logs ... || true`, so a container that could not be read produced empty output, matched no panic pattern, and returned success. The assertion's failure mode was indistinguishable from its success condition. It now reports that absence of panics is not established. deb-install's apt capture discarded the exit status, so a failed docker exec gave an empty capture that matched neither error pattern and reached the pass branch. The status is now kept and checked before the output is inspected, with the output still printed on failure. wait_for_systemd printed a warning and returned success on timeout, so every check after it read a system that may not have started its units. It now returns non-zero and the caller abandons that distro leg rather than testing an unstarted system. interop's copy of count_log_pattern carried the same defect fixed in rekey: a node whose logs could not be read contributed zero to eight expect-zero assertions. Fixed the same way, and its consumer now reports the unreadable case rather than comparing a sentinel against zero. Each validated by breaking what it guards and by confirming the healthy path is unchanged: an absent container now fails each check where it previously passed, a readable panic-free container still passes, a real panic is still caught, and a genuinely successful install still passes. |
||
|
Johnathan Corgan
|
c8a0ac5fca |
Stop a node whose logs cannot be read from counting as a clean node
count_log_pattern summed a per-node `docker logs | grep -c ... || true`, so a node the harness could not read contributed zero. The six assert_zero_count callers are negative health assertions -- no panics, no ERROR lines, no AEAD decrypt failures, no rekey msg2 failures -- and a contributed zero reads as clean, so one unreadable node silently weakened the assertion and all of them voided it. This is the family the harness-fallback issue was raised to high priority for, and it is the one remaining audit residual that can make a green rekey run lie about protocol code. The read now fails the count rather than degrading it, printing a sentinel that names the container. Both callers split the declaration from the assignment, because `local c=$(fn)` takes local's exit status and discards the function's -- which is how the original defect stayed invisible. Validated by breaking what it guards rather than by observing green: against absent containers the old reader returns 0 and assert_zero_count passes vacuously, while the new one returns rc=1 and reports a failure. Checked the other direction too, since a fix that reds a legitimately clean run is no use: a genuine zero across readable nodes still returns 0, and the sum is unchanged at 2+1+0 for a shimmed three-node read. |
||
|
Johnathan Corgan
|
55331a80b5 |
Gate on shell functions whose exit status is a log call's
A bash function returns its last command's status, so one ending in a log
call returns 0 whatever it did, and a caller written as `if func` or
`func || fail` has a gate that cannot fire. This tree found two in one
day: run_chaos ended in record, making every chaos row unconditionally
green since 2026-03-09, and build_fips_for_e2e ended in log, letting five
end-to-end legs test the previous commit's binary and report green. Both
were repaired one at a time. This is the rule that catches the next one.
What it enforces is a contract, not a bug hunt: a function whose status a
caller tests must end in an explicit return rather than leaving its
success value to whatever the last log call produced. That distinction is
deliberate and worth stating, because all three instances found in the
tree were benign — each failure path already returned early, so the
trailing echo reported a real success. Reading the function is the only
way to know that, and the next edit that puts an unguarded command before
the final log converts the benign shape into the defect with nothing to
notice. An explicit return costs a line and makes the class unreachable.
The three are given one here.
Scoped on the call sites rather than the definitions, the same way
check-log-strings.py scopes on what a grep reads rather than what its file
mentions: 315 functions scanned, 42 end in a log call, and only the ones
whose status something consumes are reported. A function that ends by
reporting and is called for its output is idiomatic and is left alone.
Without that scoping the finding list would be 42 long and would stop
being read.
Validated by construction rather than by a green run: injecting a copy of
build_fips_for_e2e's exact shape — unguarded docker cp, then a log as the
last statement, with a `|| { ...; return 1; }` caller — makes the checker
report it and exit 1.
Wired into both runners beside the parity and log-string checks.
|
||
|
Johnathan Corgan
|
3181861341 |
Assert the Link MMP pane exists before asserting its colour
mmp_focused_pane_indicator checked that the unfocused Link MMP title is not cyan with assert_ne! over fg_at. fg_at is find(..)? mapped to the cell's foreground, so it returns None for a title that was never drawn, and None != Some(Cyan). The assertion therefore passed just as happily when the pane was missing entirely as when it was present and unstyled. Assert presence first, so the colour check means "not highlighted" rather than "not there". Demonstrated rather than argued. Renaming the pane title in mmp.rs so "Link MMP" is never rendered leaves the original assertion passing, and makes the new one fail with the message it was given. Both files restored after. This is the only instance of the shape: it is the sole assert_ne! over fg_at or find in the fipstop tree, and the only assert_ne! in snapshots.rs at all. Quartet clean: fmt, build, clippy --all-targets -D warnings, test --lib at 1376 passed / 0 failed / 7 ignored. |
||
|
Johnathan Corgan
|
0f27bdbd2c |
Give the UDP microbench a reason and record the retry asymmetry
The UDP recv microbenchmark carried a bare #[ignore] while its sibling in the link module carries a self-describing one. Match it, so the reason appears wherever the test is listed rather than only in the doc comment. Record why [profile.ci] retries = 2 is not applied locally. The two gates disagree on purpose: the hosted runner retries a flaky test twice, the local sweep fails on the first failure. It exists for shared-runner packet loss, a property of that environment rather than of the code, and applying it locally would suppress a real local flake — a failure that only reproduces under load is a robustness bug to fix, not to retry past. Keeping the local sweep strict is what makes it the sharper gate. An undocumented asymmetry is indistinguishable from an oversight, which is why this is written down rather than left to be rediscovered. The cost is stated rather than hidden: a test that fails once and passes on retry is reported green with no separate signal, so a genuine intermittent failure can be absorbed. If that starts mattering the fix is to surface retried-but-passed tests, not to drop the retries. Quartet clean: fmt, build --workspace, clippy --all-targets -D warnings, and test --lib at 1376 passed / 0 failed / 7 ignored. |
||
|
Johnathan Corgan
|
d3eaad543d |
Make a skipped check visible in the verdict rather than in scrollback
Two suites could report a clean pass for work that did not run. stun-faults skips Phase 2 when tc netem is unavailable, announcing it several hundred lines above the result and then printing a bare "stun-faults-test passed". The verdict line now carries the count and names each skipped phase, and a run in which every phase was skipped fails outright, since it tested nothing. deb-install defines a skip() helper and a SKIP counter, prints "N skipped" in its summary, and calls skip() from nowhere, so the number can only ever be 0. Its exit tested FAIL alone, meaning a skip could not have failed the run even once something did set the counter. Gate on SKIP too. That changes no current outcome, which is the reason to do it now rather than later: the machinery and the reported number already existed, so the first skip path added would have printed "N skipped" beside a zero exit and read as coverage. Verified by driving the verdict logic at zero, one and three skips: clean pass, pass with the skip named, and a non-zero exit when nothing ran. |
||
|
Johnathan Corgan
|
2a6039995d |
Retire the ECN A/B test framing and record why suites are excluded
ecn-ab-test.sh was a -test.sh with no path to a failing result: it asserts nothing, applies no threshold, and nothing invokes it. It also could not have worked. It read a fixed sim-results/ecn-ab-on/ path while the runner has written timestamped directories since 2026-03-20, and not one ecn-ab result directory exists on disk, so it has found neither input for months and the "+10.2% recv throughput" figure the README carried is not reproducible from anything available. Rename it to ecn-ab-compare.sh, fix the path resolution to glob the timestamped directories, and stop swallowing a failed simulation with || true so a broken run cannot feed the comparison. Deliberately not adding the threshold assertion that would make it gateable. That needs a calibration corpus, and none exists precisely because the tool has never produced a kept result; a number invented now would assert a guess. The prerequisite is a calibration run set and a decision about what ECN is expected to deliver, which is a protocol question. Written in the script header rather than left implied. Add the exclusion block to ci-local.sh naming every suite and scenario neither runner runs, with the reason for each: interop, boringtun, iperf-test, this comparison tool, mesh-lab, and the four chaos scenarios outside CHAOS_SUITES. Until now "not in the suite list" was indistinguishable from "forgotten". |
||
|
Johnathan Corgan
|
1f149bda32 |
Run the convergence-gate unit tests in both CI runners
wait_until_connected decides whether every static suite proceeds or gives up, so a regression in it turns those suites' verdicts into noise. Its unit tests existed and were invoked by nothing. Wire them into ci-local.sh beside the parity and log-string checks, above the mode branches so --only and --test-only are gated too, and add the matching step to the ci-parity job in ci.yml. Putting them in one runner only would create exactly the drift check-ci-parity.sh exists to catch, and neither placement is visible to that checker since this is not a matrix suite. Verified by breaking what it guards rather than by observing green: setting wait_until_connected's default near-converged slack to 0 fails case4, records FAIL wait-converge, and exits the sweep 1. Restored after. They pass, which nothing had established before — 13 assertions, all green. They also take about 45 seconds rather than the "a few seconds" the header claimed, and nothing had contradicted that claim because no runner had ever invoked it. Header corrected with the measurement. The orphan sweep this called for is done and there are now no zero-reference *-test.sh files under testing/. It also settles a disagreement recorded between the audit and the task file: ecn-ab-test.sh is invoked by nothing, its one reference being a README description rather than a driver, so the audit was right and the earlier sweep wrong to call it a documented manual tool. interop-test.sh does have a driver in interop-stress.sh and iperf-test.sh one in iperf-compare-refs.sh, so those two were correctly classified. |
||
|
Johnathan Corgan
|
38a60c61da |
Record that churn-mixed's floors describe the invocation CI overrides
The baseline floors added for this scenario were calibrated from archived runs, and those runs are all the 10-node 120-second variant ci-local invokes as "churn-mixed --nodes 10 --duration 120". The file itself defaults to 20 nodes and 600 seconds, and --nodes sed-patches num_nodes in a copy before the scenario loads, so the gating run and a bare chaos.sh run are different scenarios. Nothing at the site said so. The floors hold for both, but only because the gating run is the smaller one. Retuning them against a bare 20-node run, which clears them easily at 20 answering, 1 root, 19 parented and 69 sessions, would put them past what the 10-node run reaches and turn CI red while a manual run stayed green. Say that where someone retuning them will read it. Confirmed by running the gating invocation directly: 10 answered, 2 roots, 8 parented, 18 sessions, all inside the ranges the floors were built from. churn-mixed is the only scenario CI overrides this way; the other twelve run their files as written. |
||
|
Johnathan Corgan
|
3d0a388511 |
Fail the ping test on an unknown topology profile
Every section of ping-test.sh is guarded by the profile name, so an unrecognised profile fell through all of them and the script exited 0 having run no assertion. A typo in the caller's argument produced a green run that tested nothing. Give it an else branch that names the valid profiles and exits 2. No existing leg changes behaviour: the hosted runner gates the ping step on matrix.type == 'static', which only static-mesh and static-chain carry, and ci-local passes only the two names in STATIC_SUITES. Worth noting while confirming that, though: ping-test.sh accepts a mesh-public profile that neither runner ever passes. Also record why the convergence waits are `|| true` — they are settling delays rather than assertions, and the directed-pair pings are what decides the run — and add admission-cap to the suite list in the ci-local header, which ran it without listing it. |
||
|
Johnathan Corgan
|
5d3a3d7cad |
Give the assertion-free chaos scenarios a convergence floor
Five scenarios named by the audit carried no assertions at all, so a run in which the mesh never formed exited 0 and reported green. Add a baseline assertion covering how many nodes answered, how many distinct roots they agreed on, how many took a parent, and optionally how many sessions were established, and apply it to those five plus the two cost scenarios left unasserted by the previous commit. For ethernet-only, ethernet-mesh, tcp-mesh, smoke-10, mixed-technology and depth-vs-cost the values are not calibrated: one root and N-1 parented nodes is what a spanning tree is, and all provably-completed archived runs of each show exactly that. churn-mixed is different and says so at the site. A scenario that stops and starts nodes on purpose does not hold a single tree, and its six completed runs end with two or three roots and seven or eight of ten nodes parented, so its floors sit one step outside the observed range. That leaves them catching a mesh that collapsed rather than one that churned, which is the most a sample of six supports. This gives Ethernet transport its only assertion anywhere in CI. The three tests in src/node/tests/ethernet.rs are ignored for requiring CAP_NET_RAW and neither runner passes --ignored, so until now nothing exercised that transport with a verdict attached. The floor is deliberately weak and deliberately not a substitute: it says the mesh formed, not that it formed the tree the scenario describes. Where those differ the scenario now says so in its own comments. |
||
|
Johnathan Corgan
|
e9ca741e53 |
Assert parent selection where the cost scenarios actually specify it
Four scenarios exist to test cost-based parent selection and each named its expected outcome in a comment that nothing read. Add a tree_parents assertion mapping a node to the parent it must have in the final tree snapshot, and encode it for the two scenarios whose stated outcome the implementation meets: cost-avoidance (n04 takes the fiber n03) and bottleneck-parent (n06 takes the fiber n03, n09 keeps its only parent n05). Both hold in all six provably-completed archived runs. The other two are left unasserted on purpose, and each for a different reason worth keeping distinct. mixed-technology says n06 and n08 should both pick the fiber parent n03. They do not. Across the five completed archived runs n06 picks n10 three times and n08 picks n04 four times. Encoding the criterion as written would red the scenario most runs, and encoding the observed behaviour would bless something nobody specified, so the disagreement is recorded at the site as an open question. n06 preferring n10 may well be correct and the comment stale, since n10 reaches it by fiber too. depth-vs-cost is different: its validation line does not name an outcome at all, saying only that the choice "reflects the actual cost tradeoff", which either answer satisfies. The corpus shows both occurring under one seed, three runs to two. That scenario needs a protocol decision about which parent is correct before it can have an assertion. Compare parents by address resolved from the snapshot's own my_node_addr, and fail rather than skip when a node is absent from the snapshot or still claims to be its own root. Those two states are the common ones in the older corpus and both produce the same "no match" a wrong parent does, so only separating them keeps a harness problem from reading as a routing verdict. |
||
|
Johnathan Corgan
|
5a11cf091d |
Give congestion-stress the assertions its criteria described
The scenario listed four success criteria "verified via post-run congestion snapshot". Nothing read the snapshot, so the scenario could not fail on any of them. Add a congestion_signals assertion taking a floor on the number of nodes reporting each counter, and encode three of the four. The floors are one node each because that is what the criteria say; tightening them to the counts recently observed would assert something nobody wrote down. The fourth criterion is not encoded, and that is the finding rather than an omission. No node has reported a non-zero kernel_drop_events in any of the 182 archived runs, so asserting it would red the scenario permanently. It is recorded at the site as an unmet criterion and a coverage gap: the transport drop-detection path the scenario names as its second signal is exercised by nothing. Two things about the corpus are worth carrying, both recorded in the scenario. Only six archived runs carry a status.txt and are therefore provably completed; all six meet the three encoded criteria with five to nine nodes reporting each signal. The 176 older runs meet none of them, and they are not invalid samples: each reached teardown far enough to write an analysis.txt, and ECN landed before all but one of them. What changed on 2026-07-22 is not established, since neither the scenario nor netem.py, traffic.py or control.py has been touched. Count the nodes reporting a signal rather than the magnitude of any one counter, since a single node with a large count would satisfy a magnitude test while proving the signal never propagated. A missing snapshot fails rather than reading as an absence of congestion. |
||
|
Johnathan Corgan
|
aab149e215 |
Fail a chaos run whose nodes logged errors
The simulation exit ladder reported panics and failed assertions but said nothing about ERROR-level log lines, so a run in which every node errored on every line still exited 0. Add a max_errors assertion and apply it to every scenario by default rather than having each one opt in. This is a floor on what a green run means rather than a property of an individual scenario, and a scenario that has to ask for the floor is one that can forget to. The default ceiling of 0 is what the archived corpus supports: across 2416 result directories no node log contains an ERROR-level line, while the WARN counter extracted by the same code path ranges from 0 to 1135, so the counter is known to discriminate rather than merely known to read zero. A scenario that legitimately induces errors raises the ceiling in its own YAML and says there why. Reject rust_log: off alongside it, since that is the one level that would leave the ceiling counting zero whatever the mesh did. |
||
|
Johnathan Corgan
|
73e6917341 |
Give cost-stability the parent-switch ceiling its criterion described
The scenario's success criterion existed only as a comment: count "Parent switched" in n04's log, expect at most 5. Nothing read it, and the scenario declared no assertions at all, so its exit code could only report that the mesh came up and nothing crashed. Add a max_parent_switches assertion and wire that criterion to it. The assertion takes an optional node scope, which is the part that matters. The existing sibling counts mesh-wide, and a criterion written about one node's log is a different quantity from the sum over every node: across archived runs of this scenario n04 is 1 or 2 while the mesh-wide total ranges 3 to 6. Asserting the sum against a per-node threshold would check something other than what was specified, and at this threshold would also have failed several runs that were fine. Counting nothing must not read as stability. A node id absent from the topology fails explicitly rather than matching zero log lines and sailing under the ceiling, and a scenario that declares a parent-switch assertion while setting a log level that suppresses the events it counts is rejected at load rather than passing vacuously after a full run. The parse rejects a mistyped key, a missing, negative, boolean or non-integer ceiling, and a node written empty or as a non-string -- that last one because YAML renders a bare `node:` as null, which would silently revert to the mesh-wide count. The scenario comment now records what the threshold is worth against the 11 completed archived runs rather than a single sample: 5 is well above anything observed, the daemon's own parent hold-down caps switches near 6 per run, and nearly every switch lands during initial tree formation before any mutation fires. So this catches only near-pathological reparenting. Tightening to 3 would make it a real detector, but that changes the stated criterion rather than fixing it, so it is left as a decision. Verified against a live run: the assertion evaluates n04 at 2 against mesh-wide 4 and passes, and with the ceiling temporarily set to 0 the scenario exits 3, the assertion-failure code, which it previously could not reach. |
||
|
Johnathan Corgan
|
300deb0476 |
Reject an unanswered show_mappings instead of reading it as zero
The mapping-reclaimed check expects zero mappings, and read the count
with `r.get('data',{}).get('mappings',[])`. An error response carries
no data field, so it parsed to zero and satisfied the check without the
gateway having answered at all. The expected value and the failure
value were the same number, which is the shape that lets an assertion
pass without asking anything.
Require the key to exist and be a list, and exit non-zero otherwise, so
the existing fallback turns an unanswered or malformed response into a
failure. The gateway always emits the key on success, including for an
empty set, so a genuine zero still reads as zero; confirmed against the
running gateway, where the reclamation check passes.
The earlier show_mappings poll shares the parse and is left alone: it
waits for a positive two, which no error response can produce. The
hazard was already documented there, and this is the call site that was
exposed to it.
|
||
|
Johnathan Corgan
|
700ac581ee |
Require a timeout and a drop verdict in the firewall assertions
Case (a) sends an unallowed inbound request that the ruleset should drop, and accepted any non-zero curl status as proof. A drop produces no RST, so curl can only hit its deadline and exit 28; connection refused, an unroutable address or a missing listener all fail too, with different codes, and the check counted those as a blocked connection. Require 28 exactly, so the assertion distinguishes silently dropped from failed for some other reason. Confirmed against the harness: the real run returns 28. The baseline check matched `counter packets`, which any counter rule satisfies whatever its verdict, including one that accepts. Require the rendered drop rule instead. The drop-counter read had the same weakness plus a worse one: it took field three of the first line mentioning a counter, which is the packet count only when the line begins with `counter`. On a rule such as `tcp dport 9 counter packets 42 bytes 3000 drop` it printed the port number. That shape is reachable, because the shipped ruleset includes the operator drop-in directory ahead of the trailing default deny and a drop-in may add its own counted drop. Extract by position within the matched text, and take the last match rather than the first, since case (a) falls through to the default deny that the ruleset emits as the final rule. Note in place at the baseline check that it still only proves a counted drop rule exists somewhere in the table, not that it is the trailing one; asserting rule position is a larger change than this warrants. The sample output in the README is updated to the new wording. |
||
|
Johnathan Corgan
|
98560594cf |
Prove ACL rejection per peer by matching a single log line
The four rejection checks were independent greps over the whole log: two npubs, a context and a decision. Together they proved only that node-a mentioned each npub somewhere, rejected somebody on an inbound handshake, and rejected somebody by denylist. Nothing tied a rejection to a named peer, and node-a lists both denied peers as auto_connect peers, so it logs their npubs on the outbound connect path whether or not any rejection ever happened. The actual message was never matched. Replace them with a helper that requires every given string on ONE line, and assert per denied peer: the real message, that peer's npub, and the denylist decision together. Strings match in any order by chaining fixed-string greps over the surviving lines, so the assertion does not depend on how the log formatter orders a message and its fields. A grep over empty input yields the empty string rather than anything a caller could mistake for a match, so an unreadable container times out and fails instead of passing. Two limits are written at the call sites rather than left implied. The decision conjunct discriminates nothing, since the two allowing variants return early and denylist is the only value that can reach that warning. And node-a authorizes before dialing, so it emits a fully formed rejection line for each denied peer on the outbound path; these assertions are satisfiable without the inbound check running at all, and it is the peer-count assertions that would catch that. Verified against the running harness: all three find a real matching line. The defect shape they replace was exercised offline first, with the three strings spread across three lines, where the old checks passed and the new ones fail. |
||
|
Johnathan Corgan
|
ebf34ae712 |
Read the wall clock directly for Nostr traversal timestamps
The traversal clock cached a Unix millisecond value and an Instant at first use, then served every later call by advancing the cached value with the monotonic elapsed time. A monotonic clock does not tick while the host is suspended, so once a machine had slept the returned value trailed real time by the sleep duration for the rest of the process lifetime, and never re-synced. Almost everything that clock feeds is an absolute timestamp. The NIP-40 expiration tags on adverts and traversal signals, and the issuedAt and expiresAt fields of offers and answers, are all computed as now plus a TTL; the freshness and cache-pruning paths compare it against a peer-authored, signed created_at. Once the host had slept longer than signal_ttl_secs, every offer was published already expired, relays dropped it, and the initiator timed out waiting for an answer with traversal broken until a restart. Read the wall clock on every call instead. This also removes a mismatch inside the traversal failure-state map, which was written here from the cached clock but written and read from the node lifecycle with the real one. Not platform-specific: monotonic clocks exclude suspended time on Linux and Windows as well, so this affected any host that suspends. A laptop is simply where a process lives long enough across a sleep to notice. The interval-shaped consumers hold up under a clock step. A forward step, which is what a resume produces, saturates the punch start delay to zero, and the attempt's own bounds are monotonic deadlines that are unaffected. A backward step lengthens that delay and can cost one punch attempt, which retries. Early eviction from the replay window cannot admit a replay under the shipped defaults, because the freshness window a replayed offer must also satisfy is strictly narrower than the replay window itself. The added test pins the contract and fires on a host that has genuinely suspended, but it is not a regression guard for this defect: nothing reachable from a unit test can simulate a suspend, so on a machine that has not slept the old implementation passes it too. Its comment says so rather than leaving a false sense of coverage. Reported in https://github.com/jmcorgan/fips/issues/128 |
||
|
Johnathan Corgan
|
291c4312dc |
Stop the ACL peer-count reader from turning "no answer" into zero
wait_for_peers_exact fell back to 0 when it could not read a container's peer count. Two of its six call sites are the ACL denial checks, which expect exactly 0 connected peers — so a failed docker exec satisfied them on the first iteration and the isolation property the suite exists to prove was never observed. Confirmed against a container that does not exist: the old reader returns "0" and the expect-zero comparison passes. The reader now returns the empty string when the daemon does not answer, which no numeric comparison can satisfy, and the timeout message says which of the two happened — never answered, or answered the wrong number. Same shape as admission-cap-test.sh's read_peer_count. |
||
|
Johnathan Corgan
|
d21f818659 |
Check that log strings tests match on are still ones the daemon emits
A test that greps the daemon's log for a message the daemon stopped emitting does not fail. It stops observing, and an assertion built on it — especially one expecting a count of zero — then passes because nothing can be seen rather than because nothing happened. Several findings have come from that one class, so it is now checked mechanically. testing/check-log-strings.py extracts the strings test code matches against daemon log text and requires each to exist in src/. It reads four shapes: python `"..." in line` tests, the first argument of the shell log helpers, bash associative-array pattern tables, and greps whose input is daemon log output. Patterns carry regex syntax, so each is reduced to the longest literal run of every alternation branch, with escaping resolved in the same pass — `\.` is a literal dot and `.` is a wildcard, and conflating them would let a pattern match text that is not there. Scoping is by what a grep READS, not by what its file mentions. A suite greps its own analyzer output, fipsctl JSON, Tor's log and ping output, and none of those have to correspond to a string in src/; scoping on the file produced 34 false positives. Strings that legitimately do not come from src/ are named in ALLOWED with a reason, so the exceptions are reviewable rather than invisible. It found six dead strings, three of which were not previously known: a second copy of "Excessive decrypt failures" in the interop pattern table, a dead "Handshake error" branch inside an alternation whose other branches still matched, and "bootstrap failed" in the NAT suite. All six are corrected to what the daemon emits, or dropped where the surviving branch already covers the case. Verified by breaking it two ways and confirming it reports. Also here, since it is the same failure shape one layer down: the shared log library returned an empty string for a container whose logs it could not read, so analysing a missing container reported no panics, no errors and no sessions, and exited 0 — indistinguishable from a clean run. It now reports which containers could not be read and raises, matching the fix already landed in the chaos copy. |
||
|
Johnathan Corgan
|
6c52b0e01e |
Let the static test network float so concurrent CI runs cannot collide
Two local CI runs on one host both asked docker for 172.20.0.0/24 and the second lost its whole static family to "Pool overlaps". Docker honours a fixed subnet request verbatim, so the only robust fix is to stop making one: fips-net now requests no subnet and docker assigns from its own pool, which cannot hand the same range to two runs. That means node addresses are not known before `up`, so peers address each other by container hostname instead. The generator emits node-<id>, or the topology's docker_host where the compose hostname differs — only the gateway profile, whose services are gw-*. External peers keep the address the topology gives them, since it is not ours to assign. The resolv.conf mount stays: dnsmasq is what forwards these names to docker's resolver and .fips to the daemon, so removing it would take out every .fips assertion. generated-configs is now per-run as well. A shared directory let two runs overwrite each other's node configs, which the subnet collision had been hiding by killing runs before that window opened. The generator, the compose bind mounts and env_file, the six scripts that read it, and teardown all follow FIPS_CI_NAME_SUFFIX; unset, every path renders as before. Teardown keeps the directory after a failed run, where it is the evidence of what the failing nodes were configured with. Three things this exposed that were wrong independently: admission-cap built its tcpdump patterns from the topology file's docker_ip literals. Floating the subnet makes those match nothing, which would have left its expect-zero "no Msg2 leaked" assertion passing because it could no longer see anything at all. It now reads addresses from the running containers. Restarting the denied peers together also made them swap addresses, so each peer's counts were really the pair's total; they are restarted one at a time now, and a check fails the suite outright if two denied peers ever share an address, because per-peer attribution is impossible once they do. Attribute lookups in the generator used a fixed ten-line window and read the next node's fields when a node omitted an attribute. An external node followed by an internal one was classified as internal, which under hostname peering would emit a name that resolves nowhere. Lookups are bounded to the node's own block; generated output is byte-identical for all eight topologies. The rekey outbound-only variant used to rewrite peer addresses to hostnames to set up its scenario. The generator now does that everywhere, so the rewrite matched nothing and was silently doing no work. It asserts the premise instead, and fails if a numeric address ever reappears. Verified by running three instances of this compose at once — tcp-chain plus two independent meshes — which drew 10.128.2/3/4.0/24 with no overlap while both meshes passed ping-test 20/20 over the real .fips path. tcp-chain is run by neither CI runner, so it was checked by hand: chain peer counts 1/2/1 and multi-hop .fips reachable both directions over TCP. gateway-lan still pins its own IPv4 and fd02:: ranges and is unchanged here, so the gateway profile is not yet concurrency-safe. |
||
|
Johnathan Corgan
|
428773490f |
Close the gaps an adversarial review found in the new CI guards
Four of these are places the parity guard could stop covering something without saying so, which is the failure mode it exists to prevent. Removing the deb-install suite array left it green, because the dispatch cross-check compared against a hardcoded name rather than the array it was meant to read. An arm-shaped line at an unexpected indent, or one written in quotes, was dropped silently by the arm pattern; unexpected indents are now a hard error rather than a quiet skip, and quoted arms parse. A matrix leg with a type of chaos or deb-install but no scenario key raised a Python traceback instead of reporting a problem, and a leg carrying a scenario but no suite key was skipped entirely even though scenario is now the identity for those legs. The e2e builder no longer folds docker create's stderr into the container id. Docker prints warnings on success as well as failure, so a platform-mismatch warning would have become part of the id and made every later reference to that container fail, turning a working extraction into a spurious red. Also corrects the comment about registering a new chaos assertion type, which named only one of the two key sets that actually need it. |
||
|
Johnathan Corgan
|
56f00058d4 |
Run the CI parity guard in both runners
The guard that keeps "local green" and "GitHub green" meaning the same thing was invoked by nothing. Its only entry point was an exec behind an explicit --check-parity flag, which by exec-ing could not be combined with an actual run, and no workflow step called it at all. A guard nobody runs is a guard that does not exist, which is how the mixed-profile divergence on next survived unnoticed. It now runs as the first stage of every local run and as its own job on GitHub. Locally it reports through the same result-recording path as every other stage, so a divergence sets the run's exit status instead of scrolling past. It is deliberately placed above the mode branches, so a divergence also fails --only, --test-only and --build-only runs: whichever subset was asked for, the claim that a local result means what a GitHub result means is what has broken. The GitHub side installs pyyaml explicitly rather than assuming the runner image carries it. --check-parity keeps working exactly as before. Three comment blocks that described the old folded comparison are corrected here, in the workflow, the local runner and the testing README; they were spread across three files and only two mention the guard by name, so the third is best found by searching for what it claims rather than for the script. |