fix(cli): harden hook settings atomic write - #150
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR hardens loomweave-cli’s .claude/settings.json update path to prevent a symlink-clobber primitive by replacing the predictable PID-based staging filename with an exclusive, randomized temp file and persisting it atomically.
Changes:
- Replace
.settings.json.tmp-<pid>staging withtempfile::Builder::tempfile_in+persist()for atomic swap. - Promote
tempfilefrom a dev-dependency to a runtime dependency forloomweave-cli. - Add a Unix regression test ensuring a pre-seeded predictable staging symlink does not redirect writes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/loomweave-cli/src/hooks_settings.rs | Switch to randomized, exclusive temp-file staging + add symlink regression test. |
| crates/loomweave-cli/Cargo.toml | Move tempfile to runtime dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| fn write_and_swap(dir: &Path, dest: &Path, serialized: &str) -> Result<()> { | ||
| let mut tmp = tempfile::Builder::new() | ||
| .prefix(".settings.json.tmp-") | ||
| .tempfile_in(dir) | ||
| .with_context(|| format!("create staging file in {}", dir.display()))?; |
|
Superseded by the rebase onto |
Motivation
.claudecould be pre-created as a symlink by a malicious repository and redirect writes outside the project.Description
.settings.json.tmp-<pid>staging path with an exclusive, randomized temporary file created viatempfile::Builder::tempfile_inin the.claudedirectory and atomically persist it tosettings.jsonusingpersist, avoiding following attacker symlinks.fs::write+fs::renameflow and implementwrite_and_swap(dir, dest, serialized)which creates, writes, and persists the temp file safely.tempfilefrom a test-only dev-dependency to a runtime dependency incrates/loomweave-cli/Cargo.tomlso secure temp-file creation is available at runtime.install_does_not_follow_predictable_staging_symlinkthat pre-places the old predictable symlink and verifies the external target is not modified and the installedsettings.jsonis a regular file.Testing
cargo test -p loomweave-cli hooks_settings, and all relevant tests passed (20 passed, 0 failed), including the new symlink regression test.cargo fmt --checkandgit diff --check, both of which succeeded with no reported issues.crates/loomweave-cli/src/hooks_settings.rsandcrates/loomweave-cli/Cargo.tomland committed the patch as the branch update.Codex Task