fix: make certbot email address updates user experience match the new…

[jchiarulli]

… way certbot handles updating account email addresses, set permissions for ssh authorized keys file when the file does not exist, and fix khatru pyramid owner and group setting for the users directory when backing up as root user

Summary by CodeRabbit

• New Features

  • Certbot email account management now explicitly prompts users to review and update existing account settings.

• Bug Fixes

  • SSH authorized keys file permissions now consistently set to secure defaults across all installation paths.

• Documentation

  • Updated install command description.

• Chores

  • Code formatting refinements.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors Certbot email account handling to prompt users for explicit removal or updates, enforces SSH file permissions to 0600 across all paths, adjusts file ownership path targets, expands command documentation, and performs minor formatting cleanup.

Changes

Cohort / File(s)	Summary
Certbot email management pkg/network/certbot.go	Reworked email flow in GetCertificates: replaces auto-handling of "Email contact: none" with explicit user prompts. When an account exists, now prompts user to remove or update Certbot email; if updating, collects new email and runs certbot update_account with --email and --no-eff-email flags.
File permissions and ownership pkg/network/remote_access.go, pkg/relays/khatru_pyramid/handle_exisiting_users_file.go	Ensures SSH authorized_keys file permissions explicitly set to 0600 in all creation/append paths for both root and non-root users. Adjusts SetOwnerAndGroupForAllContentUsingLinux call to target UsersFileUsersDirPath instead of UsersFilePath for root user.
Documentation and formatting cmd/install.go, pkg/network/firewall.go, pkg/verification/verify.go	Expands install command Long description text. Adds blank lines around firewall code blocks for readability. Removes extra blank lines in verification module.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 We've tidied the Certbot's account display,
Secure the SSH keys—0600, hooray!
Files owned just right, paths aligned with care,
Clean formatting brushed through the air! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title is truncated and incomplete (ends with 'match the new…'), making it unclear what the 'new' behavior or requirement is that the PR addresses.	Complete the title by clarifying what 'new' refers to, e.g., 'fix: make certbot email address updates user experience match the new certbot API' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 110b601 and 1cc8f8d.

📒 Files selected for processing (6)

• cmd/install.go
• pkg/network/certbot.go
• pkg/network/firewall.go
• pkg/network/remote_access.go
• pkg/relays/khatru_pyramid/handle_exisiting_users_file.go
• pkg/verification/verify.go

💤 Files with no reviewable changes (1)

• pkg/verification/verify.go

🧰 Additional context used

🧬 Code graph analysis (2)

pkg/relays/khatru_pyramid/handle_exisiting_users_file.go (2)

pkg/utils/directories/utils.go (1)

• SetOwnerAndGroupForAllContentUsingLinux (174-192)

pkg/relays/khatru_pyramid/constants.go (1)

• UsersFileUsersDirPath (12-12)

pkg/network/remote_access.go (2)

pkg/utils/files/utils.go (1)

• SetPermissions (123-130)

pkg/network/constants.go (1)

• RootHiddenSSHAuthorizedKeysFilePath (9-9)

🔇 Additional comments (6)

pkg/network/firewall.go (1)

17-17: LGTM!

The added blank lines improve visual separation between logical blocks.

Also applies to: 27-27, 63-63

pkg/relays/khatru_pyramid/handle_exisiting_users_file.go (1)

22-22: LGTM!

This correctly aligns the root-user path with the non-root branch (line 27), ensuring chown -R targets the users directory (UsersFileUsersDirPath) rather than a single file. This is consistent with the recursive ownership operation.

cmd/install.go (1)

28-28: LGTM!

The expanded description accurately reflects the install command's functionality.

pkg/network/remote_access.go (2)

113-116: LGTM!

Explicitly setting 0600 permissions after file creation ensures the authorized_keys file is properly secured, regardless of the system's umask. This aligns with SSH best practices.

---

126-129: LGTM!

Consistent with the root-user branch, this ensures proper 0600 permissions for the non-root authorized_keys file on creation.

pkg/network/certbot.go (1)

187-188: LGTM!

The spinner message clearly indicates to the user that an existing Certbot account was found, improving the user experience.

---

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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/network/certbot.go (1)

190-229: The prompt mentions "remove" but only implements "update" functionality.

Line 192 asks "Do you want to remove or update your Certbot email?", but the implementation (lines 213, 221) only handles the update case by passing --email <email> to certbot. Additionally, line 207 suggests "Leave email empty if you don't want to receive notifications," which would cause certbot update_account --email "" --no-eff-email to fail since the --email flag requires a valid address.

To properly implement removal, use certbot update_account --register-unsafely-without-email when the user wants to remove their email. Otherwise, clarify the prompt to only mention "update" and handle the empty email case explicitly (either reject it or implement removal).

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 110b601 and 1cc8f8d.

📒 Files selected for processing (6)

• cmd/install.go
• pkg/network/certbot.go
• pkg/network/firewall.go
• pkg/network/remote_access.go
• pkg/relays/khatru_pyramid/handle_exisiting_users_file.go
• pkg/verification/verify.go

💤 Files with no reviewable changes (1)

• pkg/verification/verify.go

🧰 Additional context used

🧬 Code graph analysis (2)

pkg/relays/khatru_pyramid/handle_exisiting_users_file.go (2)

pkg/utils/directories/utils.go (1)

• SetOwnerAndGroupForAllContentUsingLinux (174-192)

pkg/relays/khatru_pyramid/constants.go (1)

• UsersFileUsersDirPath (12-12)

pkg/network/remote_access.go (2)

pkg/utils/files/utils.go (1)

• SetPermissions (123-130)

pkg/network/constants.go (1)

• RootHiddenSSHAuthorizedKeysFilePath (9-9)

🔇 Additional comments (6)

pkg/network/firewall.go (1)

17-17: LGTM!

The added blank lines improve visual separation between logical blocks.

Also applies to: 27-27, 63-63

pkg/relays/khatru_pyramid/handle_exisiting_users_file.go (1)

22-22: LGTM!

This correctly aligns the root-user path with the non-root branch (line 27), ensuring chown -R targets the users directory (UsersFileUsersDirPath) rather than a single file. This is consistent with the recursive ownership operation.

cmd/install.go (1)

28-28: LGTM!

The expanded description accurately reflects the install command's functionality.

pkg/network/remote_access.go (2)

113-116: LGTM!

Explicitly setting 0600 permissions after file creation ensures the authorized_keys file is properly secured, regardless of the system's umask. This aligns with SSH best practices.

---

126-129: LGTM!

Consistent with the root-user branch, this ensures proper 0600 permissions for the non-root authorized_keys file on creation.

pkg/network/certbot.go (1)

187-188: LGTM!

The spinner message clearly indicates to the user that an existing Certbot account was found, improving the user experience.