Secure sandbox skill SSH config temp files

[fallintoplace]

Summary

skill install and skill remove wrote sandbox SSH configs directly under the shared temp directory using a name derived from the process id and current time. That made the path predictable enough for another local user to pre-create or race it.

This change creates a private mkdtemp directory for each sandbox skill SSH config, writes the config file inside it with exclusive create mode and 0600 permissions, and removes the whole temp directory in cleanup.

Validation

• npm test -- src/lib/actions/sandbox/skill-install.test.ts
• npx @biomejs/biome lint src/lib/actions/sandbox/skill-install.ts src/lib/actions/sandbox/skill-install.test.ts
• npm run build:cli
• npm run typecheck:cli

Summary by CodeRabbit

• Bug Fixes
  • Enhanced security and reliability of temporary SSH configuration file handling during sandbox skill installation and removal operations. Implemented stricter file permissions and exclusive file creation safeguards. Improved cleanup procedures to ensure all temporary configuration files and their parent directories are properly removed after each operation completes.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d971ba3e-3df3-4b9c-867e-962bd3938086

📥 Commits

Reviewing files that changed from the base of the PR and between 8c9ec67 and 1c75edd.

📒 Files selected for processing (2)

• src/lib/actions/sandbox/skill-install.test.ts
• src/lib/actions/sandbox/skill-install.ts

---

📝 Walkthrough

Walkthrough

Two internal helpers, createTemporarySshConfig and removeTemporarySshConfig, are extracted in skill-install.ts to manage a temp directory/file lifecycle for SSH configs with restrictive permissions and exclusive write semantics. Both removeSandboxSkill and installSandboxSkill are updated to use these helpers. Tests gain assertions for path naming, permission bits, write options, and directory cleanup.

Changes

Temp SSH Config Lifecycle Refactor

Layer / File(s)	Summary
createTemporarySshConfig and removeTemporarySshConfig helpers src/lib/actions/sandbox/skill-install.ts	Adds createTemporarySshConfig (unique temp dir, writes config with 0o600/flag: "wx", removes dir on failure) and removeTemporarySshConfig (best-effort recursive dir removal).
Call site updates and tests src/lib/actions/sandbox/skill-install.ts, src/lib/actions/sandbox/skill-install.test.ts	Replaces inline temp-file construction and manual unlink in removeSandboxSkill and installSandboxSkill with the new helpers and finally-block cleanup; extends tests to verify path naming, permission bits (0o700 dir, 0o600 file), writeFileSync arguments, and removal of both the config file and parent directory.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A temp dir born with secrets tight,
Mode six-zero-zero locks it right.
"wx" ensures no sneaky file,
The finally block cleans up in style.
Hops along, no trace in sight! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main security improvement—implementing secure, temporary SSH config file handling with restricted permissions and isolated directories.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cv]

Thanks for tackling this hardening. I opened #5517 for the full focused /tmp temp-file issue; it covers these skill-install SSH config writes via a shared mkdtemp-backed helper, and also covers the other TypeScript SSH config sites plus the targeted nemoclaw-start.sh /tmp writes.

I think this PR is superseded by #5517 so we keep the security fix in one focused patch. If #5517 lands, this one can probably be closed.