fix(postmerge-5210): 5 Copilot findings — prompt copy + comment accuracy + path + banner-password truth + sk-* FIDO key support#5214
Merged
AceHack merged 1 commit intoMay 26, 2026
Conversation
…nt accuracy + path correction + banner-password truth + sk-* FIDO key support P0 + P1 + P1 + P1 + P2 findings on PR #5210 (iter-5.4.0 just merged at e30b567). All 5 substantive; addressed: (1) P0 — prompt copy said "Press Enter to skip" but default is Y so Enter actually triggers gh-auth-login. Updated copy to "Default is YES (recommended); press Enter to proceed OR type 'n' to skip" matching the actual [Y/n] default-Y behavior. (2) P1 (comment accuracy) — block-comment said "NOT skippable if iter- 4.2 injection also failed" but impl always allows skip. Rewrote comment to reflect reality ("skippable warning-only when iter-4.2 also failed"). (3) P2 (path correction) — warning message referenced "usb-nixos-installer/nixos/installer/configuration.nix" missing the "full-ai-cluster/" prefix. Fixed to the in-repo-correct path so post-mortem debugging of missing-gh-binary stays clear. (4) P1 (banner-password truth) — install-complete banner always printed "password: zeta-change-me" but iter-5.3 (landed earlier today via #5118) lets operator set a custom password. Conditional on /mnt/etc/zeta/initial-hashedpassword existence: print "(the value you set during iter-5.3 prompt; iter-4.x default is NOT in effect)" OR fall back to documented iter-4.x default with rotation hint. (5) P1 (sk-* FIDO key support) — operator-authorized-keys.nix isKeyLine filter only accepted ssh- + ecdsa- prefixes, silently dropping sk-ssh-ed25519 / sk-ecdsa-sha2-* (FIDO/U2F security-key- backed pubkeys per RFC8709) that GitHub stores when operators register security keys. Existing operator-ssh-keys.nix explicitly documents sk-* support; this module now matches that parity. Refactored to validPrefixes list + hasValidPrefix helper for readability + future-extensibility. Substrate-inventory pass per `.claude/rules/verify-existing-substrate- before-authoring.md`: existing operator-ssh-keys.nix substrate has sk-* documented — verified before authoring the parity fix per the discipline rule landed today. Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
AceHack
deleted the
otto-cli/postmerge-5210-fixfwd-5-copilot-findings-2026-05-26
branch
There was a problem hiding this comment.
Pull request overview
This PR addresses the post-merge findings from #5210 by aligning installer UX copy with actual default behavior, fixing a referenced path in warnings, making the install-complete credential banner reflect the iter-5.3 password outcome, and ensuring operator key ingestion supports sk-* (FIDO/U2F) SSH key types.
Changes:
- Update
zeta-install.shprompts/comments to match[Y/n]default-YES behavior and correct the referenced configuration path. - Make the install-complete banner conditional on whether an operator-provided initial password hash was written.
- Expand key-prefix filtering in the operator key ingestion module to include
sk-ssh-andsk-ecdsa-.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| full-ai-cluster/usb-nixos-installer/zeta-install.sh | Fixes gh-auth prompt/copy behavior and updates install banner to reflect actual password injection state. |
| full-ai-cluster/nixos/modules/operator-authorized-keys.nix | Extends supported SSH key prefixes to include sk-* security-key-backed keys. |
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.
Summary
Addresses all 5 Copilot post-merge findings on #5210 (iter-5.4.0 just landed at e30b567). All 5 substantive; all real:
[Y/n]default-Y behaviorusb-nixos-installer/...missingfull-ai-cluster/prefixpassword: zeta-change-mebut iter-5.3 lets operator customizeisKeyLineonly accepts ssh- + ecdsa-, silently drops sk-* FIDO/U2F keys GitHub storessk-ssh-+sk-ecdsa-per existing operator-ssh-keys.nix paritySubstrate-honest note
Findings #5 (sk-* FIDO) is operationally load-bearing — operators with security-key-only GitHub setups would have had ALL their keys silently dropped. The existing operator-ssh-keys.nix module explicitly documents sk-* support; verifying that BEFORE authoring this fix (per the verify-existing-substrate-before-authoring rule landed today via #5131) caught the parity gap.
Auto-merge armed; threads resolved on #5210.
🤖 Generated with Claude Code