diff --git a/.github/wiki/cmd-deploy.md b/.github/wiki/cmd-deploy.md index 047a83095..45a55de0d 100644 --- a/.github/wiki/cmd-deploy.md +++ b/.github/wiki/cmd-deploy.md @@ -214,9 +214,13 @@ the working-tree inventory and the inventory committed on the default branch of committed copy cannot be read and the ci plugin is installed, the promotion refuses with `E499`. Git runs without `GIT_DIR`/`GIT_CONFIG_*` and replacement objects; anyone who can edit the project's `.git/config` can already run code as -the operator, so by default the project's own `origin` is trusted. To pin it, -put the expected URL in `~/.nself/ci/trust/origin.url` (first non-comment line): -a different effective origin (local config or `insteadOf`) then refuses with +the operator, so until the origin is pinned the project's own `origin` is trusted. +The first release that verifies pins it (trust on first use): `promote` writes the +effective origin URL to `~/.nself/ci/trust/origin.url` (mode 600, never replacing an +existing file) and prints the pinned URL. A URL that carries a token or password is +not written; a warning says so. To pin another URL, or before the first release, put +it in that file yourself (first non-comment line); delete the file to unpin. Once +pinned, a different effective origin (local config or `insteadOf`) then refuses with `E493` before any trust file is read, and so does a project whose `origin` was removed. A project with no `origin`, or with no committed `.nself/control-plane.yaml`, has no committed policy to consult and promotes @@ -443,9 +447,15 @@ SSH key defaults to `~/.ssh/id_ed25519`. Override with `NSELF_DEPLOY_SSH_KEY`. Agent forwarding is always disabled (`-o ForwardAgent=no`). ssh, scp and rsync get their options as separate arguments from one builder; no option, key path or remote path is ever parsed by a shell. -Before any process starts, the CLI refuses a key path outside `[a-zA-Z0-9/_.~-]` or starting with -`-`, a remote path with shell metacharacters, a leading `-` or a `..` segment, and any ssh option -holding whitespace or a quote (rsync re-splits its `-e` string on spaces). +Before a connection starts, the CLI refuses a key path outside `[a-zA-Z0-9/_.~-]` or starting with +`-`; absolute Windows drive paths such as `C:/Users/Jane/.ssh/id_ed25519` and backslash separators +are also accepted. Key paths must have no spaces. Remote paths cannot contain shell metacharacters, +a leading `-` or a `..` segment. SSH option tokens refuse whitespace and quotes except for the +canonical quoted pin-file option, which supports home directories containing spaces. + +On Windows, rsync requires the matching MSYS/Cygwin `cygpath` on `PATH`. The CLI uses `cygpath -u` +to map local sources and file lists to that runtime's mount paths, preserving directory trailing +slashes. A missing or failing converter stops the copy with an error. Native scp retains Windows paths. **Host keys.** Which policy a deploy uses depends on what it sends: @@ -453,7 +463,8 @@ holding whitespace or a quote (rsync re-splits its `-e` string on spaces). |---|---|---| | Secrets push (`.env.` copied with scp) | strict | strict | | Deploy that ships an env file (inventory deploy, `nself deploy ` push) | strict, every connection of the run | strict | -| Compose file only | `StrictHostKeyChecking=accept-new` | strict | +| Compose file only, non-production | `StrictHostKeyChecking=accept-new` | `accept-new` (P7-DEPL-29) | +| Compose file only, production through control-plane | `StrictHostKeyChecking=accept-new` | strict | | Read-only commands (probes outside a shipping run, `logs`, `health`) | `StrictHostKeyChecking=accept-new` | `accept-new` with nself's own key file (below) | Strict means the CLI scans the host's live key and continues only when that key is already trusted, @@ -496,9 +507,13 @@ Compare the fingerprint with the one your provider shows (console or `ssh-keygen `--trust-host-key`, or ssh to the host once and accept the key. Before P7-DEPL-28 and P7-DEPL-29 the v1.4 secrets push and env-shipping deploys accepted any key on first contact (`accept-new`). -IP-address hosts are pinned under the alias `nself--` (for example -`nself-203.0.113.10-22`) and ssh gets a matching `HostKeyAlias`; hosts on a non-default port use -`nself--`. A pin file holding a line the CLI did not write (a wildcard, +IP-address hosts are pinned under the alias `nself++` (for example +`nself+203.0.113.10+22`) and ssh gets a matching `HostKeyAlias`; hosts with an explicit port use +`nself++`, preserving IPv6 colons. These aliases cannot equal a real hostname. +Existing `nself--` pins remain readable when no new alias pin exists; +lookup never rewrites them. Re-enroll verified keys with `--trust-host-key` to use +the new namespace. Ordinary `nself-*` hostnames remain valid. +A pin file holding a line the CLI did not write (a wildcard, `@cert-authority` or hashed entry) or with group- or world-writable permissions is refused with `E487`; fix or remove that line and pin again. @@ -674,8 +689,12 @@ file. v1.4 mode keeps the push described above, byte for byte. One flow runs per `--env-file` list, and the sync only creates it (empty, mode 600) when the host has none. The host keeps `.nself/deploy-shipped.txt`, the list of what nSelf shipped. Before the sync the host removes only files that list names, the new set drops, and that carry the `generated by nself` - marker. A marker-carrying conf nSelf never shipped here (another project's), an unmarked file or - anything under `conf.d*/` stays. The generated `nginx/conf.d/.conf` files (today + marker. A plugin fragment under `.nself/plugins//` is a copy of your own file and has no + marker: one that the previous deploy shipped (your local record and the host list both name it) + and the new set drops is removed without the marker, behind the same symlink and realpath guard. + A marker-carrying conf nSelf never shipped here (another project's), an unmarked file outside + `.nself/plugins//` or anything under `conf.d*/` stays. The first v1.5 deploy to a v1.4 host + has no shipped list and no record yet, so it prunes nothing. The generated `nginx/conf.d/.conf` files (today `default.conf`) ship, and only that one directory: never `conf.d-/`, never a nested path, and a conf.d file is never pruned. The host decides, from the file alone: a file at that path whose first 5 lines have none starting exactly with `# Generated by nself build` (case-sensitive; a diff --git a/cmd/commands/deploy_apply_v15.go b/cmd/commands/deploy_apply_v15.go index 6f9e8c7d7..8b2edb7ec 100644 --- a/cmd/commands/deploy_apply_v15.go +++ b/cmd/commands/deploy_apply_v15.go @@ -244,8 +244,8 @@ func remoteDeployV15(ctx context.Context, workdir, host, cascade string, jsonOut ctx, stop := deploy.WithInterrupt(ctx) defer stop() key := sshKeyPath() - if key != "" && !sshKeyRe.MatchString(key) { - return fmt.Errorf("NSELF_DEPLOY_SSH_KEY contains unsafe characters (got %q): only [a-zA-Z0-9/_.~-] allowed", key) + if err := deploy.CheckKeyPath(key); err != nil { + return err } spec, err := remote.ParseHostSpec(host) if err != nil { diff --git a/cmd/commands/deploy_remote.go b/cmd/commands/deploy_remote.go index 49bd876e9..92adfd903 100644 --- a/cmd/commands/deploy_remote.go +++ b/cmd/commands/deploy_remote.go @@ -82,8 +82,8 @@ func remoteDeployPush(ctx context.Context, workdir, host, target string, jsonOut // sshKey goes into the rsync ssh command string, which rsync passes // through a shell. Reject any key path containing shell metacharacters to // prevent command injection via NSELF_DEPLOY_SSH_KEY. - if sshKey != "" && !sshKeyRe.MatchString(sshKey) { - return fmt.Errorf("NSELF_DEPLOY_SSH_KEY contains unsafe characters (got %q): only [a-zA-Z0-9/_.~-] allowed", sshKey) + if err := deploy.CheckKeyPath(sshKey); err != nil { + return err } spec, err := remote.ParseHostSpec(host) diff --git a/cmd/commands/deploy_scratch.go b/cmd/commands/deploy_scratch.go index 43fd71617..73957e869 100644 --- a/cmd/commands/deploy_scratch.go +++ b/cmd/commands/deploy_scratch.go @@ -22,6 +22,7 @@ package commands import ( "context" "fmt" + "github.com/nself-org/cli/internal/deploy/pathmap" "io" "os" "path/filepath" @@ -166,7 +167,7 @@ func stageScratch(p v15Params, scratch string) (deploy.ArtifactSet, error) { return deploy.ArtifactSet{}, err } in := deploy.StageInput{ - Map: deploy.PathMap{ScratchRoot: scratch, OperatorRoot: p.workdir, PluginDir: build.DefaultPluginDir(), Home: deploy.DefaultHome(), RemotePath: p.srv.RemotePath}, + Map: pathmap.PathMap{ScratchRoot: scratch, OperatorRoot: p.workdir, PluginDir: build.DefaultPluginDir(), Home: pathmap.DefaultHome(), RemotePath: p.srv.RemotePath}, StageDir: stage, Compose: files, EnvFiles: envs, EnvSnapshot: envSnap, EnvName: cascade, Nginx: policy, } if frontedBy != "" { diff --git a/cmd/commands/deploy_scratch_test.go b/cmd/commands/deploy_scratch_test.go index e8c7323d8..e6e4d6d5f 100644 --- a/cmd/commands/deploy_scratch_test.go +++ b/cmd/commands/deploy_scratch_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "github.com/nself-org/cli/internal/deploy/pathmap" "io/fs" "os" "path/filepath" @@ -155,7 +156,7 @@ func TestDeployScratchUntouched(t *testing.T) { t.Fatal(err) } set, err := deploy.Stage(deploy.StageInput{ - Map: deploy.PathMap{ScratchRoot: scratch, OperatorRoot: proj, PluginDir: filepath.Join(t.TempDir(), "plugins"), Home: filepath.Join(t.TempDir(), "home"), RemotePath: "/opt/nself"}, + Map: pathmap.PathMap{ScratchRoot: scratch, OperatorRoot: proj, PluginDir: filepath.Join(t.TempDir(), "plugins"), Home: filepath.Join(t.TempDir(), "home"), RemotePath: "/opt/nself"}, StageDir: stage, Compose: []string{filepath.Join(scratch, "docker-compose.yml")}, EnvFiles: []string{filepath.Join(scratch, ".env")}, EnvName: "prod", Nginx: policy, }) if err != nil { diff --git a/cmd/commands/release_manifest_flag.go b/cmd/commands/release_manifest_flag.go index 446fc3cc7..62101f3fa 100644 --- a/cmd/commands/release_manifest_flag.go +++ b/cmd/commands/release_manifest_flag.go @@ -127,6 +127,9 @@ func wirePromoteRelease(cmd *cobra.Command, workdir string, eng *cpromote.Engine Git: verify.GitCLI{Root: workdir}, Evidence: verify.ExecEvidence(binary, resolvePluginDir(), workdir), Warn: func(msg string) { _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "warning:", msg) }, + // D-1216: trust on first use for the trust origin. + PinOnFirstUse: true, + Notice: func(msg string) { _, _ = fmt.Fprintln(cmd.ErrOrStderr(), msg) }, } eng.Verify = func(ctx context.Context, s *cpromote.State) error { m, err := verify.Verify(ctx, in) diff --git a/internal/access/transport_ssh.go b/internal/access/transport_ssh.go index ef4f76c96..7df7f82a0 100644 --- a/internal/access/transport_ssh.go +++ b/internal/access/transport_ssh.go @@ -17,12 +17,12 @@ import ( "context" "fmt" "github.com/nself-org/cli/internal/compat" + "github.com/nself-org/cli/internal/hostpin" "github.com/nself-org/cli/sdk/go/v2/remote" "os" "os/exec" "path" "path/filepath" - "strconv" "strings" "time" ) @@ -92,7 +92,7 @@ func (t *SSHTransport) sshArgsContext(ctx context.Context) ([]string, error) { } args = append(args, "-o", "StrictHostKeyChecking=yes", "-o", option) if spec.Port != 0 { - alias := "nself-" + strings.ReplaceAll(spec.Host, ":", "-") + "-" + strconv.Itoa(spec.Port) + alias := hostpin.PinAlias(ctx, spec.Host, spec.Port, remote.Fingerprint) args = append(args, "-o", "HostKeyAlias="+alias) } } else { diff --git a/internal/access/transport_ssh_test.go b/internal/access/transport_ssh_test.go index e9836bbcb..f8eef38f4 100644 --- a/internal/access/transport_ssh_test.go +++ b/internal/access/transport_ssh_test.go @@ -43,7 +43,7 @@ func TestSSHTransportHostSpecPolicy(t *testing.T) { name, v15, host, want, reject string }{ {"legacy", "0", "u@example.test:2222", "StrictHostKeyChecking=accept-new", "HostKeyAlias="}, - {"strict-port", "1", "u@example.test:2222", "HostKeyAlias=nself-example.test-2222", "StrictHostKeyChecking=accept-new"}, + {"strict-port", "1", "u@example.test:2222", "HostKeyAlias=nself+example.test+2222", "StrictHostKeyChecking=accept-new"}, {"strict-default", "1", "u@example.test", "StrictHostKeyChecking=yes", "HostKeyAlias="}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/admin/connect.go b/internal/admin/connect.go index 40fa6f0c6..8f57e8414 100644 --- a/internal/admin/connect.go +++ b/internal/admin/connect.go @@ -7,6 +7,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "net" "net/http" "os" "os/exec" @@ -18,6 +19,7 @@ import ( "github.com/nself-org/cli/internal/compat" "github.com/nself-org/cli/internal/errs" + "github.com/nself-org/cli/internal/hostpin" "github.com/nself-org/cli/sdk/go/v2/remote" ) @@ -166,8 +168,8 @@ func adminHostKeyOptions(ctx context.Context, spec remote.HostSpec) ([]string, e operatorAlias = resolved.HostKeyAlias } pinAlias := spec.Host - if spec.Port != 0 && spec.Port != 22 { - pinAlias = "nself-" + strings.ReplaceAll(spec.Host, ":", "-") + "-" + strconv.Itoa(spec.Port) + if (spec.Port != 0 && spec.Port != 22) || net.ParseIP(spec.Host) != nil { + pinAlias = hostpin.PinAlias(ctx, spec.Host, spec.Port, remote.Fingerprint) } operator := adminTrustedFingerprints(ctx, operatorAlias, filepath.Join(home, ".ssh", "known_hosts")) pinned := adminTrustedFingerprints(ctx, pinAlias, pinFile) @@ -179,7 +181,7 @@ func adminHostKeyOptions(ctx context.Context, spec remote.HostSpec) ([]string, e "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", pinOption, } - if spec.Port != 0 && spec.Port != 22 && !operator[key.Fingerprint] { + if !operator[key.Fingerprint] && (spec.Port != 0 && spec.Port != 22 || net.ParseIP(spec.Host) != nil) { opts = append(opts, "-o", "HostKeyAlias="+pinAlias) } return opts, nil diff --git a/internal/admin/connect_pin_test.go b/internal/admin/connect_pin_test.go index fb47b49e8..bd5013acc 100644 --- a/internal/admin/connect_pin_test.go +++ b/internal/admin/connect_pin_test.go @@ -148,3 +148,59 @@ func TestAdminPinFileOption(t *testing.T) { }) } } + +// TestAdminSSHV15IPPort22PinOnly verifies that an IP literal on port 22 with a +// pin enrolled under the nself--22 alias emits HostKeyAlias so ssh checks +// the pin alias rather than the bare IP. Without HostKeyAlias, strict checking +// would fail because the pin file has no entry for the plain IP. +func TestAdminSSHV15IPPort22PinOnly(t *testing.T) { + compattest.Set(t, true) + e := newArgvEnv(t, "", &hostKey{alias: "nself-203.0.113.7-22", pin: true}) + if err := callEntry("verify", "u", "203.0.113.7", 22); err != nil { + t.Fatal(err) + } + got := e.argv(t) + found := false + for i, w := range got { + if w == "HostKeyAlias=nself-203.0.113.7-22" { + found = true + if i == 0 || got[i-1] != "-o" { + t.Fatalf("HostKeyAlias not preceded by -o: %q", got) + } + break + } + } + if !found { + t.Fatalf("missing HostKeyAlias for IP port 22 pin-only: %q", got) + } +} + +// TestAdminSSHV15Non22PinAliasFallback verifies that for a non-22 port, the +// admin connect path finds a pin under either the new alias or the legacy +// v1.4.13 alias. This covers the deploy.PinAlias fallback at the admin layer. +func TestAdminSSHV15Non22PinAliasFallback(t *testing.T) { + cases := []struct{ name, alias string }{ + {"new-alias", "nself+host.test+2222"}, + {"legacy-alias", "nself-host.test-2222"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + compattest.Set(t, true) + e := newArgvEnv(t, "", &hostKey{alias: tc.alias, pin: true}) + if err := callEntry("verify", "u", "host.test", 2222); err != nil { + t.Fatal(err) + } + got := e.argv(t) + found := false + for _, w := range got { + if strings.HasPrefix(w, "HostKeyAlias=") { + found = true + break + } + } + if !found { + t.Fatalf("missing HostKeyAlias for %s: %q", tc.name, got) + } + }) + } +} diff --git a/internal/controlplane/promote/verify/trust_pin.go b/internal/controlplane/promote/verify/trust_pin.go new file mode 100644 index 000000000..e24a0cb65 --- /dev/null +++ b/internal/controlplane/promote/verify/trust_pin.go @@ -0,0 +1,91 @@ +package verify + +// Purpose: trust on first use for the trust origin (D-1216). After the first +// release that verifies, the project's effective origin URL is written +// to ~/.nself/ci/trust/origin.url, so every later verification, and the +// committed-inventory lookup, holds to that origin. +// Inputs: the Git view, the operator home. +// Outputs: the pinned URL, or why nothing was pinned. +// Constraints: never replaces an existing pin file (hard link from a temp file, +// which fails when the target exists). A URL that carries a +// password or token is not written: the file is plain text and the +// URL is printed. Only the operator home is written. + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" +) + +// PinOriginOnFirstUse pins the project's origin when the operator has no pin. +// pinned is false, with a nil error, when there is nothing to do (a pin exists, +// or the project has no origin). A non-nil error means the origin was left +// unpinned and says why; it never means the release failed to verify. +func PinOriginOnFirstUse(ctx context.Context, g Git, home string) (url string, pinned bool, err error) { + if g == nil || !filepath.IsAbs(home) { + return "", false, nil + } + if _, has, err := loadOriginPin(home); has || err != nil { + return "", false, err + } + ok, err := g.HasOrigin(ctx) + if err != nil || !ok { + return "", false, err + } + got, err := g.OriginURL(ctx) + if err != nil { + return "", false, fmt.Errorf("cannot read the origin URL to pin it: %w", err) + } + got = strings.TrimSpace(got) + if !pinnableURL(got) { + return "", false, errors.New("the origin URL is empty or carries credentials, so it was not written to " + originPinName + "; put the URL without credentials there to pin it") + } + dir := filepath.Join(home, ".nself", "ci", "trust") + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", false, err + } + tmp, err := os.CreateTemp(dir, ".origin-*") + if err != nil { + return "", false, err + } + defer func() { _ = os.Remove(tmp.Name()) }() + _, werr := tmp.WriteString("# pinned by nself after the first verified release; edit or delete this file to change it\n" + got + "\n") + if cerr := tmp.Close(); werr == nil { + werr = cerr + } + if werr != nil { + return "", false, werr + } + if err := os.Link(tmp.Name(), filepath.Join(dir, originPinName)); err != nil { + if errors.Is(err, os.ErrExist) { + return "", false, nil // pinned by someone else meanwhile: theirs stands + } + return "", false, err + } + return got, true, nil +} + +// pinnableURL reports whether u is a single-line URL with no userinfo secret. +// scp-like and ssh:// forms may name a user (git@host) but never a password. +func pinnableURL(u string) bool { + if u == "" || strings.ContainsFunc(u, func(r rune) bool { return r < ' ' || r == 0x7f || r == ' ' }) { + return false + } + authority := u + if _, rest, ok := strings.Cut(u, "://"); ok { + scheme, _, _ := strings.Cut(u, "://") + authority, _, _ = strings.Cut(rest, "/") + if i := strings.LastIndex(authority, "@"); i >= 0 && scheme != "ssh" { + return false // https://token@host: the userinfo is a credential + } + } else { + authority, _, _ = strings.Cut(u, ":") + } + if i := strings.LastIndex(authority, "@"); i >= 0 && strings.Contains(authority[:i], ":") { + return false + } + return true +} diff --git a/internal/controlplane/promote/verify/trust_pin_test.go b/internal/controlplane/promote/verify/trust_pin_test.go new file mode 100644 index 000000000..e6a26c937 --- /dev/null +++ b/internal/controlplane/promote/verify/trust_pin_test.go @@ -0,0 +1,131 @@ +package verify + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" +) + +func pinFile(home string) string { + return filepath.Join(home, ".nself", "ci", "trust", originPinName) +} + +// D-1216: the first verified release pins the origin, and the pin then holds. +func TestVerifyPinsOriginOnFirstUse(t *testing.T) { + r := newRelease(t) + r.write(PayloadType, r.signer) + r.git.originURL = "git@github.com:o/r.git" + var notes, warns []string + in := r.input() + in.PinOnFirstUse, in.Notice, in.Warn = true, func(s string) { notes = append(notes, s) }, func(s string) { warns = append(warns, s) } + if _, err := Verify(context.Background(), in); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(pinFile(r.home)) + if err != nil || !strings.Contains(string(raw), "\ngit@github.com:o/r.git\n") { + t.Fatalf("pin file = %q, %v", raw, err) + } + if info, _ := os.Stat(pinFile(r.home)); info.Mode().Perm() != 0o600 { + t.Errorf("pin file mode %v, want 0600", info.Mode().Perm()) + } + if len(notes) != 1 || !strings.Contains(notes[0], "git@github.com:o/r.git") || len(warns) != 0 { + t.Errorf("notes %q warns %q", notes, warns) + } + if left, _ := filepath.Glob(filepath.Join(r.home, ".nself", "ci", "trust", ".origin-*")); len(left) != 0 { + t.Errorf("temp files left: %v", left) + } + // The pin now holds: a redirected origin refuses before trust is read. + r.git.originURL = "https://evil.example/o/r.git" + r.git.pinned = 0 + if _, err := Verify(context.Background(), in); codeOf(err) != "E493" || r.git.pinned != 0 { + t.Fatalf("redirected origin after the first-use pin: %v (trust reads %d)", err, r.git.pinned) + } + // A second verified release with the pinned origin keeps the file as it is. + r.git.originURL = "git@github.com:o/r.git" + if _, err := Verify(context.Background(), in); err != nil { + t.Fatal(err) + } + if again, _ := os.ReadFile(pinFile(r.home)); string(again) != string(raw) { + t.Errorf("pin file rewritten: %q", again) + } +} + +func TestVerifyFirstUsePinIsOptIn(t *testing.T) { + r := newRelease(t) + r.write(PayloadType, r.signer) + r.git.originURL = "git@github.com:o/r.git" + if _, err := Verify(context.Background(), r.input()); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(pinFile(r.home)); !os.IsNotExist(err) { + t.Fatalf("a library caller wrote the pin: %v", err) + } +} + +func TestVerifyFailureDoesNotPin(t *testing.T) { + r := newRelease(t) + r.exit = 1 // evidence plugin refuses + r.write(PayloadType, r.signer) + r.git.originURL = "git@github.com:o/r.git" + in := r.input() + in.PinOnFirstUse = true + if _, err := Verify(context.Background(), in); err == nil { + t.Fatal("want a refusal") + } + if _, err := os.Stat(pinFile(r.home)); !os.IsNotExist(err) { + t.Fatalf("pinned an origin on a failed verification: %v", err) + } +} + +func TestPinOriginOnFirstUseCases(t *testing.T) { + cases := []struct { + name, url string + noOrigin bool + existing string + wantPin bool + wantErr bool + }{ + {name: "https", url: "https://github.com/o/r.git", wantPin: true}, + {name: "ssh url", url: "ssh://git@host/o/r.git", wantPin: true}, + {name: "no origin", noOrigin: true}, + {name: "existing pin untouched", url: "https://github.com/o/r.git", existing: "https://other.example/x.git\n"}, + {name: "token in https userinfo", url: "https://TOKEN@github.com/o/r.git", wantErr: true}, + {name: "password in userinfo", url: "https://u:p@github.com/o/r.git", wantErr: true}, + {name: "password in ssh userinfo", url: "ssh://u:p@host/r.git", wantErr: true}, + {name: "empty url", url: "", wantErr: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + home := t.TempDir() + if tc.existing != "" { + if err := os.MkdirAll(filepath.Dir(pinFile(home)), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(pinFile(home), []byte(tc.existing), 0o600); err != nil { + t.Fatal(err) + } + } + url, pinned, err := PinOriginOnFirstUse(context.Background(), &fakeGit{noOrigin: tc.noOrigin, originURL: tc.url}, home) + if pinned != tc.wantPin || (err != nil) != tc.wantErr || (pinned && url != tc.url) { + t.Fatalf("url %q pinned %v err %v", url, pinned, err) + } + raw, rerr := os.ReadFile(pinFile(home)) + switch { + case tc.existing != "": + if string(raw) != tc.existing { + t.Errorf("existing pin replaced: %q", raw) + } + case !tc.wantPin && rerr == nil: + t.Errorf("pin file written: %q", raw) + } + if err != nil && strings.Contains(err.Error(), "TOKEN") { + t.Errorf("credential in the error: %v", err) + } + }) + } + if _, pinned, err := PinOriginOnFirstUse(context.Background(), &fakeGit{originURL: "https://github.com/o/r.git"}, "relative"); pinned || err != nil { + t.Errorf("a relative home must be a no-op: %v %v", pinned, err) + } +} diff --git a/internal/controlplane/promote/verify/verify.go b/internal/controlplane/promote/verify/verify.go index 465c763ac..99631e9f8 100644 --- a/internal/controlplane/promote/verify/verify.go +++ b/internal/controlplane/promote/verify/verify.go @@ -36,6 +36,11 @@ type Input struct { Git Git Evidence EvidenceRunner Warn func(string) + // PinOnFirstUse writes the project's origin to ~/.nself/ci/trust/origin.url + // after a release verifies and no pin exists (D-1216); Notice receives the + // one-line report. Off by default so a library caller never writes Home. + PinOnFirstUse bool + Notice func(string) } // Verify checks the release directory in the contract order. @@ -86,7 +91,25 @@ func Verify(ctx context.Context, in Input) (*Manifest, error) { if err := os.WriteFile(staged, doc, 0o600); err != nil { return nil, errs.Wrap("E497", "cannot stage the evidence for verification", err) } - return m, checkEvidence(ctx, in.Evidence, staged, m.Revision) + if err := checkEvidence(ctx, in.Evidence, staged, m.Revision); err != nil { + return m, err + } + if in.PinOnFirstUse { + pinFirstUse(ctx, in) + } + return m, nil +} + +// pinFirstUse reports the outcome of PinOriginOnFirstUse. A failure only warns: +// the release already verified. +func pinFirstUse(ctx context.Context, in Input) { + url, pinned, err := PinOriginOnFirstUse(ctx, in.Git, in.Home) + switch { + case err != nil && in.Warn != nil: + in.Warn("trust origin not pinned: " + err.Error()) + case pinned && in.Notice != nil: + in.Notice("pinned the trust origin " + url + " in ~/.nself/ci/trust/" + originPinName + "; a different origin now refuses with E493 (delete the file to unpin)") + } } func verifyEnvelope(env signing.Envelope, t Trust) ([]byte, []string, error) { diff --git a/internal/controlplane/sim/remote_apply_integration_test.go b/internal/controlplane/sim/remote_apply_integration_test.go index b0ab95dd1..3b0b28dbf 100644 --- a/internal/controlplane/sim/remote_apply_integration_test.go +++ b/internal/controlplane/sim/remote_apply_integration_test.go @@ -15,6 +15,7 @@ import ( "encoding/hex" "errors" "fmt" + "github.com/nself-org/cli/internal/deploy/pathmap" "os" "os/exec" "path/filepath" @@ -180,7 +181,7 @@ func stageSet(t *testing.T, rp, version string) (deploy.ArtifactSet, []string) { w(filepath.Join(scratch, "hasura", "metadata", "version.yaml"), "version: 3\n") w(filepath.Join(scratch, ".env.prod"), "ENV=prod\n") set, err := deploy.Stage(deploy.StageInput{ - Map: deploy.PathMap{ScratchRoot: scratch, OperatorRoot: operator, PluginDir: plugins, Home: filepath.Join(base, "home"), RemotePath: rp}, + Map: pathmap.PathMap{ScratchRoot: scratch, OperatorRoot: operator, PluginDir: plugins, Home: filepath.Join(base, "home"), RemotePath: rp}, StageDir: filepath.Join(base, "stage"), Compose: []string{filepath.Join(scratch, "docker-compose.yml"), filepath.Join(plugins, "ai", "docker-compose.plugin.yml")}, EnvFiles: []string{filepath.Join(scratch, ".env"), filepath.Join(scratch, ".nself", "compose.env")}, EnvSnapshot: filepath.Join(scratch, ".env.prod"), EnvName: "prod", }) diff --git a/internal/deploy/artifacts.go b/internal/deploy/artifacts.go index b706259a3..f86c55d3f 100644 --- a/internal/deploy/artifacts.go +++ b/internal/deploy/artifacts.go @@ -19,6 +19,7 @@ package deploy import ( "fmt" + "github.com/nself-org/cli/internal/deploy/pathmap" "io/fs" "os" "path" @@ -54,7 +55,7 @@ type ArtifactSet struct { // StageInput is what Stage reads. type StageInput struct { - Map PathMap + Map pathmap.PathMap StageDir string // a fresh empty dir Compose []string // local compose files in order EnvFiles []string // local env files in order (the compose env manifest) @@ -164,7 +165,7 @@ func Stage(in StageInput) (ArtifactSet, error) { // copyTree copies srcDir (a missing dir is fine) to / and returns // the relative file paths. Directories named conf.d* are skipped whole. rewrite // maps local roots in the file text (nginx confs); otherwise bytes are kept. -func copyTree(srcDir, stage, rel string, m PathMap, rewrite bool) ([]string, error) { +func copyTree(srcDir, stage, rel string, m pathmap.PathMap, rewrite bool) ([]string, error) { var out []string err := filepath.WalkDir(srcDir, func(p string, d fs.DirEntry, err error) error { if err != nil { @@ -206,7 +207,7 @@ func copyTree(srcDir, stage, rel string, m PathMap, rewrite bool) ([]string, err } // checkNoLocalPaths fails when any staged file still names a local root. -func checkNoLocalPaths(m PathMap, stage string, rels []string) error { +func checkNoLocalPaths(m pathmap.PathMap, stage string, rels []string) error { for _, rel := range rels { data, err := os.ReadFile(filepath.Join(stage, filepath.FromSlash(rel))) if err != nil { diff --git a/internal/deploy/artifacts_front_test.go b/internal/deploy/artifacts_front_test.go index a80e65030..b10e5d320 100644 --- a/internal/deploy/artifacts_front_test.go +++ b/internal/deploy/artifacts_front_test.go @@ -2,6 +2,7 @@ package deploy import ( "errors" + "github.com/nself-org/cli/internal/deploy/pathmap" "os" "path" "path/filepath" @@ -28,7 +29,7 @@ func frontScratch(t *testing.T, remotePath string) (StageInput, string) { writeFile(t, filepath.Join(front, "nginx", "sites", "other-stack.conf"), "# not generated by this build\n", 0o644) writeFile(t, filepath.Join(front, "nginx", "sites", "note.txt"), buildMarker, 0o644) writeFile(t, filepath.Join(front, "ssl", "certificates", "x", "fullchain.pem"), "STUB", 0o644) - m := PathMap{ScratchRoot: scratch, OperatorRoot: operator, PluginDir: filepath.Join(base, "plugins"), Home: filepath.Join(base, "home"), RemotePath: remotePath} + m := pathmap.PathMap{ScratchRoot: scratch, OperatorRoot: operator, PluginDir: filepath.Join(base, "plugins"), Home: filepath.Join(base, "home"), RemotePath: remotePath} return StageInput{ Map: m, StageDir: filepath.Join(t.TempDir(), "stage"), Compose: []string{filepath.Join(scratch, "docker-compose.yml")}, EnvFiles: []string{filepath.Join(scratch, ".env")}, EnvName: "prod", Nginx: testPolicy(), @@ -94,7 +95,7 @@ func TestStageFrontLayoutMismatchRefused(t *testing.T) { } func TestPathMapFrontRootBothDirections(t *testing.T) { - m := PathMap{ScratchRoot: "/w/proj/.nself/deploy-build/prod/nself-web/backend", OperatorRoot: "/w/proj", PluginDir: "/h/.nself/plugins", Home: "/h", RemotePath: "/srv/nself-web/backend", FrontRoot: "/w/proj/.nself/deploy-build/prod/nself-web"} + m := pathmap.PathMap{ScratchRoot: "/w/proj/.nself/deploy-build/prod/nself-web/backend", OperatorRoot: "/w/proj", PluginDir: "/h/.nself/plugins", Home: "/h", RemotePath: "/srv/nself-web/backend", FrontRoot: "/w/proj/.nself/deploy-build/prod/nself-web"} in := "a=/w/proj/.nself/deploy-build/prod/nself-web/backend/nginx b=/w/proj/.nself/deploy-build/prod/nself-web/nginx/sites c=/w/proj/.nself/deploy-build/prod/nself-web2/x" got := m.Rewrite(in) if !strings.Contains(got, "a=/srv/nself-web/backend/nginx") || !strings.Contains(got, "b=/srv/nself-web/nginx/sites") { diff --git a/internal/deploy/artifacts_nginx_test.go b/internal/deploy/artifacts_nginx_test.go index a526b3646..e9f999121 100644 --- a/internal/deploy/artifacts_nginx_test.go +++ b/internal/deploy/artifacts_nginx_test.go @@ -2,6 +2,7 @@ package deploy import ( "bytes" + "github.com/nself-org/cli/internal/deploy/pathmap" "os" "path/filepath" "strings" @@ -43,7 +44,7 @@ func nginxScratch(t *testing.T, policy *NginxPolicy) StageInput { for rel, body := range files { writeFile(t, filepath.Join(scratch, filepath.FromSlash(rel)), body, 0o644) } - m := PathMap{ScratchRoot: scratch, OperatorRoot: filepath.Join(base, "proj"), PluginDir: filepath.Join(base, "plugins"), Home: filepath.Join(base, "home"), RemotePath: "/opt/nself"} + m := pathmap.PathMap{ScratchRoot: scratch, OperatorRoot: filepath.Join(base, "proj"), PluginDir: filepath.Join(base, "plugins"), Home: filepath.Join(base, "home"), RemotePath: "/opt/nself"} return StageInput{Map: m, StageDir: t.TempDir(), EnvName: "prod", Nginx: policy} } diff --git a/internal/deploy/pathmap.go b/internal/deploy/pathmap/pathmap.go similarity index 99% rename from internal/deploy/pathmap.go rename to internal/deploy/pathmap/pathmap.go index bbd352ee0..f3c33f1a2 100644 --- a/internal/deploy/pathmap.go +++ b/internal/deploy/pathmap/pathmap.go @@ -1,4 +1,4 @@ -package deploy +package pathmap // Purpose: rewrite the absolute local paths a build leaves in compose files, // manifests and env files to the remote layout (D24 step 3), and diff --git a/internal/deploy/pathmap_test.go b/internal/deploy/pathmap_test.go index ab46abb23..1b4c20672 100644 --- a/internal/deploy/pathmap_test.go +++ b/internal/deploy/pathmap_test.go @@ -1,6 +1,7 @@ package deploy import ( + "github.com/nself-org/cli/internal/deploy/pathmap" "os" "path/filepath" "runtime" @@ -10,19 +11,19 @@ import ( // buildScratch lays out a scratch project the way a v1.5 build leaves it, with // local paths everywhere a build writes them. -func buildScratch(t *testing.T) (m PathMap, compose, envs []string, snapshot string) { +func buildScratch(t *testing.T) (m pathmap.PathMap, compose, envs []string, snapshot string) { t.Helper() return buildScratchFor(t, "/opt/nself") } -func buildScratchFor(t *testing.T, remotePath string) (m PathMap, compose, envs []string, snapshot string) { +func buildScratchFor(t *testing.T, remotePath string) (m pathmap.PathMap, compose, envs []string, snapshot string) { t.Helper() base := t.TempDir() operator := filepath.Join(base, "operator", "proj") scratch := filepath.Join(operator, ".nself", "deploy-build", "prod") plugins := filepath.Join(base, "home", ".nself", "plugins") home := filepath.Join(base, "home") - m = PathMap{ScratchRoot: scratch, OperatorRoot: operator, PluginDir: plugins, Home: home, RemotePath: remotePath} + m = pathmap.PathMap{ScratchRoot: scratch, OperatorRoot: operator, PluginDir: plugins, Home: home, RemotePath: remotePath} writeFile(t, filepath.Join(scratch, "docker-compose.yml"), "services:\n nginx:\n volumes:\n - "+scratch+"/nginx:/etc/nginx:ro\n - "+scratch+"/ssl:/etc/nginx/ssl:ro\n web:\n env_file: "+operator+"/.env\n", 0o644) writeFile(t, filepath.Join(plugins, "ai", "docker-compose.plugin.yml"), "services:\n ai:\n build:\n context: "+plugins+"/ai\n", 0o644) writeFile(t, filepath.Join(scratch, ".env"), "PROJECT=x\nNSELF_PLUGIN_DIR="+plugins+"\n", 0o600) @@ -38,7 +39,7 @@ func buildScratchFor(t *testing.T, remotePath string) (m PathMap, compose, envs []string{filepath.Join(scratch, ".env"), filepath.Join(scratch, ".nself", "compose.env")}, snapshot } -func stageIn(m PathMap, compose, envs []string, snapshot string, stage string) StageInput { +func stageIn(m pathmap.PathMap, compose, envs []string, snapshot string, stage string) StageInput { return StageInput{Map: m, StageDir: stage, Compose: compose, EnvFiles: envs, EnvSnapshot: snapshot, EnvName: "prod"} } @@ -113,7 +114,7 @@ func TestPathMapRefusesLeftoverHome(t *testing.T) { } func TestPathMapBoundaries(t *testing.T) { - m := PathMap{ScratchRoot: "/s/p", OperatorRoot: "/o/proj", PluginDir: "/h/.nself/plugins", Home: "/h", RemotePath: "/opt/nself"} + m := pathmap.PathMap{ScratchRoot: "/s/p", OperatorRoot: "/o/proj", PluginDir: "/h/.nself/plugins", Home: "/h", RemotePath: "/opt/nself"} got := m.Rewrite("a=/o/proj/x b=/o/proj2/x c=/opt/o/proj/x d=/o/proj e=\"/h/.nself/plugins/ai\"") want := "a=/opt/nself/x b=/o/proj2/x c=/opt/o/proj/x d=/opt/nself e=\"/opt/nself/.nself/plugins/ai\"" if got != want { @@ -123,7 +124,7 @@ func TestPathMapBoundaries(t *testing.T) { t.Fatalf("leftovers %v", l) } // A remote path that starts with the local home is not a leftover. - m2 := PathMap{ScratchRoot: "/s/p", OperatorRoot: "/o/proj", PluginDir: "/h/.nself/plugins", Home: "/home/u", RemotePath: "/home/u/nself"} + m2 := pathmap.PathMap{ScratchRoot: "/s/p", OperatorRoot: "/o/proj", PluginDir: "/h/.nself/plugins", Home: "/home/u", RemotePath: "/home/u/nself"} if l := m2.Leftovers("/home/u/nself/docker-compose.yml"); len(l) != 0 { t.Fatalf("remote path under the home dir flagged: %v", l) } @@ -142,7 +143,7 @@ func contains(ss []string, s string) bool { // any drive-letter case) into compose files; they must map to the remote layout and // any survivor must be reported. Pure string work: it runs the same on every OS. func TestPathMapWindowsStylePaths(t *testing.T) { - m := PathMap{ + m := pathmap.PathMap{ ScratchRoot: `C:/Users/Ann/proj/.nself/deploy-build/prod`, OperatorRoot: `C:/Users/Ann/proj`, PluginDir: `C:/Users/Ann/.nself/plugins`, diff --git a/internal/deploy/remote_exec.go b/internal/deploy/remote_exec.go index 44d9070fb..249dfdd66 100644 --- a/internal/deploy/remote_exec.go +++ b/internal/deploy/remote_exec.go @@ -20,10 +20,10 @@ import ( "context" "fmt" "os" - "os/exec" "strconv" "strings" + "github.com/nself-org/cli/internal/hostpin" "github.com/nself-org/cli/sdk/go/v2/remote" ) @@ -88,17 +88,7 @@ func parseScannedKeys(out string) []remote.HostKey { } func deployTrustedFingerprints(ctx context.Context, alias, path string) map[string]bool { - seen := map[string]bool{} - out, err := exec.CommandContext(ctx, "ssh-keygen", "-F", alias, "-f", path).Output() - if err != nil { - return seen - } - for _, line := range strings.Split(string(out), "\n") { - if fp, err := remote.Fingerprint(line); err == nil { - seen[fp] = true - } - } - return seen + return hostpin.TrustedFingerprints(ctx, alias, path, remote.Fingerprint) } // RemoteTarget describes a resolved remote host for a non-deploy command. diff --git a/internal/deploy/remote_prune.go b/internal/deploy/remote_prune.go new file mode 100644 index 000000000..3ea6b603d --- /dev/null +++ b/internal/deploy/remote_prune.go @@ -0,0 +1,133 @@ +package deploy + +// Purpose: remote prune helpers extracted from remote_sync.go to keep that +// file under the 300-line cap (TestFileSizeBudgetNotExceeded) and +// shrink internal/deploy below its package-size ceiling. The prune +// removes only files that carry the generated marker and were shipped +// by a previous deploy; nothing under conf.d* and no unmarked file +// is ever a candidate. + +import ( + "context" + "fmt" + "path" + "strings" +) + +// PrunePrevious removes the files the previous deploy shipped that the new set +// dropped and that carry the generated marker (a plugin fragment under +// .nself/plugins// needs no marker: it is a copy of the operator's file). +// What may be removed is decided by recorded, the local record of that deploy +// (deploy.json); the host's own shipped list (informational) can only narrow it. +// With no record nothing is removed. It returns the files it removed. +func PrunePrevious(ctx context.Context, r Remote, remotePath string, keep map[string]bool, recorded []string) ([]string, error) { + if len(recorded) == 0 { + return nil, nil + } + known := map[string]bool{} + for _, f := range recorded { + known[f] = true + } + prev, err := remoteLines(ctx, r, path.Join(remotePath, ShippedFile)) + if err != nil { + return nil, err + } + var cand, plugins []string + for _, f := range prev { + switch { + case !known[f] || keep[f]: + case isPluginFragment(f): + plugins = append(plugins, f) + default: + cand = append(cand, f) + } + } + removed, err := PruneMarked(ctx, r, remotePath, cand) + if err != nil { + return removed, err + } + gone, err := pruneShippedPlugins(ctx, r, remotePath, plugins) + return append(removed, gone...), err +} + +// PruneMarked removes, from the candidates (paths relative to remotePath), the +// files that carry the generated marker in their first lines. Entries that are +// not safe project-relative paths (see pruneable) are skipped. A batch in which +// any path resolves outside the project (or is itself a symlink) is refused +// whole: nothing in it is deleted and the error names the path. It returns the +// files it removed. +func PruneMarked(ctx context.Context, r Remote, remotePath string, candidates []string) ([]string, error) { + body := fmt.Sprintf(`if [ -f "$2" ] && head -n 5 -- "$2" 2>/dev/null | grep -qi %s; then rm -f -- "$2" && printf '%%s\n' "$1"; fi`, q(GeneratedMarker)) + return pruneGuarded(ctx, r, remotePath, candidates, body) +} + +// pruneShippedPlugins removes the plugin compose fragments (.nself/plugins//...) +// the previous deploy shipped and this one dropped. A fragment is copied from the +// operator's plugin dir and carries no generated marker, so the marker is not +// asked for: the candidates come from the local deploy record intersected with +// the host's shipped list (PrunePrevious), and the symlink and realpath guard is +// the one PruneMarked uses. Candidates that are not plugin fragments are skipped. +func pruneShippedPlugins(ctx context.Context, r Remote, remotePath string, candidates []string) ([]string, error) { + var cand []string + for _, f := range candidates { + if isPluginFragment(f) { + cand = append(cand, f) + } + } + return pruneGuarded(ctx, r, remotePath, cand, `if [ -f "$2" ]; then rm -f -- "$2" && printf '%s\n' "$1"; fi`) +} + +// isPluginFragment reports whether rel is a file at least one directory below +// .nself/plugins// that pruneable accepts. +func isPluginFragment(rel string) bool { + rest, ok := strings.CutPrefix(rel, ".nself/plugins/") + if !ok || !pruneable(rel) { + return false + } + slug, file, ok := strings.Cut(rest, "/") + return ok && slug != "" && file != "" +} + +// pruneGuarded runs body (see guardedScript) over the pruneable candidates in +// batches and returns what it printed as removed. +func pruneGuarded(ctx context.Context, r Remote, remotePath string, candidates []string, body string) ([]string, error) { + var cand []string + for _, f := range candidates { + if pruneable(f) { + cand = append(cand, f) + } + } + var removed []string + for len(cand) > 0 { + n := min(len(cand), 50) + out, err := r.Run(ctx, shScript(guardedScript(remotePath, cand[:n], body))) + if err != nil { + return removed, fmt.Errorf("pruning generated files on %s: %w", r.Label(), err) + } + if err := refusedPaths(out); err != nil { + return removed, err + } + for _, f := range strings.Split(strings.TrimSpace(string(out)), "\n") { + if f != "" { + removed = append(removed, f) + } + } + cand = cand[n:] + } + return removed, nil +} + +// pruneable reports whether rel is a safe project-relative file path: not empty, +// clean, no ".", ".." or empty component, not absolute, not the host's own .env, +// no newline, and not under a conf.d* dir. +func pruneable(rel string) bool { + if rel == "" || rel != path.Clean(rel) || rel == ".env" || strings.HasPrefix(rel, "/") || strings.Contains(rel, "\n") { + return false + } + for _, seg := range strings.Split(rel, "/") { + if seg == "" || seg == "." || seg == ".." || strings.HasPrefix(seg, "conf.d") { + return false + } + } + return true +} diff --git a/internal/deploy/remote_prune_plugins_test.go b/internal/deploy/remote_prune_plugins_test.go new file mode 100644 index 000000000..2b468bf68 --- /dev/null +++ b/internal/deploy/remote_prune_plugins_test.go @@ -0,0 +1,89 @@ +package deploy + +// Purpose: D-0419, the shipped-list prune also covers plugin compose fragments +// under .nself/plugins// that a deploy no longer ships. +// Inputs: a fake remote over a temp dir, a host shipped list, a local record. +// Outputs: assertions on what was removed and what stayed. +// Constraints: a fragment has no generated marker; only record-and-host-listed +// paths go, and the symlink guard refuses a batch. + +import ( + "context" + "os" + "testing" +) + +func TestPrunePreviousRemovesDroppedPluginFragment(t *testing.T) { + needTools(t) + f := newFakeRemote(t) + rp := t.TempDir() + dropped := ".nself/plugins/old/docker-compose.plugin.yml" + kept := ".nself/plugins/kept/docker-compose.plugin.yml" + unlisted := ".nself/plugins/hand/docker-compose.plugin.yml" + notRecorded := ".nself/plugins/other/docker-compose.plugin.yml" + for _, p := range []string{dropped, kept, unlisted, notRecorded} { + writeFile(t, rp+"/"+p, "services:\n x:\n image: x\n", 0o644) // no generated marker + } + writeFile(t, rp+"/"+ShippedFile, dropped+"\n"+kept+"\n"+notRecorded+"\n", 0o644) + removed, err := PrunePrevious(context.Background(), f, rp, map[string]bool{kept: true}, []string{dropped, kept, unlisted}) + if err != nil { + t.Fatal(err) + } + if len(removed) != 1 || removed[0] != dropped || exists(rp+"/"+dropped) { + t.Fatalf("removed %v, want only %s", removed, dropped) + } + for _, p := range []string{kept, unlisted, notRecorded} { + if !exists(rp + "/" + p) { + t.Errorf("%s was removed", p) + } + } +} + +func TestPrunePreviousPluginFragmentNeedsPluginPath(t *testing.T) { + needTools(t) + f := newFakeRemote(t) + rp := t.TempDir() + // Unmarked files outside .nself/plugins// keep needing the marker. + paths := []string{"hasura/metadata.yaml", ".nself/plugins/loose.yml", ".nself/compose-files.txt"} + list := "" + for _, p := range paths { + writeFile(t, rp+"/"+p, "plain\n", 0o644) + list += p + "\n" + } + writeFile(t, rp+"/"+ShippedFile, list, 0o644) + removed, err := PrunePrevious(context.Background(), f, rp, map[string]bool{}, paths) + if err != nil || len(removed) != 0 { + t.Fatalf("removed %v err %v", removed, err) + } + for _, p := range paths { + if !exists(rp + "/" + p) { + t.Errorf("unmarked %s was removed", p) + } + } +} + +func TestPrunePreviousPluginFragmentSymlinkGuard(t *testing.T) { + needTools(t) + f := newFakeRemote(t) + rp := t.TempDir() + outside := t.TempDir() + victim := outside + "/docker-compose.plugin.yml" + writeFile(t, victim, "keep me\n", 0o644) + if err := os.MkdirAll(rp+"/.nself/plugins", 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, rp+"/.nself/plugins/old"); err != nil { + t.Fatal(err) + } + rel := ".nself/plugins/old/docker-compose.plugin.yml" + writeFile(t, rp+"/"+ShippedFile, rel+"\n", 0o644) + if _, err := PrunePrevious(context.Background(), f, rp, map[string]bool{}, []string{rel}); err == nil { + t.Fatal("a plugin dir symlinked outside remote_path was pruned through") + } + if !exists(victim) { + t.Fatal("file outside remote_path was deleted") + } + if !isPluginFragment(rel) || isPluginFragment(".nself/plugins/old") || isPluginFragment(".nself/plugins//x") || isPluginFragment("nginx/a.conf") { + t.Error("isPluginFragment misjudged a path") + } +} diff --git a/internal/deploy/remote_runner.go b/internal/deploy/remote_runner.go index 3b6ffd19d..04547720c 100644 --- a/internal/deploy/remote_runner.go +++ b/internal/deploy/remote_runner.go @@ -52,8 +52,8 @@ func NewSSHRemote(ctx context.Context, host, keyPath, env, server string) (*SSHR if err != nil { return nil, errs.New("E484", err.Error()) } - if keyPath != "" && !SSHKeyRe.MatchString(keyPath) { - return nil, fmt.Errorf("ssh key path contains unsafe characters (got %q): only [a-zA-Z0-9/_.~-] allowed", keyPath) + if err := checkDeployKeyPath(keyPath); err != nil { + return nil, err } policy, err := HostKeyOptions(ctx, spec.String(), env, server, true) if err != nil { @@ -69,7 +69,7 @@ func NewSSHRemote(ctx context.Context, host, keyPath, env, server string) (*SSHR func sshOpts(keyPath string, policy []string) []string { var opts []string if keyPath != "" { - opts = append(opts, "-i", keyPath) + opts = append(opts, "-i", sshKeyPath(keyPath)) } opts = append(opts, policy...) return append(opts, "-o", "ForwardAgent=no", "-o", "BatchMode=yes", "-o", "ConnectTimeout=10") @@ -139,6 +139,14 @@ func rsyncArgv(spec remote.HostSpec, sshOpts, flags []string, filesFrom, srcDir, return nil, fmt.Errorf("rsync flag %q is not allowed (no delete, no transport override)", f) } } + filesFrom, err = rsyncLocalOperand(filesFrom) + if err != nil { + return nil, err + } + srcDir, err = rsyncLocalOperand(srcDir) + if err != nil { + return nil, err + } target := spec.String() if spec.Port != 0 { target = strings.TrimSuffix(target, ":"+strconv.Itoa(spec.Port)) diff --git a/internal/deploy/remote_sync.go b/internal/deploy/remote_sync.go index aad8fb79b..1e5cc3e17 100644 --- a/internal/deploy/remote_sync.go +++ b/internal/deploy/remote_sync.go @@ -160,80 +160,6 @@ func assertEnvModes(ctx context.Context, r Remote, set ArtifactSet) error { return nil } -// PrunePrevious removes the files the previous deploy shipped that the new set -// dropped and that carry the generated marker. What may be removed is decided by -// recorded, the local record of that deploy (deploy.json); the host's own shipped -// list (informational) can only narrow it. With no record nothing is removed. -// It returns the files it removed. -func PrunePrevious(ctx context.Context, r Remote, remotePath string, keep map[string]bool, recorded []string) ([]string, error) { - if len(recorded) == 0 { - return nil, nil - } - known := map[string]bool{} - for _, f := range recorded { - known[f] = true - } - prev, err := remoteLines(ctx, r, path.Join(remotePath, ShippedFile)) - if err != nil { - return nil, err - } - var cand []string - for _, f := range prev { - if known[f] && !keep[f] { - cand = append(cand, f) - } - } - return PruneMarked(ctx, r, remotePath, cand) -} - -// PruneMarked removes, from the candidates (paths relative to remotePath), the -// files that carry the generated marker in their first lines. Entries that are not -// safe project-relative paths (see pruneable) are skipped. A batch in which any -// path resolves outside the project (or is itself a symlink) is refused whole: nothing in it is deleted and the -// error names the path. It returns the files it removed. -func PruneMarked(ctx context.Context, r Remote, remotePath string, candidates []string) ([]string, error) { - var cand []string - for _, f := range candidates { - if pruneable(f) { - cand = append(cand, f) - } - } - var removed []string - for len(cand) > 0 { - n := min(len(cand), 50) - body := fmt.Sprintf(`if [ -f "$2" ] && head -n 5 -- "$2" 2>/dev/null | grep -qi %s; then rm -f -- "$2" && printf '%%s\n' "$1"; fi`, q(GeneratedMarker)) - out, err := r.Run(ctx, shScript(guardedScript(remotePath, cand[:n], body))) - if err != nil { - return removed, fmt.Errorf("pruning generated files on %s: %w", r.Label(), err) - } - if err := refusedPaths(out); err != nil { - return removed, err - } - for _, f := range strings.Split(strings.TrimSpace(string(out)), "\n") { - if f != "" { - removed = append(removed, f) - } - } - cand = cand[n:] - } - return removed, nil -} - -// pruneable reports whether rel is a safe project-relative file path: not empty, -// clean, no ".", ".." or empty component, not absolute, not the host's own .env, -// no newline, and not under a conf.d* dir. -func pruneable(rel string) bool { - if rel == "" || rel != path.Clean(rel) || rel == ".env" || strings.HasPrefix(rel, "/") || strings.Contains(rel, "\n") { - return false - } - for _, seg := range strings.Split(rel, "/") { - if seg == "" || seg == "." || seg == ".." || strings.HasPrefix(seg, "conf.d") { - return false - } - } - return true -} - // checkPaths returns an error naming the first entry of a locally recorded list // that is not a safe shippable path (shippable: a pruneable path or a generated // nginx/conf.d/.conf); such a list is never acted on. It gates the diff --git a/internal/deploy/ssh_alias_test.go b/internal/deploy/ssh_alias_test.go new file mode 100644 index 000000000..0e47b0c1d --- /dev/null +++ b/internal/deploy/ssh_alias_test.go @@ -0,0 +1,124 @@ +package deploy + +// Purpose: keep synthetic deploy pins separate from real hosts (D-0696). +// Inputs: fake key scans and isolated pin files, in both compatibility modes. +// Outputs: collision refusals and read-only legacy lookup assertions. +// Constraints: no network or operator pin-file changes. + +import ( + "context" + "os" + "strings" + "testing" + + "github.com/nself-org/cli/sdk/go/v2/remote" +) + +func TestDeployPinAliasIsolation(t *testing.T) { + requireKeygen(t) + for _, mode := range []string{"0", "1"} { + for _, pair := range [][2]string{ + {"203.0.113.7", "nself-203.0.113.7-22"}, + {"x:22", "nself-x-22"}, + {"[1::2]:22", "1--2:22"}, + } { + t.Run(mode+"/"+pair[0], func(t *testing.T) { + t.Setenv("NSELF_V15", mode) + t.Setenv("HOME", pinHome(t)) + installFakeKeyscan(t) + fp, err := remote.Fingerprint("h ssh-ed25519 AQID") + if err != nil { + t.Fatal(err) + } + if err := TrustHostKey(context.Background(), pair[0], fp); err != nil { + t.Fatal(err) + } + if _, err := HostKeyPolicy(context.Background(), pair[0], "live", "web", true, true); err != nil { + t.Fatalf("enrolled host refused: %v", err) + } + if _, err := HostKeyPolicy(context.Background(), pair[1], "live", "web", true, true); err == nil { + t.Fatalf("pin for %s trusted for %s", pair[0], pair[1]) + } + }) + } + } +} + +func TestDeployLegacyAliasReadOnly(t *testing.T) { + requireKeygen(t) + for _, host := range []string{"203.0.113.7", "x:22", "[1::2]:22", "nself-x-22"} { + t.Run(host, func(t *testing.T) { + t.Setenv("NSELF_V15", "1") + t.Setenv("HOME", pinHome(t)) + installFakeKeyscan(t) + spec := mustSpec(t, host) + alias := spec.Host + if spec.Port != 0 || strings.Contains(host, "203.0.113.7") { + alias = "nself-" + strings.ReplaceAll(spec.Host, ":", "-") + "-22" + } + pin := remote.PinnedHostKeys{Path: DeployKnownHostsPath()} + if err := pin.Add(alias, "ssh-ed25519 AQID"); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(pin.Path) + if err != nil { + t.Fatal(err) + } + opts, err := HostKeyPolicy(context.Background(), host, "live", "web", true, true) + if err != nil { + t.Fatal(err) + } + if alias != spec.Host && !strings.Contains(strings.Join(opts, " "), "HostKeyAlias="+alias) { + t.Fatalf("legacy alias missing: %v", opts) + } + after, err := os.ReadFile(pin.Path) + if err != nil || string(before) != string(after) { + t.Fatalf("compat lookup changed pin file: %v", err) + } + }) + } +} + +func TestDeployNewAliasOverridesLegacy(t *testing.T) { + requireKeygen(t) + t.Setenv("NSELF_V15", "1") + t.Setenv("HOME", pinHome(t)) + installFakeKeyscan(t) + pin := remote.PinnedHostKeys{Path: DeployKnownHostsPath()} + for alias, key := range map[string]string{ + "nself-203.0.113.7-22": "ssh-ed25519 AQID", + "nself+203.0.113.7+22": "ssh-ed25519 AQIE", + } { + if err := pin.Add(alias, key); err != nil { + t.Fatal(err) + } + } + if _, err := HostKeyPolicy(context.Background(), "203.0.113.7", "live", "web", true, true); err == nil { + t.Fatal("stale legacy key accepted despite a new pin") + } +} + +func TestPinAliasLegacyFallbackForOtherCallers(t *testing.T) { + requireKeygen(t) + t.Setenv("HOME", pinHome(t)) + spec := mustSpec(t, "u@example.test:2222") + if got := PinAlias(context.Background(), spec); got != "nself+example.test+2222" { + t.Fatalf("no pins: alias %q", got) + } + pin := remote.PinnedHostKeys{Path: DeployKnownHostsPath()} + if err := pin.Add("nself-example.test-2222", "ssh-ed25519 AQID"); err != nil { + t.Fatal(err) + } + if got := PinAlias(context.Background(), spec); got != "nself-example.test-2222" { + t.Fatalf("legacy-only pin: alias %q", got) + } + if err := pin.Add("nself+example.test+2222", "ssh-ed25519 AQIE"); err != nil { + t.Fatal(err) + } + if got := PinAlias(context.Background(), spec); got != "nself+example.test+2222" { + t.Fatalf("new pin must win: alias %q", got) + } + if got := PinAlias(context.Background(), mustSpec(t, "example.test")); got != "example.test" { + t.Fatalf("plain host alias %q", got) + } +} diff --git a/internal/deploy/ssh_argv.go b/internal/deploy/ssh_argv.go index 4f5ae310d..0dc7d74ff 100644 --- a/internal/deploy/ssh_argv.go +++ b/internal/deploy/ssh_argv.go @@ -13,17 +13,19 @@ package deploy import ( "fmt" + "os" "path/filepath" "regexp" "strconv" "strings" + "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/sdk/go/v2/remote" ) // SSHKeyRe is the safe alphabet for an SSH key path. The one definition: the // legacy push in cmd/commands aliases it. -var SSHKeyRe = regexp.MustCompile(`^[a-zA-Z0-9/_.~-]+$`) +var SSHKeyRe = regexp.MustCompile(`^([a-zA-Z]:[/\\][a-zA-Z0-9/\\_.~-]*|[a-zA-Z0-9/_.~-]+)$`) // sshTransport is the ssh option set for one host. opts holds [-i key], the // host-key policy and ForwardAgent=no; the port is NOT in opts (it is added @@ -33,14 +35,26 @@ type sshTransport struct { opts []string } +// CheckKeyPath is checkDeployKeyPath for callers outside the package, so every +// deploy entry point reports a bad key path the same way. +func CheckKeyPath(p string) error { return checkDeployKeyPath(p) } + // checkDeployKeyPath accepts "" (no -i) or a path in the SSHKeyRe alphabet -// that does not start with '-'. +// that does not start with '-'. A path with whitespace is E484: rsync re-splits +// its -e string on blanks, so the key cannot be passed through it. When the +// blank comes from the home directory (a Windows profile such as +// C:\Users\Jane Doe), the error names that home path. func checkDeployKeyPath(p string) error { if p == "" { return nil } + if strings.ContainsAny(p, " \t") { + return errs.New("E484", fmt.Sprintf("ssh key path %s contains whitespace%s", p, homeWhitespaceHint(p))). + WithWhy("rsync re-splits its ssh command on blanks, so a key path with whitespace cannot be passed safely."). + WithFix("Set NSELF_DEPLOY_SSH_KEY to a key path without spaces, for example a copy of the key in a directory such as C:/keys or ~/.ssh on a home without spaces.") + } if !SSHKeyRe.MatchString(p) { - return fmt.Errorf("ssh key path contains unsafe characters (got %q): only [a-zA-Z0-9/_.~-] allowed", p) + return fmt.Errorf("ssh key path contains unsafe characters (got %q): use [a-zA-Z0-9/_.~-], optionally with an absolute Windows drive prefix and separators", p) } if p[0] == '-' { return fmt.Errorf("ssh key path must not start with '-' (got %q)", p) @@ -48,6 +62,19 @@ func checkDeployKeyPath(p string) error { return nil } +// homeWhitespaceHint names the operator home directory when it holds +// whitespace and the key path sits under it. +func homeWhitespaceHint(p string) string { + home, err := os.UserHomeDir() + if err != nil || home == "" || !strings.ContainsAny(home, " \t") { + return "" + } + if strings.HasPrefix(sshKeyPath(p), sshKeyPath(home)) { + return fmt.Sprintf(" because the home directory %s does", home) + } + return "" +} + // checkTransportToken permits canonical SDK pin-file options. All other tokens // refuse blanks, controls, quotes, backslashes and backticks, which rsync splits. func checkTransportToken(tok string) error { @@ -80,7 +107,7 @@ func newSSHTransport(spec remote.HostSpec, keyPath string, policy []string) (ssh } var opts []string if keyPath != "" { - opts = append(opts, "-i", keyPath) + opts = append(opts, "-i", sshKeyPath(keyPath)) } opts = append(opts, policy...) opts = append(opts, "-o", "ForwardAgent=no") @@ -134,7 +161,7 @@ func (t sshTransport) rsyncMultiArgv(flags []string, dest string, srcs ...string } args = append(args, "-e", sshCommand, "--") for _, src := range srcs { - local, err := localOperand(src) + local, err := rsyncLocalOperand(src) if err != nil { return nil, err } @@ -190,14 +217,14 @@ func hostOperand(spec remote.HostSpec, path string) string { } // localOperand makes a relative p absolute, so scp and rsync never read it as -// a "host:path" operand or an option. A path that is already absolute (or -// starts with '/') is returned unchanged, so argv stays as it was on every OS. +// a "host:path" operand or an option. Absolute paths retain their native +// form for scp; rsyncLocalOperand also maps Windows paths for POSIX rsync. // Empty paths, newlines and NUL are refused. func localOperand(p string) (string, error) { if p == "" || strings.ContainsAny(p, "\n\x00") { return "", fmt.Errorf("local path %q is empty or contains a newline or NUL", p) } - if strings.HasPrefix(p, "/") || filepath.IsAbs(p) { + if strings.HasPrefix(p, "/") || filepath.IsAbs(p) || windowsDrivePath(p) { return p, nil } return filepath.Abs(p) diff --git a/internal/deploy/ssh_hostkeys.go b/internal/deploy/ssh_hostkeys.go index bd2e602ae..56203743b 100644 --- a/internal/deploy/ssh_hostkeys.go +++ b/internal/deploy/ssh_hostkeys.go @@ -13,7 +13,6 @@ package deploy import ( "context" "fmt" - "net" "os" "path/filepath" "strconv" @@ -21,13 +20,13 @@ import ( "github.com/nself-org/cli/internal/compat" "github.com/nself-org/cli/internal/errs" + "github.com/nself-org/cli/internal/hostpin" "github.com/nself-org/cli/sdk/go/v2/remote" ) // DeployKnownHostsPath is the dedicated pin file. The operator's SSH file is read only. func DeployKnownHostsPath() string { - home, _ := os.UserHomeDir() - return filepath.Join(home, ".config", "nself", "deploy_known_hosts") + return hostpin.KnownHostsPath() } // knownHostsOption quotes the absolute pin path for OpenSSH. @@ -35,21 +34,9 @@ func knownHostsOption() (string, error) { return remote.GlobalKnownHostsFileOption(DeployKnownHostsPath()) } -func deployHostPort(spec remote.HostSpec) int { - if spec.Port == 0 { - return 22 - } - return spec.Port -} - -// deployKeyAlias is the pin alias. A port or an IP literal needs a synthetic -// alias: the sdk refuses an IP literal as a pin alias. -func deployKeyAlias(spec remote.HostSpec) string { - if spec.Port != 0 || net.ParseIP(spec.Host) != nil { - return "nself-" + strings.ReplaceAll(spec.Host, ":", "-") + "-" + strconv.Itoa(deployHostPort(spec)) - } - return spec.Host -} +func deployHostPort(spec remote.HostSpec) int { return hostpin.HostPort(spec.Port) } +func deployKeyAlias(spec remote.HostSpec) string { return hostpin.KeyAlias(spec.Host, spec.Port) } +func legacyKeyAlias(spec remote.HostSpec) string { return hostpin.LegacyKeyAlias(spec.Host, spec.Port) } // TrustHostKey scans live keys and pins only the key with the offered fingerprint. func TrustHostKey(ctx context.Context, host, offered string) error { @@ -86,7 +73,26 @@ func pinnedFingerprints(ctx context.Context, alias string) (map[string]bool, err if _, err := pin.Lookup(alias); err != nil { return nil, errs.New("E487", fmt.Sprintf("deploy pin file %s refused: %v", DeployKnownHostsPath(), err)) } - return deployTrustedFingerprints(ctx, alias, DeployKnownHostsPath()), nil + return hostpin.TrustedFingerprints(ctx, alias, DeployKnownHostsPath(), remote.Fingerprint), nil +} + +// deployPinnedFingerprints reads legacy IP/port pins only when no new pin +// exists. It returns the alias SSH must check and never rewrites the pin file. +func deployPinnedFingerprints(ctx context.Context, spec remote.HostSpec) (string, map[string]bool, error) { + alias := deployKeyAlias(spec) + pinned, err := pinnedFingerprints(ctx, alias) + if err != nil || len(pinned) != 0 || alias == spec.Host { + return alias, pinned, err + } + legacy := legacyKeyAlias(spec) + pinned, err = pinnedFingerprints(ctx, legacy) + return legacy, pinned, err +} + +// PinAlias delegates to hostpin.PinAlias; kept here so existing deploy callers +// keep compiling without importing hostpin directly. +func PinAlias(ctx context.Context, spec remote.HostSpec) string { + return hostpin.PinAlias(ctx, spec.Host, spec.Port, remote.Fingerprint) } // hostKeyPolicy is the unexported spelling of HostKeyPolicy, kept for the @@ -120,9 +126,8 @@ func HostKeyPolicy(ctx context.Context, host, env, server string, strict, shipsS if spec.Port != 0 { operatorAlias = "[" + spec.Host + "]:" + strconv.Itoa(spec.Port) } - alias := deployKeyAlias(spec) - operator := deployTrustedFingerprints(ctx, operatorAlias, filepath.Join(home, ".ssh", "known_hosts")) - pinned, err := pinnedFingerprints(ctx, alias) + operator := hostpin.TrustedFingerprints(ctx, operatorAlias, filepath.Join(home, ".ssh", "known_hosts"), remote.Fingerprint) + alias, pinned, err := deployPinnedFingerprints(ctx, spec) if err != nil { return nil, err } diff --git a/internal/deploy/ssh_hostkeys_test.go b/internal/deploy/ssh_hostkeys_test.go index 5927f1425..f955dfeba 100644 --- a/internal/deploy/ssh_hostkeys_test.go +++ b/internal/deploy/ssh_hostkeys_test.go @@ -220,9 +220,9 @@ func TestPinnedLookupRefusesForeignLine(t *testing.T) { func TestDeployKeyAliasIPHost(t *testing.T) { requireKeygen(t) for in, want := range map[string]string{ - "u@203.0.113.7": "nself-203.0.113.7-22", - "u@[2001:db8::1]:2222": "nself-2001-db8--1-2222", - "u@host.example:2222": "nself-host.example-2222", + "u@203.0.113.7": "nself+203.0.113.7+22", + "u@[2001:db8::1]:2222": "nself+2001:db8::1+2222", + "u@host.example:2222": "nself+host.example+2222", "u@host.example": "host.example", } { if got := deployKeyAlias(mustSpec(t, in)); got != want { @@ -244,7 +244,7 @@ func TestDeployKeyAliasIPHost(t *testing.T) { if err != nil { t.Fatalf("HostKeyOptions: %v", err) } - if got := strings.Join(opts, " "); !strings.Contains(got, "HostKeyAlias=nself-203.0.113.7-22") { + if got := strings.Join(opts, " "); !strings.Contains(got, "HostKeyAlias=nself+203.0.113.7+22") { t.Errorf("options = %q", got) } } diff --git a/internal/deploy/windows_paths.go b/internal/deploy/windows_paths.go new file mode 100644 index 000000000..e0d24ea17 --- /dev/null +++ b/internal/deploy/windows_paths.go @@ -0,0 +1,55 @@ +package deploy + +import ( + "context" + "fmt" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" +) + +// windowsDrivePath identifies an absolute drive path on any host, including +// when tests exercise Windows argv on a POSIX host. Drive-relative paths are +// deliberately excluded. +func windowsDrivePath(p string) bool { + return len(p) >= 3 && ((p[0] >= 'A' && p[0] <= 'Z') || (p[0] >= 'a' && p[0] <= 'z')) && p[1] == ':' && (p[2] == '/' || p[2] == '\\') +} + +// sshKeyPath normalizes an already validated key path for ssh and rsync -e. +// Only absolute Windows drive paths admit backslash separators. +func sshKeyPath(p string) string { + if windowsDrivePath(p) { + p = strings.ReplaceAll(p, `\`, "/") + } + return filepath.ToSlash(p) +} + +// rsyncLocalOperand maps native local paths to the installed POSIX runtime. +// Inputs: a local source or file list. Outputs: an absolute local rsync path. +// Constraints: no shell; cygpath must match rsync's MSYS/Cygwin runtime. Its +// mount table is authoritative, so no drive mount prefix is hard-coded. +func rsyncLocalOperand(p string) (string, error) { + local, err := localOperand(p) + if err != nil { + return "", err + } + if runtime.GOOS != "windows" && !windowsDrivePath(local) { + return local, nil + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + out, err := exec.CommandContext(ctx, "cygpath", "-u", "--", local).Output() + if err != nil { + return "", fmt.Errorf("convert Windows local path %q for rsync: install cygpath from the same MSYS/Cygwin runtime as rsync: %w", local, err) + } + converted := strings.TrimRight(string(out), "\r\n") + if !strings.HasPrefix(converted, "/") || strings.ContainsAny(converted, "\r\n\x00") { + return "", fmt.Errorf("cygpath returned an invalid absolute local path for %q", local) + } + if strings.HasSuffix(local, "/") || strings.HasSuffix(local, `\`) { + converted = strings.TrimSuffix(converted, "/") + "/" + } + return converted, nil +} diff --git a/internal/deploy/windows_paths_test.go b/internal/deploy/windows_paths_test.go new file mode 100644 index 000000000..f6fcfd233 --- /dev/null +++ b/internal/deploy/windows_paths_test.go @@ -0,0 +1,101 @@ +package deploy + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/nself-org/cli/internal/errs" +) + +func TestWindowsDeployKeyPaths(t *testing.T) { + for _, key := range []string{`C:/Users/Jane/.ssh/id_ed25519`, `C:\Users\Jane\.ssh\id_ed25519`} { + tr, err := newSSHTransport(mustSpec(t, "host.test"), key, nil) + if err != nil { + t.Errorf("key %q: %v", key, err) + continue + } + want := strings.ReplaceAll(key, `\`, "/") + if tr.opts[1] != want { + t.Errorf("key = %q, want %q", tr.opts[1], want) + } + if got := sshOpts(key, nil)[1]; got != want { + t.Errorf("inventory key = %q, want %q", got, want) + } + } + for _, key := range []string{`C:relative`, `C:/a b/key`, `C:/key -oProxyCommand=x`, `C:/key;id`, `C:\key"`, `host:key`, `-F/tmp/key`} { + if err := checkDeployKeyPath(key); err == nil { + t.Errorf("accepted unsafe key %q", key) + } + } +} + +func TestWindowsDeployCopyPaths(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX cygpath stub") + } + dir := t.TempDir() + // Echo a mount path deliberately different from /c and /cygdrive/c: use + // the installed runtime's mapping rather than assuming its mount prefix. + script := "#!/bin/sh\n[ \"$1\" = -u ] || exit 2\n[ \"$2\" = -- ] || exit 2\nprintf '/mounted/c/%s\\n' \"${3#C:?}\" | tr '\\\\' '/'\n" + if err := os.WriteFile(filepath.Join(dir, "cygpath"), []byte(script), 0o700); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + tr, err := newSSHTransport(mustSpec(t, "host.test"), "", nil) + if err != nil { + t.Fatal(err) + } + for _, src := range []string{`C:/Jane Doe/env`, `C:\Jane Doe\env`} { + args, err := tr.rsyncArgv(src, "/remote") + if err != nil { + t.Fatal(err) + } + if got := args[len(args)-2]; got != "/mounted/c/Jane Doe/env" { + t.Errorf("rsync source = %q", got) + } + args, err = tr.scpArgv(src, "/remote") + if err != nil { + t.Fatal(err) + } + if got := args[len(args)-2]; got != src { + t.Errorf("native scp source = %q, want %q", got, src) + } + } + args, err := rsyncArgv(tr.spec, tr.opts, nil, `C:\Jane Doe\list`, `C:\Jane Doe\project\`, "/remote") + if err != nil { + t.Fatal(err) + } + if got := args[len(args)-2]; got != "/mounted/c/Jane Doe/project/" { + t.Errorf("inventory source = %q", got) + } + if !strings.Contains(strings.Join(args, "\n"), "--files-from=/mounted/c/Jane Doe/list\n") { + t.Errorf("inventory list = %q", args) + } +} + +func TestKeyPathWhitespaceIsCoded(t *testing.T) { + home := `C:\Users\Jane Doe` + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + for _, key := range []string{`C:\Users\Jane Doe\.ssh\id_ed25519`, `C:/Users/Jane Doe/.ssh/id_ed25519`} { + err := CheckKeyPath(key) + if err == nil { + t.Fatalf("key %q accepted", key) + } + var ce *errs.CLIError + if !errors.As(err, &ce) || ce.Code != "E484" { + t.Errorf("key %q: want a coded E484 error, got %v", key, err) + } + if !strings.Contains(err.Error(), home) { + t.Errorf("key %q: error does not name the home path: %v", key, err) + } + } + other := CheckKeyPath("/tmp/a key") + if other == nil || strings.Contains(other.Error(), "home directory") { + t.Errorf("a blank outside home must not blame home: %v", other) + } +} diff --git a/internal/hostpin/hostpin.go b/internal/hostpin/hostpin.go new file mode 100644 index 000000000..5282ea892 --- /dev/null +++ b/internal/hostpin/hostpin.go @@ -0,0 +1,79 @@ +// Package hostpin provides SSH host-key pin alias resolution for callers that +// do their own pin lookup (admin connect, access). It lives at L0 (stdlib +// only, the sdk fingerprint parser is injected) so L1 packages can resolve a pin +// alias without importing internal/deploy (L2). +package hostpin + +import ( + "context" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" +) + +// KnownHostsPath is the dedicated deploy pin file. The operator's SSH file is +// read only; this package never writes it. +func KnownHostsPath() string { + home, _ := os.UserHomeDir() + return filepath.Join(home, ".config", "nself", "deploy_known_hosts") +} + +// HostPort returns the effective SSH port for spec (22 when unset). +func HostPort(port int) int { + if port == 0 { + return 22 + } + return port +} + +// KeyAlias is the pin alias for host and port. A port or an IP literal needs a +// synthetic alias: the sdk refuses an IP literal as a pin alias. '+' is +// allowed in pin aliases but forbidden in host names, keeping the namespaces +// disjoint. +func KeyAlias(host string, port int) string { + if port != 0 || net.ParseIP(host) != nil { + return "nself+" + host + "+" + strconv.Itoa(HostPort(port)) + } + return host +} + +// LegacyKeyAlias is the v1.4.13 spelling of the IP/port pin alias. It can +// equal a real host name, so it is only ever read, and only when no pin exists +// under the new alias (D-0696). +func LegacyKeyAlias(host string, port int) string { + return "nself-" + strings.ReplaceAll(host, ":", "-") + "-" + strconv.Itoa(HostPort(port)) +} + +// TrustedFingerprints returns the fingerprints ssh-keygen reports for alias in +// path, each line parsed by fp (the sdk remote.Fingerprint; injected so this +// package stays at L0). Errors are swallowed: the caller treats an empty set as +// "no pin". +func TrustedFingerprints(ctx context.Context, alias, path string, fp func(string) (string, error)) map[string]bool { + seen := map[string]bool{} + out, err := exec.CommandContext(ctx, "ssh-keygen", "-F", alias, "-f", path).Output() + if err != nil { + return seen + } + for _, line := range strings.Split(string(out), "\n") { + if f, err := fp(line); err == nil { + seen[f] = true + } + } + return seen +} + +// PinAlias returns the HostKeyAlias to give ssh for a host with an explicit +// port or an IP literal. It is the new alias unless only a v1.4.13 pin exists. +func PinAlias(ctx context.Context, host string, port int, fp func(string) (string, error)) string { + alias := KeyAlias(host, port) + if alias == host || len(TrustedFingerprints(ctx, alias, KnownHostsPath(), fp)) != 0 { + return alias + } + if legacy := LegacyKeyAlias(host, port); len(TrustedFingerprints(ctx, legacy, KnownHostsPath(), fp)) != 0 { + return legacy + } + return alias +} diff --git a/internal/repoqa/testdata/layers.yaml b/internal/repoqa/testdata/layers.yaml index 3ae18cc08..6452d2747 100644 --- a/internal/repoqa/testdata/layers.yaml +++ b/internal/repoqa/testdata/layers.yaml @@ -58,6 +58,7 @@ rules: - {match: "internal/output/**", layer: L0} - {match: "internal/canon/**", layer: L0} - {match: "internal/cmdregistry/**", layer: L0} + - {match: "internal/hostpin/**", layer: L0} # SSH pin alias resolution: stdlib only; sdk fingerprint parser injected - {match: "internal/sysexec/**", layer: L0} # command runner + headless check for system-changing setup (trust, ssl, dns) # L2: domain orchestrators plus the planned ones - {match: "internal/upgrade/stack/**", layer: L2}