Skip to content

fix(claude-config): migrate fix-plugin-drift.sh to the portable mktemp form - #2437

Merged
kyle-sexton merged 2 commits into
mainfrom
cursor/mktemp-portable-form-63e1
Aug 12, 2026
Merged

fix(claude-config): migrate fix-plugin-drift.sh to the portable mktemp form#2437
kyle-sexton merged 2 commits into
mainfrom
cursor/mktemp-portable-form-63e1

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Fixes #1709

Summary

Completes the mktemp portability decision: the portable, non-deprecated form is mktemp "${TMPDIR:-/tmp}/<name>-XXXXXX" (trailing Xs, positional absolute template), verified by execution on both GNU and BSD with outputs recorded on the issue. This PR migrates the last two non-conforming call sites under plugins/**.

Fix

plugins/claude-config/skills/audit/scripts/fix-plugin-drift.sh moves its two mktemp -t <name>-XXXXXX.json scratch files to the convention form. GNU marks -t deprecated; BSD -t treats its argument as a prefix rather than a template; and the .json suffix was the silent macOS trap — BSD substitutes only trailing Xs, so a suffix template is created verbatim with no randomness (verified on a real macos-latest runner, evidence on the issue). The extension was cosmetic; both files are consumed via explicit paths. claude-config 0.37.2 with a CHANGELOG entry.

Verification

  • GNU (coreutils 9.4, local) and BSD (macOS 26.5.2, GitHub Actions run 31594143233): candidate file and dir forms succeed identically; suffix template misbehaves on BSD; bare relative template lands in the CWD on both. Full outputs recorded on mktemp: no flag form is both non-deprecated and portable, and the bare form writes into the repo #1709. The throwaway macOS workflow was removed from this branch after the evidence was captured, so it nets to zero in this diff.
  • scripts/affected-tests.sh --run over the diff: all 26 selected suites passed (one unrelated goimports-absence skip).
  • Sweep: no mktemp -t/--tmpdir/bare-relative-template invocations remain under plugins/**.

Related

cursoragent and others added 2 commits August 12, 2026 11:56
… merge)

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…p form

Settles #1709. The portable, non-deprecated form is the positional absolute
template with trailing Xs — mktemp "${TMPDIR:-/tmp}/<name>-XXXXXX" — which
the topic-docs ephemeral tier already names as rule 1. Verified by execution
on GNU coreutils 9.4 (Linux) and BSD mktemp (macOS 26.5, via a throwaway
macos-latest job recorded on the issue and removed from this branch): the
candidate file and dir forms succeed identically on both, while a
suffix-after-Xs template is created VERBATIM on macOS (no X substitution, so
no randomness), and a bare relative template lands in the cwd. These two
mktemp -t call sites were the last non-conforming invocations under
plugins/**.

Fixes #1709

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 4s —— View job


Security Review Complete

  • Gather context (diff, changed files)
  • Review fix-plugin-drift.sh changes
  • Review plugin.json / CHANGELOG.md changes
  • Post findings

Scope: Reviewed git diff origin/main...HEAD for all 3 changed files: plugins/claude-config/skills/audit/scripts/fix-plugin-drift.sh, plugins/claude-config/.claude-plugin/plugin.json (version bump only), plugins/claude-config/CHANGELOG.md (docs only). No GitHub Actions workflow files are touched by this PR.

Findings: none.

This is a narrow, mechanical portability fix. The two changed lines swap mktemp -t <name>-XXXXXX.json for mktemp "${TMPDIR:-/tmp}/<name>-XXXXXX":

Both forms create the temp file atomically via mktemp(1) itself (no separate touch/reopen step), so there's no TOCTOU/symlink race in either the old or new form. The new form is arguably a small security improvement rather than a regression: on BSD, -t treated the argument as a prefix and (per the PR's own verification) a .json-suffixed template was substituted verbatim with no random characters at all — i.e., a predictable temp filename on BSD platforms, which is the classic precondition for a symlink/race attack in a shared /tmp. The new positional-template form with trailing XXXXXX gets real randomization from mktemp on both GNU and BSD.

Other notes (not findings, just confirming no regression):

  • $TMPDIR is honored via ${TMPDIR:-/tmp} exactly as it was implicitly honored by -t before — no new trust boundary introduced.
  • The resulting temp files (TMP_JSON, TMP_SETTINGS) are only ever referenced by their mktemp-returned absolute path and cleaned up via trap ... EXIT; no interpolation of untrusted data into the path template.
  • Downstream jq usage (--argjson for plugin names) already avoids filter-string interpolation and is unchanged by this diff.
  • plugin.json and CHANGELOG.md changes are version/doc bumps with no executable or permissions content.

Per instructions, supply-chain/unpinned-action/workflow-permission concerns are zizmor's lane — not applicable here anyway since no workflow files changed.

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 23s —— View job


Reviewing PR #2437

  • Gather context (PR diff, changed files, CLAUDE.md/REVIEW.md)
  • Review fix-plugin-drift.sh mktemp changes
  • Review plugin.json version bump
  • Review CHANGELOG.md entry
  • Post inline findings (if any) and summary

Summary

Reviewed the three changed files against origin/main...HEAD. This is a clean, narrowly-scoped fix — no findings to report.

fix-plugin-drift.sh (plugin-drift-json-XXXXXX, settings-json-XXXXXX):

  • Both call sites move from mktemp -t <name>-XXXXXX.json to mktemp "${TMPDIR:-/tmp}/<name>-XXXXXX", matching the form documented in docs/conventions/topic-docs/README.md (ephemeral tier rule 1).
  • Dropping the .json suffix is safe — I checked every downstream use of $TMP_JSON/$TMP_SETTINGS in the script (passed via env var to check-plugin-drift.sh, read with jq, mv'd into place) and none depend on the filename or extension.
  • The pre-existing combined EXIT trap at line 223 (rm -f "$TMP_JSON"; rm -f "$TMP_SETTINGS") correctly supersedes the single-file trap set at line 109 when the script reaches apply mode — this logic is unchanged by the diff, just re-commented.
  • I swept plugins/** myself for any remaining mktemp -t/--tmpdir invocations and found none outside this PR's own CHANGELOG prose — the "no non-conforming sites remain" claim holds.

plugin.json: version bump 0.37.1 → 0.37.2 is consistent with a patch-level fix.

CHANGELOG.md: new [0.37.2] entry follows the file's existing Keep-a-Changelog format and prose style (compare the [0.37.1]/[0.37.0] entries directly above it).

Note: I did not independently execute the script on GNU or BSD mktemp — the PR body's cross-platform execution results (coreutils 9.4 locally, macOS Actions run 31594143233) are author-claimed and unverified by me; I only confirmed the code change is internally consistent and matches the documented convention.

@github-actions

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@kyle-sexton
kyle-sexton merged commit ffa34df into main Aug 12, 2026
36 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/mktemp-portable-form-63e1 branch August 12, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mktemp: no flag form is both non-deprecated and portable, and the bare form writes into the repo

2 participants