Skip to content

fix: write settings.toml with owner-only permissions - #887

Open
arkanoider wants to merge 1 commit into
mainfrom
fix/settings-toml-permissions
Open

fix: write settings.toml with owner-only permissions#887
arkanoider wants to merge 1 commit into
mainfrom
fix/settings-toml-permissions

Conversation

@arkanoider

@arkanoider arkanoider commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

settings.toml contains the nsec_privkey. Previously, the daemon created it with default permissions (0o644) on the headless/Docker/CI path and the wizard manual-setup path, leaving the private key readable by any user on the host.

This PR introduces a shared util::write_private_file helper that creates files with 0o600 on Unix and tightens permissions on pre-existing files, then routes all three settings.toml creation paths through it.

Changes

  • Add util::write_private_file in src/config/util.rs (Unix: OpenOptions::mode(0o600) + explicit set_permissions(0o600); non-Unix: plain write).
  • Replace std::fs::write with write_private_file in the non-interactive/headless template copy.
  • Replace the manual-setup and guided-wizard writes in src/config/wizard.rs with write_private_file.
  • Add unit tests for content writes, new-file permissions, and tightening an existing 0o644 file.

Summary by CodeRabbit

  • Security Improvements

    • Configuration files created or updated by the application now use owner-only permissions.
    • Existing configuration files have their permissions tightened automatically before content is written.
  • Bug Fixes

    • Improved handling and reporting of configuration file write errors.
    • Added coverage for file contents and permission behavior.

Introduce util::write_private_file to create files with 0o600 on Unix
and tighten permissions on existing files. Route all settings.toml
creation paths (headless/Docker, wizard manual, wizard guided) through
it so the nsec_privkey is no longer world-readable.

Add unit tests covering new files, content writes, and pre-existing
world-readable files being tightened.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9257bd87-ae31-4d0b-b38f-a195709fba95

📥 Commits

Reviewing files that changed from the base of the PR and between 09390da and 597479c.

📒 Files selected for processing (2)
  • src/config/util.rs
  • src/config/wizard.rs

Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review.


Walkthrough

The change adds a shared helper for writing owner-only configuration files. Template creation and wizard output use the helper. Tests verify file contents, new-file permissions, and permission tightening for existing files.

Changes

Private configuration files

Layer / File(s) Summary
Private-file writer and validation
src/config/util.rs
Adds write_private_file, which writes contents with 0o600 permissions on Unix, maps I/O errors, and tests new and existing files.
Configuration creation integration
src/config/util.rs, src/config/wizard.rs
Routes manual template creation and interactive wizard output through write_private_file.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 59747

This PR restricts settings.toml to owner-only permissions across the affected creation paths, reducing exposure of the private key. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: grunch, andreadiazcorreia

Poem

A rabbit guards each secret file,
With paws that set permissions right.
Templates write, the wizards sing,
Safe contents bloom from every spring.
0o600 keeps watch tonight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 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 change: writing settings.toml with owner-only permissions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/settings-toml-permissions

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@AndreaDiazCorreia AndreaDiazCorreia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@arkanoider apologies, I opened #890 a couple of hours after this one without noticing it was already covering the same ground on settings.toml. That's on me for not checking first.

The two overlap directly: both replace the same std::fs::write in config::util and the same inline OpenOptions block in config::wizard, so whichever lands first will conflict with the other. They also differ on one design point. The helper here tightens an existing file, while mine refuses to touch a path that already exists (O_EXCL, so a planted symlink can't be written through).

Would you mind taking a look at #890 so we can decide together which one to keep? Happy either way, including reworking mine on top of yours if you prefer this shape.

@arkanoider

Copy link
Copy Markdown
Collaborator Author

@arkanoider apologies, I opened #890 a couple of hours after this one without noticing it was already covering the same ground on settings.toml. That's on me for not checking first.

The two overlap directly: both replace the same std::fs::write in config::util and the same inline OpenOptions block in config::wizard, so whichever lands first will conflict with the other. They also differ on one design point. The helper here tightens an existing file, while mine refuses to touch a path that already exists (O_EXCL, so a planted symlink can't be written through).

Would you mind taking a look at #890 so we can decide together which one to keep? Happy either way, including reworking mine on top of yours if you prefer this shape.

@AndreaDiazCorreia don't worry it happens in these days...if your pr covers also this one, go and close this in favor of yours!

@AndreaDiazCorreia

Copy link
Copy Markdown
Member

I'll leave this open until #890 is actually merged, just so we don't end up with nothing fixing the settings.toml permissions if that one takes a while, and close it right after.

Thanks for catching this first.

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.

2 participants