Skip to content

test(mesh/audit): pin PSK-verified seq-seed circular-trust defence (85%->86%)#555

Merged
cagataycali merged 1 commit into
strands-labs:mainfrom
cagataycali:test/audit-seq-psk-verified-seed
Jun 20, 2026
Merged

test(mesh/audit): pin PSK-verified seq-seed circular-trust defence (85%->86%)#555
cagataycali merged 1 commit into
strands-labs:mainfrom
cagataycali:test/audit-seq-psk-verified-seed

Conversation

@cagataycali

Copy link
Copy Markdown
Member

Problem

When the audit seq sidecar is unusable (missing/corrupt/symlinked), _load_seq_counters() re-seeds _SEQ_COUNTERS by walking the persisted audit log and taking max(seq) per peer. That walk is a trust boundary: it restores the monotonic-sequence floor replay protection depends on.

The circular-trust defence on that boundary is: when STRANDS_MESH_AUDIT_PSK is configured, the seed walk trusts ONLY records whose HMAC sig it can recompute and compare_digest-match. Without it, an attacker who could append a single forged record (no PSK in dev, or a PSK exfiltrated long enough to write one line then cleared) could plant seq=10**9; on the next restart with a degraded sidecar that value would silently become the per-peer floor, denying the legitimate writer ~a billion working sequence numbers.

The existing recovery/cap tests covered only the no-PSK posture, leaving this PSK-verify branch (the actual defence) unexercised.

Change

Adds tests/mesh/test_audit_seq_psk_verified_seed.py pinning the PSK-present seed walk:

  • a correctly-signed record seeds the seq floor (and _next_seq advances past it),
  • an unsigned record and a PSK_DEGRADED poison sentinel are skipped,
  • a wrong-HMAC record fails compare_digest and is skipped (no poisoned floor),
  • an over-cap but correctly-signed record is refused by the shared _MAX_SEED_SEQ cap,
  • a log of only-unverified records seeds nothing (fail-closed, with the circular-trust WARNING).

Test-only; no source change.

Verification

  • 3 of the 5 tests FAIL on pre-defence code (verified by neutralising the if psk is not None: verify gate to trust-every-record); all 5 pass on main.
  • strands_robots/mesh/audit.py coverage 85% -> 86%; the previously-uncovered PSK-verify trust block is now exercised.
  • Full tests/mesh/ suite: 1566 passed, 2 skipped.
  • ruff check / ruff format --check / mypy all clean.

…5%->86%)

The audit-log seq-seed fallback in _load_seq_counters() restores the
per-peer monotonic-sequence floor when the seq sidecar is unusable. That
walk is a trust boundary: with STRANDS_MESH_AUDIT_PSK set it must trust
ONLY records whose HMAC sig it can recompute and compare_digest-match, so
an attacker who appended a single forged record cannot poison the seq
restore on the next restart with a degraded sidecar.

Existing recovery tests covered only the no-PSK posture, leaving the
PSK-verify branch (the actual circular-trust defence) uncovered. Pin it:
a correctly-signed record seeds the floor; unsigned, PSK_DEGRADED poison,
and wrong-HMAC records are skipped; an over-cap signed record is refused
by the shared _MAX_SEED_SEQ cap; an all-unverified log seeds nothing
(fail-closed). 3 of the 5 fail on pre-defence code (trust-every-record).

@yinsong1986 yinsong1986 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Test-only PR adding tests/mesh/test_audit_seq_psk_verified_seed.py (+155/-0), which pins the PSK-verified branch of the audit-log seq-seed fallback in strands_robots/mesh/audit.py::_load_seq_counters. The five tests exercise the circular-trust defence on the seq-seed trust boundary: a correctly-signed record seeds the floor and _next_seq advances past it; unsigned records, PSK_DEGRADED/SIGN_FAILED poison sentinels, and wrong-HMAC records are skipped; an over-cap-but-signed record is refused by the shared _MAX_SEED_SEQ cap; and an all-unverified log seeds nothing (fail-closed). No source change.

The assertions line up with the actual module behaviour: _sign uses _canonical_bytes (which excludes sig) exactly as the writer does, the over-cap test's forged log-string and the all-unverified test's unverified/NOT seeded strings match the warnings at audit.py:607 and audit.py:627, and the empty-tmpdir fixture forces the no-sidecar degraded path that triggers the walk.

What's good

  • Pins a previously-uncovered, reviewed security defence with a regression test (AGENTS.md > PR-6 "Pin regression tests for reviewed fixes"); the description confirms 3/5 fail on neutralised pre-defence code.
  • Uses monkeypatch.setenv and tmp_path with full fixture teardown rather than mutating os.environ (AGENTS.md > PR #86 testing patterns).
  • No host paths, no non-ASCII / emoji in user-facing or test strings, scope-disciplined (test-only, zero source diff).

@cagataycali
cagataycali merged commit 1f8440d into strands-labs:main Jun 20, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants