fix(wake): #941 review §2 — snapshot_ts hardening: sha precondition, epoch sanity, future-skew rejection
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
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 <[email protected]>
Claude-Session: https://claude.ai/code/session_01NsKce8iZuSuRnu3gVMCBKB
This commit is contained in:
Jason Woltje
co-authored by
Claude Fable 5
Claude Fable 5
parent
bb54e09aec
commit
a54d539eb3
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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" <<EOF
|
||||
#!/usr/bin/env bash
|
||||
set -u
|
||||
kind="\$1"; id="\$2"
|
||||
base="$stub/\${kind}_\${id}"
|
||||
[ -f "\$base" ] && cat "\$base"
|
||||
[ -f "\$base.meta" ] && { cat "\$base.meta" >&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
|
||||
|
||||
Reference in New Issue
Block a user