fix(smoke): validate remote mktemp -d before using it as an rm -rf target

[andrei-hasna]

Secondary remediation for the 2026-07-24 data-destruction incident. The primary fix is
hasna/hooks#10; this closes the same class of
defect in this repo. Separate PR by design.

The defect

scripts/smoke-machine-sync-release.mjs set remoteDir from an unchecked remote
mktemp -d, then later deleted it over ssh:

const remoteDir = runRemote(options.remote, `mktemp -d …`).trim();   // ~line 414
…
run('ssh', [options.remote, `rm -rf ${shellQuote(remoteDir)}`]);      // ~line 579

mktemp -d prints its error on stderr and nothing on stdout, so a failure leaves
remoteDir empty — the same failure mode as bun pm cache, which is what turned
rm -rf "$(bun pm cache)"/* into rm -rf /* on station02.

This is inert today only because shellQuote('') renders as '', so the delete degrades
to rm -rf ''. It is one appended /* away from catastrophic, and the value is also
interpolated into --peer-workspace. Relying on a quoting helper to neutralise a value that
should never have been accepted is exactly the assumption this incident punished.

The fix

New scripts/lib/remote-temp-dir.mjs exporting assertRemoteTempDir, applied at the
assignment and re-asserted immediately before the rm -rf.

It accepts only what the template could actually have produced. mktemp replaces the
trailing run of X in place, so a valid result is the template's fixed prefix followed
by a single non-empty path component. Checking only the template's parent directory is not
enough — that would accept /tmp/somebody-elses-dir and delete it. The bound is derived from
the template rather than hardcoded, so it stays correct if the template moves.

Rejected: empty, whitespace-only, multi-line output, non-absolute paths, /, /tmp, /usr,
/home/hasna, a sibling directory sharing the temp parent, a path escaping via .., and a
malformed template (relative, or without at least three X).

Packaging — a second, real bug this surfaced

smoke-machine-sync-release.mjs is a published file. Adding an import to it without
registering the new module would ship a package that throws ERR_MODULE_NOT_FOUND for every
installed copy. The new module is therefore registered in all three deliberate allowlists
(package.json files, scripts/validate-public-package.mjs, tests/package-release.test.ts)
— that triplication is a review tripwire, so it is honoured rather than worked around.

The existing allowlists only catch the opposite mistake (shipping something unreviewed).
Nothing caught "a published script imports something unpublished", so this PR adds that test.

It immediately found pre-existing breakage. scripts/apply-cloud-migrations.mjs:23
statically imports ../src/storage.ts, but src/ has never been in files. Verified against
a packed tarball:

$ npm pack && tar xzf hasna-knowledge-0.2.91.tgz && cd package
$ bun scripts/apply-cloud-migrations.mjs --help
error: Cannot find module '../src/storage.ts' from '…/package/scripts/apply-cloud-migrations.mjs'

That path carries a single exemption named in the test, tracked as todos task 1eb481d5;
removing the exemption is that task's acceptance criterion. It is not fixed here because
choosing between repointing at dist, shipping src, or unpublishing the script is a
separate decision that deserves its own review — and silently deleting the test to make it
green would be precisely the "guard that passes while protecting nothing" trap.

Evidence

$ bun test tests/machine-smoke-script.test.ts tests/package-release.test.ts
  14 pass, 0 fail

$ node scripts/validate-public-package.mjs
  Public package validation passed for hasna-knowledge-0.2.91.tgz.
  Public scripts included: 7.

$ npm pack && tar xzf … && bun scripts/smoke-machine-sync-release.mjs --help
  resolves and runs from the tarball

$ gitleaks git --staged --no-banner --redact …
  no leaks found

Full suite with HASNA_KNOWLEDGE_API_URL/_KEY unset: 227 pass, 2 skip, 1 fail. That one
failure (tests/cli.test.ts — "context pack and proposal context commands return bounded
agent JSON") reproduces on the untouched baseline via git stash, so it is pre-existing and
unrelated. With those env vars set, 99 tests fail because the cloud-API flip disables the
local sqlite catalog — also environmental, also unrelated.

Negative controls

Guards are proven to fail on bad input, not merely to pass on good:

• assertRemoteTempDir throws for every rejected value listed above, each an explicit test case.
• Removing the new module from the allowlist makes the packaging test fail with the exact
  missing path:

  error: scripts/smoke-machine-sync-release.mjs imports ./lib/remote-temp-dir.mjs,
         which is not in the published files list
  

No rm is executed by any test, at any scope. Only the validator is exercised.

Scope

grep confirms this was the only remote mktemp -d and the only ssh … rm -rf in the repo.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

CI on this PR is red. It is not caused by this PR — both failing tests already fail on main, and one of them is a dated time bomb that went off on 2026-07-26. Evidence below so a reviewer/merger does not have to re-derive it.

Failure 1 — knowledge cli > context pack and proposal context commands return bounded agent JSON

tests/cli.test.ts:2667-2668 seed evidence hardcoded to 2026-06-25T00:00:00.000Z; line 2675 queries it with --since 30d; line 2680 asserts the evidence is found. The fixture simply aged out of the window:

date	fixture age	inside --since 30d?	observed
2026-07-24	29d	yes	main CI ubuntu-bun passed
2026-07-26	31d	no	this PR failed
2026-07-27	32d	no	fails locally

Controlled A/B on untouched main @ 2023a26 in a detached worktree, changing only the fixture date:

A) main as-is, today
   bun test tests/cli.test.ts -t "context pack and proposal context"
   -> 1 fail

B) main + ONLY 2026-06-25 -> 2026-07-24 in the fixture, nothing else
   same command
   -> 0 fail

So the calendar is the sole variable. Tracked as todos d1b93867. The fix should seed the fixture relative to now or freeze time — bumping the date only re-arms the bomb.

Failure 2 — knowledge machine sync ledger > sync export redacts local file and workspace refs from bundles (macOS only)

Also fails on main’s own CI run 30106566377, same commit as this PR’s base. Not caused by this PR; needs its own diagnosis.

This PR’s own tests pass in CI

(pass) remote temp dir guard > rejects the empty output a failing mktemp -d leaves behind
(pass) remote temp dir guard > the failure names the path and says why it matters

This PR touches only scripts/lib/remote-temp-dir.mjs, scripts/smoke-machine-sync-release.mjs, scripts/validate-public-package.mjs, the two test files, and package.json. None of them are imported by the CLI or by tests/cli.test.ts.

[andrei-hasna]

Round 2 — review findings fixed in 0902c4b

F2 first, because it corrects this PR’s own premise

The defect I originally described was not reachable. runRemote → runChecked throws on a non-zero remote exit, so a failing mktemp -d (exit 1, empty stdout) aborts the run and never assigns "". Verified:

failing mktemp -d : exit=1  stdout=""  -> runChecked THROWS; the pre-PR line never assigned ""

The reachable hole is the remote exiting zero with stdout that is not the path — an ssh Banner, a chatty login profile, or a ForceCommand wrapper. Verified:

echo banner; mktemp -d ... : exit=0  stdout="Welcome to linux-node-a\n/tmp/knowledge-banner-uqj7wT"
guard: REJECTED -> path spans multiple lines

The wrong claim appeared in four places. Corrected in the module docstring and the test comments here; this comment corrects the PR narrative. An incident-remediation artifact that misstates the defect it fixes corrupts the incident record — that is worse than the bug.

F1 — the guard degraded into the parent-only check it exists to avoid

When a template’s final component is nothing but X, the derived prefix is a bare directory and every sibling validated. Reproduced:

!! ACCEPT  template=/tmp/XXXXXX          dir=/tmp/somebody-elses-dir
!! ACCEPT  template=/XXXXXX              dir=/etc
!! ACCEPT  template=/XXXXXX              dir=/home
!! ACCEPT  template=/home/hasna/XXXXXX   dir=/home/hasna/.ssh

/tmp/XXXXXX is an ordinary template. The line that should have caught it — prefix.includes("/") === false — can never fire, since "/tmp/".includes("/") is true; mutation testing showed it was both ineffective and untested. Templates without a fixed prefix in their final component are now refused; all five attack cases reject, the legitimate template still passes.

Remaining findings

• F3 — templates containing .. passed with a meaningless bound, and the template is built from a caller-supplied version string. Templates must now be normalized.
• F4 — the re-assertion sits in a finally, where a throw replaces the in-flight exception and skips the cleanup: it would mask a real smoke failure and guarantee a remote leak. Now contained — reports and skips the delete, never masks.
• F5 — the new import test only scanned relative specifiers, so it missed that apply-cloud-migrations.mjs has a second unresolvable import: @hasna/contracts/auth, a devDependency only. Task 1eb481d5 would have closed on a false green with the published script still throwing. Bare specifiers are now checked against runtime dependencies; both imports are exempted by name and both must go for that task to be done.
• F6 — regex missed side-effect imports and fired inside comments. Comments and template literals are stripped, side-effect imports matched, node:/bun: builtins excluded — found by the new check correctly flagging bun:sqlite.
• F7/F8 — the GNU suffix form is documented as deliberately refused; the local temp dir can no longer leak on the throw path, because the remote dir is created first.

Evidence

bun test <touched files>   18 pass, 0 fail   (was 14)
validate-public-package    passed, 7 public scripts
npm pack + extract + --help  resolves and runs
full suite                 231 pass, 2 skip, 1 fail  <- the dated time bomb, task d1b93867
gitleaks --staged          no leaks found

Mutation retest — the three mutations that SURVIVED the reviewer’s run are now all caught:

CAUGHT | M5 parent-only bound        CAUGHT | M6 bare prefix accepted
CAUGHT | M7 absolute check dropped   CAUGHT | M8 template normalization dropped

No rm executed by any test. Requesting re-review of bb9cf2b..0902c4b.

[andrei-hasna]

Round 3 — re-review findings fixed in a15850f

The re-review returned APPROVE-WITH-FIXES and caught that round 2 introduced a defect in the same class it was closing. That one first.

N1 — my own guard-rail test could be silenced by one glob string

The import test stripped comments with /\*[\s\S]*?\*\//g, which treats the /* inside an ordinary glob string like **/*.ts as a comment opener and deletes everything up to the next */. Reproduced:

raw specifiers:             [ "ghost-package", "./ok.mjs" ]
what the test actually saw: [ "./ok.mjs" ]

One innocuous glob line before a broken import and the check goes green — precisely the passes-while-broken failure this test exists to catch, in the test itself. Replaced with a character scanner that tracks string and template literals. Verified by planting const g = **/*.ts plus a broken import: now fails with the exact missing specifier.

N9 — the bare-specifier check false-positived on legitimate imports

Builtins imported without the node: prefix, the package’s own name (valid via exports), and #-prefixed imports subpaths are now allowed. The self-reference case was live, not hypothetical: scripts/smoke-machines-adapter.mjs already imports @hasna/knowledge and was passing only by accident of the old template-literal strip.

N3 — three accuracy defects in text rewritten for accuracy

• (a) The docstring claimed the guard accepts “only what this exact template could have produced.” False — any suffix of any length or charset was accepted (Z, ZZZZZZZZZZ, $(id), ..). mktemp emits exactly N characters from [A-Za-z0-9]; that is now enforced, so the claim is true.
• (b) A comment presented a paraphrase as a verbatim transcript. echo banner cannot print Welcome to linux-node-a, and mktemp never prints the unexpanded XXXXXX. Replaced with the actual measured output. This is the same defect F2 was raised for, in text I rewrote to fix F2.
• (c) “interpolated into --peer-workspace” was loose — that site passes a separate argv element. The real shell interpolations are the remote cd <value> && … and rm -rf <value>.

N4/N5/N6 — the finally block is now actually contained

Round 2 wrapped only the assertion — the one statement that cannot currently fail — while leaving rmSync and the ssh spawn able to throw, replace the in-flight exception, and skip the remaining cleanup. The reviewer modelled it: rmSync throw masks; ssh NUL-arg throw masks. All three are individually contained now, and a non-zero remote cleanup exit is reported instead of silently discarded. Both temp dirs are created inside the try with null guards — round 2’s reordering had only moved the leak from local to remote.

Evidence

Mutation retest — all 10 checks CAUGHT, including M8 (X{3,} → X{1,}) which survived the reviewer’s run, and M5 which my own new charset check had silently subsumed until a message-ordering test pinned it.

bun test <touched files>   22 pass, 0 fail   (was 18)
validate-public-package    passed, 7 public scripts
npm pack + extract + --help  resolves and runs
full suite                 235 pass, 2 skip, 1 fail
gitleaks --staged          no leaks found

CI correction (N2) — stated plainly

My earlier “231 pass / 1 fail” was Linux-only. CI is red with two failures. The second is macOS-only: knowledge machine sync ledger > sync export redacts local file and workspace refs from bundles. Both reproduce on main’s own CI run 30106566377 for this PR’s base commit, and git diff origin/main...HEAD -- src/ tests/cli.test.ts is empty — so neither is caused by this PR. Both are recorded on todos d1b93867.

This PR cannot merge on a green CI until those two are fixed independently. Saying that rather than reporting a clean-looking local number.

Also surfaced: test-matrix (windows-latest, bun) is cancelled by fail-fast in every recent run, so no test has executed on Windows for some time — the new tests included.

[andrei-hasna]

Round 4 — REJECTED at a15850f; fixed in 17f328f

My fix for N1 reintroduced N1. But the finding that matters is structural, not the parsing bug.

F2 — three versions of this check shipped in a row, none of them pinned

Mutation run on a15850f:

SURVIVED  isAlwaysResolvable() always true  (guard fully disabled)
SURVIVED  drop the "#" allowance
SURVIVED  drop the builtinModules allowance
SURVIVED  stripComments -> identity
SURVIVED  stripComments -> ""            (strips the entire file)
SURVIVED  stripComments -> the OLD regex from 0902c4b

You could restore the exact regex N1 condemned and every test stayed green. The tests meant to pin the check exercised a private copy of the logic instead of the logic itself. The operating mode — silent green — never changed; only the implementation moved. That is why the same defect class shipped three rounds running.

Fix: extracted unresolvableImportsIn(script, source), and both the enforcement test and the pinning tests now call it. Mutation retest — all CAUGHT, all previously SURVIVED:

CAUGHT  isAlwaysResolvable always true      CAUGHT  extractor reports nothing
CAUGHT  exemptions widened to everything    CAUGHT  relative check disabled
CAUGHT  bare-dep check disabled             CAUGHT  "bun:" blessed as a class again

F1 — stopped hand-rolling a JS parser

The scanner had no regex-literal state. A quote inside a regex literal enters string mode; the phase shift makes a later ordinary string's /* read as a block-comment opener — and that branch deletes to EOF. Now Bun.Transpiler().scan(), verified against every shape that defeated both strippers. Extracting imports is a parsing problem; anything short of a parser is a guess that fails silently.

F3/F4/F5 — the parser removed the reason for three allowances, so all three are deleted

• ownName existed only to mask a phantom the regex stripper invented from a template literal that generates a child script. It also waved through @hasna/knowledge/<subpath> for subpaths absent from exports (ERR_PACKAGE_PATH_NOT_EXPORTED).
• unprefixed builtins — builtinModules under bun test is Bun's list and contains ws, undici, bun: real npm names, not Node builtins. Verified absent from Node 22's list; all three ERR_MODULE_NOT_FOUND from the tarball.
• #-prefixed — no imports map exists, so every one is guaranteed to fail. It could only ever hide breakage.

bun: is no longer blessed as a class either — verified ERR_UNSUPPORTED_ESM_URL_SCHEME under node from the packed tarball. Exempted by name, filed as 104f993d.

F6/F7 — containment was correct, one frame too low

runSyncSmoke's finally was contained, but its two callers used a raw rmSync, discarding the exception it takes care to preserve — in exactly the --no-machines-sync paths the release smoke runs. And report itself was unguarded: process.stderr.write is synchronous on a pipe and throws EPIPE when the consumer exits, masking the in-flight error and skipping the remote cleanup. Both are now shared, individually-contained helpers; zero raw rmSync remains in any finally.

F8 — the two unpinned validator checks

A lone \r, and the suffix charset with 6-character fixtures so only the charset rule can reject them. The earlier fixtures were all the wrong length, leaving the shell-metacharacter backstop pinned by nothing.

Evidence

touched-file tests       24 pass, 0 fail   (was 22)
import-check mutations   6/6 CAUGHT        (6/6 previously SURVIVED)
validator mutations      2/2 CAUGHT
validator                passes
npm pack + extract + --help under NODE     resolves and runs
full suite               239 pass, 2 skip, 1 fail

The one failure remains the pre-existing dated time bomb (d1b93867), untouched by this branch.

Left standing as sound by the reviewer: the N3a suffix tightening (verified against 400 GNU mktemp samples and against glibc/musl/BSD padding sets), the runSyncSmoke control flow across all 8 modelled paths, and the N3b docstring correction.

Requesting re-review of a15850f..17f328f.