Skip to content

fix(security): use try-finally for temp file cleanup in update-check#3307

Merged
louisgv merged 1 commit intomainfrom
fix/issue-3306
Apr 15, 2026
Merged

fix(security): use try-finally for temp file cleanup in update-check#3307
louisgv merged 1 commit intomainfrom
fix/issue-3306

Conversation

@la14-1
Copy link
Copy Markdown
Member

@la14-1 la14-1 commented Apr 15, 2026

Why: Temp file cleanup race in performAutoUpdate — if process crashes after write but before cleanup, temp script files persist indefinitely. Restructured so cleanup is unconditionally reached after tryCatch captures any exec error, guaranteeing temp file removal on both success and failure paths.

Fixes #3306

-- refactor/security-auditor

Restructure temp file write-execute-cleanup in performAutoUpdate so
cleanup is unconditionally reached after tryCatch captures any exec
error. Previously, the Windows and Unix paths each had separate
tryCatch+cleanup+rethrow sequences that could diverge under future
edits. Now a single tryCatch wraps the platform-branching exec, with
cleanup always running before any error is re-thrown.

Fixes #3306

Agent: security-auditor
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@la14-1 la14-1 marked this pull request as ready for review April 15, 2026 05:22
Copy link
Copy Markdown
Member

@louisgv louisgv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security Review

Verdict: APPROVED
Commit: 31b893b

Findings

None. This PR improves security by guaranteeing temp file cleanup in all code paths.

Security Assessment

  • Temp file handling: Uses unique filenames (Date.now()), proper permissions (0o700 on Unix)
  • Command execution: Uses execFileSync with argument arrays (no shell injection risk)
  • Cleanup guarantee: tryCatch wrapper ensures cleanup runs even on error (fixes potential temp file leak)
  • Version bump: 1.0.11 → 1.0.12 (follows policy)

Tests

  • bash -n: N/A (no shell scripts changed)
  • bun test: PASS (2068 tests, 0 failures)

This refactor is a security improvement over the prior code.


-- security/pr-reviewer

@louisgv louisgv added the security-approved Security review approved label Apr 15, 2026
@louisgv louisgv merged commit d1d51fb into main Apr 15, 2026
6 checks passed
@louisgv louisgv deleted the fix/issue-3306 branch April 15, 2026 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security-approved Security review approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: temp file cleanup race in update-check.ts performAutoUpdate

2 participants