fix: unnecessary password prompts for hosts and cert

[pilat]

Fix unnecessary password prompts for hosts and cert operations

Problem: DevBox prompted for password on every devbox up even when no changes were needed.

Root causes:

1. Hosts: Non-deterministic map iteration order caused host entries to be generated in different order each run, triggering false "content changed" detection
2. Hosts: hosts.Save added empty markers when entries were empty, causing unnecessary writes
3. Cert (macOS): isSynced() only checked the first certificate returned by keychain, failing when multiple certs with similar names existed

Fixes:

• Sort IPs before generating host entries for deterministic output
• Skip adding markers when entries are empty
• Check all PEM blocks when verifying cert is installed in keychain

Summary by CodeRabbit

• Bug Fixes

  • Improved hosts file update reliability with automatic retry on permission failures and clearer update notifications
  • Enhanced macOS certificate validation to correctly handle multiple certificates returned by the system
  • Prevented unnecessary writes by detecting when hosts content is unchanged

• Refactor

  • Deterministic ordering of host entries for more stable and predictable hosts file generation

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Hosts management now reports whether the hosts file changed (Save returns (bool, error)), runHostsUpdate retries via sudo on first-time save errors and prints a conditional update message. Project state no longer stores HasHosts and host entries are deterministically sorted. macOS cert check now iterates multiple PEM blocks.

Changes

Cohort / File(s)	Change Summary
Host file API and behavior internal/hosts/hosts.go	Save and internal save signatures changed to return (bool, error) indicating whether the hosts file was modified. Error returns updated to (false, err) for stat/read/write failures; unchanged/no-op cases return (false, nil); successful writes return (true, nil). End-of-file and write error paths adjusted accordingly.
Host update orchestration cmd/devbox/up.go	runHostsUpdate removed early-return guard for zero hosts, now captures (changed, err) from hosts.Save. On first-time save error, added a sudo retry path (prints a notification before retry). When changed == true, prints "[*] Updated hosts file". Removed updates to p.HasHosts and p.SaveState.
Project state & host ordering internal/project/project.go, internal/project/struct.go	Removed HasHosts from Project and stateFileStruct. SaveState no longer persists HasHosts. loadState only sets LocalMounts when state.Mounts is non-nil. applyHosts now builds host entries deterministically: IPs are collected and sorted, and hostnames per IP are sorted before creating HostEntities. Added slices import for sorting.
macOS certificate parsing internal/cert/cert_darwin.go	Added encoding/pem import and changed certificate check to iterate over multiple PEM blocks returned by the security command, parse each certificate block, and compare parsed certificates to the in-memory certificate until a match is found or blocks are exhausted.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas needing extra attention:

• Call sites for hosts.Save across the repo to ensure the new (bool, error) return is correctly handled.
• Permission/privilege handling and messaging in runHostsUpdate sudo retry path to avoid unnecessary elevation or confusing logs.
• Migration and backward-compatibility implications from removing HasHosts from persisted state files.
• Deterministic sorting in applyHosts for any callers that might rely on previous ordering.
• Correct parsing and comparison logic in the macOS PEM iteration to avoid false negatives/positives.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 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 summarizes the main fix: eliminating unnecessary password prompts caused by hosts file and certificate verification issues.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/hosts-sync

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65ad91b and 3d88a09.

⛔ Files ignored due to path filters (1)

• internal/hosts/hosts_test.go is excluded by !**/*_test.go

📒 Files selected for processing (5)

• cmd/devbox/up.go
• internal/cert/cert_darwin.go
• internal/hosts/hosts.go
• internal/project/project.go
• internal/project/struct.go

🧰 Additional context used

📓 Path-based instructions (3)

internal/**/*.go

⚙️ CodeRabbit configuration file

internal/**/*.go: Core packages providing project functionality:

• project/: Project configuration, Docker Compose extensions (x-devbox-*)
• manager/: Project/service autodetection from current directory
• git/: Git operations (clone, sparse checkout, sync)
• cert/: SSL certificate generation
• hosts/: /etc/hosts management with project-scoped markers
• table/: CLI table output formatting

Review for:

• Clean interfaces and proper encapsulation
• Error wrapping with context
• No interface{} - use 'any' instead (enforced by linter)
• YAGNI principle - no speculative features or premature abstractions
• Security considerations for host file and certificate operations

Files:

• internal/project/struct.go
• internal/cert/cert_darwin.go
• internal/hosts/hosts.go
• internal/project/project.go

internal/project/**/*.go

⚙️ CodeRabbit configuration file

internal/project/**/*.go: Project configuration extending Docker Compose with x-devbox extensions:

• x-devbox-sources: Repository definitions with branch and sparse checkout
• x-devbox-scenarios: Named command shortcuts
• x-devbox-hosts: /etc/hosts entries
• x-devbox-cert: SSL certificate domains

State is persisted in .devboxstate JSON file.
Mount system replaces volume binds with local paths.

Review for proper parsing and validation of compose extensions.

Files:

• internal/project/struct.go
• internal/project/project.go

cmd/devbox/**/*.go

⚙️ CodeRabbit configuration file

cmd/devbox/**/*.go: This directory contains CLI commands using Cobra framework.
Each command is in a separate file with an init() function that registers via root.AddCommand().

Review for:

• Proper error handling with context wrapping (fmt.Errorf with %w)
• Consistent command structure following existing patterns
• Appropriate use of runWrapper for context handling
• Clear, concise command descriptions
• No over-engineering - only implement what's needed

Files:

• cmd/devbox/up.go

🧬 Code graph analysis (1)

cmd/devbox/up.go (4)

internal/hosts/hosts.go (1)

• Save (19-21)

internal/hosts/hosts_test.go (1)

• TestSave (10-62)

cmd/devbox/update_hosts.go (1)

• init (11-38)

internal/project/config.go (1)

• IP (24-27)

🔇 Additional comments (10)

internal/cert/cert_darwin.go (1)

22-46: LGTM! Properly handles multiple certificates from keychain.

The loop over PEM blocks correctly addresses the root cause where only the first certificate was checked. The pattern of consuming data while pem.Decode returns non-nil blocks is idiomatic. Silently continuing on parse errors is acceptable here since this is a sync-check, not a validation routine.

internal/hosts/hosts.go (4)

17-21: Clean API change with proper documentation.

The signature change to return (bool, error) is a good design choice - it allows callers to determine if actual changes occurred without re-reading the file. The doc comment clearly explains the contract.

---

57-59: Correctly avoids writing empty marker blocks.

When entries is empty during replacement, the code now skips writing the BEGIN/END markers entirely. This prevents the "empty markers cause unnecessary writes" issue described in the PR.

---

76-82: Correct guard for appending new entries.

The condition !replaced && len(entries) > 0 ensures markers are only appended when there are actual entries to add and no existing block was found. This completes the fix for empty entries handling.

---

84-92: Change detection and error wrapping look good.

The content comparison before write prevents unnecessary file operations. The error at line 90 is now properly wrapped with context, addressing the previous review feedback.

internal/project/struct.go (1)

3-5: State simplified appropriately.

Removing HasHosts from persisted state is correct since the hosts file change detection is now handled by hosts.Save returning a changed flag. No need to track this across runs.

internal/project/project.go (3)

232-243: Deterministic output achieved with proper sorting.

Both IPs and hostnames per IP are now sorted, ensuring consistent output regardless of map iteration order. This directly fixes the root cause of false "content changed" detection mentioned in the PR objectives.

---

186-188: Defensive nil check prevents map overwrite.

The nil check before assigning state.Mounts to p.LocalMounts is correct. Without it, a state file with "mounts": null would replace the initialized map with nil, potentially causing nil map panics on subsequent writes.

---

120-123: State persistence simplified.

SaveState now only persists Mounts, consistent with the removal of HasHosts from stateFileStruct.

cmd/devbox/up.go (1)

192-211: Integration with new hosts.Save signature is correct.

The logic properly handles all cases:

• Permission error on first run → sudo retry
• Other errors → propagate
• Success with changes → notify user
• Success without changes → silent (no unnecessary password prompt)

This achieves the PR's goal of eliminating unnecessary password prompts.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 099b2a8 and 65ad91b.

⛔ Files ignored due to path filters (1)

• internal/hosts/hosts_test.go is excluded by !**/*_test.go

📒 Files selected for processing (5)

• cmd/devbox/up.go
• internal/cert/cert_darwin.go
• internal/hosts/hosts.go
• internal/project/project.go
• internal/project/struct.go

🧰 Additional context used

📓 Path-based instructions (3)

internal/**/*.go

⚙️ CodeRabbit configuration file

internal/**/*.go: Core packages providing project functionality:

• project/: Project configuration, Docker Compose extensions (x-devbox-*)
• manager/: Project/service autodetection from current directory
• git/: Git operations (clone, sparse checkout, sync)
• cert/: SSL certificate generation
• hosts/: /etc/hosts management with project-scoped markers
• table/: CLI table output formatting

Review for:

• Clean interfaces and proper encapsulation
• Error wrapping with context
• No interface{} - use 'any' instead (enforced by linter)
• YAGNI principle - no speculative features or premature abstractions
• Security considerations for host file and certificate operations

Files:

• internal/project/struct.go
• internal/hosts/hosts.go
• internal/cert/cert_darwin.go
• internal/project/project.go

internal/project/**/*.go

⚙️ CodeRabbit configuration file

internal/project/**/*.go: Project configuration extending Docker Compose with x-devbox extensions:

• x-devbox-sources: Repository definitions with branch and sparse checkout
• x-devbox-scenarios: Named command shortcuts
• x-devbox-hosts: /etc/hosts entries
• x-devbox-cert: SSL certificate domains

State is persisted in .devboxstate JSON file.
Mount system replaces volume binds with local paths.

Review for proper parsing and validation of compose extensions.

Files:

• internal/project/struct.go
• internal/project/project.go

cmd/devbox/**/*.go

⚙️ CodeRabbit configuration file

cmd/devbox/**/*.go: This directory contains CLI commands using Cobra framework.
Each command is in a separate file with an init() function that registers via root.AddCommand().

Review for:

• Proper error handling with context wrapping (fmt.Errorf with %w)
• Consistent command structure following existing patterns
• Appropriate use of runWrapper for context handling
• Clear, concise command descriptions
• No over-engineering - only implement what's needed

Files:

• cmd/devbox/up.go

🧬 Code graph analysis (1)

cmd/devbox/up.go (1)

internal/hosts/hosts.go (1)

• Save (19-21)

🔇 Additional comments (6)

internal/project/struct.go (1)

3-5: LGTM!

Clean simplification of the state struct. Removing HasHosts aligns with the deterministic hosts handling approach—the hosts file content itself is now the source of truth rather than persisted state.

internal/project/project.go (2)

186-188: LGTM!

Good defensive check. Prevents overwriting the initialized empty map (from line 85) with nil when the state file exists but has no mounts.

---

120-123: LGTM!

State persistence is now minimal and correct—only Mounts needs to be persisted since hosts file changes are detected by content comparison.

internal/hosts/hosts.go (2)

17-21: LGTM!

Clean API change. Returning (changed bool, error) is idiomatic Go for operations where callers need to know if an action occurred.

---

57-59: LGTM!

Correct handling of empty entries—existing blocks are removed without leaving empty markers, and no new blocks are added when there's nothing to write. Fixes the unnecessary writes issue.

Also applies to: 76-82

internal/cert/cert_darwin.go (1)

22-46: Solid fix for the multi-certificate matching issue.

The loop correctly iterates all PEM blocks returned by security find-certificate -a -p, continuing past parse errors and non-certificate blocks. Using Equal() for comparison is the right approach, and pemTypeCertificate is properly defined as a constant in cert.go.