fix: disable Electron SUID sandbox for obsidian headless wrapper - #1433
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughA shell script was modified to pass an additional Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Mesa DescriptionTL;DRDisable Electron SUID sandbox for the obsidian headless wrapper to fix SUID sandbox crash on NixOS. What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the obsidian-headless.sh script to include the --no-sandbox flag when executing Obsidian. The review feedback recommends using the more targeted --disable-setuid-sandbox flag instead of disabling the entire sandbox, which improves security while still addressing the specific SUID-related crashes in Nix environments.
| @@ -1,2 +1,2 @@ | |||
| #!/usr/bin/env bash | |||
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian "$@" | |||
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian --no-sandbox "$@" | |||
There was a problem hiding this comment.
The --no-sandbox flag disables all security sandboxing in Electron. Since the pull request description identifies the SUID sandbox as the specific cause of the crash (due to Nix store limitations), using --disable-setuid-sandbox is a more targeted approach. This allows the kernel-based namespace sandbox to remain active if the host system supports it, maintaining a better security posture while still resolving the SUID-related crash.
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian --no-sandbox "$@" | |
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian --disable-setuid-sandbox "$@" |
There was a problem hiding this comment.
Pull request overview
This PR updates the Obsidian headless CLI wrapper to avoid Electron’s SUID sandbox crash on NixOS by passing an explicit sandbox-disabling flag.
Changes:
- Add
--no-sandboxto theobsidianinvocation inside thexvfb-runwrapper.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,2 +1,2 @@ | |||
| #!/usr/bin/env bash | |||
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian "$@" | |||
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian --no-sandbox "$@" | |||
There was a problem hiding this comment.
Using --no-sandbox fully disables Chromium/Electron sandboxing, which is a significant security reduction even in headless mode (e.g., malicious plugins/vault content could gain more privileges). If the intent is only to avoid the failing SUID sandbox on NixOS, consider switching to --disable-setuid-sandbox (or gating --no-sandbox behind an env var/host check) so other sandbox mechanisms can still apply where available.
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian --no-sandbox "$@" | |
| exec @xvfbRun@/bin/xvfb-run -a @obsidian@/bin/obsidian --disable-setuid-sandbox "$@" |
Summary
--no-sandboxto the obsidian headless wrapper to fix SUID sandbox crash on NixOSchrome-sandboxalways failsTest plan
obsidianCLI on kyber and verify it no longer crashes with SUID sandbox errorobsidian --helpand vault commands work underxvfb-runSummary by cubic
Disable Electron SUID sandbox in the Obsidian headless wrapper to stop crashes on NixOS. Adds
--no-sandboxto theobsidiancall underxvfb-run, which is safe for our headless CLI usage.Written for commit 8c60e55. Summary will update on new commits.