forked from mosaicstack/stack
fix(wake): #941 review §2 round 2 — validate WAKE_SNAPSHOT_TS_FUTURE_SLACK before arithmetic
The operator's slack knob was interpolated raw into $((...)) under set -u:
'300s'/'5m' -> 'value too great', 'abc' -> 'unbound variable' — all FATAL to
the poll, falsifying the PR's own 'poll never fails' invariant, and a negative
value silently inverted the guard into deny-all. Now resolved once into a
local, validated ^[0-9]{1,9}$, loud fallback to 300 on mismatch. New D13
group: '300s'/'abc'/negative all fall back and KEEP valid metadata; a valid
tightened knob (0) still rejects a future ts. Manifest states the skew
guarantee explicitly: a surviving snapshot_ts is attested only to within SLACK
seconds of the DETECTOR's clock — bounded negative age at render is possible;
treat age<=0 as effectively-current, never proof of freshness.
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
8710d0f6d7
commit
313f3087e6
@@ -291,10 +291,19 @@ _poll_source() {
|
||||
# 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
|
||||
# The OPERATOR's knob gets the same discipline as the adapter's ts: it
|
||||
# is interpolated into $((...)) under set -u, so a non-numeric value
|
||||
# ('300s', '5m', 'abc') would be FATAL to the poll — the one thing this
|
||||
# block must never be. Resolve it ONCE, validate, fall back loudly.
|
||||
local now_s slack
|
||||
slack="${WAKE_SNAPSHOT_TS_FUTURE_SLACK:-300}"
|
||||
if ! printf '%s' "$slack" | grep -Eq '^[0-9]{1,9}$'; then
|
||||
echo "detector.sh: WAKE_SNAPSHOT_TS_FUTURE_SLACK='$slack' is not a plain non-negative integer (seconds) — falling back to 300, poll continues (#940)." >&2
|
||||
slack=300
|
||||
fi
|
||||
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
|
||||
if [ "$snap_ts" -gt $((now_s + slack)) ]; then
|
||||
echo "detector.sh: source '$kind/$id' snapshot_ts is beyond the ${slack}s future-skew allowance (ts=$snap_ts now=$now_s) — snapshot_ts DROPPED, poll continues (#940)." >&2
|
||||
snap_ts=""
|
||||
fi
|
||||
fi
|
||||
|
||||
@@ -274,14 +274,27 @@
|
||||
# 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
|
||||
# exact failure class #940 fixes. The SLACK knob itself is
|
||||
# operator input interpolated into arithmetic under set -u, so it
|
||||
# gets the same discipline (#941 §2 round 2): validated
|
||||
# ^[0-9]{1,9}$, else LOUD fallback to 300 — a malformed knob
|
||||
# ('300s', '5m', 'abc') must never kill the poll, and a negative
|
||||
# one must never invert the guard into deny-all. SKEW GUARANTEE
|
||||
# (stated, not implied): the future-skew check runs against the
|
||||
# DETECTOR's clock; consumer-side age arithmetic runs on the
|
||||
# consumer's. A surviving snapshot_ts is therefore attested only
|
||||
# to within SLACK seconds of the detector's clock, plus whatever
|
||||
# skew the consumer's own clock adds — a small NEGATIVE age at
|
||||
# render is bounded, not impossible; treat age <= 0 as
|
||||
# "effectively current," never as proof of freshness.
|
||||
# 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).
|
||||
# D10/D11/D12/D13, test-wake-digest-quarantine.sh Q10).
|
||||
component=wake
|
||||
version=0.6.12
|
||||
|
||||
|
||||
@@ -588,6 +588,74 @@ EOF
|
||||
[ "$(det_seq)" = "3" ] || fail_msg "D12: all three real deltas must still have enqueued, got seq $(det_seq)"
|
||||
) && ok
|
||||
|
||||
echo "== D13: WAKE_SNAPSHOT_TS_FUTURE_SLACK is operator input — a malformed knob must fall back to 300 loudly, never kill the poll or invert the guard =="
|
||||
(
|
||||
WAKE_STATE_HOME="$(fresh_state d13)"
|
||||
export WAKE_STATE_HOME
|
||||
unset WAKE_AGENT
|
||||
stub="$TMP_ROOT/d13stub"
|
||||
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/d13.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 "D13: baseline pass failed"
|
||||
goodsha="0123abc4567890def0123abc4567890def012345"
|
||||
# Every sub-case ships a VALID sha + CURRENT ts: the metadata itself is good,
|
||||
# only the operator's knob is broken, so the correct outcome is fallback-and-keep.
|
||||
# (a) '300s' — the natural duration-suffix mistake. Under set -u this used to be
|
||||
# FATAL inside \$((...)): rc=1, wake never fires. Now: loud fallback, ts kept.
|
||||
printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$(date +%s)" >"$stub/repo_r1.meta"
|
||||
printf 'SHA-BBB\n' >"$stub/repo_r1"
|
||||
err="$(WAKE_SNAPSHOT_TS_FUTURE_SLACK='300s' "$DET" poll-once 2>&1 >/dev/null)"; rc=$?
|
||||
[ "$rc" -eq 0 ] || fail_msg "D13: SLACK='300s' must NEVER fail the poll (rc=$rc)"
|
||||
echo "$err" | grep -qi 'falling back to 300' || fail_msg "D13: malformed slack 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")' >/dev/null 2>&1 \
|
||||
|| fail_msg "D13: valid metadata must SURVIVE a malformed knob (fallback, not drop) [$entry]"
|
||||
# (b) 'abc' — bare word: under set -u, arithmetic dies on 'abc: unbound variable'.
|
||||
printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$(date +%s)" >"$stub/repo_r1.meta"
|
||||
printf 'SHA-CCC\n' >"$stub/repo_r1"
|
||||
err="$(WAKE_SNAPSHOT_TS_FUTURE_SLACK='abc' "$DET" poll-once 2>&1 >/dev/null)"; rc=$?
|
||||
[ "$rc" -eq 0 ] || fail_msg "D13: SLACK='abc' must NEVER fail the poll (rc=$rc)"
|
||||
echo "$err" | grep -qi 'falling back to 300' || fail_msg "D13: non-numeric slack 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")' >/dev/null 2>&1 \
|
||||
|| fail_msg "D13: valid metadata must SURVIVE a non-numeric knob [$entry]"
|
||||
# (c) negative slack — arithmetic would ACCEPT it and silently invert the guard
|
||||
# into deny-all (a CURRENT ts reads as 'future'). Must fall back and keep the ts.
|
||||
printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$(date +%s)" >"$stub/repo_r1.meta"
|
||||
printf 'SHA-DDD\n' >"$stub/repo_r1"
|
||||
err="$(WAKE_SNAPSHOT_TS_FUTURE_SLACK='-99999999' "$DET" poll-once 2>&1 >/dev/null)"; rc=$?
|
||||
[ "$rc" -eq 0 ] || fail_msg "D13: negative SLACK must NEVER fail the poll (rc=$rc)"
|
||||
echo "$err" | grep -qi 'falling back to 300' || fail_msg "D13: negative slack 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")' >/dev/null 2>&1 \
|
||||
|| fail_msg "D13: negative slack must NOT invert the guard into deny-all [$entry]"
|
||||
# (d) a VALID knob is still honored: slack=0 with a ts 60s ahead -> future-skew drop.
|
||||
printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$(( $(date +%s) + 60 ))" >"$stub/repo_r1.meta"
|
||||
printf 'SHA-EEE\n' >"$stub/repo_r1"
|
||||
err="$(WAKE_SNAPSHOT_TS_FUTURE_SLACK='0' "$DET" poll-once 2>&1 >/dev/null)"; rc=$?
|
||||
[ "$rc" -eq 0 ] || fail_msg "D13: valid SLACK=0 must not fail the poll (rc=$rc)"
|
||||
echo "$err" | grep -qi 'future-skew' || fail_msg "D13: a valid tightened slack must still reject a future ts [$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 "D13: valid SLACK=0 must drop the future ts but keep the sha [$entry]"
|
||||
[ "$(det_seq)" = "4" ] || fail_msg "D13: all four 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