The cooldown guards validate syntax but not range: a digits-only marker ahead of now sticks the throttle ON permanently, with no diagnostic

[fdaviddpt]

Found by a release-gate security audit of v0.16.0..main while cutting v0.17.0. Pre-existing rather than introduced by #322/#325 — but it is the missing half of the guard that release exists to add, and the release note asserts the opposite of it.

Mechanism

case "$LAST_MOD" in ''|*[!0-9]*) LAST_MOD=0 ;; esac accepts any all-digit string. It rejects the syntax error; it does not bound the value.

A marker holding a future epoch — clock set back after an NTP correction, a VM snapshot restore, a container clock jump — or an overflowing integer produces a negative ELAPSED. [ "$ELAPSED" -lt "$SAVE_COOLDOWN" ] is then true, and the exit 0 fires.

The exit 0 sits above the line that rewrites the marker. So the self-heal that the whole fix rests on is unreachable on exactly the path that needs it:

$ /bin/bash -c 'for M in 99999999999999999999 9223372036854775807; do
    LAST_MOD=$M; case "$LAST_MOD" in ""|*[!0-9]*) LAST_MOD=0;; esac
    E=$(( $(date +%s) - 10#$LAST_MOD )); printf "marker=%-22s ELAPSED=%s " "$M" "$E"
    if [ "$E" -lt 120 ]; then echo "-> COOLDOWN ENGAGED (exit 0, marker NOT rewritten)"; else echo "-> proceeds"; fi; done'
marker=99999999999999999999   ELAPSED=-7766279629666055955 -> COOLDOWN ENGAGED (exit 0, marker NOT rewritten)
marker=9223372036854775807    ELAPSED=-9223372035068589843 -> COOLDOWN ENGAGED (exit 0, marker NOT rewritten)

Every subsequent run skips, permanently, until the wall clock catches up or someone runs --force or deletes the file by hand. No stderr, nothing in hook-errors.log.

Four sites, and on each the rewrite is unreachable on the stuck path

Site	Stuck state	Rewrite that never runs
scripts/save-session.sh:169-177	no session is ever saved again	:187
scripts/save-session.sh:600-604	NDC compression never runs again; now.md grows unbounded	:622, inside if [ "$RUN_NDC" = true ]
hooks.d/after_save/50-git-backup.sh:169-176	git backup permanently stopped — #258's original outage, reached through a value the guard accepts	_gb_stamp_cooldown, :583/:586
scripts/post-tool-hook.sh:316-326	never forks a save again; compounds the first row	this file never writes the marker

Why this outranks the bug that was just fixed

The syntax-error case self-heals and leaves one line in the log. This one is permanent and mute — the class where a user discovers it by noticing that nothing has been saved for a week.

Class: fails-to-preserve. The data is still on the machine; what stops is the preservation.

The release note currently says the opposite

CHANGELOG.md and the code comment at scripts/save-session.sh:155-158 both state that both gates self-heal and that the cost is "one skipped cooldown per corruption event, not a throttle stuck off". True for the syntax-error case, false for this one — and the confident scoping is exactly what would make a maintainer deprioritise this report. Corrected in the v0.17.0 release commit.

Judgment call for whoever takes this

A range check is not simply "clamp negatives to 0". Decide what a future-dated marker means and say it out loud:

• Treat ELAPSED < 0 as "the clock moved" and proceed (self-healing, matches the existing philosophy), or
• treat it as corruption, reset the marker to now, and emit a diagnostic.

The second is more honest but changes behaviour on a laptop that legitimately suspends across a DST or timezone shift. Whichever you pick, the marker rewrite has to become reachable on the stuck path — that is the structural half, and it is the same shape at all four sites.

Pin it with a test that asserts the behaviour (a second run does save), not the diagnostic — a fallback to 0 makes a corrupt marker read as "very old", so corrupt and clean-but-ancient reach the same decision and only a side effect separates broken from fixed.

[fdaviddpt]

Reopening — this was closed by PR #328 (the v0.17.0 release commit), which referenced it rather than fixing it. The code is unchanged.

Verified just now on main (3353c1f):

scripts/save-session.sh
  155-    # Scope it honestly: `date +%s > "$COOLDOWN_MARKER"` sits below the `fi`, so
  156:    # it runs regardless and the marker SELF-HEALS. One corruption costs one
  157-    # skipped cooldown, not a throttle stuck off.

That comment is the claim this issue exists to refute, still in the source. No commit between eeb7a62 and 3353c1f touches save-session.sh:169-177, and the range guard is still case … *[!0-9]*) LAST_MOD=0 with no upper bound.

Two separate things were conflated when it closed:

• The CHANGELOG was corrected at v0.17.0 — that part is real and is what the release PR did.
• The comment in the source was not, and neither was the guard. Anyone reading save-session.sh still gets the confident wrong scoping.

Found by the agent building v0.18.0, which noticed the issue's own body says the claim was "corrected in the v0.17.0 release commit" and then checked whether the code matched. It did not.

Worth naming the mechanism, because it will recur: an issue referenced in a release PR body can be closed by that PR without a line of its subject matter changing. The post-merge check I run verifies that a linked issue closed; it does not verify the issue was fixed. Those are different questions and only the second one matters.