build(deploy-camunda): address review round 2 on secrets/env preflight

- config env: flip --show-origin default to false so the default output is
  the machine-parseable NAME/VALUE pair; --show-origin adds the ORIGIN column,
  matching git config precedent.
- config init: suppress terminal echo for promptSecret via x/term.ReadPassword
  on a TTY; piped/test input keeps the buffered line reader.
- config: add ErrDeploymentExists sentinel; config init matches it with
  errors.Is instead of substring-matching the error text.
- preflight: add a chart-path check that resolves the path like root.go and
  fails red on a bad path, so doctor no longer reports falsely green when its
  PersistentPreRunE is skipped.
- merge.go / preflight.go / test-secret-mapping.yaml: correct the SkipPreflight
  comment (matrix leaves it false), trim why-narration per the HOW-only policy,
  and drop the self-contradictory "no recompile needed" note.
- preflight_test: isolate buildScenarioEnv seed test from a real .env via
  t.Chdir; add TestCheckChartPath.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: eamonnmoloney

scripts/deploy-camunda/cmd/config_env.go

@@ -12,8 +12,9 @@ import (
 )
 
 // newEnvCommand creates `config env`: prints the effective environment a deploy
-// would see and which layer each value came from, with secret values masked.
-// Mirrors `git config --show-origin`.
+// would see, with secret values masked. Default output is two machine-parseable
+// columns (NAME, VALUE); --show-origin adds an ORIGIN column, mirroring
+// `git config` (plain by default, origin column only when asked).
 func newEnvCommand() *cobra.Command {
 	var showOrigin bool
 	var unmask bool
@@ -55,7 +56,7 @@ unless --unmask is passed.`,
 		},
 	}
 
-	cmd.Flags().BoolVar(&showOrigin, "show-origin", true, "Show which layer each value came from")
+	cmd.Flags().BoolVar(&showOrigin, "show-origin", false, "Add an ORIGIN column showing which layer each value came from")
 	cmd.Flags().BoolVar(&unmask, "unmask", false, "Show secret values in clear text (key/secret/password/token)")
 	return cmd
 }

scripts/deploy-camunda/cmd/config_init.go

@@ -19,6 +19,7 @@ import (
 	"scripts/prepare-helm-values/pkg/env"
 
 	"github.com/spf13/cobra"
+	"golang.org/x/term"
 )
 
 // newInitCommand creates the `config init` wizard: an interactive first-run
@@ -77,7 +78,7 @@ checklist without prompting.`,
 			}
 			if err := config.CreateDeployment(cfgRes.Path, name); err != nil {
 				// Already-exists is fine — we'll update it in place.
-				if !strings.Contains(err.Error(), "already exists") {
+				if !errors.Is(err, config.ErrDeploymentExists) {
 					return err
 				}
 				fmt.Fprintf(out, "  profile %q already exists — updating it\n", name)
@@ -246,16 +247,47 @@ func promptLine(ctx context.Context, out io.Writer, r *bufio.Reader, label, def
 }
 
 func promptSecret(ctx context.Context, out io.Writer, r *bufio.Reader, label string) (string, error) {
-	// Note: input is not hidden (matches existing env.Prompt behavior); we simply
-	// never echo the value back afterwards.
 	fmt.Fprintf(out, "%s: ", label)
+	// On a real terminal, read with echo disabled so the secret never appears on
+	// screen or in scrollback. Piped/redirected input (tests, scripts) has
+	// nothing to hide and falls back to the buffered line reader.
+	stdinFd := int(os.Stdin.Fd())
+	if r.Buffered() == 0 && term.IsTerminal(stdinFd) {
+		secret, err := readSecretCtx(ctx, stdinFd)
+		fmt.Fprintln(out)
+		return secret, err
+	}
 	line, err := readLineCtx(ctx, r)
 	if err != nil {
 		return "", err
 	}
 	return strings.TrimSpace(line), nil
 }
 
+// readSecretCtx reads an echo-suppressed line from the terminal fd, aborting if
+// ctx is cancelled. Like readLineCtx, the parked term.ReadPassword goroutine is
+// an accepted leak for this single-run CLI.
+func readSecretCtx(ctx context.Context, fd int) (string, error) {
+	type res struct {
+		b   []byte
+		err error
+	}
+	ch := make(chan res, 1)
+	go func() {
+		b, err := term.ReadPassword(fd)
+		ch <- res{b, err}
+	}()
+	select {
+	case <-ctx.Done():
+		return "", ctx.Err()
+	case rr := <-ch:
+		if rr.err != nil && !errors.Is(rr.err, io.EOF) {
+			return "", rr.err
+		}
+		return strings.TrimSpace(string(rr.b)), nil
+	}
+}
+
 func promptYesNo(ctx context.Context, out io.Writer, r *bufio.Reader, label string, def bool) (bool, error) {
 	suffix := "y/N"
 	if def {

scripts/deploy-camunda/config/config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -629,6 +630,10 @@ func GetValue(cfgPath, key string) (string, error) {
 	return formatValue(val), nil
 }
 
+// ErrDeploymentExists is returned by CreateDeployment when the named deployment
+// is already present. Callers match it with errors.Is to take an update path.
+var ErrDeploymentExists = errors.New("deployment already exists")
+
 // CreateDeployment creates a new empty deployment configuration.
 func CreateDeployment(cfgPath, name string) error {
 	content, err := os.ReadFile(cfgPath)
@@ -655,7 +660,7 @@ func CreateDeployment(cfgPath, name string) error {
 
 	// Check if deployment already exists
 	if _, exists := deployments[name]; exists {
-		return fmt.Errorf("deployment %q already exists", name)
+		return fmt.Errorf("deployment %q: %w", name, ErrDeploymentExists)
 	}
 
 	// Create empty deployment

scripts/deploy-camunda/config/merge.go

@@ -190,10 +190,9 @@ type RuntimeFlags struct {
 	ConfigFound bool
 
 	// SkipPreflight disables the fail-fast secrets/env preflight that
-	// deploy.Execute runs before any cluster mutation. The matrix runner sets
-	// this true because it performs its own pre-dispatch docker login and
-	// kube-context warmup; direct deploys leave it false so missing inputs
-	// surface up front rather than mid-deploy.
+	// deploy.Execute runs before any cluster mutation. Set via --skip-preflight
+	// as an escape hatch; both direct deploys and matrix entries leave it false
+	// so missing inputs surface up front rather than mid-deploy.
 	SkipPreflight bool
 
 	// ChangedFlags tracks which CLI flags were explicitly set by the user.

scripts/deploy-camunda/deploy/data/test-secret-mapping.yaml

@@ -1,9 +1,10 @@
 # Vault secret mapping scaffolded by --auto-generate-secrets and `config init`.
 #
 # This file is the single source of truth for which environment variables are
-# packed into the test Secret. Edit it directly (no recompile needed beyond a
-# rebuild of the embedding binary) instead of the old hardcoded string in
-# secrets.go. All variables share one vault path, so the path is declared once.
+# packed into the test Secret, replacing the old hardcoded string in secrets.go.
+# It is baked into the binary via //go:embed at compile time, so any edit here
+# only takes effect after rebuilding deploy-camunda. All variables share one
+# vault path, so the path is declared once.
 #
 # generateTestSecrets random-fills the four DISTRO_QA_E2E_TESTS_* values when
 # they are unset; the remaining IDP_*/OPENAI_* values are read from the

scripts/deploy-camunda/deploy/preflight.go

@@ -1,10 +1,5 @@
-// Package deploy preflight.go implements a self-diagnosing check of the
-// secrets/environment setup a deployment needs, mirroring tools like
-// `flyctl doctor` and `gh auth status`. The same checks back both the
-// standalone `deploy-camunda doctor` command and the fail-fast guard that runs
-// before any cluster mutation, so a missing credential surfaces up front with a
-// remediation hint instead of mid-deploy as an ImagePullBackOff or a
-// `kubectl apply` parse error.
+// Package deploy preflight.go runs the secrets/environment checks that back
+// both `deploy-camunda doctor` and the fail-fast guard in deploy.Execute.
 package deploy
 
 import (
@@ -101,10 +96,16 @@ func Preflight(ctx context.Context, flags *config.RuntimeFlags, opts PreflightOp
 	// and companion placeholder checks don't false-positive on values the deploy
 	// supplies itself. Vault-mapping and docker checks use baseEnv — those vars are
 	// never deploy-computed.
+	// Resolve the chart path first: doctor skips root.go's PersistentPreRunE, so
+	// the check both validates the path and rewrites flags.Chart.ChartPath to the
+	// resolved directory the scenario checks below depend on.
+	chartCheck := checkChartPath(flags)
+
 	baseEnv := effectiveEnv(flags)
 	deployEnv := scenarioDeployEnv(flags, baseEnv)
 
 	r.Checks = append(r.Checks, checkConfigFile(opts))
+	r.Checks = append(r.Checks, chartCheck)
 	r.Checks = append(r.Checks, checkKubeContext(ctx, flags, opts))
 	r.Checks = append(r.Checks, checkDockerCredentials(flags, baseEnv)...)
 	r.Checks = append(r.Checks, checkVaultMapping(flags, baseEnv))
@@ -312,6 +313,36 @@ func checkVaultMapping(flags *config.RuntimeFlags, envMap map[string]string) Che
 	}
 }
 
+// checkChartPath resolves the configured chart path the same way root.go's
+// PersistentPreRunE does — a relative path is joined against repoRoot — and
+// fails when it does not resolve to a directory. On success it rewrites
+// flags.Chart.ChartPath to the resolved path so the scenario/companion checks
+// scan the same directory a deploy would.
+func checkChartPath(flags *config.RuntimeFlags) Check {
+	path := strings.TrimSpace(flags.Chart.ChartPath)
+	if path == "" {
+		return Check{Name: "chart path", Status: StatusOK, Detail: "no chart configured"}
+	}
+	if !filepath.IsAbs(path) && flags.Chart.RepoRoot != "" {
+		if _, err := os.Stat(path); err != nil {
+			resolved := filepath.Join(flags.Chart.RepoRoot, path)
+			if fi, err := os.Stat(resolved); err == nil && fi.IsDir() {
+				path = resolved
+			}
+		}
+	}
+	if fi, err := os.Stat(path); err != nil || !fi.IsDir() {
+		return Check{
+			Name:        "chart path",
+			Status:      StatusFail,
+			Detail:      fmt.Sprintf("%q does not exist or is not a directory", path),
+			Remediation: "set --repo-root/--chart/--version or --chart-path to a valid chart directory",
+		}
+	}
+	flags.Chart.ChartPath = path
+	return Check{Name: "chart path", Status: StatusOK, Detail: path}
+}
+
 // checkScenarioEnv scans the values file(s) for the selected scenario(s) and
 // reports any $PLACEHOLDER env vars that are unset. Resolution failures are a
 // soft warning rather than a hard fail so `doctor` is still useful when no

scripts/deploy-camunda/deploy/preflight_test.go

@@ -204,6 +204,10 @@ func TestCheckScenarioEnvScansLayeredFiles(t *testing.T) {
 // scenarios fail locally on the $VENOM_CLIENT_ID/$CONNECTORS_CLIENT_ID
 // placeholders. flags.ExtraEnv (used by OIDC/Entra) must still override.
 func TestBuildScenarioEnvSeedsKeycloakClientIDs(t *testing.T) {
+	// Isolate from any real .env in the checkout: buildScenarioEnv now defaults
+	// EnvFile to ".env" in CWD, which would otherwise leak local values over the
+	// seed-override assertions below.
+	t.Chdir(t.TempDir())
 	ctx := &ScenarioContext{}
 	t.Run("seeds keycloak defaults", func(t *testing.T) {
 		env, err := buildScenarioEnv(ctx, &config.RuntimeFlags{})
@@ -230,6 +234,41 @@ func TestBuildScenarioEnvSeedsKeycloakClientIDs(t *testing.T) {
 	})
 }
 
+func TestCheckChartPath(t *testing.T) {
+	dir := t.TempDir()
+
+	t.Run("no chart configured is OK", func(t *testing.T) {
+		if c := checkChartPath(&config.RuntimeFlags{}); c.Status != StatusOK {
+			t.Errorf("status = %q, want ok", c.Status)
+		}
+	})
+
+	t.Run("nonexistent path fails red", func(t *testing.T) {
+		f := &config.RuntimeFlags{}
+		f.Chart.ChartPath = filepath.Join(dir, "does-not-exist")
+		if c := checkChartPath(f); c.Status != StatusFail {
+			t.Errorf("status = %q, want fail", c.Status)
+		}
+	})
+
+	t.Run("relative path resolves against repoRoot", func(t *testing.T) {
+		chart := filepath.Join(dir, "charts", "camunda-platform-8.10")
+		if err := os.MkdirAll(chart, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		f := &config.RuntimeFlags{}
+		f.Chart.RepoRoot = dir
+		f.Chart.ChartPath = filepath.Join("charts", "camunda-platform-8.10")
+		c := checkChartPath(f)
+		if c.Status != StatusOK {
+			t.Fatalf("status = %q, want ok", c.Status)
+		}
+		if f.Chart.ChartPath != chart {
+			t.Errorf("ChartPath = %q, want resolved %q", f.Chart.ChartPath, chart)
+		}
+	})
+}
+
 func TestCheckCompanionEnv(t *testing.T) {
 	withCharts := func(charts ...config.CompanionChart) *config.RuntimeFlags {
 		return &config.RuntimeFlags{CompanionCharts: charts}

scripts/deploy-camunda/go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/mattn/go-runewidth v0.0.24
 	github.com/spf13/cobra v1.10.2
 	github.com/spf13/pflag v1.0.10
+	golang.org/x/term v0.44.0
 	gopkg.in/yaml.v3 v3.0.1
 	scripts/camunda-core v0.0.0
 	scripts/camunda-deployer v0.0.0
@@ -60,7 +61,6 @@ require (
 	golang.org/x/oauth2 v0.27.0 // indirect
 	golang.org/x/sync v0.20.0 // indirect
 	golang.org/x/sys v0.46.0 // indirect
-	golang.org/x/term v0.44.0 // indirect
 	golang.org/x/text v0.23.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect