From a361b71c40ca14b8762888f026a74819cd66792f Mon Sep 17 00:00:00 2001 From: Joakim Persson Date: Thu, 27 Aug 2026 23:29:30 +0200 Subject: [PATCH] ship: don't trust the mtime the exporter deliberately backdates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rsync --update skips a file whose mtime is not strictly newer than the receiver's. The stage file's mtime IS the source transcript's mtime (os.utime() at :903, "preserve session mtime for dedup stability"), so re-exporting a session that has not been appended to since its last ship produces a mtime that is not newer than what's already at the receiver — exactly the case a redactor upgrade needs to ship, because content differs while mtime does not. --update reports success and sends nothing. Reported and patched by pi@mbp-m1-2020 (evt_20260827T211925_9674a31da0b4, artifact art_20260827T211839_fa52af563105, sha256 f217e47e…), measured live: a scrubbed re-export of a dormant session (pi_01a03022-…f542) sat unshipped in the palace host's inbox while every local signal reported a clean stage, saved only because a host-side sweep happened to rewrite the remote copy independently that same day. --checksum compares content and ignores size/mtime entirely. Dropping --update outright was considered and rejected: rsync's default quick check already transfers on a SIZE difference alone, which is why the observed case (33-byte placeholder vs a 43-byte token) would have been masked as "fixed" by a change that only works until a redaction whose placeholder happens to match the secret's length. os.utime() at :903 is untouched — its backdating is a separate, load-bearing design call for dedup stability, out of scope for this fix. Added scripts/test-rsync-ship-idempotency.sh: ships a file, rewrites its content to an EQUAL-LENGTH string while restoring the original mtime (what os.utime() does), ships again, asserts the receiver's sha256 changed. Equal length is deliberate, not cosmetic — mismatched lengths would pass via the quick check alone and prove nothing about --checksum specifically; this is the same reasoning that ruled out dropping --update. Verified the test discriminates: passes against today's --checksum, fails against --update (checked by temporarily substituting the flag in a copy, not committed). Runs offline — a local rsync destination path exercises the same size/mtime/checksum comparison as the ssh transfer, no palace or network needed. --- bin/mempalace-pi-session | 15 ++++- scripts/test-rsync-ship-idempotency.sh | 88 ++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 2 deletions(-) create mode 100755 scripts/test-rsync-ship-idempotency.sh diff --git a/bin/mempalace-pi-session b/bin/mempalace-pi-session index 894bef5..30634cf 100755 --- a/bin/mempalace-pi-session +++ b/bin/mempalace-pi-session @@ -974,15 +974,26 @@ fi # ── Ship to the palace host (remote mode only) ─────────────────────── # mempalace_mine expands its source path in the SERVER process, so in remote -# mode the exports have to physically exist over there. rsync --update is the +# mode the exports have to physically exist over there. --checksum is the # idempotent half; the mine is the other half. +# +# NOT --update: the stage file's mtime is deliberately the SOURCE transcript's +# mtime (see the os.utime() in the exporter, "preserve session mtime for dedup +# stability"), so a re-export of a session that has not been appended to since +# the last ship carries an mtime that is NOT newer than the receiver copy. With +# --update rsync then SKIPS it silently -- which is exactly the case that must +# ship after a redactor change, because the content differs while the mtime does +# not. Measured on mbp-m1-2020 2026-08-27: a scrubbed re-export of a dormant +# session was skipped and unscrubbed bytes stayed in the palace host's inbox, +# while every local signal reported a clean stage. --checksum compares content +# and keeps the ship idempotent without trusting timestamps. MINE_SOURCE="$STAGE" if [[ "$MODE" == "remote" ]]; then ssh_cmd="ssh" [[ -n "$SSH_CONFIG" ]] && ssh_cmd="ssh -F $SSH_CONFIG" echo "" echo "Shipping stage to ${SSH_TARGET%/}/$DEVICE/ ..." - if ! rsync -a --update --no-owner --no-group \ + if ! rsync -a --checksum --no-owner --no-group \ -e "$ssh_cmd" \ --include='*.jsonl' --exclude='*' \ "$STAGE/" "${SSH_TARGET%/}/$DEVICE/"; then diff --git a/scripts/test-rsync-ship-idempotency.sh b/scripts/test-rsync-ship-idempotency.sh new file mode 100755 index 0000000..f57239a --- /dev/null +++ b/scripts/test-rsync-ship-idempotency.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# test-rsync-ship-idempotency.sh — regression test for the ship-step bug fixed +# 2026-08-27 (bin/mempalace-pi-session: rsync --update -> --checksum). +# +# THE BUG: the stage file's mtime is deliberately the SOURCE transcript's mtime +# (os.utime() at :903, "preserve session mtime for dedup stability"), so a +# re-export of a session that has not been appended to since the last ship +# carries a mtime that is NOT newer than the receiver copy. rsync --update +# skips a file whose mtime is not strictly newer than the destination's, so a +# scrubbed re-export of a DORMANT session was silently dropped: content +# differed, mtime did not, "success" was reported, and unscrubbed bytes stayed +# in the palace host's inbox indefinitely. Measured on mbp-m1-2020 2026-08-27. +# +# THE ASSERTION mirrors that exactly and needs no ssh, no palace, no secret: +# stage a file, ship it, rewrite the content while RESTORING the original +# mtime (the same thing os.utime() does), ship again, assert the receiver's +# sha256 changed. On the pre-fix flag (--update) this fails; with --checksum +# (content comparison, mtime-independent) it passes. +# +# Deliberately does NOT invoke bin/mempalace-pi-session itself: that needs a +# live ssh target, a palace, MEMPALACE_* env — disproportionate scaffolding for +# what is, at its core, one rsync flag. Pulls the flag list out of the script +# instead of hand-copying it, so a future change to the ship command either +# updates this test's expectation or fails it loudly rather than drifting +# silently out of sync with what actually ships. +# +# Usage: scripts/test-rsync-ship-idempotency.sh (exit 0 = fix still holds) +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +SHIP_SCRIPT="$REPO_ROOT/bin/mempalace-pi-session" +SRC="$(mktemp -d)"; DST="$(mktemp -d)" +trap 'rm -rf "$SRC" "$DST"' EXIT + +EXPECT='rsync -a --checksum --no-owner --no-group' +if ! grep -qF "$EXPECT" "$SHIP_SCRIPT"; then + echo "FAIL: $SHIP_SCRIPT no longer ships with '$EXPECT'." >&2 + echo " Either the fix regressed, or the flags changed -- update this" >&2 + echo " test's EXPECT string to match, don't just delete the test." >&2 + exit 1 +fi + +ship() { + rsync -a --checksum --no-owner --no-group \ + --include='*.jsonl' --exclude='*' \ + "$SRC/" "$DST/" +} + +# Same BYTE LENGTH on purpose, mirroring mbp-m1-2020's own reasoning for why +# dropping --update outright would not be enough: rsync's default quick check +# (no --update, no --checksum) already transfers when SIZE differs, so a test +# with mismatched lengths would pass by accident and prove nothing about +# --checksum specifically. A real redaction whose placeholder happens to match +# the secret's length is exactly the case that stays silently undetected unless +# content itself, not size or mtime, is compared. +UNSCRUBBED='live: MEMPALACE_REMOTE_TOKEN=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA' +SCRUBBED='live: MEMPALACE_REMOTE_TOKEN=' +if [[ ${#UNSCRUBBED} -ne ${#SCRUBBED} ]]; then + echo "FAIL: test fixture bug -- the two fixture strings must be equal length (${#UNSCRUBBED} vs ${#SCRUBBED}); this test would not exercise --checksum at all otherwise." >&2 + exit 1 +fi + +# 1. Baseline: ship a session, receiver now matches sender. +printf '%s\n' "$UNSCRUBBED" > "$SRC/session.jsonl" +touch -d '2026-08-23T23:53:18' "$SRC/session.jsonl" +ship +before="$(sha256sum "$DST/session.jsonl" | cut -d' ' -f1)" + +# 2. The bug's exact shape: rewrite content (same length!) as a scrubbed +# re-export would, then restore the ORIGINAL mtime, as the exporter's +# os.utime() does. +printf '%s\n' "$SCRUBBED" > "$SRC/session.jsonl" +touch -d '2026-08-23T23:53:18' "$SRC/session.jsonl" + +ship +after="$(sha256sum "$DST/session.jsonl" | cut -d' ' -f1)" + +if [[ "$before" == "$after" ]]; then + echo "FAIL: receiver sha256 unchanged after a content-only re-export at a held-constant mtime." >&2 + echo " This is the exact defect --checksum was added to fix." >&2 + exit 1 +fi +if ! grep -q '&2 + exit 1 +fi + +echo "PASS: ship step transfers changed content even when mtime is deliberately held constant (before=$before after=$after)"