From a54d539eb352e8e0625c2188f46ba11edc5b76e4 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 30 Jul 2026 05:42:01 -0500 Subject: [PATCH] =?UTF-8?q?fix(wake):=20#941=20review=20=C2=A72=20?= =?UTF-8?q?=E2=80=94=20snapshot=5Fts=20hardening:=20sha=20precondition,=20?= =?UTF-8?q?epoch=20sanity,=20future-skew=20rejection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A ts without a valid sha is unverifiable dating — dropped loudly. A future ts yields a negative age (stale reads fresher-than-fresh, the failure class this PR exists to fix) — rejected beyond WAKE_SNAPSHOT_TS_FUTURE_SLACK (default 300s). Epoch sanity (^[0-9]{1,12}$) is validated BEFORE the shell integer comparison so an absurd value cannot error past the guard. All three paths covered by new D12. Manifest now records that snapshot_* absence is deliberately not diagnostic (reviewer observation, PR #941 comment 19475). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NsKce8iZuSuRnu3gVMCBKB --- .../mosaic/framework/tools/wake/detector.sh | 26 ++++++++ .../mosaic/framework/tools/wake/manifest.txt | 18 +++++- .../tools/wake/test-wake-detector.sh | 59 +++++++++++++++++++ 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/framework/tools/wake/detector.sh b/packages/mosaic/framework/tools/wake/detector.sh index e12c107c..ec29c5ef 100755 --- a/packages/mosaic/framework/tools/wake/detector.sh +++ b/packages/mosaic/framework/tools/wake/detector.sh @@ -272,6 +272,32 @@ _poll_source() { snap_sha="" snap_ts="" fi + # A ts must be a sane positive epoch BEFORE any arithmetic touches it: a + # negative or absurdly large value would make the shell integer comparison + # below error out and silently KEEP the bad ts — validate first, compare after. + if [ -n "$snap_ts" ] && ! printf '%s' "$snap_ts" | grep -Eq '^[0-9]{1,12}$'; then + echo "detector.sh: source '$kind/$id' snapshot_ts rejected (not a sane positive epoch) — snapshot_ts DROPPED, poll continues (#940)." >&2 + snap_ts="" + fi + # ts is only meaningful anchored to a revision the consumer can re-verify — + # a bare number with no sha behind it is the weakest possible attestation, + # so ts requires a valid sha (#940 review §2). + if [ -n "$snap_ts" ] && [ -z "$snap_sha" ]; then + echo "detector.sh: source '$kind/$id' snapshot_ts without a valid snapshot_sha — snapshot_ts DROPPED (unverifiable dating), poll continues (#940)." >&2 + snap_ts="" + fi + # A FUTURE ts renders a stale snapshot fresher-than-fresh (negative age) — + # wrong in the reassuring direction, the exact failure class #940 fixes. + # Cross-host NTP skew of a few seconds is the NORMAL case, so allow a small + # slack; beyond it, drop the ts (the sha stays: independently verifiable). + if [ -n "$snap_ts" ]; then + local now_s + now_s="$(date +%s)" + if [ "$snap_ts" -gt $((now_s + ${WAKE_SNAPSHOT_TS_FUTURE_SLACK:-300})) ]; then + echo "detector.sh: source '$kind/$id' snapshot_ts is beyond the ${WAKE_SNAPSHOT_TS_FUTURE_SLACK:-300}s future-skew allowance (ts=$snap_ts now=$now_s) — snapshot_ts DROPPED, poll continues (#940)." >&2 + snap_ts="" + fi + fi else echo "detector.sh: source '$kind/$id' wrote UNPARSEABLE snapshot metadata on fd 3 — DROPPED, poll continues (#940)." >&2 fi diff --git a/packages/mosaic/framework/tools/wake/manifest.txt b/packages/mosaic/framework/tools/wake/manifest.txt index 031433ec..f1050ea5 100644 --- a/packages/mosaic/framework/tools/wake/manifest.txt +++ b/packages/mosaic/framework/tools/wake/manifest.txt @@ -266,8 +266,22 @@ # the consumer — zero round trips. Watch-list schema UNTOUCHED # ([1,1] unchanged — adapter contract + locator vocabulary, not # watch-list config). store.sh/reconcile.sh/beacon.sh UNCHANGED. -# Changed: detector.sh, digest.sh (+ test-wake-detector.sh D10/D11, -# test-wake-digest-quarantine.sh Q10). +# Review hardening (#941 §2): snapshot_ts additionally requires a +# VALID snapshot_sha (a bare number with no revision to re-verify +# against is the weakest attestation — dropped loudly), must be a +# sane positive epoch (^[0-9]{1,12}$ — validated BEFORE the shell +# integer comparison so an absurd value cannot error past it), and +# must not sit beyond a future-skew allowance +# (WAKE_SNAPSHOT_TS_FUTURE_SLACK, default 300 s): a future ts +# yields a NEGATIVE age — stale-reads-fresher-than-fresh, the +# exact failure class #940 fixes. NOTE for consumers: these fields +# are ADVISORY and their ABSENCE IS DELIBERATELY NOT DIAGNOSTIC — +# a pre-#940 adapter and a dropped-as-malformed attestation render +# identically (no snapshot_* fields); the drop is loud only in the +# detector's own stderr. Do not build load-bearing logic on the +# absence of these fields. +# Changed: detector.sh, digest.sh (+ test-wake-detector.sh +# D10/D11/D12, test-wake-digest-quarantine.sh Q10). component=wake version=0.6.12 diff --git a/packages/mosaic/framework/tools/wake/test-wake-detector.sh b/packages/mosaic/framework/tools/wake/test-wake-detector.sh index ef1ba371..b3adffbd 100755 --- a/packages/mosaic/framework/tools/wake/test-wake-detector.sh +++ b/packages/mosaic/framework/tools/wake/test-wake-detector.sh @@ -529,6 +529,65 @@ EOF [ "$(det_seq)" = "2" ] || fail_msg "D11: both real deltas must still have enqueued (advisory metadata never suppresses a wake), got seq $(det_seq)" ) && ok +echo "== D12: snapshot_ts guards (#940 review §2) — ts requires a sha, a future ts is dropped, an absurd ts cannot reach the comparison ==" +( + WAKE_STATE_HOME="$(fresh_state d12)" + export WAKE_STATE_HOME + unset WAKE_AGENT + stub="$TMP_ROOT/d12stub" + mkdir -p "$stub" + cat >"$stub/adapter.sh" <&3; } 2>/dev/null +exit 0 +EOF + chmod +x "$stub/adapter.sh" + export WAKE_DETECTOR_SOURCE_CMD="$stub/adapter.sh" + wl="$TMP_ROOT/d12.json" + write_watchlist "$wl" 1 + export WAKE_WATCH_LIST="$wl" + printf 'SHA-AAA\n' >"$stub/repo_r1" + printf '## LANE-X\ndecision: hold\n' >"$stub/board_file_b1" + "$DET" poll-once >/dev/null 2>&1 || fail_msg "D12: baseline pass failed" + goodsha="0123abc4567890def0123abc4567890def012345" + # (a) ts WITHOUT sha — the weakest attestation: an unverifiable number with no + # revision to re-verify against. Dropped loudly; the wake still fires. + printf '{"snapshot_ts":1753850000}\n' >"$stub/repo_r1.meta" + printf 'SHA-BBB\n' >"$stub/repo_r1" + err="$("$DET" poll-once 2>&1 >/dev/null)"; rc=$? + [ "$rc" -eq 0 ] || fail_msg "D12: ts-without-sha must NEVER fail the poll (rc=$rc)" + echo "$err" | grep -qi 'without a valid snapshot_sha' || fail_msg "D12: dropping ts-without-sha must be LOUD on stderr [$err]" + entry="$("$STORE" drain | tail -1)" + echo "$entry" | jq -e '.locators | has("snapshot_sha") or has("snapshot_ts")' >/dev/null 2>&1 \ + && fail_msg "D12: ts-without-sha must emit NO snapshot fields [$entry]" + # (b) valid sha + FUTURE ts — a negative age would read fresher-than-fresh, + # wrong in the reassuring direction. ts dropped, sha kept (independently verifiable). + future=$(( $(date +%s) + 9999 )) + printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$future" >"$stub/repo_r1.meta" + printf 'SHA-CCC\n' >"$stub/repo_r1" + err="$("$DET" poll-once 2>&1 >/dev/null)"; rc=$? + [ "$rc" -eq 0 ] || fail_msg "D12: future ts must NEVER fail the poll (rc=$rc)" + echo "$err" | grep -qi 'future-skew' || fail_msg "D12: dropping a future ts must be LOUD on stderr [$err]" + entry="$("$STORE" drain | tail -1)" + echo "$entry" | jq -e --arg s "$goodsha" 'select(.locators.snapshot_sha==$s) | .locators | has("snapshot_ts") | not' >/dev/null 2>&1 \ + || fail_msg "D12: future ts must drop ts but KEEP the sha [$entry]" + # (c) valid sha + absurdly large ts — must be rejected by the sanity regex BEFORE + # the shell integer comparison (which would error out and silently keep it). + printf '{"snapshot_sha":"%s","snapshot_ts":99999999999999999999}\n' "$goodsha" >"$stub/repo_r1.meta" + printf 'SHA-DDD\n' >"$stub/repo_r1" + err="$("$DET" poll-once 2>&1 >/dev/null)"; rc=$? + [ "$rc" -eq 0 ] || fail_msg "D12: absurd ts must NEVER fail the poll (rc=$rc)" + echo "$err" | grep -qi 'sane positive epoch' || fail_msg "D12: rejecting an absurd ts must be LOUD on stderr [$err]" + entry="$("$STORE" drain | tail -1)" + echo "$entry" | jq -e --arg s "$goodsha" 'select(.locators.snapshot_sha==$s) | .locators | has("snapshot_ts") | not' >/dev/null 2>&1 \ + || fail_msg "D12: absurd ts must drop ts but KEEP the sha [$entry]" + [ "$(det_seq)" = "3" ] || fail_msg "D12: all three real deltas must still have enqueued, got seq $(det_seq)" +) && ok + echo if [ -s "$FAILFILE" ]; then echo "wake detector harness: FAILED ($(grep -c . "$FAILFILE") assertion(s))" >&2