Fix config guards for unset secure_dir and shell_bin - #25
Merged
Merged
Conversation
Guard against empty secure_dir before constructing hosts_file path. Prevents loading /hosts if it exists on the system when secure_dir is not configured.
Guard against empty shell_bin before checking if file exists. Prevents confusing 'does not exist!' error message when shell_bin is not configured.
There was a problem hiding this comment.
🟢 Approval recommended
The reviewed changes address the identified unset-configuration cases with no unresolved blocking issues.
Pull request overview
Adds guards for unset shell_bin and secure_dir values in script/setup.
Changes:
- Skip shell setup when
shell_binis empty. - Skip hosts setup when
secure_diris empty. - Validate
secure_dirbefore constructing the hosts path.
File summaries
| File | Description |
|---|---|
script/setup |
Adds configuration guards before shell and hosts setup operations. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two functions in
script/setupdid not guard against empty config values before using them, leading to unexpected behavior when those values were unset:setup_hosts(line 141): Whensecure_dirwas unset,config_get secure_dir ""returned empty string, makinghosts_fileresolve to/hosts. If that file existed on the system,sudo hostile loadwould load an unrelated system-level file.setup_shell(line 73): Whenshell_binwas unset, the empty value would fail the file existence check with a confusing error message (does not exist!) before exiting.Solution
Added early-return guards in both functions to check for empty config values before constructing paths or performing operations:
setup_hosts: Return with warning ifsecure_diris emptysetup_shell: Return with warning ifshell_binis emptyThis matches the pattern already used in
setup_secure_files(line 44), which correctly guards with[ -n "$secure_dir" ]before use.Changes
script/setup: Added empty-value guards tosetup_hostsandsetup_shellFixes #24