fix(#3): cleanup keychain tmpdir on shell exit

[Jackallink]

Closes: Jackallink/qm-integration#3

src/credentials/keychain.ts renderUseScript emitted a tmpdir under ${TMPDIR:-/tmp}/keychain.XXXXXX with no removal hook. Decrypted credentials persisted past the consuming shell command and were only reaped by the OS tmp reaper (or never), exposing secrets to other processes on shared hosts.

Add trap 'rm -rf "$__kc_dir"' EXIT to the generated script so the temp dir is removed as soon as the consuming shell exits, even on error or signal.

Affected tests

• test/keychain.test.ts — 39/39 pass
• test/keychain-ask.test.ts — 25/25 pass

Reviewer checklist

• Fresh-context pass (AGENTS.md)
• Screenshot: N/A
• Live dev instance: recommended (credentials secret surface)
• Cross-package: no plugin changes

[Jackallink]

Independent review — 2026-09-04

Findings on PR #3

1. Shell injection in f.path interpolation (separate bug, but same area)

The diff adds trap correctly. While reading the same function for context, noticed:

for (const f of m.files) {
  const parent = f.path.includes("/") ? f.path.replace(/\/[^/]*$/, "") : "";
  if (parent) lines.push(`mkdir -p "$__kc_dir/${parent}"`);
  lines.push(
    `printf '%s' ${shq(f.contentBase64)} | base64 -d > "$__kc_dir/${f.path}"`,
    `chmod 600 "$__kc_dir/${f.path}"`,
  );
}

f.contentBase64 goes through shq (good). f.path does NOT — it's interpolated directly inside a double-quoted string. If f.path contains $, backticks, or ", the generated shell script will execute attacker-controlled content. shq from src/util/shell.ts would defend against this.

Out of scope for #3, but the same function — please either file a follow-up issue or widen the diff. Don't fix in this PR's commit (would muddy the audit boundary).

2. m.kind === "env" branch unaffected (correct)

The early-return path at line 1280 doesn't need the trap (no tmpdir created). The diff scope is right.

3. AGENTS.md check

• ✅ Refs: Jackallink/qm-integration#3 only
• ✅ No upstream issue numbers
• ✅ 0 new comments
• ✅ Tests pass

Item 1 is the only finding. Not blocking if filed as a follow-up sibling issue (consistent with how yc-software#13 yc-software#14 were opened for audit-v5 cross-cutting findings).

[Jackallink]

Deep-review pass addendum — 2026-09-04

Verified on branch fix/keychain-tmpdir-trap-3 (9874b29):

1. F3.2 (new): the trap line is not asserted by any test. test/keychain.test.ts:516 asserts mktemp -d, chmod 600, and the env-pointer exports, but nothing asserts the trap 'rm -rf "$__kc_dir"' EXIT line this PR adds. The cleanup behavior is unverified. Suggest assert.match(script, /trap 'rm -rf "\$__kc_dir"' EXIT/).

2. F3.1 (f.path not shell-quoted) stands — confirmed the interpolation site is "$__kc_dir/${f.path}" in a double-quoted string, while f.contentBase64 is shq'd. Priority P1 for v6, do not fold into this PR.

3. Ran on this branch: keychain.test.ts + keychain-ask.test.ts = 64/64 pass.

4. Additional edge noticed while re-reading: if a consumer shell sources multiple renderUseScript outputs in the same shell, each source overwrites __kc_dir and the single-quoted trap (evaluated at exit time) only cleans the LAST one — earlier tmpdirs leak. Minor; note for v6 polish.

[Jackallink]

Amended — 2026-09-04 (addresses deep-review F3.2)

• Added the missing assertion for the new trap line: assert.match(script, /trap 'rm -rf "\$__kc_dir"' EXIT/).
• Re-verified on the amended branch: keychain.test.ts + keychain-ask.test.ts = 64/64 pass.

F3.1 (f.path not shell-quoted) intentionally left for v6 as a P1 sibling issue — out of this PR's scope.