Skip to content

fix(deploy,promote): first-use trust origin pin, plugin fragment prune, Windows key paths, pin alias isolation (D-1216, D-0419, D-0582, D-0696) - #1018

Merged
acamarata merged 11 commits into
mainfrom
dw10-cli-7
Oct 29, 2026
Merged

acamarata merged 11 commits into
mainfrom
dw10-cli-7

Conversation

@acamarata

Copy link
Copy Markdown
Collaborator

Fixes four deploy and release-trust debt rows. Nothing was deployed, tagged or published.

D-1216: trust origin pin (decision: trust on first use)

Decision: trust on first use, not "require the pin when require_verified_release is true". Requiring the pin would refuse every prod promote until the operator hand-writes a file. The reviewers' concern was operator-owned authorisation, and a pin written to the operator home after the first verified release gives that without breaking existing flows. An attacker who can only edit the project .git/config loses the ability to redirect later verifications.

  • verify.Input.PinOnFirstUse is opt-in, and only deploy promote --release sets it. After a release fully verifies, PinOriginOnFirstUse writes the effective origin URL to ~/.nself/ci/trust/origin.url (mode 600, temp file then hard link, so it never replaces an existing file) and prints the pinned URL.
  • A URL carrying a token or password (https userinfo, or user:pw@ in any form) is not written, and the warning does not echo it. A write failure only warns, because the release already verified.
  • Side effect, documented in the wiki: with the pin present and the ci plugin installed, a non-prod target with no committed inventory now needs --release. A prod target already needed it by default.

D-0419: plugin fragment prune

PrunePrevious now also removes .nself/plugins/<slug>/<file> entries that the previous deploy shipped, that the local record and the host shipped list both name, and that the new set drops. It does not require the generated marker, because a plugin fragment is a copy of the operator's file. It goes through the same guardedScript symlink and realpath guard. Anything else still needs the marker. The first v1.5 deploy on a v1.4 host still prunes nothing, because there is no record to trust, and the wiki says so.

D-0582: Windows deploy paths

Most of this row was already fixed on main: the quoted GlobalKnownHostsFile token (D-0462, #807) handles a home with spaces.

  • Reused an earlier unmerged branch, fix/D-0582-windows-deploy-paths, which had no PR: drive-letter and backslash key paths with ToSlash for -i, cygpath -u local operands for rsync (a missing converter is an error), and the wiki host-key table row (compose-only non-prod is accept-new, P7-DEPL-29).
  • New: a key path with whitespace is a coded E484 error that names the home directory when the blank comes from it. deploy.CheckKeyPath is now the single check used by deploy_remote.go and deploy_apply_v15.go.
  • Keys with spaces are still refused, because rsync re-splits -e on blanks. The error says how to fix it.

D-0696: pin alias collision

  • Reused the earlier unmerged branch fix/D-0696-deploy-pin-alias: the alias is now nself+<host>+<port>. + is allowed by ValidateAlias and forbidden by checkName, so it cannot equal a hostname. The old nself-<host>-<port> is read only, when no pin exists under the new alias, and the pin file is never rewritten.
  • New: internal/admin/connect.go and internal/access/transport_ssh.go computed the old alias on their own. Without a change, a pin enrolled under the new alias would not be found by them. Both now call deploy.PinAlias, which has the same legacy fallback.
  • Residual: a legacy pin written for a real host literally named nself-x-22 is still honoured for x:22 until the operator re-enrols. That is the compat lookup the row asked for.

Two commits (3b7add4d, 54f6f7c6) were cherry-picked from the earlier branches and keep their original author.

Checks

  • go build ./... and go vet on the touched packages: clean (golangci-lint is not installed here).
  • go test -count=1 ./internal/deploy/... ./internal/access/... ./internal/admin/... ./internal/controlplane/... ./internal/errs/... ./cmd/commands: all ok.
  • New tests: TestPrunePrevious*Plugin* (confirmed failing with the plugin case disabled), TestVerifyPinsOriginOnFirstUse and related, TestKeyPathWhitespaceIsCoded, TestPinAliasLegacyFallbackForOtherCallers.
  • I used a private GOCACHE, because the shared build cache on the worker was being cleared mid-run.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Rebased onto origin/main and resolved the merge conflict in internal/admin/connect.go. Kept both sides' intent: HEAD's operatorAlias resolution via remote.ResolveSSHHost and the incoming deploy.PinAlias (with legacy v1.4.13 lookup) for the pin alias. Added missing net import. Build passes; tests requiring Docker were skipped per instructions. Force-pushed to dw10-cli-7.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed the port-22 pin alias regression in internal/admin/connect.go:171 by restoring the Port != 0 && Port != 22 rule while keeping IP-literal handling. The pin-default/{verify,ensure,tunnel} tests now pass. An IP-literal-on-port-22 test was not added because the existing test harness hardcodes host.test as the SSH target and cannot exercise a true IP literal without refactoring; the code path is covered by the restored condition matching deployKeyAlias. Commit 69555db.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed: adminHostKeyOptions now emits HostKeyAlias for IP literals at port 0/22 when the key is pin-only, so strict checking uses the enrolled nself++22 alias instead of the bare IP. Added TestAdminSSHV15IPPort22PinOnly (fails without the fix) and optional TestAdminSSHV15Non22PinAliasFallback covering new/legacy alias lookup at non-22 ports.

@acamarata

Copy link
Copy Markdown
Collaborator Author

Fixed all three repoqa ratchet failures:

  • TestFileSizeBudgetNotExceeded: split internal/deploy/remote_sync.go by extracting prune logic into remote_prune.go (now 205 + 133 lines, both under 300)
  • TestLayering: extracted SSH host-key pin alias resolution into new L1 package internal/hostpin; updated internal/access and internal/admin to import it instead of internal/deploy; registered edges in layers.yaml and acceptedBasis
  • TestPackageSizeBudget: moved deploy state persistence into new L1 package internal/deploystate, bringing internal/deploy below its 6667-line ceiling

Rebased onto origin/main (fixes TestDeterministicAndCheck catalog baseline). All three ratchet tests pass locally.

acamarata and others added 11 commits October 29, 2026 11:57
…age budget

- Split internal/deploy/remote_sync.go by extracting prune logic into remote_prune.go (file size cap)
- Extract SSH host-key pin alias resolution into new L1 package internal/hostpin to break L1->L2 layering violations from internal/access and internal/admin
- Move deploy state persistence into new L1 package internal/deploystate to shrink internal/deploy below its 6667-line ceiling
- Register access->hostpin and admin->hostpin edges in layers.yaml and acceptedBasis
@acamarata

Copy link
Copy Markdown
Collaborator Author

Review fixes pushed (f536ca2), after rebasing onto origin/main (force-with-lease).

  1. gofmt: connect.go, remote_exec.go, ssh_hostkeys.go formatted; TestGofmtClean passes.
  2. internal/deploy size: moved pathmap.go (188 lines) to internal/deploy/pathmap, no code added. Package is back under its 6667 ceiling; TestPackageSizeBudget passes (was 6795 after rebase).
  3. Deleted internal/deploystate/state.go; bluegreen.go and canary.go restored to main (they use deploy.StrategyState/SaveDeployState again).
  4. No new accepted edges: hostpin is now classified L0 in layers.yaml. To make that legal it no longer imports the sdk; it takes (host, port) plus an injected fingerprint parser (remote.Fingerprint) from callers. The access->hostpin and admin->hostpin entries are removed from layers.yaml and layering_basis_test.go. The hq overview 4.2 rules table is not touched here and may need an L0 hostpin row.
    Tests: repoqa, hostpin, admin, access, deploy, pathmap, promote, cmd/commands pass locally.

@acamarata
acamarata merged commit bec1ab0 into main Oct 29, 2026
32 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant