From 76c60c45e06f74075b631d3cc3f403953f4cef71 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Mon, 7 Sep 2026 23:41:59 -0700 Subject: [PATCH] fix(node): validate the profile on a bootstrap dry run resolveClient is the only thing that calls Profile.Validate, and a dry run skips it. The server URL is then concatenated unconditionally into the client.rb the script writes, so a profile naming no Cinc Server, such as a Supermarket-only one, produced a script pointing nowhere and exited zero: chef_server_url '/organizations/' Dry run exists to show what a real run would do, so it has to apply the same requirement rather than print something that could never work. Validate the resolved profile before building the command, on both paths. Signed-off-by: Tim Smith --- apps/cinc/cmd/node.go | 7 +++++++ apps/cinc/cmd/node_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/apps/cinc/cmd/node.go b/apps/cinc/cmd/node.go index 1114ed8..eaf23d4 100644 --- a/apps/cinc/cmd/node.go +++ b/apps/cinc/cmd/node.go @@ -285,6 +285,13 @@ cinc node bootstrap web01.example.com --ssh-user ubuntu --policy-name base --pol if err != nil { return err } + // The bootstrap script hard-codes the server URL into the client.rb + // it writes, so the profile must actually name a server. A real run + // gets this from resolveClient below, but a dry run never calls it + // and would otherwise print a script pointing at "/organizations/". + if err := profile.Validate(); err != nil { + return err + } var privateKey string if !flags.dryRun { c, err := resolveClient(cmd) diff --git a/apps/cinc/cmd/node_test.go b/apps/cinc/cmd/node_test.go index 462bdf3..8529d0a 100644 --- a/apps/cinc/cmd/node_test.go +++ b/apps/cinc/cmd/node_test.go @@ -440,6 +440,38 @@ func TestNodeBootstrapRejectsEnvironmentWithPolicy(t *testing.T) { } } +// TestNodeBootstrapDryRunRejectsProfileWithoutServer covers a profile that is +// perfectly valid on disk but names no Cinc Server, such as a Supermarket-only +// one. A real bootstrap catches this in resolveClient; a dry run never calls +// it, and used to emit a script whose client.rb pointed at "/organizations/". +func TestNodeBootstrapDryRunRejectsProfileWithoutServer(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "credentials") + cfg := fmt.Sprintf(`[default] +supermarket_site = "https://supermarket.cinc.sh" +client_name = "tim" +client_key = %q +`, writeTestKey(t)) + if err := os.WriteFile(cfgPath, []byte(cfg), 0o600); err != nil { + t.Fatal(err) + } + + root := newRootCmd() + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(new(bytes.Buffer)) + root.SetArgs([]string{ + "node", "bootstrap", "web01.example.test", + "--ssh-user", "ubuntu", "--config", cfgPath, "--dry-run", + }) + + if err := root.Execute(); err == nil { + t.Fatalf("dry run accepted a profile with no server URL and printed:\n%s", out.String()) + } + if strings.Contains(out.String(), "chef_server_url") { + t.Errorf("dry run emitted a bootstrap script anyway:\n%s", out.String()) + } +} + func TestNodeBootstrapDryRunCommand(t *testing.T) { cfgPath := writeCommandConfig(t, "https://cinc.example.test") root := newRootCmd()