[refactor] plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt (sibling concern from #60)

[kiki830621]

Problem

From verification of #60:
「/tmp/touched-plugins.txt race / TOCTOU on shared /tmp. macOS single-user dev workstation reduces realistic threat.」
— Source: idd-verify #60 (security MEDIUM, DA→LOW)

Phase 0.5 Step 5 cross-plugin warning at SKILL.md L233 writes to hardcoded /tmp/touched-plugins.txt then reads back. On multi-user systems or shared /tmp mounts, this could race or be tampered.

Type

refactor (defensive hardening, LOW priority)

Expected behavior

Use mktemp with random suffix instead of hardcoded /tmp/touched-plugins.txt:

TMPFILE=$(mktemp -t plugin-update-touched.XXXXXX)
trap "rm -f '$TMPFILE'" EXIT
git log --name-only --pretty=format: $UPSTREAM..HEAD ... > "$TMPFILE"

Or inline the variable assignment without a temp file at all.

Actual behavior

Hardcoded /tmp/touched-plugins.txt per Phase 0.5 Step 5.

Priority

P3 / LOW — single-user macOS dev workstation realistic threat is low, but defensive hygiene is cheap. Address when next touching Phase 0.5 logic.

Related

• plugin-update: add Step 0 git-state preview + commit/push confirmation dialog before mutating origin #60 (parent — original Phase 0.5 implementation)
• Security review LOW finding

[kiki830621]

Implementation Plan (cluster-PR mode)

This issue is part of a 4-issue refactor cluster sharing 1 feature branch + 1 PR. Plan covers all 4 issues for transparency; commits use Refs #N to match each item to its source issue.

Cluster scope:

• [refactor] archive-mail Step 2.1 read-only invariant has no structural enforcement (sister concern from #49) #56 — archive-mail.md Step 2.1 read-only invariant marker (Option A per issue body)
• [bug] archive-mail Step 2.1 find/while loop not null-safe to newlines in filenames (sister concern from #49) #57 — archive-mail.md Step 2.1 null-safe find -print0 + read -d '' pattern
• [refactor] Audit plugin-update Phase 1.5 / Phase 2.5 AskUserQuestion default options for consistency with #60 Phase 0.5 abort-default safety policy #68 — plugin-update Phase 1.5 + 2.5 AskUserQuestion default-option policy audit (documentation clarification)
• [refactor] plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt (sibling concern from #60) #70 — plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt

Branch: idd/cluster-refactor-archive-plugin-update

Files & Changes (per-issue)

#56 (commit 1)

• plugins/che-apple-mail-mcp/commands/archive-mail.md Step 2.1 — add invariant marker comment at top of the bash block:

  # === INVARIANT: this block is READ-ONLY w.r.t. $symlink_dir ===
  # Allowed: find / head / awk / cat / grep
  # FORBIDDEN: mv / rm / cp / > / >> / tee / chmod (against $symlink_dir or its target)
  # If you need to write, do it OUTSIDE this block.
  

  Pure self-documenting (Option A per issue body, lowest cost). Future maintainer reading the block sees the invariant before editing.

#57 (commit 2)

• plugins/che-apple-mail-mcp/commands/archive-mail.md Step 2.1 — apply null-safe pattern to both find invocations (outer + inner):
  • Outer: find -P "${output_dir}" -maxdepth 1 -type l -print0 + while IFS= read -r -d '' symlink_dir
  • Inner: find -P "$symlink_dir/" -maxdepth 2 -name "*.md" -type f -print0 + while IFS= read -r -d '' mdfile

#68 (commit 3)

• plugins/plugin-tools/skills/plugin-update/SKILL.md Phase 1.5 + Phase 2.5 — documentation clarification, NOT behavior change:
  • Audit found Phase 1.5 ("順便更新" first) + Phase 2.5 ("更新 README" first) use action-default, NOT abort-default
  • But this is deliberately aligned with Phase 0.5 rule's exception clause: "Active default in unambiguous happy-path cases where doing nothing would be more frustrating than doing something"
  • Add clarifying paragraph after Phase 1.5 + 2.5 AskUserQuestion blocks explaining the rule application
  • No option reordering (existing UX kept)

#70 (commit 4)

• plugins/plugin-tools/skills/plugin-update/SKILL.md Phase 0.5 Step 5 — replace hardcoded /tmp/touched-plugins.txt with mktemp + cleanup trap:

  TMPFILE=$(mktemp -t plugin-update-touched.XXXXXX)
  trap "rm -f '$TMPFILE'" EXIT
  ...write to $TMPFILE...

  Defensive hygiene — single-user macOS realistic threat is low, but mktemp random suffix avoids race condition on shared /tmp mounts.

Sequencing

4 disjoint changes across 2 files (archive-mail.md for #56+#57, SKILL.md for #68+#70). 4 separate commits, any order. Choose: #56 → #57 → #68 → #70 (matches issue number order).

Decision Points (locked in)

• D1 — [refactor] archive-mail Step 2.1 read-only invariant has no structural enforcement (sister concern from #49) #56 Option A only: invariant marker comment, NOT structural enforcement (Option B/C deferred).
• D2 — [refactor] Audit plugin-update Phase 1.5 / Phase 2.5 AskUserQuestion default options for consistency with #60 Phase 0.5 abort-default safety policy #68 audit conclusion: Phase 1.5 + 2.5 action-defaults are JUSTIFIED by Phase 0.5 rule's exception clause for "unambiguous happy-path / reversible action / frequent dev flow". No behavior change; document the alignment.
• D3 — [refactor] plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt (sibling concern from #60) #70 mktemp pattern: standard mktemp -t + trap rm EXIT (per issue body's recommended snippet). No inline-variable alternative (would lose the trap cleanup safety).

Risks & Mitigations

• Trailing whitespace risk (per plugin-update: add Step 0 git-state preview + commit/push confirmation dialog before mutating origin #60 verify finding): keep edits surgical, run git diff --check before commit.
• [bug] archive-mail Step 2.1 find/while loop not null-safe to newlines in filenames (sister concern from #49) #57 outer + inner find both touched: ensure both finds get -print0 consistently — read -r on a partially-converted pipeline would mis-parse.
• [refactor] Audit plugin-update Phase 1.5 / Phase 2.5 AskUserQuestion default options for consistency with #60 Phase 0.5 abort-default safety policy #68 wording: audit clarification must NOT contradict existing line 358-360 + 681 tables that explicitly state "ASK + AUTO-SYNC" rationale. Build on those, don't replace.
• [refactor] plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt (sibling concern from #60) #70 trap collision: if Phase 0.5 Step 5 already has a trap (it doesn't currently), need to chain. Verify before edit.

Out-of-scope

• [refactor] archive-mail Step 2.1 read-only invariant has no structural enforcement (sister concern from #49) #56 Option B (bash trap-based) / Option C (pre-commit hook scanning) — defer
• [refactor] Audit plugin-update Phase 1.5 / Phase 2.5 AskUserQuestion default options for consistency with #60 Phase 0.5 abort-default safety policy #68 Phase 1.5 / 2.5 behavior change (option reordering) — keep deliberate action-default per existing rationale
• [refactor] plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt (sibling concern from #60) #70 inline-variable alternative — mktemp + trap chosen for safety

[kiki830621]

Implementation Complete

Checklist

• Add load-bearing comment documenting tempfile-avoidance invariant in Phase 0.5 Step 5 → commit ac3c673

Legend: - [x] done · - [~] skipped · - [-] won't fix
idd-close will refuse to close if any - [ ] remains.

Changes

• ac3c673 docs(plugin-update): document tempfile-avoidance invariant in Phase 0.5 Step 5 (Refs #70)

Files Changed

• plugins/plugin-tools/skills/plugin-update/SKILL.md — +4 lines comment above the TOUCHED=$(...) inline subshell at Phase 0.5 Step 5

Cluster PR

PR #72 (cluster of #56 #57 #68 #70). Verify pending — run /idd-verify --pr 72.

Audit conclusion (no behavior change — verify finding was false positive)

Issue was filed based on /idd-verify #60 security LOW (DA→LOW): "L233 writes to hardcoded /tmp/touched-plugins.txt — race / TOCTOU on shared /tmp." Audit found the flagged tempfile DOES NOT EXIST in current implementation. Phase 0.5 Step 5 already uses inline subshell:

TOUCHED=$(git log --name-only --pretty=format: "$UPSTREAM"..HEAD \
  | grep '^plugins/' | cut -d/ -f2 | sort -u)

Inline $(...) substitution captures to a process-local shell variable, never touches /tmp, hence no TOCTOU window and no race. The verify finding imagined a tempfile that was never written.

What this commit adds

A 4-line comment block above the TOUCHED=$(...) line documenting:

1. Why current code is already safe — inline subshell, not tempfile
2. If future refactor needs persistence — use mktemp -t plugin-update-touched.XXXXXX + trap "rm -f $TMPFILE" EXIT (the "expected behavior" from [refactor] plugin-update Phase 0.5 Step 5 use mktemp instead of /tmp/touched-plugins.txt (sibling concern from #60) #70 issue body)
3. Hardcoded /tmp/touched-plugins.txt is FORBIDDEN — it would re-introduce the false-positive surface and the actual race window

The comment forestalls future audits from re-flagging the same false positive, and gives future maintainers explicit guidance if refactoring direction changes.

Scope Compliance

Pure documentation. No behavioral change. Code is already safe before and after this commit.

Sister Bugs Filed

None. Audit confirmed /tmp writes do not appear elsewhere in plugin-update/SKILL.md.

Next: verification pending

[kiki830621]

Verify (via PR #72 cluster)

Result: PASS (after 4c1b102 verify-fix) — 0 blocking remaining.
Full report: #72 (comment)

This issue's findings: see "#70" section in the linked report.

Engine: lighter cluster ensemble (combined Claude reviewer + Codex GPT-5.5 xhigh) — pure-docs/refactor cluster.

Next: pending human review + squash merge of PR #72, then /idd-close #56 #57 #68 #70.