From 313f3087e641398143cd5687ad0a0fce5a551c89 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 30 Jul 2026 05:55:10 -0500 Subject: [PATCH 1/2] =?UTF-8?q?fix(wake):=20#941=20review=20=C2=A72=20roun?= =?UTF-8?q?d=202=20=E2=80=94=20validate=20WAKE=5FSNAPSHOT=5FTS=5FFUTURE=5F?= =?UTF-8?q?SLACK=20before=20arithmetic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01NsKce8iZuSuRnu3gVMCBKB --- .../mosaic/framework/tools/wake/detector.sh | 15 +++- .../mosaic/framework/tools/wake/manifest.txt | 17 ++++- .../tools/wake/test-wake-detector.sh | 68 +++++++++++++++++++ 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/packages/mosaic/framework/tools/wake/detector.sh b/packages/mosaic/framework/tools/wake/detector.sh index ec29c5ef..f35d229b 100755 --- a/packages/mosaic/framework/tools/wake/detector.sh +++ b/packages/mosaic/framework/tools/wake/detector.sh @@ -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 diff --git a/packages/mosaic/framework/tools/wake/manifest.txt b/packages/mosaic/framework/tools/wake/manifest.txt index f1050ea5..2c089c31 100644 --- a/packages/mosaic/framework/tools/wake/manifest.txt +++ b/packages/mosaic/framework/tools/wake/manifest.txt @@ -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 diff --git a/packages/mosaic/framework/tools/wake/test-wake-detector.sh b/packages/mosaic/framework/tools/wake/test-wake-detector.sh index b3adffbd..126559bb 100755 --- a/packages/mosaic/framework/tools/wake/test-wake-detector.sh +++ b/packages/mosaic/framework/tools/wake/test-wake-detector.sh @@ -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" <&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 -- 2.54.0 From 667402457e6a42dd84f6293b39b2626c8d0a9388 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 30 Jul 2026 06:04:48 -0500 Subject: [PATCH 2/2] =?UTF-8?q?fix(wake):=20#942=20review=20=E2=80=94=20fo?= =?UTF-8?q?rce=20base-10=20on=20the=20slack=20knob;=20shape=20validation?= =?UTF-8?q?=20is=20not=20radix=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit '08'/'09' pass ^[0-9]{1,9}$ yet are fatal octal in $((...)) — the same poll-killing mode this PR closes — and '0300' silently means 192, changing the operator's stated intent with no warning. slack=$((10#$slack)) after the regex (which guarantees pure digits, so 10# is safe) makes zero-padded knobs mean what was written instead of rejecting them. Two new D13 sub-cases: '08' survives with metadata kept, and the discrimination test — '0300' with a ts +250s ahead KEEPS the ts (decimal 300) where octal 192 would have dropped it. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01NsKce8iZuSuRnu3gVMCBKB --- .../mosaic/framework/tools/wake/detector.sh | 5 +++++ .../mosaic/framework/tools/wake/manifest.txt | 7 ++++++- .../tools/wake/test-wake-detector.sh | 20 ++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/mosaic/framework/tools/wake/detector.sh b/packages/mosaic/framework/tools/wake/detector.sh index f35d229b..4eea6248 100755 --- a/packages/mosaic/framework/tools/wake/detector.sh +++ b/packages/mosaic/framework/tools/wake/detector.sh @@ -301,6 +301,11 @@ _poll_source() { 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 + # Shape validation is not radix validation: bash reads a leading zero as + # OCTAL, so '08'/'09' pass the regex yet are FATAL in $((...)), and + # '0300' silently means 192. Force base-10 so the knob means what the + # operator wrote (safe: the regex above guarantees pure digits). + slack=$((10#$slack)) now_s="$(date +%s)" 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 diff --git a/packages/mosaic/framework/tools/wake/manifest.txt b/packages/mosaic/framework/tools/wake/manifest.txt index 2c089c31..02830656 100644 --- a/packages/mosaic/framework/tools/wake/manifest.txt +++ b/packages/mosaic/framework/tools/wake/manifest.txt @@ -279,7 +279,12 @@ # 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 +# one must never invert the guard into deny-all. Shape validation +# is NOT radix validation (#942 review): bash reads leading zeros +# as OCTAL, so '08'/'09' pass the regex yet are fatal in $((...)) +# and '0300' silently means 192 — the knob is therefore forced +# base-10 (10#) after validation, so it means what the operator +# wrote. 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 diff --git a/packages/mosaic/framework/tools/wake/test-wake-detector.sh b/packages/mosaic/framework/tools/wake/test-wake-detector.sh index 126559bb..9ec80e68 100755 --- a/packages/mosaic/framework/tools/wake/test-wake-detector.sh +++ b/packages/mosaic/framework/tools/wake/test-wake-detector.sh @@ -644,6 +644,24 @@ EOF 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]" + # (d2-pre) ZERO-PADDED knobs — shape-valid, radix-hostile. '08' passes the + # regex but is fatal octal in \$((...)) without the 10# normalization; '0300' + # silently means 192 (octal), so a ts +250s ahead would be WRONGLY dropped. + # With 10#: '08' means 8 and survives; '0300' means 300 and the +250s ts is KEPT. + printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$(date +%s)" >"$stub/repo_r1.meta" + printf 'SHA-DD2\n' >"$stub/repo_r1" + err="$(WAKE_SNAPSHOT_TS_FUTURE_SLACK='08' "$DET" poll-once 2>&1 >/dev/null)"; rc=$? + [ "$rc" -eq 0 ] || fail_msg "D13: SLACK='08' (octal-fatal without 10#) must NEVER fail the poll (rc=$rc) [$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: SLACK='08' with a current ts must keep the metadata [$entry]" + printf '{"snapshot_sha":"%s","snapshot_ts":%s}\n' "$goodsha" "$(( $(date +%s) + 250 ))" >"$stub/repo_r1.meta" + printf 'SHA-DD3\n' >"$stub/repo_r1" + err="$(WAKE_SNAPSHOT_TS_FUTURE_SLACK='0300' "$DET" poll-once 2>&1 >/dev/null)"; rc=$? + [ "$rc" -eq 0 ] || fail_msg "D13: SLACK='0300' must not fail the poll (rc=$rc)" + 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: SLACK='0300' must mean 300 (decimal), so a +250s ts is KEPT — octal 192 would have dropped it [$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" @@ -653,7 +671,7 @@ EOF 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)" + [ "$(det_seq)" = "6" ] || fail_msg "D13: all six real deltas must still have enqueued, got seq $(det_seq)" ) && ok echo -- 2.54.0