ship: don't trust the mtime the exporter deliberately backdates
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.
This commit is contained in:
@@ -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
|
||||
|
||||
Executable
+88
@@ -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=<redacted: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 '<redacted:MEMPALACE_REMOTE_TOKEN' "$DST/session.jsonl"; then
|
||||
echo "FAIL: receiver does not contain the scrubbed content." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "PASS: ship step transfers changed content even when mtime is deliberately held constant (before=$before after=$after)"
|
||||
Reference in New Issue
Block a user