From 91349acac1a156d711cdb71d0c16b1eed95bf4b7 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Tue, 21 Jul 2026 11:49:32 +0000 Subject: [PATCH] fix: the import count guard closes the octal hole, not just the garbage one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three reviewers landed on the same line. Reproduced on this head under `set -euo pipefail`: prev_n=08 [ -n "$prev_n" ] && [ "$prev_n" -eq "$prev_n" ] 2>/dev/null || prev_n=0 # -> guard PASSES: test parses 08 as decimal n=$((prev_n + 1)) # -> bash: 08: value too great for base (error token is "08") Two holes that had to close together. `test -eq` reads decimal, arithmetic reads a leading zero as octal, so a value can pass the guard and still abort the arithmetic it was guarding. The abort lands after the physical `incus import` and before the stamp, the placement correction and the start — the exact window the degrade-never-die contract exists to protect, and the same side box_provenance()'s schema check falls on. case "$prev_n" in ''|*[!0-9]*) prev_n=0 ;; esac n=$((10#$prev_n + 1)) Digits-only closes sign and garbage; 10# forces base ten. Checked across 08 -> 9, 007 -> 8, 5 -> 6, and '' / not-a-number / -3 / 3x / 00 -> 1. The existing not-a-number fixture could never have caught this: that value fails the guard and degrades, so it exercises the path that already worked. Added a count=08 fixture beside it, asserting both halves — the import does not fail, AND the count advances to 9 rather than degrading to 1, because 08 is a real previous total and reading it as anything else would be its own small lie. A zero-padded count is not exotic; it is what any external tool that formats numbers writes. Verified by mutation: with the old guard restored both new assertions fail. Also drops user.box.mode.asked from the IMPCLONE fixture (grok's nit). Since #129 the clone path clears that key, so a fixture built from the mint shape that kept it described a box the clone path cannot produce. Nothing asserts it today — which is precisely why it would have rotted unnoticed. --- bin/box | 10 ++++++++-- test/cli.sh | 21 ++++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/bin/box b/bin/box index 51ecda3..bb61369 100755 --- a/bin/box +++ b/bin/box @@ -1708,8 +1708,14 @@ cmd_import() { # config, a foreign user.box.imported.count) must never fail an import that # has already happened — arithmetic on it under 'set -e' would. Same side the # schema check in box_provenance() falls on: degrade, never die. - [ -n "$prev_n" ] && [ "$prev_n" -eq "$prev_n" ] 2>/dev/null || prev_n=0 - n=$((prev_n + 1)); now="$(mint_time)" + # Two holes, and they have to close together. `[ 08 -eq 08 ]` PASSES — test + # parses decimal — and then `$((08 + 1))` aborts with "value too great for + # base", because arithmetic reads a leading zero as octal. That abort lands + # after the physical 'incus import' and before the stamp, the placement fix + # and the start: precisely the window this guard exists to protect. So the + # digits-only case closes sign and garbage, and 10# forces base ten. + case "$prev_n" in ''|*[!0-9]*) prev_n=0 ;; esac + n=$((10#$prev_n + 1)); now="$(mint_time)" local istamp=( user.box.imported.last="$now" user.box.imported.last.by="$(box_version)" diff --git a/test/cli.sh b/test/cli.sh index 912dfdb..3223018 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -1558,6 +1558,21 @@ check "re-import: a non-integer count does not fail the import (#131)" 0 "import importbox "$BLOG" "$BADN" check "re-import: ...it restarts the count rather than inventing a total (#131)" 0 "" \ import_set "$BLOG" 'user\.box\.imported\.count=1' +# A leading zero is the hole the non-integer fixture CANNOT catch: '08' passes +# an -eq guard (test parses decimal) and then dies in arithmetic, which reads +# it as octal. That abort would land after the physical 'incus import' and +# before the stamp, the placement fix and the start — the exact window the +# degrade-never-die contract exists to protect. A zero-padded count is not +# exotic either: it is what any external tool that formats numbers writes. +ZEROPAD="$IWORK/zeropad.cfg" +{ cat "$MINTED_ART"; echo 'user.box.imported.count 08'; } > "$ZEROPAD" +ZLOG="$IWORK/zeropad.log" +check "re-import: a zero-padded count does not fail the import (#131)" 0 "imported work" \ + importbox "$ZLOG" "$ZEROPAD" +# Counted as decimal 8, not degraded to 0 and not read as octal: '08' is a +# real previous total, so the honest next value is 9. +check "re-import: ...and counts it as decimal, so 08 advances to 9 (#131)" 0 "" \ + import_set "$ZLOG" 'user\.box\.imported\.count=9' # --- a legacy artifact with no stamp at all --------------------------------- # A pre-stamp box export, or a hand-rolled 'incus export' of an unmanaged VM. @@ -1619,7 +1634,11 @@ check "info: ...and the first one, with the count (#131)" \ # An imported CLONE reads as a clone that also travelled — the two facts sit # side by side, neither having eaten the other. IMPCLONE="$MWORK/imported-clone.cfg" -{ grep -v '^user.box.origin ' "$IMPCFG" +# mode.asked is dropped, not merely unasserted: since #129 the clone path +# clears it (nobody asked THIS box anything), so a fixture built from the mint +# shape that kept the key would describe a box the clone path cannot produce. +# No assertion here reads it — which is exactly why it would rot unnoticed. +{ grep -v '^user.box.origin ' "$IMPCFG" | grep -v '^user.box.mode.asked ' echo 'user.box.origin clone' echo 'user.box.origin.from work/authed'; } > "$IMPCLONE" check "info: an imported clone is still a clone (#131)" \