Ship a hardened, key-enrolled installer image; retire debugAccess as the default#60
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR replaces the debug-profile installer bring-up with a hardened, key-only LAN SSH admin-access flow: new ChangesKey-only admin access and installer hardening
CI Workflow Scheduling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant install-keepnode
participant Disk
participant sshd
participant admin-access module
Operator->>install-keepnode: run with --ssh-key <pubkey|file>
install-keepnode->>install-keepnode: validate ssh key format
install-keepnode->>Disk: resolve/validate whole-disk device
install-keepnode->>Disk: partition, format, mount /mnt
install-keepnode->>Disk: write key to /mnt/etc/keepnode/admin_authorized_keys
Disk-->>Operator: install complete, key-only SSH ready
Operator->>sshd: SSH as keepadmin using private key
sshd->>admin-access module: check authorizedKeysFile + AllowUsers policy
admin-access module-->>sshd: allow (mesh or lanBringup RFC1918 range)
sshd-->>Operator: session established, no password auth
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
README.md (2)
70-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTypo: stray space-before-comma.
Line 70: "same LAN , there is no password login" — reads oddly; likely meant to be an em dash or period.
✏️ Proposed fix
-4. After first boot, reach the node over **key-only SSH** from a machine on the same LAN , there is no password login: +4. After first boot, reach the node over **key-only SSH** from a machine on the same LAN -- there is no password login:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 70 - 77, Fix the stray space-before-comma in the README onboarding sentence by updating the wording around the key-only SSH step in the README section; adjust the phrase in the affected paragraph so it reads naturally (for example by replacing the odd comma break with proper punctuation) while keeping the rest of the SSH and Vaultwarden instructions unchanged.
65-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify how the SSH public key file gets onto the live installer session.
The example
install-keepnode /dev/sda --ssh-key ~/.ssh/id_ed25519.pubassumes that path exists on the booted USB session, which it typically won't on a fresh boot. Consider adding a note that the key can be pasted inline as a string, or copied in viascp/a second USB.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 65 - 68, The README’s install example implies the SSH key file path already exists in the live installer session, which can confuse users. Update the documentation around the install-keepnode example to explain how the key is provided to the booted session, referencing the install-keepnode usage and --ssh-key flag, and note acceptable ways to supply it such as pasting the public key inline or copying it in beforehand via scp or a second USB.nixos/admin-access.nix (1)
37-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTypo: stray space-before-comma reads as a grammar error.
Line 47: "does NOT satisfy the anti-lockout check , the installer is responsible" — the space before the comma breaks the sentence; likely meant to be an em dash. Since this is a public option
description(surfaced innixos-option/manual), worth cleaning up.✏️ Proposed fix
- Complements `authorizedKeys`. The file may be absent/empty at build time, so on its own it does - NOT satisfy the anti-lockout check , the installer is responsible for populating it and refusing - to proceed with no key. + Complements `authorizedKeys`. The file may be absent/empty at build time, so on its own it does + NOT satisfy the anti-lockout check -- the installer is responsible for populating it and refusing + to proceed with no key.Logic-wise, the updated assertion (accepting a non-null
authorizedKeysFileas sufficient at build time, trusting the installer to populate it) and the anti-lockout precedence checks are otherwise correct.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nixos/admin-access.nix` around lines 37 - 76, Fix the typo in the admin-access option text by updating the `authorizedKeysFile` description in `admin-access.nix`: remove the stray space before the comma in the anti-lockout sentence and rephrase it cleanly (for example with an em dash or period) so the `lib.mkOption` description reads naturally in `nixos-option`/manual output. Keep the existing meaning intact and only adjust the wording in the `authorizedKeysFile` description block.docs/deployment.md (1)
94-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTypo: stray space-before-comma (two instances).
Lines 97-98: "generic ISO , where the operator key isn't known at build time , `install-keepnode..." — both spots read oddly, likely meant as em dashes.
✏️ Proposed fix
-Mesh-only admin assumes your laptop is already a rostered mesh peer. For a co-owned cluster you author -that in from the start (step 2). For a box installed from the generic ISO , where the operator key -isn't known at build time , `install-keepnode --ssh-key <pubkey|file>` enrolls your key at install -time into a runtime file (`keepNode.adminAccess.authorizedKeysFile`) that the fixed closure reads. +Mesh-only admin assumes your laptop is already a rostered mesh peer. For a co-owned cluster you author +that in from the start (step 2). For a box installed from the generic ISO -- where the operator key +isn't known at build time -- `install-keepnode --ssh-key <pubkey|file>` enrolls your key at install +time into a runtime file (`keepNode.adminAccess.authorizedKeysFile`) that the fixed closure reads.Rest of the section (hardened-default framing,
lanBringupexplanation, redeploy guidance) reads correctly and matches the code changes inadmin-access.nix/installer.nix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/deployment.md` around lines 94 - 109, Fix the wording in the “Installer bring-up (generic ISO)” section by removing the stray spaces before the commas and tightening the sentence around install-keepnode; update the sentence containing “generic ISO” and “install-keepnode --ssh-key” so it reads naturally, and keep the surrounding references to keepNode.adminAccess.authorizedKeysFile and keepNode.adminAccess.lanBringup unchanged.nixos/installer.nix (1)
126-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify how the operator gets a key file onto the live installer environment.
The MOTD example (
--ssh-key ~/.ssh/id_ed25519.pub) implies the key file already exists on the booted installer environment's filesystem, but a freshly-booted live ISO won't have the operator's laptop home directory. Since the script also accepts the key as an inline string, the MOTD/docs should clarify the expected workflow (e.g. paste the key inline,scpit in, or copy via a second USB) to avoid confusing first-time users.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nixos/installer.nix` around lines 126 - 127, Clarify the installer MOTD/docs in installer.nix so it does not imply the live ISO already has the operator’s laptop home directory available. Update the install-keepnode guidance to reference the inline SSH key option and explicitly describe how to provide the key on the installer environment, such as pasting it directly, copying it over with scp, or transferring it via a second USB, so users understand the expected workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/deployment.md`:
- Around line 94-109: Fix the wording in the “Installer bring-up (generic ISO)”
section by removing the stray spaces before the commas and tightening the
sentence around install-keepnode; update the sentence containing “generic ISO”
and “install-keepnode --ssh-key” so it reads naturally, and keep the surrounding
references to keepNode.adminAccess.authorizedKeysFile and
keepNode.adminAccess.lanBringup unchanged.
In `@nixos/admin-access.nix`:
- Around line 37-76: Fix the typo in the admin-access option text by updating
the `authorizedKeysFile` description in `admin-access.nix`: remove the stray
space before the comma in the anti-lockout sentence and rephrase it cleanly (for
example with an em dash or period) so the `lib.mkOption` description reads
naturally in `nixos-option`/manual output. Keep the existing meaning intact and
only adjust the wording in the `authorizedKeysFile` description block.
In `@nixos/installer.nix`:
- Around line 126-127: Clarify the installer MOTD/docs in installer.nix so it
does not imply the live ISO already has the operator’s laptop home directory
available. Update the install-keepnode guidance to reference the inline SSH key
option and explicitly describe how to provide the key on the installer
environment, such as pasting it directly, copying it over with scp, or
transferring it via a second USB, so users understand the expected workflow.
In `@README.md`:
- Around line 70-77: Fix the stray space-before-comma in the README onboarding
sentence by updating the wording around the key-only SSH step in the README
section; adjust the phrase in the affected paragraph so it reads naturally (for
example by replacing the odd comma break with proper punctuation) while keeping
the rest of the SSH and Vaultwarden instructions unchanged.
- Around line 65-68: The README’s install example implies the SSH key file path
already exists in the live installer session, which can confuse users. Update
the documentation around the install-keepnode example to explain how the key is
provided to the booted session, referencing the install-keepnode usage and
--ssh-key flag, and note acceptable ways to supply it such as pasting the public
key inline or copying it in beforehand via scp or a second USB.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02c33a98-5fb3-4827-afd8-5fad29605017
📒 Files selected for processing (7)
.github/workflows/ci.ymlREADME.mddocs/deployment.mdflake.nixnixos/admin-access.nixnixos/installer.nixtests/adminaccess-bringup.nix
1 participant
Final onboarding increment (bead keep-node-4r4): the generic installer ISO now ships a HARDENED, key-enrolled image instead of the insecure debug profile, closing keep-node-6et. A fresh box is reachable with no known password.
Key constraint that shaped this:
install-keepnodeinstalls a PRE-BUILT closure (nixos-install --system, offline, no eval), so the operator's key cannot be baked into the Nix config at install time. It must be provisioned into a runtime file that a generic closure reads.nixos/admin-access.nix: two options extendkeepNode.adminAccess:authorizedKeysFile, a mutable runtime authorized_keys path sshd also honours (added toauthorizedKeysFiles), for a key provisioned AFTER install. The anti-lockout assertion accepts it as a key source.lanBringup, also expose the key-only SSH on the LAN (dropping the@10.44/16source restriction), because a fresh node has no mesh to be reached over yet. Still key-only, no password. Off by default, so a declarative deploy stays mesh-only.flake.nix: akeepnodeBringupSystemclosure (hardened appliance +adminAccesswithlanBringup+ the runtime key file, NO debug-access); the installer embeds THAT instead ofkeepnodeDebugSystem.nixos/installer.nix:install-keepnode --ssh-key <pubkey|file>(required) enrolls the operator key into/etc/keepnode/admin_authorized_keyson the target; refuses without a key (no unreachable box). Messaging updated (no moreroot/keepnode).tests/adminaccess-bringup.nix: a node withlanBringup+ a key in the runtime file (no inline keys) , the operator keys in over the LAN with passwordless sudo, root is refused, password auth is off. Proves the exact mechanism the installer uses.mesh-admin-sshregression confirms the defaults keep the mesh-only path intact.--ssh-key, key-only SSH, Vaultwarden via SSH tunnel);docs/deployment.mdgains the generic-ISO bring-up path. The legacykeepnode-debugprofile remains an explicit opt-in for keyless evaluation, never the installed default.Validated: adminaccess-bringup + mesh-admin-ssh green, installer closure evaluates, install-keepnode syntax-checked, mdBook builds. A full ISO build is running as final confirmation.
Summary by CodeRabbit