fix(onboard): replace predictable temp filenames with mkdtempSync

[latenighthackathon]

Summary

Probe functions and writeSandboxConfigSyncFile construct temp filenames using Date.now() and Math.random(), both of which are predictable. A local attacker can race the file creation with a symlink to redirect curl output (API responses) or inject a malicious sync script. This replaces all 6 sites with fs.mkdtempSync(), matching the secure pattern already used elsewhere in the same file.

Related Issue

Closes #1093

Changes

• Replaced predictable temp filename construction in 6 functions with fs.mkdtempSync():
  • writeSandboxConfigSyncFile — now creates nemoclaw-sync-<random>/sync.sh
  • probeOpenAiLikeEndpoint — now creates nemoclaw-probe-<random>/body.json
  • probeAnthropicEndpoint — now creates nemoclaw-anthropic-probe-<random>/body.json
  • fetchNvidiaEndpointModels — now creates nemoclaw-nvidia-models-<random>/body.json
  • fetchOpenAiLikeModels — now creates nemoclaw-openai-models-<random>/body.json
  • fetchAnthropicModels — now creates nemoclaw-anthropic-models-<random>/body.json
• Updated all finally blocks to clean up the temp directory ({ recursive: true }) instead of just the file.
• Updated caller in setupOpenclaw to remove the temp directory instead of unlinking the file.
• Updated existing test to match new function signature and verify file permissions (0o600).

Testing

• npm test passes (no local environment)
• Unit tests updated

Executed:

• Verified all 6 mkdtempSync calls use the correct prefix to preserve debuggability
• Verified all 5 finally blocks reference probeDir (not bodyFile) for recursive cleanup
• Verified writeSandboxConfigSyncFile caller at line 2373 cleans up via path.dirname(scriptFile)
• Confirmed fs.mkdtempSync is already used at lines 1764 and 2680 in the same file — no new API dependency

Checklist

• Follows Conventional Commits
• Commits are signed

Summary by CodeRabbit

• Chores

  • Use isolated per-invocation temporary directories for sandbox config and HTTP probe/model fetch operations, writing stable script/output filenames and performing recursive cleanup to improve reliability and avoid collisions; ensure generated scripts have secure permissions.

• Tests

  • Updated tests to verify the new temp-directory layout, script placement, and file permission behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0cb66c9f-aa05-42a5-9161-1dbf1750087c

📥 Commits

Reviewing files that changed from the base of the PR and between 9e49e57 and 9841578.

📒 Files selected for processing (1)

• bin/lib/onboard.js

✅ Files skipped from review due to trivial changes (1)

• bin/lib/onboard.js

---

📝 Walkthrough

Walkthrough

Replaced predictable timestamp/random temp filenames with per-invocation unique directories created via fs.mkdtempSync() in onboard probe and sandbox-sync routines; writes use stable filenames inside those dirs (e.g., body.json, sync.sh) and cleanup removes the temp directory recursively. writeSandboxConfigSyncFile drops its now parameter.

Changes

Cohort / File(s)	Summary
Onboard temp-file hardening bin/lib/onboard.js	Replaced ad-hoc predictable temp filenames with fs.mkdtempSync() directories for probeOpenAiLikeEndpoint, probeAnthropicEndpoint, fetchNvidiaEndpointModels, fetchOpenAiLikeModels, fetchAnthropicModels, and writeSandboxConfigSyncFile. Each call writes to stable filenames (e.g., body.json, sync.sh) inside the temp dir and now removes the directory recursively in finally. writeSandboxConfigSyncFile signature changed (removed now).
Tests updated for nondeterministic paths & perms test/onboard.test.js	Test no longer expects deterministic numeric ID in path; asserts returned path matches nemoclaw-sync-<anything>/sync.sh, verifies script content, and on non-Windows platforms checks file mode is 0o600.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I dug a new, snug hollow den,

sync.sh tucked where no one can ken,
No guessed filenames to spy or pry,
mkdtemp whispers, "let danger fly",
A quiet hop — secure again.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.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 directly and clearly describes the main change: replacing predictable temp filenames with mkdtempSync, which is the primary objective of the PR.
Linked Issues check	✅ Passed	The PR addresses all coding objectives from issue #1093: replaced predictable temp filename construction with fs.mkdtempSync in six functions, updated cleanup logic for recursive directory removal, and ensured appropriate file permissions.
Out of Scope Changes check	✅ Passed	All changes are directly related to the objectives of issue #1093 with no extraneous modifications beyond fixing the identified security issue.

_{✏️ 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.}