From 3cd20a3eef98634a735eb145248b6793947a820c Mon Sep 17 00:00:00 2001 From: Anthony Wong Date: Wed, 9 Sep 2026 14:35:41 +0000 Subject: [PATCH] feat(cli): port provisioning hardening to v1.0 --- .gitignore | 4 + SPEC.md | 206 ------ cmd/cli/cli.go | 324 +++++++-- cmd/cli/cli_provision_test.go | 239 ++++++ cmd/cli/commands.go | 65 +- cmd/cli/commands_diag.go | 473 ++++++++++++ cmd/cli/commands_diag_test.go | 429 +++++++++++ cmd/cli/commands_start_test.go | 152 ++++ cmd/cli/hostname.go | 70 +- cmd/cli/hostname_test.go | 62 ++ cmd/cli/prog.go | 30 +- cmd/cli/prog_intercept_fallback_test.go | 2 +- cmd/cli/provision_boundary_guard_test.go | 243 +++++++ cmd/cli/provision_input_test.go | 417 +++++++++++ cmd/cli/provision_result.go | 121 +++- cmd/cli/provision_result_test.go | 24 +- cmd/cli/provision_terminal_paths_test.go | 244 +++++++ docs/provisioning-failure-codes.md | 137 +++- internal/controld/config.go | 45 +- internal/controld/config_test.go | 110 +++ internal/controld/reachability.go | 33 + internal/controld/reachability_test.go | 14 + tasks/plan.md | 213 ------ tasks/todo.md | 21 - .../darwin/test-pkg-intercept-mode.sh | 20 +- .../test-postinstall-provision-failure.sh | 680 +++++++++++++++--- 26 files changed, 3706 insertions(+), 672 deletions(-) delete mode 100644 SPEC.md create mode 100644 cmd/cli/commands_diag.go create mode 100644 cmd/cli/commands_diag_test.go create mode 100644 cmd/cli/provision_boundary_guard_test.go create mode 100644 cmd/cli/provision_input_test.go create mode 100644 cmd/cli/provision_terminal_paths_test.go create mode 100644 internal/controld/reachability.go create mode 100644 internal/controld/reachability_test.go delete mode 100644 tasks/plan.md delete mode 100644 tasks/todo.md diff --git a/.gitignore b/.gitignore index 799011f..e9121bf 100644 --- a/.gitignore +++ b/.gitignore @@ -14,3 +14,7 @@ ctrld-* cmd/cli/rsrc_*.syso ctrld ctrld.exe + +# Local planning artifacts - never commit +/SPEC.md +/tasks/ diff --git a/SPEC.md b/SPEC.md deleted file mode 100644 index 706baee..0000000 --- a/SPEC.md +++ /dev/null @@ -1,206 +0,0 @@ -# SPEC: Stable customer-visible provisioning failure codes - -Issue: [#586](https://gitlab.int.windscribe.com/controld/clients/ctrld/-/issues/586) -Requested by: Catt Garrod (@catt). Scope expanded by: Anthony Wong (@anthony). - -## 1. Objective - -Terminal provisioning failures in ctrld — bootstrap/API setup, listener -binding, and service installation/startup — must produce a stable, -support-facing failure identifier that survives process exit and reaches -both manual CLI users and MDM-driven installs. A customer or admin reports -one code; Support maps it to a scenario and a next action without asking -for reruns or verbose logs. - -Motivating incident (v1.5.5, macOS): provisioning reached the Control D -API, then died with only `FTL listener.0 could not find available listen -ip and port`. The per-address UDP/TCP bind errors existed only at Info -level in an in-memory logger and vanished on exit. The macOS pkg -`postinstall` discards ctrld's stdout/stderr entirely and judges success -by plist existence, so nothing useful reached the MDM log. - -**Users:** end customers and IT admins reporting failures; Support agents -triaging them; MDM/RMM operators reading installer logs. - -### Failure contract (agreed design) - -Three surfaces, all carrying the same identifier: - -1. **Result file** — on terminal provisioning failure, ctrld writes a - small redacted JSON file (atomic write: temp + rename) in the ctrld - home directory (same base dir as the internal `ctrld.log`, - via `absHomeDir`). Removed/overwritten on later successful - provisioning so stale failures don't mislead. Schema: - - ```json - { - "version": 1, - "timestamp": "2026-08-18T12:00:00Z", - "stage": "listener", - "code": "LISTENER_BIND_FAILED", - "exit_code": 41, - "message": "could not find available listen ip and port", - "detail": { - "attempts": [ - {"addr": "127.0.0.1:53", "proto": "udp", "os_error": "address already in use"} - ] - } - } - ``` - - `detail` is bounded (cap recorded bind attempts; cap string lengths) - and redacted by construction: no provisioning tokens, resolver IDs, - config contents, or unrelated host data. - -2. **Exit code + final stderr line** — the installer-facing command - (`ctrld start`, and `ctrld run` when run manually in the foreground) - exits with a stage-scoped code and prints one final line containing - the string code and stage, e.g. - `provisioning failed: stage=listener code=LISTENER_BIND_FAILED (exit 41)`. - -3. **Installer log (MDM path)** — `scripts/pkg/postinstall` stops - discarding the signal: it captures `ctrld start`'s output to a - private temp file, extracts only the fixed-charset identifier line - (`stage=[a-z]* code=[A-Z_]* (exit [0-9]*)` — structurally unable to - carry the token), and echoes it with the exit code into the - installer log. The result file's `message`/`detail` fields are - deliberately never surfaced there. The plist-existence check remains - the final success gate. - -### Identifier format - -- **Primary identifier: stable string codes.** Initial set — - bootstrap: `API_UNREACHABLE`, `API_REJECTED`, `API_DEVICE_INVALID`; - listener: `LISTENER_BIND_FAILED`, `LISTENER_CONFIGURED_ADDR_UNAVAILABLE`; - service: `SERVICE_INSTALL_FAILED`, `SERVICE_START_FAILED`, - `SERVICE_SELFCHECK_FAILED`. Codes are append-only; renames are new - codes plus a deprecation note in the mapping doc. -- **Secondary: stage-scoped process exit codes** as a coarse machine - signal: bootstrap 30–39, listener 40–49, service install/start 50–59. - Each string code owns one exit code. Existing contracts are untouched: - `ctrld status` 0–3, deactivation-pin 126, success 0. -- One underlying failure maps to one code on every path (manual CLI and - MDM), on both branches. - -### Propagation (daemon → installer) - -The listener/bootstrap fatals fire inside the daemon process -(`ctrld run` under launchd/systemd/SCM), not in `ctrld start`. The -daemon writes the result file before exiting; the existing log-socket -exit notification (`notifyExitToLogServer`) already unblocks `ctrld -start`'s self-check. `ctrld start` then reads the result file, prints -the identifier, and exits with the mapped stage exit code. The daemon's -own exit-status semantics toward service managers are preserved — -in particular the deliberate exit-0 on permanent API rejection that -protects the restart-policy budget; the result file carries the failure -identity in that case. - -### Support mapping - -`docs/provisioning-failure-codes.md` in this repo: one row per code — -code, stage, exit code, failure scenario, next safe troubleshooting -action or evidence request. Updated in the same MR whenever a code is -added or changed. - -### Branch scope - -Full implementation on **both** `v1.0` (release line for v1.5.5) and -`master`. The branches diverge heavily (`v1.0`: zerolog fork, -`commands.go`, `service_status.go`, macOS pkg scripts; `master`: zap, -inline commands, no pkg scripts), so this is one shared contract -(codes, exit-code ranges, file schema, doc) implemented twice, as two -MRs referencing #586. - -## 2. Commands - -- Build: `go build ./...` -- Test: `go test ./cmd/cli/...` (full: `go test ./...`) -- Vet: `go vet ./...` -- Branch workflow: feature branch off `v1.0` for the v1.0 MR; separate - feature branch off `master` for the port MR. Rebase, never merge the - base branch in. - -## 3. Project structure - -New and touched files on `v1.0` (master port mirrors the same contract -at its equivalent emission points in its `cli.go`): - -- `cmd/cli/provision_result.go` (new) — stage + code enums, exit-code - mapping, result-file schema, atomic write/read/clear helpers, - bounded/redacted detail builders. Pattern follows `service_status.go` - (small file: named constants + classifier + dedicated tests). -- `cmd/cli/provision_result_test.go` (new). -- `cmd/cli/cli.go` — emission points: `run()` bootstrap failure branches - (permanent rejection, invalid-device, fatal fetch), and - `tryUpdateListenerConfig` / `tryUpdateListenerConfigIntercept` fatals, - which now record per-attempt `{addr, proto, os_error}` bind detail. -- `cmd/cli/commands.go` — `initStartCmd`: doTasks install/start failures - and the self-check failure branch read the result file, print the - identifier, and exit with the stage code (replacing bare `os.Exit(1)` - on those paths). -- `scripts/pkg/postinstall` — propagate exit code + result-file contents - into the installer log (v1.0 only; master has no pkg scripts). -- `docs/provisioning-failure-codes.md` (new) — support mapping. - -## 4. Code style - -- Per repo conventions and global rules: guard clauses, small functions, - descriptive names, explicit error handling — never weaken existing - handling (e.g. keep the permanent-rejection exit-0 rationale intact). -- Comments only for non-obvious constraints (e.g. why the daemon must - still exit 0 on permanent rejection), simple-english, self-contained — - no issue/MR references in code. -- Match each branch's logging idiom: zerolog fork on `v1.0`, zap on - `master`. No new dependencies. -- Conventional Commits; MR titles in simple-english; both MRs reference - #586 (release-line MR carries `Closes #586`). - -## 5. Testing strategy - -Test-first where the harness allows. Coverage required by the issue: - -- **Code/mapping unit tests** — every string code maps to exactly one - stage and one in-range exit code; ranges don't collide with existing - contracts (0–3 status, 126 pin). -- **Result file round-trip** — write/read/clear; atomic write; stale - file removed on success. -- **Redaction** — serialize a result built from inputs containing a - provision token, resolver ID, and config content; assert none appear. -- **Listener bind failure (regression test for the incident)** — occupy - a port, drive the listener-config path to exhaustion, assert the - result records `LISTENER_BIND_FAILED` with attempted address, UDP/TCP - operation, and OS error (`address already in use`-class). -- **Bootstrap failures** — mock API: permanent 4xx → `API_REJECTED`; - invalid-device 40402 → `API_DEVICE_INVALID`; unreachable → - `API_UNREACHABLE`. -- **Service install/start/self-check failures** — injected task - failures assert code selection and `ctrld start` exit code. -- **MDM surface** — shell-level check of `postinstall` failure branch - (result file present → correct log line and exit), aligned with the - existing `test-scripts/` approach; manual pkg verification steps - documented in the MR. -- Both branches: the shared contract tests exist on both; branch-specific - emission tests match each branch's structure. - -## 6. Boundaries - -**Always:** -- Redact tokens, resolver IDs, config contents, host data from every - customer-visible surface (result file, stderr line, installer log). -- Preserve existing exit-code contracts (`ctrld status` 0–3, pin 126) - and the daemon's service-manager-facing exit semantics. -- Bound all recorded detail (attempt counts, string lengths). -- Keep codes append-only once merged. - -**Ask first:** -- Changing the daemon's (`ctrld run` under a service manager) exit codes - or restart-relevant behavior beyond writing the result file. -- Adding any persisted file outside the ctrld home directory. -- Expanding scope to runtime (post-provisioning) failures — this ticket - owns terminal provisioning failures only. - -**Never:** -- Print or persist the provisioning token (the reason postinstall - discards output today — the replacement surface must stay token-free). -- Auto-detect or kill conflicting processes (explicitly out of scope). -- Break `ctrld status`'s documented exit-code contract. diff --git a/cmd/cli/cli.go b/cmd/cli/cli.go index 98dc763..2c4a6a3 100644 --- a/cmd/cli/cli.go +++ b/cmd/cli/cli.go @@ -24,6 +24,7 @@ import ( "strings" "sync/atomic" "time" + "unicode" "github.com/Masterminds/semver/v3" "github.com/cuonglm/osinfo" @@ -140,6 +141,7 @@ func initCLI() { initClientsCmd() initUpgradeCmd() initLogCmd() + initDiagCmd() } // isMobile reports whether the current OS is a mobile platform. @@ -183,8 +185,12 @@ func isStableVersion(vs string) bool { // RunCobraCommand runs ctrld cli. func RunCobraCommand(cmd *cobra.Command) { noConfigStart = isNoConfigStart(cmd) - checkStrFlagEmpty(cmd, cdUidFlagName) - checkStrFlagEmpty(cmd, cdOrgFlagName) + if !checkStrFlagEmpty(cmd, cdUidFlagName) { + return + } + if !checkStrFlagEmpty(cmd, cdOrgFlagName) { + return + } run(nil, make(chan struct{})) } @@ -223,7 +229,8 @@ func CheckDeactivationPin(pin int64, stopCh chan struct{}) int { // run runs ctrld cli with given app callback and stop channel. func run(appCallback *AppCallback, stopCh chan struct{}) { if stopCh == nil { - mainLog.Load().Fatal().Msg("stopCh is nil") + failRunUnclassified(mainLog.Load().Error(), "run() called with a nil stop channel", nil) + return } waitCh := make(chan struct{}) p := &prog{ @@ -261,14 +268,10 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { } else { mainLog.Load().Warn().Err(err).Msgf("unable to resolve socket address: %s", sockPath) } - notifyExitToLogServer := func() { - if p.logConn != nil { - _, _ = p.logConn.Write([]byte(msgExit)) - } - } if daemon && runtime.GOOS == "windows" { - mainLog.Load().Fatal().Msg("Cannot run in daemon mode. Please install a Windows service.") + failRunUnclassified(mainLog.Load().Error(), "cannot run in daemon mode; please install a Windows service", p.notifyExitToLogServer) + return } if !daemon { @@ -277,7 +280,8 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { go func() { s, err := newService(p, svcConfig) if err != nil { - mainLog.Load().Fatal().Err(err).Msg("failed create new service") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to create new service: %v", err), p.notifyExitToLogServer) + return } if err := s.Run(); err != nil { mainLog.Load().Error().Err(err).Msg("failed to start service") @@ -288,18 +292,20 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { tryReadingConfig(writeDefaultConfig) if err := readBase64Config(configBase64); err != nil { - mainLog.Load().Fatal().Err(err).Msg("failed to read base64 config") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to read base64 config: %v", err), p.notifyExitToLogServer) + return } processNoConfigFlags(noConfigStart) // After s.Run() was called, if ctrld is going to be terminated for any reason, // write msgExit to p.logConn so others (like "ctrld start") won't have to wait for timeout. p.mu.Lock() - if err := v.Unmarshal(&cfg); err != nil { - notifyExitToLogServer() - mainLog.Load().Fatal().Msgf("failed to unmarshal config: %v", err) - } + unmarshalErr := v.Unmarshal(&cfg) p.mu.Unlock() + if unmarshalErr != nil { + failRunUnclassified(mainLog.Load().Error(), fmt.Sprintf("failed to unmarshal config: %v", unmarshalErr), p.notifyExitToLogServer) + return + } processLogAndCacheFlags(v, &cfg) @@ -312,8 +318,8 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { // Wait for network up. if !ctrldnet.Up() { - notifyExitToLogServer() - mainLog.Load().Fatal().Msg("network is not up yet") + failRunUnclassified(mainLog.Load().Error(), "network is not up yet", p.notifyExitToLogServer) + return } p.router = router.New(&cfg, cdUID != "") @@ -327,8 +333,8 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { // time for validating server certificate. Some routers need NTP synchronization // to set the current time, so this check must happen before processCDFlags. if err := p.router.PreRun(); err != nil { - notifyExitToLogServer() - mainLog.Load().Fatal().Err(err).Msg("failed to perform router pre-run check") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to perform router pre-run check: %v", err), p.notifyExitToLogServer) + return } oldLogPath := cfg.Service.LogPath @@ -336,7 +342,9 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { cdUID = uid } if cdUID != "" { - validateCdUpstreamProtocol() + if !validateCdUpstreamProtocol(p.notifyExitToLogServer) { + return + } // Bound API preflight by the service lifetime. Without this, a stop request // arriving while the API is unreachable leaves this retry/backoff loop running // after "service stopped" was logged, so the process keeps working on behalf of @@ -353,7 +361,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { // not see a failed start and apply its restart policy to a service the // operator just asked to stop. mainLog.Load().Notice().Msg("stop requested while fetching resolver config, shutting down") - notifyExitToLogServer() + p.notifyExitToLogServer() return case pf.err != nil: if isMobile() { @@ -361,7 +369,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { return } - handleAPIPreflightFailure(p, pf.err, notifyExitToLogServer) + handleAPIPreflightFailure(p, pf.err, p.notifyExitToLogServer) return default: p.mu.Lock() @@ -370,7 +378,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { } } - updated := updateListenerConfig(&cfg, notifyExitToLogServer) + updated := updateListenerConfig(&cfg, p.notifyExitToLogServer) // Bootstrap and listener binding both succeeded, so an earlier run's // recorded failure no longer describes this install. @@ -390,11 +398,10 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { if updated { if err := writeConfigFile(&cfg); err != nil { - notifyExitToLogServer() - mainLog.Load().Fatal().Err(err).Msg("failed to write config file") - } else { - mainLog.Load().Info().Msg("writing config file to: " + defaultConfigFile) + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to write config file: %v", err), p.notifyExitToLogServer) + return } + mainLog.Load().Info().Msg("writing config file to: " + defaultConfigFile) } if newLogPath := cfg.Service.LogPath; newLogPath != "" && oldLogPath != newLogPath { @@ -412,31 +419,28 @@ func run(appCallback *AppCallback, stopCh chan struct{}) { } if err := validateConfig(&cfg); err != nil { - notifyExitToLogServer() - os.Exit(1) + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("invalid config: %v", err), p.notifyExitToLogServer) + return } initCache() if daemon { exe, err := os.Executable() if err != nil { - mainLog.Load().Error().Err(err).Msg("failed to find the binary") - notifyExitToLogServer() - os.Exit(1) + failRunUnclassified(mainLog.Load().Error().Err(err), "failed to find the binary", p.notifyExitToLogServer) + return } curDir, err := os.Getwd() if err != nil { - mainLog.Load().Error().Err(err).Msg("failed to get current working directory") - notifyExitToLogServer() - os.Exit(1) + failRunUnclassified(mainLog.Load().Error().Err(err), "failed to get current working directory", p.notifyExitToLogServer) + return } // If running as daemon, re-run the command in background, with daemon off. cmd := exec.Command(exe, append(os.Args[1:], "-d=false")...) cmd.Dir = curDir if err := cmd.Start(); err != nil { - mainLog.Load().Error().Err(err).Msg("failed to start process as daemon") - notifyExitToLogServer() - os.Exit(1) + failRunUnclassified(mainLog.Load().Error().Err(err), "failed to start process as daemon", p.notifyExitToLogServer) + return } mainLog.Load().Info().Int("pid", cmd.Process.Pid).Msg("DNS proxy started") os.Exit(0) @@ -741,9 +745,10 @@ func permanentAPIRejection(err error) (*controld.ErrorResponse, bool) { } // apiFailureCode maps a bootstrap preflight error to its provisioning code. -// A deleted device gets its own code because it triggers self-uninstall; -// other permanent rejections are generic; anything else counts as -// reachability trouble worth retrying. +// A deleted device gets its own code because it triggers self-uninstall; a +// permanent rejection with a known token reason gets its own code; other +// permanent rejections are generic; anything else counts as reachability +// trouble worth retrying. func apiFailureCode(err error) (provisionFailureCode, bool) { if err == nil { return "", false @@ -752,12 +757,62 @@ func apiFailureCode(err error) (provisionFailureCode, bool) { if errors.As(err, &uer) && uer.ErrorField.Code == controld.InvalidConfigCode { return provisionCodeAPIDeviceInvalid, true } - if _, ok := permanentAPIRejection(err); ok { + if rejection, ok := permanentAPIRejection(err); ok { + if code, ok := tokenFailureCodeForReason(rejection.ErrorField.Metadata.Reason); ok { + return code, true + } return provisionCodeAPIRejected, true } return provisionCodeAPIUnreachable, true } +// tokenFailureCodeForReason maps error.metadata.reason on a provisioning-token +// rejection to its failure code. An empty or unrecognized reason is not +// classified here; the caller falls back to the generic rejection code. +func tokenFailureCodeForReason(reason string) (provisionFailureCode, bool) { + switch reason { + case controld.ReasonTokenInvalid: + return provisionCodeTokenInvalid, true + case controld.ReasonTokenExpired: + return provisionCodeTokenExpired, true + case controld.ReasonTokenLimitReached: + return provisionCodeTokenLimitReached, true + case controld.ReasonTokenDisabled: + return provisionCodeTokenDisabled, true + default: + return "", false + } +} + +// tokenRejectionMessage returns the human message for a TOKEN_* code: what is +// wrong with the provisioning code and one concrete next action. Empty for +// any other code, so callers know to fall back to a generic message. Never +// includes the token itself. +func tokenRejectionMessage(code provisionFailureCode) string { + switch code { + case provisionCodeTokenInvalid: + return "the provisioning code is not valid; check the code and re-enter it exactly as given" + case provisionCodeTokenExpired: + return "the provisioning code has expired; get a new provisioning code from your administrator" + case provisionCodeTokenLimitReached: + return "the provisioning code reached its device limit; free up a device slot or use a different code" + case provisionCodeTokenDisabled: + return "the provisioning code was invalidated; download a profile from an active provisioning code" + default: + return "" + } +} + +// provisionTokenFailureMessage returns the result-file message for a failed +// provision-token exchange: a token-specific message when the API gave a +// known reason, the generic exchange-failure summary otherwise. +func provisionTokenFailureMessage(code provisionFailureCode, err error) string { + if msg := tokenRejectionMessage(code); msg != "" { + return msg + } + return fmt.Sprintf("provision token exchange failed: %v", err) +} + // apiRejectionSummary reports the HTTP status only. The API's raw error body // can echo back the value the caller sent, so it stays out of the artifact. func apiRejectionSummary(statusCode int) string { @@ -1215,7 +1270,15 @@ func selfCheckResolveDomain(ctx context.Context, addr, scope string, domain stri return errSelfCheckNoAnswer } -func userHomeDir() (string, error) { +// unixServiceHomeDir is where a ctrld service with root rights keeps its +// files on macOS, Linux, and the BSDs. +const unixServiceHomeDir = "/etc/controld" + +// serviceHomeDir returns the directory where a ctrld service with root or +// administrator rights keeps its files. It has no fallback to the home +// directory of the current user. A caller without root that only reads can +// thus look where the service wrote. +func serviceHomeDir() (string, error) { dir, err := router.HomeDir() if err != nil { return "", err @@ -1237,7 +1300,17 @@ func userHomeDir() (string, error) { if isMobile() { return homedir, nil } - dir = "/etc/controld" + return unixServiceHomeDir, nil +} + +// userHomeDir is serviceHomeDir with a fallback: when the current user +// cannot write to the unix service home, it returns the home directory of +// that user instead. +func userHomeDir() (string, error) { + dir, err := serviceHomeDir() + if err != nil || dir != unixServiceHomeDir { + return dir, err + } if err := os.MkdirAll(dir, 0750); err != nil { return os.UserHomeDir() // fallback to user home directory } @@ -1898,6 +1971,84 @@ func osVersion() string { return oi.String() } +// provisionTokenMinLen, provisionTokenMaxLen, and provisionTokenExpectedPrefix +// describe the shape of a --cd-org value the input stage checks before any +// network call. +const ( + provisionTokenMinLen = 6 + provisionTokenMaxLen = 64 + provisionTokenExpectedPrefix = "org-v1-" +) + +// provisionTokenShapeValid reports whether token looks like something the API +// could parse: a length between provisionTokenMinLen and provisionTokenMaxLen, +// with no whitespace or control characters. It does not require the +// "org-v1-" prefix - legacy provisioning codes predate that convention and +// are handled with a warning, not a failure. +func provisionTokenShapeValid(token string) bool { + if len(token) < provisionTokenMinLen || len(token) > provisionTokenMaxLen { + return false + } + for _, r := range token { + if unicode.IsSpace(r) || unicode.IsControl(r) { + return false + } + } + return true +} + +// validateProvisionTokenShape classifies an obviously malformed --cd-org +// value (PROVISION_TOKEN_MALFORMED) before any network call, so a typo or a +// pasted URL fails fast instead of going through a doomed API round trip. A +// token missing the "org-v1-" prefix only gets a logged warning: legacy +// provisioning codes may lack it and still work. +func validateProvisionTokenShape() bool { + if !provisionTokenShapeValid(cdOrg) { + msg := fmt.Sprintf("--cd-org provisioning code is malformed: must be %d-%d characters with no whitespace or control characters", provisionTokenMinLen, provisionTokenMaxLen) + failProvision(newProvisionResult(provisionCodeProvisionTokenMalformed, msg, nil, provisionSecrets()...), nil) + return false + } + if !strings.HasPrefix(cdOrg, provisionTokenExpectedPrefix) { + mainLog.Load().Warn().Msg("--cd-org provisioning code does not have the expected org-v1- prefix; continuing, since legacy codes may lack it") + } + return true +} + +// validateCustomHostnameFlag classifies an invalid --custom-hostname value +// (CUSTOM_HOSTNAME_INVALID) before any network call. ctrld's accept/reject +// rule (validHostname) is unchanged - this only names the field, the +// offending character(s), and the allowed format instead of a bare fatal +// exit. ControlD's device-name formatting folds or strips some characters +// ctrld still accepts (dot, space, plus), so a hostname using one of those +// gets a one-line notice instead: the registered device name may differ from +// what was requested. +func validateCustomHostnameFlag() bool { + if customHostname == "" { + return true + } + if !validHostname(customHostname) { + failProvision(newProvisionResult(provisionCodeCustomHostnameInvalid, customHostnameFailureMessage(customHostname), nil, provisionSecrets()...), nil) + return false + } + if hostnameMayBeFoldedByServer(customHostname) { + mainLog.Load().Notice().Msgf("device name %q contains characters ControlD may fold or strip when it registers the device; the registered name may differ", customHostname) + } + return true +} + +// validateInterceptModeFlag classifies an invalid --intercept-mode value +// (INTERCEPT_MODE_INVALID) on the provisioning boundary, so a typo fails with +// a stable code instead of a bare fatal exit an installer wrapper cannot tell +// apart from any other crash. An empty mode means the flag was not set. +func validateInterceptModeFlag(mode string) bool { + if mode == "" || validInterceptMode(mode) { + return true + } + msg := fmt.Sprintf("--intercept-mode %q is not valid: must be 'off', 'dns', or 'hard'", mode) + failProvision(newProvisionResult(provisionCodeInterceptModeInvalid, msg, nil, provisionSecrets()...), nil) + return false +} + // cdUIDFromProvToken fetch UID from ControlD API using provision token. func cdUIDFromProvToken() string { // --cd flag supersedes --cd-org, ignore it if both are supplied. @@ -1908,9 +2059,11 @@ func cdUIDFromProvToken() string { if cdOrg == "" { return "" } - // Validate custom hostname if provided. - if customHostname != "" && !validHostname(customHostname) { - mainLog.Load().Fatal().Msgf("invalid custom hostname: %q", customHostname) + if !validateProvisionTokenShape() { + return "" + } + if !validateCustomHostnameFlag() { + return "" } req := &controld.UtilityOrgRequest{ @@ -1926,7 +2079,7 @@ func cdUIDFromProvToken() string { code, _ := apiFailureCode(err) mainLog.Load().Error().Msgf("failed to fetch resolver uid with provision token: %s: %s", redactToken(cdOrg), redactSecrets(err.Error(), provisionSecrets()...)) - failProvision(newProvisionResult(code, fmt.Sprintf("provision token exchange failed: %v", err), nil, provisionSecrets()...), nil) + failProvision(newProvisionResult(code, provisionTokenFailureMessage(code, err), nil, provisionSecrets()...), nil) return "" } return resolverConfig.UID @@ -2032,32 +2185,55 @@ func newSocketControlClientMobile(dir string, stopCh chan struct{}) *controlClie } // checkStrFlagEmpty validates if a string flag was set to an empty string. -// If yes, emitting a fatal error message. -func checkStrFlagEmpty(cmd *cobra.Command, flagName string) { +// An explicit empty --cd-org is a malformed provisioning token, classified on +// the provisioning boundary; every other flag keeps the bare fatal, since +// this helper is shared with flags outside that boundary. Returns false when +// the caller must stop: provisionExit is stubbed out under test, so nothing +// else stops execution from falling through. +func checkStrFlagEmpty(cmd *cobra.Command, flagName string) bool { fl := cmd.Flags().Lookup(flagName) - if !fl.Changed || fl.Value.Type() != "string" { - return + if !fl.Changed || fl.Value.Type() != "string" || fl.Value.String() != "" { + return true } - if fl.Value.String() == "" { - mainLog.Load().Fatal().Msgf(`flag "--%s" value must be non-empty`, fl.Name) + if flagName == cdOrgFlagName { + msg := fmt.Sprintf("--%s provisioning code is malformed: value must be non-empty", cdOrgFlagName) + failProvision(newProvisionResult(provisionCodeProvisionTokenMalformed, msg, nil, provisionSecrets()...), nil) + return false } + mainLog.Load().Fatal().Msgf(`flag "--%s" value must be non-empty`, fl.Name) + return false } -func validateCdUpstreamProtocol() { +// validateCdUpstreamProtocol validates the Control D upstream protocol, +// classified on the provisioning boundary since it only runs once --cd is +// set. notify unblocks a waiting "ctrld start" on the daemon side; the parent +// process, which has no one waiting, passes nil. Returns false when the +// caller must stop. +func validateCdUpstreamProtocol(notify func()) bool { if cdUID == "" { - return + return true } switch cdUpstreamProto { case ctrld.ResolverTypeDOH, ctrld.ResolverTypeDOH3: + return true default: - mainLog.Load().Fatal().Msg(`flag "--protocol" must be "doh" or "doh3"`) + msg := fmt.Sprintf("--proto %q is not valid: must be 'doh' or 'doh3'", cdUpstreamProto) + failProvision(newProvisionResult(provisionCodeInvalidFlagCombination, msg, nil, provisionSecrets()...), notify) + return false } } -func validateCdAndNextDNSFlags() { +// validateCdAndNextDNSFlags validates that Control D and NextDNS flags are +// not used together, classified on the provisioning boundary since it only +// fires once --cd or --cd-org is set. Returns false when the caller must +// stop. +func validateCdAndNextDNSFlags() bool { if (cdUID != "" || cdOrg != "") && nextdns != "" { - mainLog.Load().Fatal().Msgf("--%s/--%s could not be used with --%s", cdUidFlagName, cdOrgFlagName, nextdnsFlagName) + msg := fmt.Sprintf("--%s/--%s cannot be used together with --%s", cdUidFlagName, cdOrgFlagName, nextdnsFlagName) + failProvision(newProvisionResult(provisionCodeInvalidFlagCombination, msg, nil, provisionSecrets()...), nil) + return false } + return true } // removeNextDNSFromArgs removes the --nextdns from command line arguments. @@ -2274,7 +2450,25 @@ func runningIface(s service.Service) *ifaceResponse { return nil } +// apiRejectionMessage returns the result-file message for a permanent API +// rejection: the HTTP status for a device-invalid or generic rejection, or +// the raw error for anything else (network trouble, an unreachable API). +func apiRejectionMessage(err error) string { + var uer *controld.ErrorResponse + if errors.As(err, &uer) && uer.ErrorField.Code == controld.InvalidConfigCode { + return apiRejectionSummary(uer.StatusCode) + } + if rejection, ok := permanentAPIRejection(err); ok { + return apiRejectionSummary(rejection.StatusCode) + } + return fmt.Sprintf("failed to fetch resolver config: %v", err) +} + // doValidateCdRemoteConfig fetches and validates custom config for cdUID. +// fatal distinguishes the two callers: a direct install-time caller passes +// true and classifies a fetch failure on the provisioning boundary; the +// restart path passes false and gets the error back to decide for itself, +// with no process exit. func doValidateCdRemoteConfig(cdUID string, fatal bool) error { // Username is only sent during initial provisioning (cdUIDFromProvToken). // All subsequent calls use lightweight metadata to avoid EDR triggers. @@ -2283,16 +2477,16 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error { Version: rootCmd.Version, Metadata: ctrld.SystemMetadataRuntime(context.Background()), } - rc, err := controld.FetchResolverConfig(context.Background(), req, cdDev) + rc, err := fetchResolverConfig(context.Background(), req, cdDev) if err != nil { - logger := mainLog.Load().Fatal() - if !fatal { - logger = mainLog.Load().Warn() - } - logger.Err(err).Err(err).Msgf("failed to fetch resolver uid: %s", cdUID) if !fatal { + mainLog.Load().Warn().Err(err).Msgf("failed to fetch resolver config for %s", redactToken(cdUID)) return err } + code, _ := apiFailureCode(err) + mainLog.Load().Error().Err(err).Msgf("failed to fetch resolver config for %s", redactToken(cdUID)) + failProvision(newProvisionResult(code, apiRejectionMessage(err), nil, provisionSecrets()...), nil) + return err } // return earlier if there's no custom config. diff --git a/cmd/cli/cli_provision_test.go b/cmd/cli/cli_provision_test.go index 34628da..0eeae63 100644 --- a/cmd/cli/cli_provision_test.go +++ b/cmd/cli/cli_provision_test.go @@ -2,6 +2,8 @@ package cli import ( "context" + "encoding/json" + "errors" "fmt" "net" "net/http" @@ -73,6 +75,140 @@ func TestApiFailureCode(t *testing.T) { } } +// TestApiFailureCodeMapsRejectionReason covers the token_* reasons the API sends in +// error.metadata.reason on a provisioning-token rejection. An absent or unknown +// reason must fall back to the generic API_REJECTED exactly as before this reason +// field existed. +func TestApiFailureCodeMapsRejectionReason(t *testing.T) { + rejectionWithReason := func(reason string) error { + e := &controld.ErrorResponse{StatusCode: http.StatusBadRequest} + e.ErrorField.Code = 40003 + e.ErrorField.Message = "invalid token" + e.ErrorField.Metadata.Reason = reason + return e + } + + tests := []struct { + name string + reason string + wantCode provisionFailureCode + }{ + {name: "token_invalid", reason: "token_invalid", wantCode: provisionCodeTokenInvalid}, + {name: "token_expired", reason: "token_expired", wantCode: provisionCodeTokenExpired}, + {name: "token_limit_reached", reason: "token_limit_reached", wantCode: provisionCodeTokenLimitReached}, + {name: "token_disabled", reason: "token_disabled", wantCode: provisionCodeTokenDisabled}, + {name: "reason absent falls back", reason: "", wantCode: provisionCodeAPIRejected}, + {name: "unknown reason falls back", reason: "some_future_reason", wantCode: provisionCodeAPIRejected}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + code, ok := apiFailureCode(rejectionWithReason(tc.reason)) + if !ok { + t.Fatal("apiFailureCode() ok = false, want true") + } + if code != tc.wantCode { + t.Errorf("apiFailureCode() code = %s, want %s", code, tc.wantCode) + } + }) + } +} + +// TestApiFailureCodeSurvivesMalformedReasonType covers a rejection body whose +// metadata.reason is the wrong JSON type end to end: decode it exactly as +// internal/controld does (json.Unmarshal into the same exported type), then +// classify it. Before the metadata decode fix, this body failed the whole +// decode and apiFailureCode never saw an *ErrorResponse at all, so it fell +// back to API_UNREACHABLE - the retryable bootstrap code - instead of the +// permanent rejection this HTTP 400 with a known error code actually is. +func TestApiFailureCodeSurvivesMalformedReasonType(t *testing.T) { + tests := []struct { + name string + body string + }{ + {name: "reason as a number", body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":12345}}}`}, + {name: "reason as an object", body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":{"inner":"value"}}}}`}, + {name: "reason as null", body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":null}}}`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + e := &controld.ErrorResponse{StatusCode: http.StatusBadRequest} + if err := json.Unmarshal([]byte(tc.body), e); err != nil { + t.Fatalf("a malformed reason must not fail the whole decode: %v", err) + } + code, ok := apiFailureCode(e) + if !ok { + t.Fatal("apiFailureCode() ok = false, want true") + } + if code != provisionCodeAPIRejected { + t.Errorf("apiFailureCode() code = %s, want %s (not %s)", code, provisionCodeAPIRejected, provisionCodeAPIUnreachable) + } + }) + } +} + +// TestCdUIDFromProvTokenReasonCodes covers the full path from an API rejection +// reason to a persisted result file: each known reason gets its own code, exit +// code, and stage, with a message naming the field and a next action but never +// echoing the token. Absent and unknown reasons keep the generic rejection. +func TestCdUIDFromProvTokenReasonCodes(t *testing.T) { + const secretToken = "org-secret-token-999" + + tests := []struct { + name string + reason string + wantCode provisionFailureCode + wantContains string + }{ + {name: "token_invalid", reason: "token_invalid", wantCode: provisionCodeTokenInvalid, wantContains: "provisioning code"}, + {name: "token_expired", reason: "token_expired", wantCode: provisionCodeTokenExpired, wantContains: "expired"}, + {name: "token_limit_reached", reason: "token_limit_reached", wantCode: provisionCodeTokenLimitReached, wantContains: "limit"}, + {name: "token_disabled", reason: "token_disabled", wantCode: provisionCodeTokenDisabled, wantContains: "invalidated"}, + {name: "reason absent", reason: "", wantCode: provisionCodeAPIRejected, wantContains: ""}, + {name: "unknown reason", reason: "brand_new_reason", wantCode: provisionCodeAPIRejected, wantContains: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldFetch, oldHostname := fetchResolverUIDFn, customHostname + t.Cleanup(func() { fetchResolverUIDFn, customHostname = oldFetch, oldHostname }) + cdUID = "" + cdOrg = secretToken + customHostname = "" + + rejected := &controld.ErrorResponse{StatusCode: http.StatusBadRequest} + rejected.ErrorField.Code = 40003 + rejected.ErrorField.Message = "invalid token " + secretToken + rejected.ErrorField.Metadata.Reason = tc.reason + fetchResolverUIDFn = func(context.Context, *controld.UtilityOrgRequest, string, bool) (*controld.ResolverConfig, error) { + return nil, rejected + } + + if got := cdUIDFromProvToken(); got != "" { + t.Errorf("cdUIDFromProvToken() = %q, want empty on failure", got) + } + if *exitCode != provisionExitCodeForCode[tc.wantCode] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[tc.wantCode]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(tc.wantCode) { + t.Errorf("code = %q, want %q", r.Code, tc.wantCode) + } + if r.Stage != string(provisionStageBootstrap) { + t.Errorf("stage = %q, want bootstrap", r.Stage) + } + if tc.wantContains != "" && !strings.Contains(r.Message, tc.wantContains) { + t.Errorf("message = %q, want it to contain %q", r.Message, tc.wantContains) + } + if strings.Contains(r.Message, secretToken) { + t.Errorf("token leaked into result message: %q", r.Message) + } + }) + } +} + func stubProvisionGlobals(t *testing.T) (exitCode *int, notified *bool) { t.Helper() oldCdUID, oldCdOrg := cdUID, cdOrg @@ -179,6 +315,109 @@ func TestHandleAPIPreflightFailure(t *testing.T) { }) } +// TestDoValidateCdRemoteConfigClassifiesAPIFailure covers the fatal=true +// caller: a fetch failure must classify on the provisioning boundary with +// the same per-class codes as the daemon-side preflight, instead of a bare +// fatal. +func TestDoValidateCdRemoteConfigClassifiesAPIFailure(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldFetch := fetchResolverConfig + t.Cleanup(func() { fetchResolverConfig = oldFetch }) + + deviceInvalid := func() error { + e := &controld.ErrorResponse{StatusCode: http.StatusNotFound} + e.ErrorField.Code = controld.InvalidConfigCode + e.ErrorField.Message = "device does not exist" + return e + } + rejected := func() error { + e := &controld.ErrorResponse{StatusCode: http.StatusUnauthorized} + e.ErrorField.Message = "bad token" + return e + } + + tests := []struct { + name string + err error + wantCode provisionFailureCode + }{ + {name: "device invalid", err: deviceInvalid(), wantCode: provisionCodeAPIDeviceInvalid}, + {name: "permanent rejection", err: rejected(), wantCode: provisionCodeAPIRejected}, + {name: "unreachable", err: retryableNetworkErr(), wantCode: provisionCodeAPIUnreachable}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fetchResolverConfig = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + return nil, tc.err + } + if err := doValidateCdRemoteConfig("device-uid-123", true); err == nil { + t.Error("doValidateCdRemoteConfig() error = nil, want the fetch error back") + } + if *exitCode != provisionExitCodeForCode[tc.wantCode] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[tc.wantCode]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(tc.wantCode) { + t.Errorf("code = %q, want %q", r.Code, tc.wantCode) + } + }) + } +} + +// TestDoValidateCdRemoteConfigNonFatalReturnsError proves the restart path +// (fatal=false) is unaffected: it still just warns and hands the error back, +// with no process exit and no result file. +func TestDoValidateCdRemoteConfigNonFatalReturnsError(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldFetch := fetchResolverConfig + t.Cleanup(func() { fetchResolverConfig = oldFetch }) + + wantErr := errors.New("network unreachable") + fetchResolverConfig = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + return nil, wantErr + } + + if err := doValidateCdRemoteConfig("device-uid-123", false); !errors.Is(err, wantErr) { + t.Errorf("doValidateCdRemoteConfig() error = %v, want %v", err, wantErr) + } + if *exitCode != -1 { + t.Errorf("provisionExit called with %d, want no exit", *exitCode) + } + if _, err := readProvisionResult(); err == nil { + t.Error("expected no provision result written for the non-fatal path") + } +} + +// TestDoValidateCdRemoteConfigDoesNotSelfUninstall proves a device-invalid +// failure on this path never triggers self-uninstall: unlike the daemon-side +// bootstrap preflight, callers passing fatal=true run before any service is +// installed, so there is nothing to remove. +func TestDoValidateCdRemoteConfigDoesNotSelfUninstall(t *testing.T) { + _, _ = stubProvisionGlobals(t) + oldFetch, oldUninstall := fetchResolverConfig, uninstallInvalidCdUIDFn + t.Cleanup(func() { fetchResolverConfig, uninstallInvalidCdUIDFn = oldFetch, oldUninstall }) + + uninstallCalled := false + uninstallInvalidCdUIDFn = func(*prog, zerolog.Logger, bool) bool { + uninstallCalled = true + return true + } + e := &controld.ErrorResponse{StatusCode: http.StatusNotFound} + e.ErrorField.Code = controld.InvalidConfigCode + fetchResolverConfig = func(context.Context, *controld.ResolverConfigRequest, bool) (*controld.ResolverConfig, error) { + return nil, e + } + + _ = doValidateCdRemoteConfig("device-uid-123", true) + + if uninstallCalled { + t.Error("doValidateCdRemoteConfig triggered self-uninstall; nothing is installed yet on this path") + } +} + func TestCdUIDFromProvTokenFailureEmitsCode(t *testing.T) { exitCode, _ := stubProvisionGlobals(t) oldFetch, oldHostname := fetchResolverUIDFn, customHostname diff --git a/cmd/cli/commands.go b/cmd/cli/commands.go index 6ebf5c7..9b057b4 100644 --- a/cmd/cli/commands.go +++ b/cmd/cli/commands.go @@ -342,9 +342,20 @@ NOTE: running "ctrld start" without any arguments will start already installed c return nil }, Run: func(cmd *cobra.Command, args []string) { - checkStrFlagEmpty(cmd, cdUidFlagName) - checkStrFlagEmpty(cmd, cdOrgFlagName) - validateCdAndNextDNSFlags() + // Clear before any check runs, not just before doTasksE: a result from a + // previous attempt must never survive to mislead diag/postinstall on this + // one, even if this attempt fails before reaching doTasksE. + clearProvisionResult() + + if !checkStrFlagEmpty(cmd, cdUidFlagName) { + return + } + if !checkStrFlagEmpty(cmd, cdOrgFlagName) { + return + } + if !validateCdAndNextDNSFlags() { + return + } sc := &service.Config{} *sc = *svcConfig osArgs := os.Args[2:] @@ -359,8 +370,8 @@ NOTE: running "ctrld start" without any arguments will start already installed c // Without this, a typo like "--intercept-mode fds" would install the service, // the child process would Fatal() on the invalid value, and the parent would // then uninstall — confusing and destructive. - if interceptMode != "" && !validInterceptMode(interceptMode) { - mainLog.Load().Fatal().Msgf("invalid --intercept-mode value %q: must be 'off', 'dns', or 'hard'", interceptMode) + if !validateInterceptModeFlag(interceptMode) { + return } p := &prog{ @@ -370,6 +381,9 @@ NOTE: running "ctrld start" without any arguments will start already installed c s, err := newService(p, sc) if err != nil { mainLog.Load().Error().Msg(err.Error()) + // A bare return would exit 0 with no result file, so support + // could not tell this failure from a start that never ran. + failProvisionUnclassified("initialize service: "+err.Error(), nil) return } p.preRun() @@ -397,7 +411,8 @@ NOTE: running "ctrld start" without any arguments will start already installed c // An explicit "off" argument must override a previously persisted config // value while the service clears that value on startup. if err := removeServiceFlag("--intercept-mode"); err != nil { - mainLog.Load().Fatal().Err(err).Msg("failed to remove existing intercept mode from service arguments") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to remove existing intercept mode from service arguments: %v", err), nil) + return } if interceptMode == "off" { @@ -406,10 +421,12 @@ NOTE: running "ctrld start" without any arguments will start already installed c mainLog.Load().Notice().Msgf("Existing service detected — appending --intercept-mode %s to service arguments", interceptMode) } if err := appendServiceFlag("--intercept-mode"); err != nil { - mainLog.Load().Fatal().Err(err).Msg("failed to append intercept flag to service arguments") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to append intercept flag to service arguments: %v", err), nil) + return } if err := appendServiceFlag(interceptMode); err != nil { - mainLog.Load().Fatal().Err(err).Msg("failed to append intercept mode value to service arguments") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to append intercept mode value to service arguments: %v", err), nil) + return } // Stop the service if running (bypasses ctrld pin — this is an @@ -538,7 +555,8 @@ NOTE: running "ctrld start" without any arguments will start already installed c if startOnly && isCtrldInstalled { tryReadingConfigWithNotice(false, true) if err := v.Unmarshal(&cfg); err != nil { - mainLog.Load().Fatal().Msgf("failed to unmarshal config: %v", err) + failRunUnclassified(mainLog.Load().Error(), fmt.Sprintf("failed to unmarshal config: %v", err), nil) + return } // if already running, dont restart @@ -565,8 +583,6 @@ NOTE: running "ctrld start" without any arguments will start already installed c {s.Start, true, "Start"}, {noticeWritingControlDConfig, false, "Notice writing ControlD config"}, } - // Any result found later must come from this attempt, not a stale run. - clearProvisionResult() startAttemptAt := time.Now() mainLog.Load().Notice().Msg("Starting existing ctrld service") failedTask, taskErr := doTasksE(tasks) @@ -575,12 +591,15 @@ NOTE: running "ctrld start" without any arguments will start already installed c failProvision(newProvisionResult(code, serviceTaskErrorSummary(failedTask, taskErr), nil, provisionSecrets()...), nil) return } - os.Exit(1) + // Not a service-stage task. doTasksE already logged the cause; classify + // UNCLASSIFIED instead of the old silent fall-through that exited 1. + failProvisionUnclassified(serviceTaskErrorSummary(failedTask, taskErr), nil) + return } sockDir, err := socketDir() if err != nil { - mainLog.Load().Warn().Err(err).Msg("Failed to get socket directory") - os.Exit(1) + failRunUnclassified(mainLog.Load().Error(), fmt.Sprintf("failed to get socket directory: %v", err), nil) + return } // The daemon can start and still fail provisioning (for example a @@ -623,11 +642,14 @@ NOTE: running "ctrld start" without any arguments will start already installed c sc.Arguments = append(sc.Arguments, "--cd="+cdUID) } if cdUID != "" { - validateCdUpstreamProtocol() + if !validateCdUpstreamProtocol(nil) { + return + } } if err := p.router.ConfigureService(sc); err != nil { - mainLog.Load().Fatal().Err(err).Msg("failed to configure service on router") + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("failed to configure service on router: %v", err), nil) + return } if configPath != "" { @@ -637,7 +659,8 @@ NOTE: running "ctrld start" without any arguments will start already installed c tryReadingConfigWithNotice(writeDefaultConfig, true) if err := v.Unmarshal(&cfg); err != nil { - mainLog.Load().Fatal().Msgf("failed to unmarshal config: %v", err) + failRunUnclassified(mainLog.Load().Error(), fmt.Sprintf("failed to unmarshal config: %v", err), nil) + return } initInteractiveLogging() @@ -682,8 +705,6 @@ NOTE: running "ctrld start" without any arguments will start already installed c // generated after s.Start, so we notice users here for consistent with nextdns mode. {noticeWritingControlDConfig, false, "Notice writing ControlD config"}, } - // Any result found later must come from this attempt, not a stale run. - clearProvisionResult() startAttemptAt := time.Now() mainLog.Load().Notice().Msg("Starting service") failedTask, taskErr := doTasksE(tasks) @@ -692,9 +713,9 @@ NOTE: running "ctrld start" without any arguments will start already installed c failProvision(newProvisionResult(code, serviceTaskErrorSummary(failedTask, taskErr), nil, provisionSecrets()...), nil) return } - // Not a service-stage task. doTasksE already logged the cause; exit - // non-zero instead of the old silent fall-through that exited 0. - os.Exit(1) + // Not a service-stage task. doTasksE already logged the cause; classify + // UNCLASSIFIED instead of the old silent fall-through that exited 0. + failProvisionUnclassified(serviceTaskErrorSummary(failedTask, taskErr), nil) return } diff --git a/cmd/cli/commands_diag.go b/cmd/cli/commands_diag.go new file mode 100644 index 0000000..566bc5f --- /dev/null +++ b/cmd/cli/commands_diag.go @@ -0,0 +1,473 @@ +package cli + +import ( + "context" + "crypto/tls" + "encoding/json" + "errors" + "fmt" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" + "unicode/utf8" + + "github.com/kardianos/service" + "github.com/spf13/cobra" + + "github.com/Control-D-Inc/ctrld/internal/controld" +) + +// diag reports the facts that support asks for on every provisioning +// ticket. A customer or admin can then paste the output of one command +// instead of a hunt through logs and preference panes. It runs without +// root. A section that needs data the current user cannot read reports +// that, instead of a failure of the whole command. On an installed device +// those are the provisioning-result and the service-state sections, because +// the service wrote the file as root and the service manager answers only +// root. +const diagCmdLong = `Collect diagnostics for a provisioning failure. + +Reports the client version, MDM-managed preferences (macOS only), the last +provisioning result, service state, and whether the Control D API is +reachable. Safe to paste into a support ticket: it never prints the +provisioning token itself, only whether one is present. + +Run it with root or administrator rights on an installed device to include +the last provisioning result and the service state. Without those rights, +the two sections report permission denied.` + +// diagAPIProbeTimeout bounds the API reachability check. diag must return +// promptly even when the network is unreachable. +const diagAPIProbeTimeout = 5 * time.Second + +// diagOverallTimeout bounds the whole report. No single probe, however +// wedged, may keep "ctrld diag" from returning. +const diagOverallTimeout = 15 * time.Second + +// diagServiceStateTimeout bounds one service-state probe. systemctl or +// launchctl can hang against a wedged service manager; the kardianos +// service package gives us no way to cancel that call, so we race it +// against this timer in a goroutine instead. +const diagServiceStateTimeout = 5 * time.Second + +// diagFieldMaxLen bounds any single field pulled from outside ctrld's own +// control (a managed-preferences value), so a misconfigured profile cannot +// blow up the report's size. +const diagFieldMaxLen = 256 + +// managedPrefsDomain is the MDM-managed preferences domain ctrld reads its +// provisioning settings from. +const managedPrefsDomain = "/Library/Managed Preferences/com.controld.ctrld" + +// managedPrefsBin is invoked with an absolute path so diag never depends on +// PATH. +const managedPrefsBin = "/usr/bin/defaults" + +// managedPrefsSupported reports whether this platform has managed +// preferences to read. A var so tests can exercise the macOS-shaped report +// on any OS. +var managedPrefsSupported = func() bool { return runtime.GOOS == "darwin" } + +// managedPrefsRead runs `defaults read [key]` and returns the +// trimmed value, or ok=false if the domain or key does not exist. A var so +// tests seam it instead of shelling out for real. It honors the caller's +// ctx so a near-expired overall deadline cuts this short too. +var managedPrefsRead = func(ctx context.Context, domain, key string) (string, bool) { + args := []string{"read", domain} + if key != "" { + args = append(args, key) + } + probeCtx, cancel := context.WithTimeout(ctx, 3*time.Second) + defer cancel() + out, err := exec.CommandContext(probeCtx, managedPrefsBin, args...).Output() + if err != nil { + return "", false + } + return strings.TrimSpace(string(out)), true +} + +// diagProbeReachability makes the one HTTPS probe api_reachability reports +// on. A var so tests replace it instead of hitting the network. +var diagProbeReachability = controld.ProbeReachability + +type diagReport struct { + ClientVersion string `json:"client_version"` + Commit string `json:"commit"` + ManagedPreferences diagManagedPreferences `json:"managed_preferences"` + ProvisionResult diagProvisionResult `json:"provision_result"` + ServiceState diagServiceState `json:"service_state"` + APIReachability diagAPIReachability `json:"api_reachability"` +} + +type diagManagedPreferences struct { + Applicable bool `json:"applicable"` + ProfilePresent bool `json:"profile_present"` + ProvisionToken string `json:"provision_token"` // "present" or "absent"; never the value + CustomHostname string `json:"custom_hostname"` + InterceptMode string `json:"intercept_mode"` + Note string `json:"note"` +} + +// diagProvisionResult mirrors the on-disk provision_result.json for report +// purposes. AgeSeconds is -1 when no age applies (nothing recorded, or the +// timestamp did not parse). Message and Attempts are re-bounded on read: the +// file was written bounded, but diag must not trust that a file on disk +// still is (a mismatched version, or hand-edited). +type diagProvisionResult struct { + Status string `json:"status"` // none, recorded, untrusted, unreadable, corrupt + Stage string `json:"stage"` + Code string `json:"code"` + ExitCode int `json:"exit_code"` + Message string `json:"message"` + Attempts []provisionBindAttempt `json:"attempts,omitempty"` + AgeSeconds int64 `json:"age_seconds"` +} + +type diagServiceState struct { + Status string `json:"status"` // running, stopped, not_installed, unknown + Note string `json:"note"` +} + +type diagAPIReachability struct { + Reachable bool `json:"reachable"` + ErrorClass string `json:"error_class"` // empty when reachable +} + +// diagProvisionResultPath locates the result file where the service wrote +// it. The shared home-dir resolver has a fallback to the home directory of +// the current user when /etc/controld is not writable, and every run +// without root hits that fallback. A diag that used it would report "none +// recorded" for a file it never looked at. A var so tests can point it at +// a temp dir. +var diagProvisionResultPath = func() string { + if homedir != "" { + return filepath.Join(homedir, provisionResultFileName) + } + dir, err := serviceHomeDir() + if err != nil { + return provisionResultPath() + } + return filepath.Join(dir, provisionResultFileName) +} + +// diagServiceStateFn collects service_state. A var so tests can supply a +// fixed state instead of depending on whatever service happens to be +// installed on the machine running the tests. +var diagServiceStateFn = collectServiceStateReal + +// buildDiagReport bounds the whole report at diagOverallTimeout: whatever +// deadline the caller passed in, a single probe wedging past this must not +// keep "ctrld diag" from returning. +func buildDiagReport(ctx context.Context) diagReport { + ctx, cancel := context.WithTimeout(ctx, diagOverallTimeout) + defer cancel() + return diagReport{ + ClientVersion: curVersion(), + Commit: commit, + ManagedPreferences: collectManagedPreferences(ctx), + ProvisionResult: collectProvisionResultDiag(), + ServiceState: collectServiceStateBounded(ctx), + APIReachability: collectAPIReachability(ctx), + } +} + +func collectManagedPreferences(ctx context.Context) diagManagedPreferences { + if !managedPrefsSupported() { + return diagManagedPreferences{Note: "not applicable on this platform"} + } + m := diagManagedPreferences{Applicable: true} + if _, ok := managedPrefsRead(ctx, managedPrefsDomain, ""); !ok { + m.Note = "configuration profile not found" + return m + } + m.ProfilePresent = true + // An empty value reads as absent: the postinstall refuses to provision + // on an empty ProvisionToken, so "present" would send support the wrong + // way. + if v, ok := managedPrefsRead(ctx, managedPrefsDomain, "ProvisionToken"); ok && v != "" { + m.ProvisionToken = "present" + } else { + m.ProvisionToken = "absent" + } + if v, ok := managedPrefsRead(ctx, managedPrefsDomain, "CustomHostname"); ok { + m.CustomHostname = boundedDiagField(v) + } + if v, ok := managedPrefsRead(ctx, managedPrefsDomain, "InterceptMode"); ok { + m.InterceptMode = boundedDiagField(v) + } + return m +} + +// boundedDiagField caps a value from outside ctrld's control to a size that +// keeps the report bounded, cutting on a rune boundary so it never splits a +// multi-byte character. +func boundedDiagField(s string) string { + if utf8.RuneCountInString(s) <= diagFieldMaxLen { + return s + } + runes := []rune(s) + return string(runes[:diagFieldMaxLen]) +} + +// collectProvisionResultDiag reads provision_result.json from the service +// home through the same trusted reader "ctrld start" uses, so diag never +// reports a code that contract validation would reject. A read the current +// user is not permitted to make is its own status: the file is not +// corrupt, the reader lacks root. +func collectProvisionResultDiag() diagProvisionResult { + r, err := readProvisionResultAt(diagProvisionResultPath()) + if err != nil { + if os.IsNotExist(err) { + return diagProvisionResult{Status: "none", AgeSeconds: -1} + } + if os.IsPermission(err) { + return diagProvisionResult{Status: "unreadable", AgeSeconds: -1} + } + return diagProvisionResult{Status: "corrupt", AgeSeconds: -1} + } + if !provisionResultTrusted(r) { + return diagProvisionResult{Status: "untrusted", AgeSeconds: -1} + } + age := int64(-1) + if ts, parseErr := time.Parse(time.RFC3339, r.Timestamp); parseErr == nil { + if d := time.Since(ts); d >= 0 { + age = int64(d.Round(time.Second).Seconds()) + } else { + age = 0 + } + } + var attempts []provisionBindAttempt + if r.Detail != nil { + attempts = boundedDiagAttempts(r.Detail.Attempts) + } + return diagProvisionResult{ + Status: "recorded", + Stage: r.Stage, + Code: r.Code, + ExitCode: r.ExitCode, + Message: boundedDiagField(r.Message), + Attempts: attempts, + AgeSeconds: age, + } +} + +// boundedDiagAttempts re-applies the same attempt-count cap the file was +// written with, and bounds each attempt's fields, so a result file from a +// mismatched or tampered version cannot make the report unbounded. +func boundedDiagAttempts(attempts []provisionBindAttempt) []provisionBindAttempt { + if len(attempts) > maxProvisionBindAttempts { + attempts = attempts[:maxProvisionBindAttempts] + } + bounded := make([]provisionBindAttempt, len(attempts)) + for i, a := range attempts { + bounded[i] = provisionBindAttempt{ + Addr: boundedDiagField(a.Addr), + Proto: boundedDiagField(a.Proto), + OSError: boundedDiagField(a.OSError), + } + } + return bounded +} + +// collectServiceStateReal is diagServiceStateFn's production implementation. +// It reuses the same service-manager wrapper "ctrld status" does, so a +// permission-limited run reports the OS's own permission error rather than a +// wrong status. +func collectServiceStateReal() diagServiceState { + s, err := newService(&prog{}, svcConfig) + if err != nil { + return diagServiceState{Status: "unknown", Note: "could not set up the service manager"} + } + status, statusErr := s.Status() + switch { + case errors.Is(statusErr, service.ErrNotInstalled): + return diagServiceState{Status: "not_installed"} + case statusErr != nil: + return diagServiceState{Status: "unknown", Note: boundedDiagField(statusErr.Error())} + case status == service.StatusRunning: + return diagServiceState{Status: "running"} + case status == service.StatusStopped: + return diagServiceState{Status: "stopped"} + default: + return diagServiceState{Status: "unknown"} + } +} + +// collectServiceStateBounded runs diagServiceStateFn in the background and +// races it against ctx and diagServiceStateTimeout, so a wedged service +// manager reports "timed out" instead of hanging the whole diag report. The +// goroutine is left running if the probe never returns; that is harmless +// since the process exits shortly after diag prints its report. +func collectServiceStateBounded(ctx context.Context) diagServiceState { + done := make(chan diagServiceState, 1) + go func() { done <- diagServiceStateFn() }() + + timer := time.NewTimer(diagServiceStateTimeout) + defer timer.Stop() + + select { + case s := <-done: + return s + case <-ctx.Done(): + return diagServiceState{Status: "unknown", Note: "timed out"} + case <-timer.C: + return diagServiceState{Status: "unknown", Note: "timed out"} + } +} + +func collectAPIReachability(ctx context.Context) diagAPIReachability { + probeCtx, cancel := context.WithTimeout(ctx, diagAPIProbeTimeout) + defer cancel() + if err := diagProbeReachability(probeCtx, cdDev); err != nil { + return diagAPIReachability{Reachable: false, ErrorClass: classifyReachabilityError(err)} + } + return diagAPIReachability{Reachable: true} +} + +// classifyReachabilityError turns a probe failure into a coarse class safe +// to print: no host, no address, no request details, just what kind of +// failure it was. +func classifyReachabilityError(err error) string { + if err == nil { + return "" + } + if errors.Is(err, context.DeadlineExceeded) { + return "timeout" + } + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return "timeout" + } + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) { + return "dns" + } + var certErr *tls.CertificateVerificationError + if errors.As(err, &certErr) { + return "tls" + } + var opErr *net.OpError + if errors.As(err, &opErr) { + return "connection" + } + return "other" +} + +// diagElevateHint names the step that gets root or administrator rights on +// this OS, for the text report. +func diagElevateHint() string { + if runtime.GOOS == "windows" { + return "run again from an administrator prompt" + } + return "run again with sudo" +} + +func displayOrDefault(s, def string) string { + if s == "" { + return def + } + return s +} + +// renderDiagText writes the text-mode report in a fixed section order, so +// output stays stable across runs and safe to diff or paste into a ticket. +func renderDiagText(w io.Writer, r diagReport) { + fmt.Fprintf(w, "client version: %s (commit %s)\n\n", r.ClientVersion, r.Commit) + + fmt.Fprintln(w, "managed preferences:") + switch { + case !r.ManagedPreferences.Applicable: + fmt.Fprintf(w, " %s\n", displayOrDefault(r.ManagedPreferences.Note, "not applicable")) + case !r.ManagedPreferences.ProfilePresent: + fmt.Fprintf(w, " %s\n", r.ManagedPreferences.Note) + default: + fmt.Fprintf(w, " provision token: %s\n", r.ManagedPreferences.ProvisionToken) + fmt.Fprintf(w, " custom hostname: %s\n", displayOrDefault(r.ManagedPreferences.CustomHostname, "(not set)")) + fmt.Fprintf(w, " intercept mode: %s\n", displayOrDefault(r.ManagedPreferences.InterceptMode, "(not set)")) + } + fmt.Fprintln(w) + + fmt.Fprintln(w, "last provisioning result:") + switch r.ProvisionResult.Status { + case "none": + fmt.Fprintln(w, " none recorded") + case "untrusted": + fmt.Fprintln(w, " result file present but not trusted (contents ignored)") + case "unreadable": + fmt.Fprintf(w, " permission denied (%s)\n", diagElevateHint()) + case "corrupt": + fmt.Fprintln(w, " result file present but could not be read (contents ignored)") + default: + fmt.Fprintf(w, " stage: %s\n", r.ProvisionResult.Stage) + fmt.Fprintf(w, " code: %s\n", r.ProvisionResult.Code) + fmt.Fprintf(w, " exit code: %d\n", r.ProvisionResult.ExitCode) + fmt.Fprintf(w, " message: %s\n", r.ProvisionResult.Message) + for _, a := range r.ProvisionResult.Attempts { + fmt.Fprintf(w, " attempt: %s/%s: %s\n", a.Addr, a.Proto, a.OSError) + } + fmt.Fprintf(w, " age: %s\n", (time.Duration(r.ProvisionResult.AgeSeconds) * time.Second).String()) + } + fmt.Fprintln(w) + + fmt.Fprintln(w, "service state:") + fmt.Fprintf(w, " status: %s\n", r.ServiceState.Status) + if r.ServiceState.Note != "" { + fmt.Fprintf(w, " note: %s\n", r.ServiceState.Note) + } + fmt.Fprintln(w) + + fmt.Fprintln(w, "api reachability:") + fmt.Fprintf(w, " reachable: %t\n", r.APIReachability.Reachable) + if r.APIReachability.ErrorClass != "" { + fmt.Fprintf(w, " error class: %s\n", r.APIReachability.ErrorClass) + } +} + +func writeDiagJSON(w io.Writer, r diagReport) error { + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(r) +} + +// initDiagCmd registers "ctrld diag" alongside the other top-level commands. +func initDiagCmd() *cobra.Command { + diagCmd := newDiagCmd() + rootCmd.AddCommand(diagCmd) + return diagCmd +} + +// newDiagCmd builds the "diag" command without registering it anywhere, so +// tests can attach it to an isolated root command instead of the package's +// shared one. +func newDiagCmd() *cobra.Command { + var asJSON bool + diagCmd := &cobra.Command{ + Use: "diag", + Short: "Collect diagnostics for a provisioning failure", + Long: diagCmdLong, + Args: cobra.NoArgs, + // diag always exits 0 once it ran: a failure it finds is reported, + // not turned into a nonzero exit. RunE returning an error would exit + // 1 (see Main), so every branch below reports instead of erroring. + RunE: func(cmd *cobra.Command, args []string) error { + report := buildDiagReport(context.Background()) + if asJSON { + if err := writeDiagJSON(cmd.OutOrStdout(), report); err != nil { + // A closed pipe (e.g. `ctrld diag --json | head -1`) must + // not turn into a nonzero exit; the report already ran. + mainLog.Load().Debug().Err(err).Msg("could not write diag JSON report") + } + return nil + } + renderDiagText(cmd.OutOrStdout(), report) + return nil + }, + } + diagCmd.Flags().BoolVar(&asJSON, "json", false, "print the report as JSON") + return diagCmd +} diff --git a/cmd/cli/commands_diag_test.go b/cmd/cli/commands_diag_test.go new file mode 100644 index 0000000..d347fc3 --- /dev/null +++ b/cmd/cli/commands_diag_test.go @@ -0,0 +1,429 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + "unicode/utf8" + + "github.com/spf13/cobra" +) + +// fakeProvisionToken stands in for a real provisioning code. Tests assert it +// never reaches either output mode. +const fakeProvisionToken = "org-v1-FAKE00000000000000000000TOKEN" + +func withManagedPrefsSeam(t *testing.T, supported bool, values map[string]string) { + t.Helper() + oldSupported := managedPrefsSupported + oldRead := managedPrefsRead + managedPrefsSupported = func() bool { return supported } + managedPrefsRead = func(_ context.Context, _, key string) (string, bool) { + v, ok := values[key] + return v, ok + } + t.Cleanup(func() { + managedPrefsSupported = oldSupported + managedPrefsRead = oldRead + }) +} + +// overrideDiagProvisionResultPath points the writer and diag at one temp +// file, so a test can write a result and read it back through diag. +func overrideDiagProvisionResultPath(t *testing.T) string { + t.Helper() + path := overrideProvisionResultPath(t) + old := diagProvisionResultPath + diagProvisionResultPath = func() string { return path } + t.Cleanup(func() { diagProvisionResultPath = old }) + return path +} + +func withServiceStateSeam(t *testing.T, state diagServiceState) { + t.Helper() + old := diagServiceStateFn + diagServiceStateFn = func() diagServiceState { return state } + t.Cleanup(func() { diagServiceStateFn = old }) +} + +func withAPIProbeSeam(t *testing.T, err error) { + t.Helper() + old := diagProbeReachability + diagProbeReachability = func(context.Context, bool) error { return err } + t.Cleanup(func() { diagProbeReachability = old }) +} + +func seedTrustedProvisionResult(t *testing.T, age time.Duration) { + t.Helper() + overrideDiagProvisionResultPath(t) + r := newProvisionResult(provisionCodeTokenExpired, "the provisioning code has expired", nil) + r.Timestamp = time.Now().Add(-age).UTC().Format(time.RFC3339) + if err := writeProvisionResult(r); err != nil { + t.Fatal(err) + } +} + +// seedOversizedProvisionResult writes a trusted result file straight to disk +// (bypassing newProvisionResult's own bounding), standing in for a file left +// by a mismatched or tampered version of ctrld. +func seedOversizedProvisionResult(t *testing.T) { + t.Helper() + overrideDiagProvisionResultPath(t) + attempts := make([]provisionBindAttempt, maxProvisionBindAttempts*3) + for i := range attempts { + attempts[i] = provisionBindAttempt{ + Addr: "0.0.0.0:53", + Proto: "udp", + OSError: strings.Repeat("e", diagFieldMaxLen*3), + } + } + r := &provisionResult{ + Version: 1, + Timestamp: time.Now().UTC().Format(time.RFC3339), + Stage: string(provisionStageListener), + Code: string(provisionCodeListenerBindFailed), + ExitCode: provisionExitCodeForCode[provisionCodeListenerBindFailed], + Message: strings.Repeat("m", diagFieldMaxLen*3), + Detail: &provisionDetail{Attempts: attempts}, + } + if err := writeProvisionResult(r); err != nil { + t.Fatal(err) + } +} + +func TestDiagProvisionResultBoundsOversizedFields(t *testing.T) { + seedOversizedProvisionResult(t) + + got := collectProvisionResultDiag() + + if n := utf8.RuneCountInString(got.Message); n > diagFieldMaxLen { + t.Errorf("message length = %d, want <= %d", n, diagFieldMaxLen) + } + if len(got.Attempts) > maxProvisionBindAttempts { + t.Errorf("attempts length = %d, want <= %d", len(got.Attempts), maxProvisionBindAttempts) + } + for _, a := range got.Attempts { + if n := utf8.RuneCountInString(a.OSError); n > diagFieldMaxLen { + t.Errorf("attempt os_error length = %d, want <= %d", n, diagFieldMaxLen) + } + } +} + +func TestDiagTextReportNeverLeaksToken(t *testing.T) { + withManagedPrefsSeam(t, true, map[string]string{ + "": "", // domain probe: profile present + "ProvisionToken": fakeProvisionToken, + "CustomHostname": "corp-laptop.example.com", + "InterceptMode": "intercept-dns", + }) + seedTrustedProvisionResult(t, 3*time.Minute+12*time.Second) + withServiceStateSeam(t, diagServiceState{Status: "stopped"}) + withAPIProbeSeam(t, nil) + + report := buildDiagReport(context.Background()) + var buf bytes.Buffer + renderDiagText(&buf, report) + out := buf.String() + + if strings.Contains(out, fakeProvisionToken) { + t.Fatalf("text output leaked the provision token: %s", out) + } + wantLines := []string{ + "provision token: present", + "custom hostname: corp-laptop.example.com", + "intercept mode: intercept-dns", + "stage: bootstrap", + "code: TOKEN_EXPIRED", + "exit code: 34", + "status: stopped", + "reachable: true", + } + for _, want := range wantLines { + if !strings.Contains(out, want) { + t.Errorf("text output missing %q, got:\n%s", want, out) + } + } +} + +func TestDiagJSONReportNeverLeaksToken(t *testing.T) { + withManagedPrefsSeam(t, true, map[string]string{ + "": "", + "ProvisionToken": fakeProvisionToken, + "CustomHostname": "corp-laptop.example.com", + "InterceptMode": "standard", + }) + seedTrustedProvisionResult(t, time.Minute) + withServiceStateSeam(t, diagServiceState{Status: "running"}) + withAPIProbeSeam(t, errors.New("dial tcp: connect: connection refused")) + + report := buildDiagReport(context.Background()) + var buf bytes.Buffer + if err := writeDiagJSON(&buf, report); err != nil { + t.Fatal(err) + } + out := buf.String() + if strings.Contains(out, fakeProvisionToken) { + t.Fatalf("JSON output leaked the provision token: %s", out) + } + + var decoded diagReport + if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil { + t.Fatalf("could not decode JSON report: %v", err) + } + if decoded.ManagedPreferences.ProvisionToken != "present" { + t.Errorf("provision_token = %q, want present", decoded.ManagedPreferences.ProvisionToken) + } + if decoded.ManagedPreferences.CustomHostname != "corp-laptop.example.com" { + t.Errorf("custom_hostname = %q", decoded.ManagedPreferences.CustomHostname) + } + if decoded.ProvisionResult.Status != "recorded" || decoded.ProvisionResult.Code != "TOKEN_EXPIRED" { + t.Errorf("provision_result = %+v", decoded.ProvisionResult) + } + if decoded.ServiceState.Status != "running" { + t.Errorf("service_state = %+v", decoded.ServiceState) + } + if decoded.APIReachability.Reachable { + t.Error("api_reachability.reachable = true, want false") + } + if decoded.APIReachability.ErrorClass == "" { + t.Error("api_reachability.error_class empty for an unreachable API") + } +} + +func TestDiagEmptyMachine(t *testing.T) { + overrideDiagProvisionResultPath(t) // temp dir, no result file written + withManagedPrefsSeam(t, false, nil) + withServiceStateSeam(t, diagServiceState{Status: "not_installed"}) + withAPIProbeSeam(t, context.DeadlineExceeded) + + report := buildDiagReport(context.Background()) + + if report.ManagedPreferences.Applicable { + t.Error("managed preferences reported applicable with no profile on this platform") + } + if report.ProvisionResult.Status != "none" { + t.Errorf("provision result status = %q, want none", report.ProvisionResult.Status) + } + if report.ServiceState.Status != "not_installed" { + t.Errorf("service state = %q, want not_installed", report.ServiceState.Status) + } + if report.APIReachability.Reachable { + t.Error("api reachability reported reachable with a forced timeout") + } + if report.APIReachability.ErrorClass != "timeout" { + t.Errorf("error class = %q, want timeout", report.APIReachability.ErrorClass) + } + + var buf bytes.Buffer + renderDiagText(&buf, report) + if !strings.Contains(buf.String(), "none recorded") { + t.Errorf("text output missing 'none recorded': %s", buf.String()) + } +} + +// Without root, diag must look where the root-run service wrote the result +// file, not in the home directory of the current user. +func TestDiagProvisionResultPathIgnoresUserHome(t *testing.T) { + want := "/etc/controld/" + provisionResultFileName + if runtime.GOOS == "windows" { + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + want = filepath.Join(filepath.Dir(exe), provisionResultFileName) + } + if got := diagProvisionResultPath(); got != want { + t.Errorf("diag provision result path = %q, want %q", got, want) + } +} + +// Diag must read where a daemon started with --homedir wrote, so it obeys +// the same override as the writer. +func TestDiagProvisionResultPathHonorsHomedir(t *testing.T) { + old := homedir + homedir = t.TempDir() + t.Cleanup(func() { homedir = old }) + want := filepath.Join(homedir, provisionResultFileName) + if got := diagProvisionResultPath(); got != want { + t.Errorf("diag provision result path = %q, want %q", got, want) + } +} + +// A result file the current user cannot read must report that, not +// "corrupt": the file is fine, the reader lacks root. +func TestDiagProvisionResultUnreadable(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file modes do not deny reads on Windows") + } + if os.Geteuid() == 0 { + t.Skip("root can read a 0000 file") + } + path := overrideDiagProvisionResultPath(t) + if err := os.WriteFile(path, []byte("{}"), 0o000); err != nil { + t.Fatal(err) + } + + r := collectProvisionResultDiag() + if r.Status != "unreadable" { + t.Errorf("status = %q, want unreadable", r.Status) + } + if r.AgeSeconds != -1 { + t.Errorf("age_seconds = %d, want -1", r.AgeSeconds) + } + + var buf bytes.Buffer + renderDiagText(&buf, diagReport{ProvisionResult: r}) + if !strings.Contains(buf.String(), "permission denied ("+diagElevateHint()+")") { + t.Errorf("text output does not name permission denied and the elevation step: %s", buf.String()) + } +} + +func TestDiagServiceStateHangYieldsTimeout(t *testing.T) { + old := diagServiceStateFn + // started closes the instant the background probe goroutine reads and + // invokes our stub. Cleanup waits for that before restoring the global: + // otherwise a slow-to-schedule goroutine can still be reading + // diagServiceStateFn when Cleanup writes to it, a data race on the shared + // package var (this test's stub is left running past the test's own + // return, same as production - see collectServiceStateBounded's doc). + started := make(chan struct{}) + diagServiceStateFn = func() diagServiceState { + close(started) + time.Sleep(2 * time.Second) // stand in for a wedged systemctl/launchctl + return diagServiceState{Status: "running"} + } + t.Cleanup(func() { + <-started + diagServiceStateFn = old + }) + + withManagedPrefsSeam(t, false, nil) + overrideDiagProvisionResultPath(t) + withAPIProbeSeam(t, nil) + + // A short deadline stands in for the overall 15s budget already having + // run low; the probe must still yield within it instead of hanging. + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + + start := time.Now() + report := buildDiagReport(ctx) + elapsed := time.Since(start) + + if elapsed > time.Second { + t.Fatalf("buildDiagReport took %s, want bounded well under the 2s hang", elapsed) + } + if report.ServiceState.Status != "unknown" { + t.Errorf("service state status = %q, want unknown", report.ServiceState.Status) + } + if !strings.Contains(report.ServiceState.Note, "timed out") { + t.Errorf("service state note = %q, want it to mention timing out", report.ServiceState.Note) + } +} + +func TestDiagManagedPrefsProfileAbsent(t *testing.T) { + withManagedPrefsSeam(t, true, map[string]string{}) // domain read fails: profile absent + m := collectManagedPreferences(context.Background()) + if m.ProfilePresent { + t.Error("profile reported present when the domain read failed") + } + if m.Note == "" { + t.Error("expected a note explaining the absent profile") + } +} + +func TestDiagManagedPrefsTokenAbsent(t *testing.T) { + withManagedPrefsSeam(t, true, map[string]string{"": ""}) // profile present, no keys set + m := collectManagedPreferences(context.Background()) + if !m.ProfilePresent { + t.Fatal("profile should be present") + } + if m.ProvisionToken != "absent" { + t.Errorf("provision token = %q, want absent", m.ProvisionToken) + } +} + +// An empty ProvisionToken value must read as absent: the postinstall refuses +// to provision on an empty token, so diag must not call it present. +func TestDiagManagedPrefsTokenEmpty(t *testing.T) { + withManagedPrefsSeam(t, true, map[string]string{"": "", "ProvisionToken": ""}) + m := collectManagedPreferences(context.Background()) + if !m.ProfilePresent { + t.Fatal("profile should be present") + } + if m.ProvisionToken != "absent" { + t.Errorf("provision token = %q, want absent", m.ProvisionToken) + } +} + +func TestClassifyReachabilityError(t *testing.T) { + if got := classifyReachabilityError(nil); got != "" { + t.Errorf("nil error class = %q, want empty", got) + } + if got := classifyReachabilityError(context.DeadlineExceeded); got != "timeout" { + t.Errorf("deadline exceeded class = %q, want timeout", got) + } +} + +func TestDiagCommandJSONFlag(t *testing.T) { + withManagedPrefsSeam(t, false, nil) + overrideDiagProvisionResultPath(t) + withServiceStateSeam(t, diagServiceState{Status: "not_installed"}) + withAPIProbeSeam(t, nil) + + testRootCmd := &cobra.Command{Use: "ctrld"} + testRootCmd.AddCommand(newDiagCmd()) + var buf bytes.Buffer + testRootCmd.SetOut(&buf) + testRootCmd.SetArgs([]string{"diag", "--json"}) + if err := testRootCmd.Execute(); err != nil { + t.Fatalf("diag --json returned error: %v", err) + } + var decoded diagReport + if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil { + t.Fatalf("diag --json did not print valid JSON: %v\n%s", err, buf.String()) + } +} + +// writerFailingAfter accepts the first n bytes written to it, then fails +// every write after that - standing in for a pipe closed by a downstream +// reader (`ctrld diag --json | head -1`). +type writerFailingAfter struct { + n int + written int +} + +func (w *writerFailingAfter) Write(p []byte) (int, error) { + if w.written >= w.n { + return 0, errors.New("write: broken pipe") + } + remaining := w.n - w.written + if len(p) > remaining { + w.written += remaining + return remaining, errors.New("write: broken pipe") + } + w.written += len(p) + return len(p), nil +} + +func TestDiagJSONWriteErrorStillExitsZero(t *testing.T) { + withManagedPrefsSeam(t, false, nil) + overrideDiagProvisionResultPath(t) + withServiceStateSeam(t, diagServiceState{Status: "not_installed"}) + withAPIProbeSeam(t, nil) + + testRootCmd := &cobra.Command{Use: "ctrld"} + testRootCmd.AddCommand(newDiagCmd()) + testRootCmd.SetOut(&writerFailingAfter{n: 10}) + testRootCmd.SetArgs([]string{"diag", "--json"}) + if err := testRootCmd.Execute(); err != nil { + t.Fatalf("diag --json with a failing writer returned error %v, want nil per the always-exit-0 contract", err) + } +} diff --git a/cmd/cli/commands_start_test.go b/cmd/cli/commands_start_test.go index 882fac2..2ac2ec4 100644 --- a/cmd/cli/commands_start_test.go +++ b/cmd/cli/commands_start_test.go @@ -1,10 +1,162 @@ package cli import ( + "go/ast" + "go/parser" + "go/token" + "os" + "strings" "testing" "time" ) +// startRunFuncSource extracts the source text of initStartCmd's "Run:" +// closure body from commands.go. Driving the closure itself end-to-end for +// every early-return branch is not practical in a unit test: within a few +// lines of any check failing, it reaches into the real OS service manager. +// Some invariants about its shape are cheaper and more reliable to pin by +// reading the source than by executing it. +func startRunFuncSource(t *testing.T) string { + t.Helper() + file := packageSourcePath(t, "commands.go") + fset := token.NewFileSet() + node, err := parser.ParseFile(fset, file, nil, 0) + if err != nil { + t.Fatalf("could not parse %s: %v", file, err) + } + var runLit *ast.FuncLit + ast.Inspect(node, func(n ast.Node) bool { + fn, ok := n.(*ast.FuncDecl) + if !ok || fn.Name.Name != "initStartCmd" { + return true + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + kv, ok := n.(*ast.KeyValueExpr) + if !ok { + return true + } + // initStartCmd defines a second, unrelated "Run:" field further down + // for a start command alias; take only the first match, which is the + // real start command's closure this test cares about. + if runLit == nil { + if ident, ok := kv.Key.(*ast.Ident); ok && ident.Name == "Run" { + if lit, ok := kv.Value.(*ast.FuncLit); ok { + runLit = lit + } + } + } + return true + }) + return false + }) + if runLit == nil { + t.Fatalf("Run: func literal not found in initStartCmd in %s", file) + } + src, err := os.ReadFile(file) + if err != nil { + t.Fatalf("could not read %s: %v", file, err) + } + start := fset.Position(runLit.Body.Lbrace).Offset + end := fset.Position(runLit.Body.Rbrace).Offset + return string(src[start:end]) +} + +// TestStartCommandClearsProvisionResultBeforeAnyCheck pins the ordering fix: +// clearProvisionResult() must run before every check in the start command's +// Run closure that can fail or return early, not just before doTasksE. +// Without this, a check between the top of the closure and the old call +// sites could return early (whether by writing its own classified failure +// or, like the "service already running" and service-manager-init-error +// paths, by writing nothing at all) while a previous attempt's result file +// was still sitting there to mislead diag/postinstall on retry. +func TestStartCommandClearsProvisionResultBeforeAnyCheck(t *testing.T) { + body := startRunFuncSource(t) + + clearIdx := strings.Index(body, "clearProvisionResult()") + if clearIdx == -1 { + t.Fatal("start command no longer calls clearProvisionResult()") + } + + // Every check or step that can return out of the closure before reaching + // doTasksE. Each must appear after the entry clear. + earlyChecks := []string{ + "checkStrFlagEmpty(", + "validateCdAndNextDNSFlags(", + "validateInterceptModeFlag(", + "doTasksE(", + } + for _, check := range earlyChecks { + idx := strings.Index(body, check) + if idx == -1 { + t.Fatalf("expected the start command to still call %s", check) + } + if idx < clearIdx { + t.Errorf("%s appears before clearProvisionResult(): a failure there could leave a stale result file behind", check) + } + } +} + +// TestStartCommandClassifiesServiceInitFailure pins the fix for a bare +// return: when newService fails in the start command, the closure must fail +// through failProvisionUnclassified, so a result file and the identifier +// line exist, instead of a plain return that exits 0. +func TestStartCommandClassifiesServiceInitFailure(t *testing.T) { + body := startRunFuncSource(t) + initIdx := strings.Index(body, "newService(p, sc)") + if initIdx == -1 { + t.Fatal("start command no longer calls newService(p, sc)") + } + branchEnd := strings.Index(body[initIdx:], "p.preRun()") + if branchEnd == -1 { + t.Fatal("could not find the end of the service init branch") + } + if !strings.Contains(body[initIdx:initIdx+branchEnd], "failProvisionUnclassified(") { + t.Error("service init failure does not fail through failProvisionUnclassified") + } +} + +// TestStartCommandReplacesStaleResultOnEarlyClassifiedFailure is a +// behavioral companion to the structural test above: it drives the real +// start command through its earliest classified failure (an invalid +// --intercept-mode) and checks the file left behind names the new attempt, +// not a stale one seeded beforehand. +func TestStartCommandReplacesStaleResultOnEarlyClassifiedFailure(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldIntercept, oldCdUID, oldCdOrg, oldNextdns := interceptMode, cdUID, cdOrg, nextdns + t.Cleanup(func() { + interceptMode, cdUID, cdOrg, nextdns = oldIntercept, oldCdUID, oldCdOrg, oldNextdns + }) + + // initStartCmd binds these globals to flag defaults as it registers them + // (StringVarP writes the default straight into the pointer), so the + // command must exist before the test overrides the values it drives the + // closure with. + cmd := initStartCmd() + cdUID, cdOrg, nextdns = "", "", "" + interceptMode = "bogus" // fails validateInterceptModeFlag before any OS work + + if err := writeProvisionResult(newProvisionResult(provisionCodeServiceStartFailed, "a previous failed attempt", nil)); err != nil { + t.Fatal(err) + } + + cmd.Run(cmd, nil) + + wantExit := provisionExitCodeForCode[provisionCodeInterceptModeInvalid] + if *exitCode != wantExit { + t.Fatalf("exit = %d, want %d (validateInterceptModeFlag should have run)", *exitCode, wantExit) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code == string(provisionCodeServiceStartFailed) { + t.Fatal("stale result from a previous attempt survived the new attempt") + } + if r.Code != string(provisionCodeInterceptModeInvalid) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeInterceptModeInvalid) + } +} + func TestServiceStageFailureCode(t *testing.T) { tests := []struct { taskName string diff --git a/cmd/cli/hostname.go b/cmd/cli/hostname.go index d28435d..21f33dc 100644 --- a/cmd/cli/hostname.go +++ b/cmd/cli/hostname.go @@ -1,6 +1,10 @@ package cli -import "regexp" +import ( + "fmt" + "regexp" + "strings" +) // validHostname reports whether hostname is a valid hostname. // A valid hostname contains 3 -> 64 characters and conform to RFC1123. @@ -12,3 +16,67 @@ func validHostname(hostname string) bool { validHostnameRfc1123 := regexp.MustCompile(`^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$`) return validHostnameRfc1123.MatchString(hostname) } + +// isHostnameChar reports whether r is part of validHostname's accepted +// charset (letters, digits, hyphen, dot). It does not check position, so a +// hostname can fail validHostname on structure (length, leading/trailing +// hyphen) while every one of its characters passes here. +func isHostnameChar(r rune) bool { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + return true + case r == '-' || r == '.': + return true + } + return false +} + +// offendingHostnameChars returns the distinct characters in hostname that +// validHostname's charset does not accept, in first-seen order. Empty when +// every character is accepted - a rejection can still come from structure +// alone (too short, too long, a leading or trailing hyphen). +func offendingHostnameChars(hostname string) string { + seen := make(map[rune]bool) + var bad []rune + for _, r := range hostname { + if isHostnameChar(r) || seen[r] { + continue + } + seen[r] = true + bad = append(bad, r) + } + return string(bad) +} + +// serverFoldedHostnameChars are the characters ControlD's +// DevicesTableModel.formatDeviceName folds to '-' (or strips) when it +// registers a device name server-side. Only '.' can actually reach +// hostnameMayBeFoldedByServer through validateCustomHostnameFlag's guarded +// path: validHostname runs first and already rejects any hostname +// containing a space or a '+' as CUSTOM_HOSTNAME_INVALID, so those two never +// get here from an explicit --custom-hostname value. They stay in this set +// for completeness: a mobile caller can set CustomHostname to an +// OS-derived default directly, without going through +// validateCustomHostnameFlag at all, so a space or '+' can still reach the +// API unvalidated by this client. +const serverFoldedHostnameChars = ". +" + +// hostnameMayBeFoldedByServer reports whether hostname contains a character +// ControlD may fold or strip when it registers the device, so the name ctrld +// accepted may not be the name the dashboard ends up showing. +func hostnameMayBeFoldedByServer(hostname string) bool { + return strings.ContainsAny(hostname, serverFoldedHostnameChars) +} + +// customHostnameFailureMessage names the field, the offending character(s) +// when there are any, and the allowed format for CUSTOM_HOSTNAME_INVALID. +// ctrld's accept/reject rule (validHostname) is unchanged - this only +// explains a rejection that used to be a bare fatal exit. +func customHostnameFailureMessage(hostname string) string { + const allowedFormat = "3-64 characters of letters, digits, hyphens, and dots (RFC1123 hostname format)" + reason := "is not a valid hostname" + if bad := offendingHostnameChars(hostname); bad != "" { + reason = fmt.Sprintf("contains characters a hostname cannot use: %q", bad) + } + return fmt.Sprintf("--custom-hostname (CustomHostname) %q %s; allowed format: %s", hostname, reason, allowedFormat) +} diff --git a/cmd/cli/hostname_test.go b/cmd/cli/hostname_test.go index f44b231..b096215 100644 --- a/cmd/cli/hostname_test.go +++ b/cmd/cli/hostname_test.go @@ -33,3 +33,65 @@ func Test_validHostname(t *testing.T) { }) } } + +// TestOffendingHostnameChars pins the characters surfaced in the +// CUSTOM_HOSTNAME_INVALID message, so the failure names what is actually +// wrong instead of a bare "invalid hostname". +func TestOffendingHostnameChars(t *testing.T) { + tests := []struct { + name string + hostname string + want string + }{ + {"single offender", "foo@bar", "@"}, + {"space", "foo bar", " "}, + {"dot is allowed", "foo.bar", ""}, + {"hyphen is allowed", "foo-bar", ""}, + {"distinct offenders in order", "a!b!c#d", "!#"}, + {"structurally invalid but no bad char", strings.Repeat("a", 65), ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := offendingHostnameChars(tc.hostname); got != tc.want { + t.Errorf("offendingHostnameChars(%q) = %q, want %q", tc.hostname, got, tc.want) + } + }) + } +} + +// TestHostnameMayBeFoldedByServer pins the characters ControlD's +// DevicesTableModel.formatDeviceName folds or strips when it registers a +// device, so a ctrld-accepted name using one gets a heads-up notice instead +// of silently registering under a different name. +func TestHostnameMayBeFoldedByServer(t *testing.T) { + tests := []struct { + name string + hostname string + want bool + }{ + {"dot", "foo.bar", true}, + {"space", "foo bar", true}, + {"plus", "foo+bar", true}, + {"plain", "foobar", false}, + {"hyphen only", "foo-bar", false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := hostnameMayBeFoldedByServer(tc.hostname); got != tc.want { + t.Errorf("hostnameMayBeFoldedByServer(%q) = %v, want %v", tc.hostname, got, tc.want) + } + }) + } +} + +// TestCustomHostnameFailureMessage pins the message shape the T6 contract +// requires: the flag/field name, the offending character(s), and the +// allowed format. +func TestCustomHostnameFailureMessage(t *testing.T) { + msg := customHostnameFailureMessage("foo@bar") + for _, want := range []string{"--custom-hostname", "CustomHostname", "@", "allowed format"} { + if !strings.Contains(msg, want) { + t.Errorf("customHostnameFailureMessage() = %q, want it to contain %q", msg, want) + } + } +} diff --git a/cmd/cli/prog.go b/cmd/cli/prog.go index 082269e..a838221 100644 --- a/cmd/cli/prog.go +++ b/cmd/cli/prog.go @@ -637,6 +637,25 @@ func (p *prog) setupUpstream(cfg *ctrld.Config) { p.ptrNameservers = ptrNameservers } +// notifyExitToLogServer writes msgExit to the log connection, if one is +// open, so a waiting "ctrld start" sees this run ended instead of waiting +// out the full timeout. A terminal provisioning path calls this right +// before failing. +func (p *prog) notifyExitToLogServer() { + if p.logConn != nil { + _, _ = p.logConn.Write([]byte(msgExit)) + } +} + +// reportServeDNSFailure classifies a listener that bound successfully - the +// LISTENER_* codes already rule out a bind conflict - but failed to actually +// serve DNS. No dedicated code exists for this, so it falls back to +// UNCLASSIFIED. notifyExitToLogServer unblocks a waiting "ctrld start" +// before the process exits. +func (p *prog) reportServeDNSFailure(listenerNum string, err error) { + failRunUnclassified(mainLog.Load().Error().Err(err), fmt.Sprintf("unable to start dns proxy on listener.%s: %v", listenerNum, err), p.notifyExitToLogServer) +} + // run runs the ctrld main components. // // The reload boolean indicates that the function is run when ctrld first start @@ -754,7 +773,7 @@ func (p *prog) run(reload bool, reloadCh chan struct{}) { addr := net.JoinHostPort(listenerConfig.IP, strconv.Itoa(listenerConfig.Port)) mainLog.Load().Info().Msgf("starting DNS server on listener.%s: %s", listenerNum, addr) if err := p.serveDNS(listenerNum); err != nil { - mainLog.Load().Fatal().Err(err).Msgf("unable to start dns proxy on listener.%s", listenerNum) + p.reportServeDNSFailure(listenerNum, err) } mainLog.Load().Debug().Msgf("end of serveDNS listener.%s: %s", listenerNum, addr) }(listenerNum) @@ -927,8 +946,11 @@ var ( initializeOsResolverWithSystemNameserversFn = ctrld.InitializeOsResolverWithSystemNameservers setDnsForRunningIfaceFn = (*prog).setDnsForRunningIface resetDNSFn = (*prog).resetDNS - refuseFallbackFatal = func(format string, v ...any) { - mainLog.Load().Fatal().Msgf(format, v...) + // refuseFallbackFatal reports a startup failure the interface-DNS fallback + // cannot safely paper over, then exits. No dedicated code exists for this, + // so it falls back to UNCLASSIFIED. + refuseFallbackFatal = func(p *prog, format string, v ...any) { + failRunUnclassified(mainLog.Load().Error(), fmt.Sprintf(format, v...), p.notifyExitToLogServer) } ) @@ -1030,7 +1052,7 @@ func (p *prog) setDNS(systemNameservers []string) { // Leave the host resolvable: restore static settings or DHCP rather than // exiting with an interface still pointed at a ctrld that is not serving. resetDNSFn(p, false, true) - refuseFallbackFatal("Refusing to fall back to interface DNS: it cannot direct queries to %s:%d, which would leave this host with no working resolver. Free port 53 for ctrld, or resolve the intercept failure, then start again.", lc.IP, lc.Port) + refuseFallbackFatal(p, "Refusing to fall back to interface DNS: it cannot direct queries to %s:%d, which would leave this host with no working resolver. Free port 53 for ctrld, or resolve the intercept failure, then start again.", lc.IP, lc.Port) // Unreachable in production - the line above exits - but returning // explicitly keeps the refusal from depending on that, so nothing can // fall through to installing the fallback this just rejected. diff --git a/cmd/cli/prog_intercept_fallback_test.go b/cmd/cli/prog_intercept_fallback_test.go index 0cf98ab..06f7124 100644 --- a/cmd/cli/prog_intercept_fallback_test.go +++ b/cmd/cli/prog_intercept_fallback_test.go @@ -135,7 +135,7 @@ func newInterceptFallbackHarness(t *testing.T, lc *ctrld.ListenerConfig) *interc return nil } resetDNSFn = func(_ *prog, _ bool, _ bool) { h.resetCalls++ } - refuseFallbackFatal = func(format string, v ...any) { + refuseFallbackFatal = func(_ *prog, format string, v ...any) { h.refusals = append(h.refusals, fmt.Sprintf(format, v...)) } diff --git a/cmd/cli/provision_boundary_guard_test.go b/cmd/cli/provision_boundary_guard_test.go new file mode 100644 index 0000000..a458506 --- /dev/null +++ b/cmd/cli/provision_boundary_guard_test.go @@ -0,0 +1,243 @@ +package cli + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "runtime" + "testing" +) + +// packageSourcePath resolves a file in this package's source directory, or +// skips the test when the source is not available. The Windows CI job runs +// pre-built test binaries with no checkout, so neither the working directory +// nor the build-time path recorded by runtime.Caller reaches the source +// there. Skipping is safe for these static checks: they analyze the same +// source on every platform, and the jobs with a checkout still enforce them. +func packageSourcePath(t *testing.T, name string) string { + t.Helper() + if _, err := os.Stat(name); err == nil { + return name + } + if _, thisFile, _, ok := runtime.Caller(0); ok { + p := filepath.Join(filepath.Dir(thisFile), name) + if _, err := os.Stat(p); err == nil { + return p + } + } + t.Skipf("package source for %s not available (pre-built test binary without a checkout); this static check runs where source is present", name) + return "" +} + +// provisionBoundaryFiles are the files this lane's provisioning-hardening +// work audited: every terminal path they can reach must report a classified +// failure (a result file, an output line, a stage-scoped exit code), not a +// bare crash installer tooling cannot read. TestProvisioningTerminalPathAudit +// documents the audit by scenario; this test enforces it structurally so a +// later change cannot reintroduce a bare fatal without either classifying it +// or adding it here with a reason. +// +// v1.0 keeps the start command, the run command, and every other top-level +// command (log, stop, reload, status, uninstall, clients, upgrade) in one +// commands.go, where master splits each into its own file. That collapses +// several master file names into "commands.go" here, and it means the +// allowlist below has one entry per out-of-scope command instead of one +// excluded file per command. +// +// Scope: the check sees direct calls (".Fatal()", "os.Exit", "panic") in the +// files listed here. A helper defined in one of these files is caught once, +// at the body that holds the call, so an indirect exit through it needs no +// second rule. A helper defined outside the list, such as +// checkHasElevatedPrivilege in service.go wired as the "start" PreRun, is +// outside the reach of this check. TestProvisioningTerminalPathAudit lists +// those by hand in its exclusion comment. +var provisionBoundaryFiles = []string{ + "cli.go", + "commands.go", + "prog.go", + "provision_result.go", +} + +// provisionBoundaryFatalAllowlist is every call in the files above that +// still reaches a bare ".Fatal()" (on any logger chain), "os.Exit", or +// "panic", keyed +// by "file.go:FuncName" (FuncName includes the receiver type for methods, +// e.g. "prog.setDNS"). Each entry names why it is not a provisioning +// failure, or why it predates and sits outside the --cd-org boundary this +// lane hardened. +var provisionBoundaryFatalAllowlist = map[string]string{ + "cli.go:RunMobile": `panic on a nil AppConfig is a programming error in the mobile host ` + + `app, at an entry point the CLI command tree never calls. Not reachable from ` + + `"ctrld start --cd-org".`, + + "cli.go:run": `os.Exit(0) is the successful exit of the daemon-respawn launcher, right ` + + `after it starts the real background process. Not a failure.`, + + "cli.go:readConfigFile": `shared CLI config-parsing helper used the same way by every ` + + `invocation mode (config-file, no-config, nextdns, --cd, --cd-org). Predates and is ` + + `orthogonal to the --cd-org provisioning boundary.`, + + "cli.go:processNoConfigFlags": `shared CLI helper enforcing --listen/--primary_upstream in ` + + `no-config mode. Applies uniformly across every invocation mode, not specifically to ` + + `provisioning.`, + + "cli.go:processListenFlag": `shared CLI helper parsing --listen. Applies uniformly across ` + + `every invocation mode, not specifically to provisioning.`, + + "cli.go:readConfigWithNotice": `shared CLI helper (userHomeDir failure while locating the ` + + `default config file). Applies uniformly across every invocation mode, not specifically ` + + `to provisioning.`, + + "cli.go:checkStrFlagEmpty": `shared flag-emptiness helper. Only an explicit empty --cd-org ` + + `is a provisioning-token value, classified separately inside this function; every other ` + + `flag (currently --cd) keeps its bare fatal.`, + + "cli.go:uninstall": `the "ctrld uninstall" command's own router re-configuration step. ` + + `Outside the --cd-org provisioning boundary this lane hardened.`, + + "commands.go:initStartCmd": `two unrelated bare exits nested in the same top-level func: ` + + `the deactivation pin check's os.Exit(126) (out of scope for this lane by contract - ` + + `"pin-check 126 untouched"), and the hidden "router setup" alias command's own ` + + `os.Executable/re-exec failure handling (generic CLI plumbing for a re-exec helper, not ` + + `a provisioning outcome). Every check on the actual --cd-org boundary in this function ` + + `already classifies through failProvision/failRunUnclassified.`, + + "commands.go:initLogCmd": `the "ctrld log" command. Outside the --cd-org provisioning ` + + `boundary this lane hardened.`, + + "commands.go:initStopCmd": `the "ctrld stop" command. Outside the --cd-org provisioning ` + + `boundary this lane hardened.`, + + "commands.go:initReloadCmd": `the "ctrld reload"/"ctrld restart" commands. Outside the ` + + `--cd-org provisioning boundary this lane hardened.`, + + "commands.go:initStatusCmd": `the "ctrld status" command. Outside the --cd-org ` + + `provisioning boundary this lane hardened.`, + + "commands.go:initUninstallCmd": `the "ctrld uninstall" command. Outside the --cd-org ` + + `provisioning boundary this lane hardened.`, + + "commands.go:initClientsCmd": `the "ctrld clients" command. Outside the --cd-org ` + + `provisioning boundary this lane hardened.`, + + "commands.go:initUpgradeCmd": `the "ctrld upgrade" command. Outside the --cd-org ` + + `provisioning boundary this lane hardened.`, + + "prog.go:prog.Stop": `the deactivation pin check's os.Exit(126). Out of scope for this ` + + `lane by contract ("pin-check 126 untouched").`, + + "prog.go:prog.setDNS": `the runtime --intercept-mode validation (validInterceptMode), ` + + `distinct from the early "ctrld start" check that already reports ` + + `INTERCEPT_MODE_INVALID. Left alone per T6's own doc comment.`, +} + +// funcDeclKey names a function declaration the way +// provisionBoundaryFatalAllowlist keys it: "FuncName" for a plain function, +// "ReceiverType.FuncName" for a method. +func funcDeclKey(fn *ast.FuncDecl) string { + if fn.Recv == nil || len(fn.Recv.List) == 0 { + return fn.Name.Name + } + recvType := fn.Recv.List[0].Type + if star, ok := recvType.(*ast.StarExpr); ok { + recvType = star.X + } + if ident, ok := recvType.(*ast.Ident); ok { + return ident.Name + "." + fn.Name.Name + } + return fn.Name.Name +} + +// isBareFatalOrExitCall reports whether call is ".Fatal(...)" on any +// logger chain, "os.Exit(...)", or a bare "panic(...)". +func isBareFatalOrExitCall(call *ast.CallExpr) bool { + if ident, ok := call.Fun.(*ast.Ident); ok && ident.Name == "panic" { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + if sel.Sel.Name == "Fatal" { + return true + } + if sel.Sel.Name == "Exit" { + if ident, ok := sel.X.(*ast.Ident); ok && ident.Name == "os" { + return true + } + } + return false +} + +// TestProvisionBoundaryHasNoUnlistedBareFatal parses every provisioning +// boundary file and flags a bare Fatal/os.Exit that is not in the allowlist +// above. A new one appearing here means a change added a crash the +// provisioning boundary's contract does not allow: classify it through +// failProvision/failProvisionUnclassified, or add it to the allowlist with a +// reason if it genuinely sits outside the boundary. +func TestProvisionBoundaryHasNoUnlistedBareFatal(t *testing.T) { + fset := token.NewFileSet() + for _, file := range provisionBoundaryFiles { + node, err := parser.ParseFile(fset, packageSourcePath(t, file), nil, 0) + if err != nil { + t.Fatalf("could not parse %s: %v", file, err) + } + for _, decl := range node.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil { + continue + } + key := file + ":" + funcDeclKey(fn) + ast.Inspect(fn.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || !isBareFatalOrExitCall(call) { + return true + } + if _, allowed := provisionBoundaryFatalAllowlist[key]; !allowed { + pos := fset.Position(call.Pos()) + t.Errorf("%s:%d: unlisted bare fatal/exit in %s - classify it through "+ + "failProvision/failProvisionUnclassified, or add it to "+ + "provisionBoundaryFatalAllowlist with a reason", file, pos.Line, key) + } + return true + }) + } + } +} + +// TestProvisionBoundaryAllowlistHasNoStaleEntries is the converse check: every +// allowlisted key must still name a real bare Fatal/os.Exit. A stale entry +// (the call was removed or reclassified) would silently widen the allowlist +// and hide a real regression the next time this test runs. +func TestProvisionBoundaryAllowlistHasNoStaleEntries(t *testing.T) { + fset := token.NewFileSet() + found := make(map[string]bool, len(provisionBoundaryFatalAllowlist)) + for _, file := range provisionBoundaryFiles { + node, err := parser.ParseFile(fset, packageSourcePath(t, file), nil, 0) + if err != nil { + t.Fatalf("could not parse %s: %v", file, err) + } + for _, decl := range node.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil { + continue + } + key := file + ":" + funcDeclKey(fn) + ast.Inspect(fn.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || !isBareFatalOrExitCall(call) { + return true + } + found[key] = true + return true + }) + } + } + for key := range provisionBoundaryFatalAllowlist { + if !found[key] { + t.Errorf("provisionBoundaryFatalAllowlist[%q] no longer matches any bare fatal/exit; remove the stale entry", key) + } + } +} diff --git a/cmd/cli/provision_input_test.go b/cmd/cli/provision_input_test.go new file mode 100644 index 0000000..0713907 --- /dev/null +++ b/cmd/cli/provision_input_test.go @@ -0,0 +1,417 @@ +package cli + +import ( + "context" + "strings" + "testing" + + "github.com/rs/zerolog" + "github.com/spf13/cobra" + + "github.com/Control-D-Inc/ctrld" + "github.com/Control-D-Inc/ctrld/internal/controld" +) + +// captureMainLog swaps mainLog for a test-local buffer logger and restores +// the previous logger on cleanup. Asserting on the shared logOutput sink is +// order-dependent: a test that ran earlier can re-store mainLog (Windows' +// Test_validInterfaces calls initConsoleLogging), leaving the shared buffer +// stale for every later test. +func captureMainLog(t *testing.T) *syncBuffer { + t.Helper() + buf := &syncBuffer{} + l := zerolog.New(buf) + old := mainLog.Load() + mainLog.Store(&l) + t.Cleanup(func() { mainLog.Store(old) }) + return buf +} + +func TestProvisionTokenShapeValid(t *testing.T) { + tests := []struct { + name string + token string + want bool + }{ + {"too short", "abcde", false}, + {"minimum length", "abcdef", true}, + {"maximum length", strings.Repeat("a", 64), true}, + {"too long", strings.Repeat("a", 65), false}, + {"contains space", "org-v1- abc", false}, + {"contains tab", "org-v1-\tabc", false}, + {"contains newline", "org-v1-\nabc", false}, + {"contains control char", "org-v1-\x00abc", false}, + {"sane with prefix", "org-v1-abcdef123456", true}, + {"sane without prefix", "legacytoken123", true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := provisionTokenShapeValid(tc.token); got != tc.want { + t.Errorf("provisionTokenShapeValid(%q) = %v, want %v", tc.token, got, tc.want) + } + }) + } +} + +// TestCdUIDFromProvTokenMalformedToken proves a malformed --cd-org value is +// classified before any network attempt: no call reaches fetchResolverUIDFn, +// and the malformed value itself never appears in the persisted message. +func TestCdUIDFromProvTokenMalformedToken(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldFetch, oldHostname := fetchResolverUIDFn, customHostname + t.Cleanup(func() { fetchResolverUIDFn, customHostname = oldFetch, oldHostname }) + + cdUID = "" + const malformed = "ab cd" + cdOrg = malformed + customHostname = "" + + called := false + fetchResolverUIDFn = func(context.Context, *controld.UtilityOrgRequest, string, bool) (*controld.ResolverConfig, error) { + called = true + return nil, nil + } + + if got := cdUIDFromProvToken(); got != "" { + t.Errorf("cdUIDFromProvToken() = %q, want empty on failure", got) + } + if called { + t.Error("fetchResolverUIDFn was called; malformed token must be rejected before any network attempt") + } + if *exitCode != provisionExitCodeForCode[provisionCodeProvisionTokenMalformed] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeProvisionTokenMalformed]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeProvisionTokenMalformed) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeProvisionTokenMalformed) + } + if r.Stage != string(provisionStageInput) { + t.Errorf("stage = %q, want %q", r.Stage, provisionStageInput) + } + if strings.Contains(r.Message, malformed) { + t.Errorf("malformed token leaked into result message: %q", r.Message) + } +} + +// TestCdUIDFromProvTokenPrefixlessTokenProceeds proves a token missing the +// "org-v1-" prefix is not rejected - only warned about - since legacy codes +// may lack it. +func TestCdUIDFromProvTokenPrefixlessTokenProceeds(t *testing.T) { + _, _ = stubProvisionGlobals(t) + logBuf := captureMainLog(t) + oldFetch, oldHostname := fetchResolverUIDFn, customHostname + t.Cleanup(func() { fetchResolverUIDFn, customHostname = oldFetch, oldHostname }) + + cdUID = "" + cdOrg = "legacytoken123" + customHostname = "" + + called := false + fetchResolverUIDFn = func(context.Context, *controld.UtilityOrgRequest, string, bool) (*controld.ResolverConfig, error) { + called = true + return &controld.ResolverConfig{UID: "resolved-uid"}, nil + } + + if got := cdUIDFromProvToken(); got != "resolved-uid" { + t.Errorf("cdUIDFromProvToken() = %q, want resolved-uid", got) + } + if !called { + t.Error("fetchResolverUIDFn was not called; a prefixless-but-sane token must still proceed") + } + if !strings.Contains(logBuf.String(), "org-v1-") { + t.Error("expected a warning mentioning the org-v1- prefix") + } +} + +// TestCdUIDFromProvTokenInvalidCustomHostname proves an invalid +// --custom-hostname value is classified before any network attempt, with a +// message that names the field and format, and never leaks the provision +// token. +func TestCdUIDFromProvTokenInvalidCustomHostname(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldFetch, oldHostname := fetchResolverUIDFn, customHostname + t.Cleanup(func() { fetchResolverUIDFn, customHostname = oldFetch, oldHostname }) + + cdUID = "" + const secretToken = "org-v1-secret-token-42" + cdOrg = secretToken + customHostname = "foo@bar" + + called := false + fetchResolverUIDFn = func(context.Context, *controld.UtilityOrgRequest, string, bool) (*controld.ResolverConfig, error) { + called = true + return nil, nil + } + + if got := cdUIDFromProvToken(); got != "" { + t.Errorf("cdUIDFromProvToken() = %q, want empty on failure", got) + } + if called { + t.Error("fetchResolverUIDFn was called; invalid custom hostname must be rejected before any network attempt") + } + if *exitCode != provisionExitCodeForCode[provisionCodeCustomHostnameInvalid] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeCustomHostnameInvalid]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeCustomHostnameInvalid) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeCustomHostnameInvalid) + } + if !strings.Contains(r.Message, "allowed format") { + t.Errorf("message = %q, want it to contain the allowed format", r.Message) + } + if strings.Contains(r.Message, secretToken) { + t.Errorf("provision token leaked into result message: %q", r.Message) + } +} + +// TestCdUIDFromProvTokenFoldableHostnameLogsNotice proves a hostname that +// ctrld accepts, but ControlD's device-name formatting would fold or strip, +// gets a notice - not a failure - and provisioning still proceeds. +func TestCdUIDFromProvTokenFoldableHostnameLogsNotice(t *testing.T) { + _, _ = stubProvisionGlobals(t) + logBuf := captureMainLog(t) + oldFetch, oldHostname := fetchResolverUIDFn, customHostname + t.Cleanup(func() { fetchResolverUIDFn, customHostname = oldFetch, oldHostname }) + + cdUID = "" + cdOrg = "org-v1-abcdef123456" + customHostname = "foo.bar" + + fetchResolverUIDFn = func(context.Context, *controld.UtilityOrgRequest, string, bool) (*controld.ResolverConfig, error) { + return &controld.ResolverConfig{UID: "resolved-uid"}, nil + } + + if got := cdUIDFromProvToken(); got != "resolved-uid" { + t.Errorf("cdUIDFromProvToken() = %q, want resolved-uid", got) + } + if !strings.Contains(logBuf.String(), "foo.bar") { + t.Error("expected a notice naming the foldable hostname") + } +} + +func TestValidateInterceptModeFlag(t *testing.T) { + oldMode := interceptMode + t.Cleanup(func() { interceptMode = oldMode }) + + t.Run("valid values proceed", func(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + for _, mode := range []string{"", "off", "dns", "hard"} { + interceptMode = mode + if !validateInterceptModeFlag(mode) { + t.Errorf("validateInterceptModeFlag(%q) = false, want true", mode) + } + if *exitCode != -1 { + t.Errorf("mode %q: provisionExit called with %d, want no exit", mode, *exitCode) + } + } + }) + + t.Run("invalid value is classified", func(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + interceptMode = "fds" + if validateInterceptModeFlag("fds") { + t.Error("validateInterceptModeFlag(\"fds\") = true, want false") + } + if *exitCode != provisionExitCodeForCode[provisionCodeInterceptModeInvalid] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeInterceptModeInvalid]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeInterceptModeInvalid) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeInterceptModeInvalid) + } + for _, want := range []string{"off", "dns", "hard"} { + if !strings.Contains(r.Message, want) { + t.Errorf("message = %q, want it to contain %q", r.Message, want) + } + } + }) +} + +// TestCheckStrFlagEmptyClassifiesEmptyCdOrg proves an explicit empty --cd-org +// is classified as a malformed provisioning token rather than a bare fatal. +func TestCheckStrFlagEmptyClassifiesEmptyCdOrg(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + cmd := &cobra.Command{} + cmd.Flags().String(cdOrgFlagName, "", "") + if err := cmd.Flags().Set(cdOrgFlagName, ""); err != nil { + t.Fatal(err) + } + + if checkStrFlagEmpty(cmd, cdOrgFlagName) { + t.Error("checkStrFlagEmpty() = true, want false for an explicit empty --cd-org") + } + if *exitCode != provisionExitCodeForCode[provisionCodeProvisionTokenMalformed] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeProvisionTokenMalformed]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeProvisionTokenMalformed) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeProvisionTokenMalformed) + } + if r.Stage != string(provisionStageInput) { + t.Errorf("stage = %q, want %q", r.Stage, provisionStageInput) + } + if !strings.Contains(r.Message, cdOrgFlagName) { + t.Errorf("message = %q, want it to name --%s", r.Message, cdOrgFlagName) + } +} + +// TestCheckStrFlagEmptyProceedsWhenNotChangedOrNotEmpty proves the two cases +// that must not classify: the flag was never set, and it was set to a +// non-empty value. +func TestCheckStrFlagEmptyProceedsWhenNotChangedOrNotEmpty(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + + t.Run("flag never set", func(t *testing.T) { + cmd := &cobra.Command{} + cmd.Flags().String(cdOrgFlagName, "", "") + if !checkStrFlagEmpty(cmd, cdOrgFlagName) { + t.Error("checkStrFlagEmpty() = false, want true when the flag was never set") + } + }) + + t.Run("flag set to a non-empty value", func(t *testing.T) { + cmd := &cobra.Command{} + cmd.Flags().String(cdOrgFlagName, "", "") + if err := cmd.Flags().Set(cdOrgFlagName, "org-v1-abc"); err != nil { + t.Fatal(err) + } + if !checkStrFlagEmpty(cmd, cdOrgFlagName) { + t.Error("checkStrFlagEmpty() = false, want true for a non-empty value") + } + }) + + if *exitCode != -1 { + t.Errorf("provisionExit called with %d, want no exit", *exitCode) + } +} + +// TestValidateCdAndNextDNSFlagsClassifiesConflict proves --cd or --cd-org +// combined with --nextdns is classified as INVALID_FLAG_COMBINATION, naming +// every flag involved. +func TestValidateCdAndNextDNSFlagsClassifiesConflict(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldCdUID, oldCdOrg, oldNextdns := cdUID, cdOrg, nextdns + t.Cleanup(func() { cdUID, cdOrg, nextdns = oldCdUID, oldCdOrg, oldNextdns }) + + t.Run("non-conflicting combinations proceed", func(t *testing.T) { + cases := []struct{ cdUID, cdOrg, nextdns string }{ + {"", "", ""}, + {"uid123", "", ""}, + {"", "org-v1-abc", ""}, + {"", "", "nextdns-id"}, + } + for _, tc := range cases { + cdUID, cdOrg, nextdns = tc.cdUID, tc.cdOrg, tc.nextdns + if !validateCdAndNextDNSFlags() { + t.Errorf("validateCdAndNextDNSFlags() = false for cdUID=%q cdOrg=%q nextdns=%q, want true", tc.cdUID, tc.cdOrg, tc.nextdns) + } + } + if *exitCode != -1 { + t.Errorf("provisionExit called with %d, want no exit", *exitCode) + } + }) + + t.Run("cd-org with nextdns is classified", func(t *testing.T) { + cdUID, cdOrg, nextdns = "", "org-v1-abc", "nextdns-id" + if validateCdAndNextDNSFlags() { + t.Error("validateCdAndNextDNSFlags() = true, want false") + } + if *exitCode != provisionExitCodeForCode[provisionCodeInvalidFlagCombination] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeInvalidFlagCombination]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeInvalidFlagCombination) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeInvalidFlagCombination) + } + if r.Stage != string(provisionStageInput) { + t.Errorf("stage = %q, want %q", r.Stage, provisionStageInput) + } + for _, want := range []string{cdUidFlagName, cdOrgFlagName, nextdnsFlagName} { + if !strings.Contains(r.Message, want) { + t.Errorf("message = %q, want it to name --%s", r.Message, want) + } + } + }) + + t.Run("cd with nextdns is classified", func(t *testing.T) { + cdUID, cdOrg, nextdns = "uid123", "", "nextdns-id" + if validateCdAndNextDNSFlags() { + t.Error("validateCdAndNextDNSFlags() = true, want false") + } + if *exitCode != provisionExitCodeForCode[provisionCodeInvalidFlagCombination] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeInvalidFlagCombination]) + } + }) +} + +// TestValidateCdUpstreamProtocol proves an invalid --proto value is +// classified only once --cd is in play, and that notify runs when given. +func TestValidateCdUpstreamProtocol(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + oldCdUID, oldProto := cdUID, cdUpstreamProto + t.Cleanup(func() { cdUID, cdUpstreamProto = oldCdUID, oldProto }) + + t.Run("no --cd proceeds regardless of protocol", func(t *testing.T) { + cdUID = "" + cdUpstreamProto = "garbage" + if !validateCdUpstreamProtocol(nil) { + t.Error("validateCdUpstreamProtocol(nil) = false, want true when --cd is not set") + } + if *exitCode != -1 { + t.Errorf("provisionExit called with %d, want no exit", *exitCode) + } + }) + + t.Run("valid protocols proceed", func(t *testing.T) { + cdUID = "uid123" + for _, proto := range []string{ctrld.ResolverTypeDOH, ctrld.ResolverTypeDOH3} { + cdUpstreamProto = proto + if !validateCdUpstreamProtocol(nil) { + t.Errorf("validateCdUpstreamProtocol(nil) = false for proto %q, want true", proto) + } + } + if *exitCode != -1 { + t.Errorf("provisionExit called with %d, want no exit", *exitCode) + } + }) + + t.Run("invalid protocol is classified and notifies", func(t *testing.T) { + cdUID = "uid123" + cdUpstreamProto = "quic" + notified := false + if validateCdUpstreamProtocol(func() { notified = true }) { + t.Error("validateCdUpstreamProtocol() = true, want false") + } + if !notified { + t.Error("notify not called") + } + if *exitCode != provisionExitCodeForCode[provisionCodeInvalidFlagCombination] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeInvalidFlagCombination]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeInvalidFlagCombination) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeInvalidFlagCombination) + } + if !strings.Contains(r.Message, "quic") || !strings.Contains(r.Message, "doh") { + t.Errorf("message = %q, want it to name the given and allowed values", r.Message) + } + }) +} diff --git a/cmd/cli/provision_result.go b/cmd/cli/provision_result.go index bd513be..606b8a4 100644 --- a/cmd/cli/provision_result.go +++ b/cmd/cli/provision_result.go @@ -8,6 +8,8 @@ import ( "strings" "time" "unicode/utf8" + + "github.com/rs/zerolog" ) // A terminal provisioning failure reports the same stable code on three @@ -20,6 +22,7 @@ import ( type provisionStage string const ( + provisionStageInput provisionStage = "input" provisionStageBootstrap provisionStage = "bootstrap" provisionStageListener provisionStage = "listener" provisionStageService provisionStage = "service" @@ -28,50 +31,87 @@ const ( type provisionFailureCode string const ( - provisionCodeAPIUnreachable provisionFailureCode = "API_UNREACHABLE" - provisionCodeAPIRejected provisionFailureCode = "API_REJECTED" - provisionCodeAPIDeviceInvalid provisionFailureCode = "API_DEVICE_INVALID" - provisionCodeListenerBindFailed provisionFailureCode = "LISTENER_BIND_FAILED" - provisionCodeListenerAddrUnavail provisionFailureCode = "LISTENER_CONFIGURED_ADDR_UNAVAILABLE" - provisionCodeServiceInstall provisionFailureCode = "SERVICE_INSTALL_FAILED" - provisionCodeServiceStartFailed provisionFailureCode = "SERVICE_START_FAILED" - provisionCodeServiceSelfCheck provisionFailureCode = "SERVICE_SELFCHECK_FAILED" + provisionCodeProvisionTokenMalformed provisionFailureCode = "PROVISION_TOKEN_MALFORMED" + provisionCodeCustomHostnameInvalid provisionFailureCode = "CUSTOM_HOSTNAME_INVALID" + provisionCodeInterceptModeInvalid provisionFailureCode = "INTERCEPT_MODE_INVALID" + provisionCodeInvalidFlagCombination provisionFailureCode = "INVALID_FLAG_COMBINATION" + provisionCodeAPIUnreachable provisionFailureCode = "API_UNREACHABLE" + provisionCodeAPIRejected provisionFailureCode = "API_REJECTED" + provisionCodeAPIDeviceInvalid provisionFailureCode = "API_DEVICE_INVALID" + provisionCodeTokenInvalid provisionFailureCode = "TOKEN_INVALID" + provisionCodeTokenExpired provisionFailureCode = "TOKEN_EXPIRED" + provisionCodeTokenLimitReached provisionFailureCode = "TOKEN_LIMIT_REACHED" + provisionCodeTokenDisabled provisionFailureCode = "TOKEN_DISABLED" + provisionCodeListenerBindFailed provisionFailureCode = "LISTENER_BIND_FAILED" + provisionCodeListenerAddrUnavail provisionFailureCode = "LISTENER_CONFIGURED_ADDR_UNAVAILABLE" + provisionCodeServiceInstall provisionFailureCode = "SERVICE_INSTALL_FAILED" + provisionCodeServiceStartFailed provisionFailureCode = "SERVICE_START_FAILED" + provisionCodeServiceSelfCheck provisionFailureCode = "SERVICE_SELFCHECK_FAILED" + provisionCodeUnclassified provisionFailureCode = "UNCLASSIFIED" ) var allProvisionFailureCodes = []provisionFailureCode{ + provisionCodeProvisionTokenMalformed, + provisionCodeCustomHostnameInvalid, + provisionCodeInterceptModeInvalid, + provisionCodeInvalidFlagCombination, provisionCodeAPIUnreachable, provisionCodeAPIRejected, provisionCodeAPIDeviceInvalid, + provisionCodeTokenInvalid, + provisionCodeTokenExpired, + provisionCodeTokenLimitReached, + provisionCodeTokenDisabled, provisionCodeListenerBindFailed, provisionCodeListenerAddrUnavail, provisionCodeServiceInstall, provisionCodeServiceStartFailed, provisionCodeServiceSelfCheck, + provisionCodeUnclassified, } var provisionStageForCode = map[provisionFailureCode]provisionStage{ - provisionCodeAPIUnreachable: provisionStageBootstrap, - provisionCodeAPIRejected: provisionStageBootstrap, - provisionCodeAPIDeviceInvalid: provisionStageBootstrap, - provisionCodeListenerBindFailed: provisionStageListener, - provisionCodeListenerAddrUnavail: provisionStageListener, - provisionCodeServiceInstall: provisionStageService, - provisionCodeServiceStartFailed: provisionStageService, - provisionCodeServiceSelfCheck: provisionStageService, + provisionCodeProvisionTokenMalformed: provisionStageInput, + provisionCodeCustomHostnameInvalid: provisionStageInput, + provisionCodeInterceptModeInvalid: provisionStageInput, + provisionCodeInvalidFlagCombination: provisionStageInput, + provisionCodeAPIUnreachable: provisionStageBootstrap, + provisionCodeAPIRejected: provisionStageBootstrap, + provisionCodeAPIDeviceInvalid: provisionStageBootstrap, + provisionCodeTokenInvalid: provisionStageBootstrap, + provisionCodeTokenExpired: provisionStageBootstrap, + provisionCodeTokenLimitReached: provisionStageBootstrap, + provisionCodeTokenDisabled: provisionStageBootstrap, + provisionCodeListenerBindFailed: provisionStageListener, + provisionCodeListenerAddrUnavail: provisionStageListener, + provisionCodeServiceInstall: provisionStageService, + provisionCodeServiceStartFailed: provisionStageService, + provisionCodeServiceSelfCheck: provisionStageService, + provisionCodeUnclassified: provisionStageService, } -// Exit codes are grouped by stage (bootstrap 30-39, listener 40-49, service -// 50-59) so the exit code alone names the failed stage. 0-3 belong to -// "ctrld status" and 126 to the deactivation pin check; never reuse those. +// Exit codes are grouped by stage (input 20-29, bootstrap 30-39, listener +// 40-49, service 50-59) so the exit code alone names the failed stage. 0-3 +// belong to "ctrld status" and 126 to the deactivation pin check; never +// reuse those. var provisionExitCodeForCode = map[provisionFailureCode]int{ - provisionCodeAPIUnreachable: 30, - provisionCodeAPIRejected: 31, - provisionCodeAPIDeviceInvalid: 32, - provisionCodeListenerBindFailed: 41, - provisionCodeListenerAddrUnavail: 42, - provisionCodeServiceInstall: 51, - provisionCodeServiceStartFailed: 52, - provisionCodeServiceSelfCheck: 53, + provisionCodeProvisionTokenMalformed: 21, + provisionCodeCustomHostnameInvalid: 22, + provisionCodeInterceptModeInvalid: 23, + provisionCodeInvalidFlagCombination: 24, + provisionCodeAPIUnreachable: 30, + provisionCodeAPIRejected: 31, + provisionCodeAPIDeviceInvalid: 32, + provisionCodeTokenInvalid: 33, + provisionCodeTokenExpired: 34, + provisionCodeTokenLimitReached: 35, + provisionCodeTokenDisabled: 36, + provisionCodeListenerBindFailed: 41, + provisionCodeListenerAddrUnavail: 42, + provisionCodeServiceInstall: 51, + provisionCodeServiceStartFailed: 52, + provisionCodeServiceSelfCheck: 53, + provisionCodeUnclassified: 59, } const ( @@ -213,7 +253,11 @@ func writeProvisionResult(r *provisionResult) error { } func readProvisionResult() (*provisionResult, error) { - buf, err := os.ReadFile(provisionResultPath()) + return readProvisionResultAt(provisionResultPath()) +} + +func readProvisionResultAt(path string) (*provisionResult, error) { + buf, err := os.ReadFile(path) if err != nil { return nil, err } @@ -245,3 +289,24 @@ func failProvision(r *provisionResult, notify func()) { } provisionExit(r.ExitCode) } + +// failProvisionUnclassified persists an UNCLASSIFIED result (service stage, +// exit 59) and exits. It is the fallback for a terminal path that predates a +// stable code - a config unmarshal, a file-system or environment failure - +// so support still gets a result file and a stage-scoped exit code instead +// of a bare crash with nothing to read. No terminal path on the +// provisioning boundary may bypass this or an existing classified code. +func failProvisionUnclassified(message string, notify func()) { + failProvision(newProvisionResult(provisionCodeUnclassified, message, nil, provisionSecrets()...), notify) +} + +// failRunUnclassified logs msg on ev, then fails provisioning as UNCLASSIFIED +// and unblocks a waiting "ctrld start" via notify (nil if none). Callers +// return immediately after this call: provisionExit is stubbed out under +// test, so nothing stops execution from falling through otherwise. ev +// carries whatever the caller already chained onto it (for example .Err()), +// so each call site keeps its own log fields. +func failRunUnclassified(ev *zerolog.Event, msg string, notify func()) { + ev.Msg(msg) + failProvisionUnclassified(msg, notify) +} diff --git a/cmd/cli/provision_result_test.go b/cmd/cli/provision_result_test.go index 3003904..f7f95d3 100644 --- a/cmd/cli/provision_result_test.go +++ b/cmd/cli/provision_result_test.go @@ -20,8 +20,21 @@ func overrideProvisionResultPath(t *testing.T) string { return path } +// The result file must obey the same homedir override as the log and the +// config file, so a daemon started with --homedir writes it next to them. +func TestProvisionResultPathHonorsHomedir(t *testing.T) { + old := homedir + homedir = t.TempDir() + t.Cleanup(func() { homedir = old }) + want := filepath.Join(homedir, provisionResultFileName) + if got := provisionResultPath(); got != want { + t.Errorf("provisionResultPath() = %q, want %q", got, want) + } +} + func TestProvisionCodesMapToOneStageAndInRangeExit(t *testing.T) { stageRanges := map[provisionStage][2]int{ + provisionStageInput: {20, 29}, provisionStageBootstrap: {30, 39}, provisionStageListener: {40, 49}, provisionStageService: {50, 59}, @@ -55,8 +68,8 @@ func TestProvisionCodesMapToOneStageAndInRangeExit(t *testing.T) { } seenExits[exit] = code } - if len(allProvisionFailureCodes) != 8 { - t.Errorf("expected 8 codes, got %d", len(allProvisionFailureCodes)) + if len(allProvisionFailureCodes) != 17 { + t.Errorf("expected 17 codes, got %d", len(allProvisionFailureCodes)) } } @@ -166,14 +179,17 @@ func TestFailureCodeDocTableMatchesConstants(t *testing.T) { t.Fatalf("could not read the failure-code doc: %v", err) } doc := string(buf) + // Package-stage rows document identifiers scripts/pkg/postinstall emits + // on its own; they are not in this binary's failure-code registry, so + // they sit outside the one-row-per-code count below. rows := 0 for _, line := range strings.Split(doc, "\n") { - if strings.HasPrefix(line, "| `") { + if strings.HasPrefix(line, "| `") && !strings.Contains(line, "| package |") { rows++ } } if rows != len(allProvisionFailureCodes) { - t.Errorf("doc table has %d code rows, want %d", rows, len(allProvisionFailureCodes)) + t.Errorf("doc table has %d non-package code rows, want %d", rows, len(allProvisionFailureCodes)) } for _, code := range allProvisionFailureCodes { row := "| `" + string(code) + "` | " + string(provisionStageForCode[code]) + " | " + strconv.Itoa(provisionExitCodeForCode[code]) + " |" diff --git a/cmd/cli/provision_terminal_paths_test.go b/cmd/cli/provision_terminal_paths_test.go new file mode 100644 index 0000000..7e59222 --- /dev/null +++ b/cmd/cli/provision_terminal_paths_test.go @@ -0,0 +1,244 @@ +package cli + +import ( + "errors" + "net" + "strings" + "testing" + "time" +) + +// TestFailProvisionUnclassified pins the fallback every terminal path on the +// provisioning boundary uses when it predates a stable code: it persists +// UNCLASSIFIED, calls notify, and exits at the code's exit number. +func TestFailProvisionUnclassified(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + notified := false + + failProvisionUnclassified("something failed with no dedicated code", func() { notified = true }) + + if !notified { + t.Error("notify not called") + } + if *exitCode != provisionExitCodeForCode[provisionCodeUnclassified] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeUnclassified]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeUnclassified) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeUnclassified) + } + if r.Stage != string(provisionStageService) { + t.Errorf("stage = %q, want %q", r.Stage, provisionStageService) + } + if !strings.Contains(r.Message, "something failed") { + t.Errorf("message = %q, want it to contain the given message", r.Message) + } +} + +// TestNotifyExitToLogServer pins the method every run()-side failure uses to +// unblock a waiting "ctrld start": it writes msgExit to the log connection +// exactly when one is set, and is a no-op otherwise. +func TestNotifyExitToLogServer(t *testing.T) { + t.Run("nil connection is a no-op", func(t *testing.T) { + p := &prog{} + p.notifyExitToLogServer() // must not panic + }) + + t.Run("writes msgExit to a set connection", func(t *testing.T) { + server, client := net.Pipe() + t.Cleanup(func() { _ = server.Close(); _ = client.Close() }) + p := &prog{logConn: client} + + received := make(chan string, 1) + go func() { + buf := make([]byte, 64) + n, _ := server.Read(buf) + received <- string(buf[:n]) + }() + + p.notifyExitToLogServer() + + select { + case got := <-received: + if got != msgExit { + t.Errorf("wrote %q, want %q", got, msgExit) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for notifyExitToLogServer to write") + } + }) +} + +// TestReportServeDNSFailure covers the one prog.go path where a listener +// bound successfully (LISTENER_* already ruled that out) but the proxy +// itself failed to start serving - a distinct failure with no dedicated +// code. +func TestReportServeDNSFailure(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + server, client := net.Pipe() + t.Cleanup(func() { _ = server.Close(); _ = client.Close() }) + go func() { + buf := make([]byte, 64) + _, _ = server.Read(buf) + }() + p := &prog{logConn: client} + + p.reportServeDNSFailure("0", errors.New("bind lost mid-flight")) + + if *exitCode != provisionExitCodeForCode[provisionCodeUnclassified] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeUnclassified]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeUnclassified) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeUnclassified) + } + if !strings.Contains(r.Message, "listener.0") { + t.Errorf("message = %q, want it to name the listener", r.Message) + } +} + +// TestRefuseFallbackFatalDefault covers the production refuseFallbackFatal +// closure (not the test-harness override other tests install): it +// classifies UNCLASSIFIED and notifies through the given prog's log +// connection. +func TestRefuseFallbackFatalDefault(t *testing.T) { + exitCode, _ := stubProvisionGlobals(t) + server, client := net.Pipe() + t.Cleanup(func() { _ = server.Close(); _ = client.Close() }) + go func() { + buf := make([]byte, 64) + _, _ = server.Read(buf) + }() + p := &prog{logConn: client} + + refuseFallbackFatal(p, "cannot fall back: %s", "port 53 is unavailable") + + if *exitCode != provisionExitCodeForCode[provisionCodeUnclassified] { + t.Errorf("exit = %d, want %d", *exitCode, provisionExitCodeForCode[provisionCodeUnclassified]) + } + r, err := readProvisionResult() + if err != nil { + t.Fatalf("no provision result written: %v", err) + } + if r.Code != string(provisionCodeUnclassified) { + t.Errorf("code = %q, want %q", r.Code, provisionCodeUnclassified) + } + if !strings.Contains(r.Message, "port 53 is unavailable") { + t.Errorf("message = %q, want it to contain the formatted detail", r.Message) + } +} + +// terminalPathCase documents one terminal path this lane audited under +// "ctrld start --cd-org": where it lives, what triggers it, and the code it +// now reports. reachable names a test that exercises the real call site +// end-to-end (through the seams this package already exposes); a path deep +// inside the start command's Run closure / run() with no seam to trigger it +// in isolation is documented here with reachable == "" and relies on the +// shared helper tests above (TestFailProvisionUnclassified, +// TestNotifyExitToLogServer, TestReportServeDNSFailure, +// TestRefuseFallbackFatalDefault) to prove the wiring it calls into. +type terminalPathCase struct { + file string + scenario string + code provisionFailureCode + reachable string // name of the test proving this exact site, or "" if only the shared helper is exercised +} + +// TestProvisioningTerminalPathAudit is the committed enumeration required for +// "ctrld start --cd-org": every Fatal/os.Exit found by re-deriving the +// requirement (grepping commands.go's initStartCmd, cli.go's run() and its +// callers cdUIDFromProvToken/handleAPIPreflightFailure, and prog.go's +// pre-daemonization paths), reconciled against a code. None of them may +// reach a bare Fatal/os.Exit any more; each row's code is asserted against +// the known contract maps so a typo'd code name fails this test. +func TestProvisioningTerminalPathAudit(t *testing.T) { + cases := []terminalPathCase{ + // commands.go initStartCmd (parent "ctrld start" process). v1.0 keeps + // what master splits into commands_service_start.go in the same + // commands.go as every other command. + {"commands.go (initStartCmd)", "--intercept-mode is not off/dns/hard", provisionCodeInterceptModeInvalid, "TestValidateInterceptModeFlag"}, + {"commands.go (initStartCmd) / cli.go RunCobraCommand", "explicit empty --cd-org", provisionCodeProvisionTokenMalformed, "TestCheckStrFlagEmptyClassifiesEmptyCdOrg"}, + {"commands.go (initStartCmd)", "--cd/--cd-org used together with --nextdns", provisionCodeInvalidFlagCombination, "TestValidateCdAndNextDNSFlagsClassifiesConflict"}, + {"commands.go (initStartCmd)", "newService fails building the service handle", provisionCodeUnclassified, "TestStartCommandClassifiesServiceInitFailure"}, + {"commands.go (initStartCmd) / cli.go run()", "--proto is not doh/doh3 once --cd is set", provisionCodeInvalidFlagCombination, "TestValidateCdUpstreamProtocol"}, + {"commands.go (initStartCmd)", "removeServiceFlag fails while upgrading an existing service's intercept mode", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "appendServiceFlag(\"--intercept-mode\") fails during the same upgrade", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "appendServiceFlag(mode) fails during the same upgrade", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "config unmarshal fails restarting an already-installed service", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "doTasksE fails on a non-service-stage task restarting an existing service", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "socketDir() fails restarting an existing service", provisionCodeUnclassified, ""}, + // v1.0-only: the DNS intercept-mode router abstraction (predates and is + // unrelated to issue-595) configures itself before a fresh install; + // already routed through failRunUnclassified, not a bare fatal. + {"commands.go (initStartCmd)", "router.ConfigureService fails configuring the router before a fresh install", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "config unmarshal fails on a fresh install", provisionCodeUnclassified, ""}, + {"commands.go (initStartCmd)", "doTasksE fails on a non-service-stage task on a fresh install", provisionCodeUnclassified, ""}, + + // cli.go run() (the "ctrld run" child process --cd-org spawns, or a direct "ctrld run --cd-org" invocation) + {"cli.go run()", "called with a nil stop channel", provisionCodeUnclassified, ""}, + {"cli.go run()", "--daemon on windows", provisionCodeUnclassified, ""}, + {"cli.go run()", "newService fails building the background service handle", provisionCodeUnclassified, ""}, + {"cli.go run()", "readBase64Config fails", provisionCodeUnclassified, ""}, + {"cli.go run()", "config unmarshal fails", provisionCodeUnclassified, ""}, + {"cli.go run()", "network is not up", provisionCodeUnclassified, ""}, + // v1.0-only: same router abstraction as above, on the daemon side. + {"cli.go run()", "router.PreRun fails performing the router pre-run check", provisionCodeUnclassified, ""}, + {"cli.go run()", "writeConfigFile fails", provisionCodeUnclassified, ""}, + {"cli.go run()", "validateConfig fails", provisionCodeUnclassified, ""}, + {"cli.go run()", "os.Executable fails in daemon respawn", provisionCodeUnclassified, ""}, + {"cli.go run()", "os.Getwd fails in daemon respawn", provisionCodeUnclassified, ""}, + {"cli.go run()", "cmd.Start fails in daemon respawn", provisionCodeUnclassified, ""}, + + // prog.go (pre-daemonization: listener startup and DNS-intercept setup) + {"prog.go (p *prog) run()", "serveDNS fails after the listener already bound", provisionCodeUnclassified, "TestReportServeDNSFailure"}, + {"prog.go (p *prog) setDNS()", "DNS intercept fails and the interface-DNS fallback cannot reach a non-53 listener", provisionCodeUnclassified, "TestRefuseFallbackFatalDefault"}, + } + + for _, tc := range cases { + if _, ok := provisionStageForCode[tc.code]; !ok { + t.Errorf("%s: %s: code %s is not a known contract code", tc.file, tc.scenario, tc.code) + } + if tc.reachable != "" { + continue + } + t.Logf("documented (exercised only via shared helper wiring, not a standalone seam): %s: %s -> %s", tc.file, tc.scenario, tc.code) + } + + // Out of scope, left as bare Fatal/os.Exit deliberately, with reasons: + // + // - cli.go run(): stopCh nil check is still bare-Fatal-free (converted + // above) but is genuinely unreachable from any CLI invocation - the + // CLI always constructs a fresh channel. Converted anyway above for + // consistency, not because it is reachable. + // - prog.go (p *prog) setDNS() line ~962's own --intercept-mode check: + // explicitly left alone per T6 - it is the "runtime validation in + // prog.go" validInterceptMode's own doc comment calls out as distinct + // from "the early start command check" that got INTERCEPT_MODE_INVALID. + // - prog.go (p *prog) Stop() and commands.go initStartCmd's own + // deactivation-pin os.Exit: explicitly out of scope per the lane + // contract ("pin-check 126 untouched"). + // - cli.go: doValidateCdRemoteConfig's Fatal is not reachable from the + // --cd-org start path at all on v1.0 - the start command defers --cd + // validation to processCDFlags in the "ctrld run" child instead (see + // the comment in initStartCmd), a divergence from master that predates + // and is unrelated to this lane. General CLI/config-parsing helpers + // (readConfigFile, processNoConfigFlags, processListenFlag, + // readConfigWithNotice's userHomeDir failure): these predate and apply + // uniformly across every ctrld invocation mode (config-file mode, + // no-config mode, nextdns mode), not specifically to --cd-org + // provisioning. Out of scope for this lane's --cd-org enumeration. + // - checkStrFlagEmpty for --cd (cdUidFlagName) keeps its bare fatal: + // only the --cd-org case above is a provisioning-token value. + // - cli.go uninstall()'s bare fatal on a router re-configuration + // failure: the "ctrld uninstall" command, not "ctrld start --cd-org". + // + // No firewall-mode flag exists on v1.0, so master's INTERCEPT_MODE_INVALID- + // adjacent "--firewall-mode is not off/on" rows (parent and daemon-side) + // have no equivalent here. +} diff --git a/docs/provisioning-failure-codes.md b/docs/provisioning-failure-codes.md index 6d8b299..41954bc 100644 --- a/docs/provisioning-failure-codes.md +++ b/docs/provisioning-failure-codes.md @@ -13,9 +13,10 @@ stable code on three surfaces: `provisioning failed: stage= code= (exit )`. The macOS pkg `postinstall` extracts exactly this line into the installer log, so MDM consoles see it without any ctrld log configuration. -- **Exit code** — stage-scoped: bootstrap 30–39, listener 40–49, - service 50–59. Unrelated existing contracts are unchanged - (`ctrld status` exits 0–3; invalid deactivation pin exits 126). +- **Exit code** — stage-scoped: input 20–29, bootstrap 30–39, + listener 40–49, service 50–59. This does not change other existing + exit-code contracts: `ctrld status` still exits 0–3, and an invalid + deactivation pin still exits 126. A customer or administrator only needs to report the code (or the whole output line). The table below is the maintained support mapping; it must @@ -23,16 +24,109 @@ stay in sync with `cmd/cli/provision_result.go` and changes in the same MR. ## Codes +For every code below, first run `sudo ctrld diag` (`sudo ctrld diag --json` +gives a paste-safe, machine-readable copy). Get that output before you do +anything else. The command needs no running service. Without root, the +last-provisioning-result and service-state sections report permission +denied, so run it with `sudo`. It repeats the same stage, code, and message +the customer already reported. +The **Next action / evidence** column lists what to look at next. + | Code | Stage | Exit | Failure scenario | Next action / evidence | |---|---|---|---|---| -| `API_UNREACHABLE` | bootstrap | 30 | The Control D API could not be reached or answered with a retryable error (network failure, proxy interference, 5xx, timeout) and retries ran out. The service manager may retry the service later. | Check the device's network path to `api.controld.com` (DNS, proxy, firewall, captive portal). Ask for the result file's `message` and whether other TLS traffic works. | -| `API_REJECTED` | bootstrap | 31 | The API answered and permanently rejected the configuration (4xx other than 408/429): bad or revoked token, malformed request. ctrld exits without burning service-manager restarts because retrying cannot change the answer. | Verify the provision token / org configuration in the Control D dashboard. Re-push after fixing credentials. Evidence: HTTP status in the result file `message`. | -| `API_DEVICE_INVALID` | bootstrap | 32 | The API reports the device/resolver no longer exists (error code 40402). ctrld self-uninstalls its service because the identity is gone server-side. | Confirm the device was deleted or re-provisioned in the dashboard; re-provision with a current token. No local evidence needed beyond the code. | -| `LISTENER_BIND_FAILED` | listener | 41 | No listen address could be bound after all fallbacks (configured address, 0.0.0.0:53, localhost:53, port 5354, random) were exhausted. `detail.attempts` records each tried address with the UDP/TCP OS error, e.g. `address already in use` (another DNS service owns the port) or `can't assign requested address` (address not on any interface). | Read `detail.attempts`: `address already in use` → find the process owning the port (`sudo lsof -i :53 -nP`); `can't assign requested address` → the configured IP is not present on the device. Then fix the conflict or the listener config. | +| `PROVISION_TOKEN_MALFORMED` | input | 21 | The `--cd-org` value is not a provisioning code: it is empty, too short, too long, or it contains whitespace or control characters. ctrld checks this before any network call. A missing `org-v1-` prefix is not this code. That case only logs a warning, because legacy codes can lack the prefix. | Make sure that the provisioning code was copied in full, with no extra whitespace. The result file never echoes the value. | +| `CUSTOM_HOSTNAME_INVALID` | input | 22 | The `--custom-hostname` value fails ctrld's own hostname rule (`validHostname`): it is not 3 to 64 characters, or it is not in RFC1123 hostname format. ctrld checks this before any network call. The API itself does not reject a bad hostname during provisioning. When ControlD registers the device name, it folds or strips characters like space, `+`, and `.`. Because of this, a value ctrld accepts can still register under an adjusted name. That case only logs a notice, not a failure. | Fix `--custom-hostname` following the message: it names the offending characters and the allowed format. | +| `INTERCEPT_MODE_INVALID` | input | 23 | The `--intercept-mode` value is not one of `off`, `dns`, or `hard`. ctrld checks this before it installs the service. | Re-run with a valid `--intercept-mode` value. | +| `INVALID_FLAG_COMBINATION` | input | 24 | `--cd` or `--cd-org` is used together with `--nextdns`, or `--proto` is set to anything other than `doh`/`doh3` while `--cd` is in play. ctrld checks this before any network call or service install. | Re-run without the conflicting flag, or fix the invalid value. The message names the exact flags involved. | +| `API_UNREACHABLE` | bootstrap | 30 | The Control D API could not be reached or answered with a retryable error (network failure, proxy interference, 5xx, timeout) and retries ran out. The service manager may retry the service later. | Make sure that the device has network access to `api.controld.com` (DNS, proxy, firewall, captive portal). `ctrld diag`'s `api_reachability` block already ran this same probe. Ask whether other TLS traffic works. | +| `API_REJECTED` | bootstrap | 31 | The API answered and permanently rejected the configuration (4xx other than 408/429): bad or revoked token, malformed request. ctrld exits without burning service-manager restarts because retrying cannot change the answer. This is also the fallback for the four `TOKEN_*` codes below. Those codes need the API response to carry `error.metadata.reason`. An API deployment that does not yet populate that field reports every provisioning-code rejection as `API_REJECTED` instead. | Verify the provision token / org configuration in the Control D dashboard. Re-push after fixing credentials. Evidence: HTTP status in the result file `message`. | +| `API_DEVICE_INVALID` | bootstrap | 32 | The API reports the device/resolver no longer exists (error code 40402). When the daemon's own bootstrap preflight discovers this, it self-uninstalls, because the identity is gone server-side. A direct install-time config fetch can also hit this code before the service exists. On that path there is nothing to uninstall yet. | Confirm the device was deleted or re-provisioned in the dashboard; re-provision with a current token. No local evidence needed beyond the code. | +| `TOKEN_INVALID` | bootstrap | 33 | The API rejected the `--cd-org` provisioning code with reason `token_invalid`: the code is not recognized. This code needs the deployed API to send `error.metadata.reason: "token_invalid"`. See `API_REJECTED` above for the fallback when it does not. | Make sure that the code is correct. Re-enter it exactly as given. | +| `TOKEN_EXPIRED` | bootstrap | 34 | The API rejected the `--cd-org` provisioning code with reason `token_expired`. This has the same API-reason dependency as `TOKEN_INVALID`. | Get a new provisioning code from your administrator. | +| `TOKEN_LIMIT_REACHED` | bootstrap | 35 | The API rejected the `--cd-org` provisioning code with reason `token_limit_reached`: it reached its device limit. This has the same API-reason dependency as `TOKEN_INVALID`. | Free up a device slot or use a different provisioning code. | +| `TOKEN_DISABLED` | bootstrap | 36 | The API rejected the `--cd-org` provisioning code with reason `token_disabled`: the code was invalidated. This has the same API-reason dependency as `TOKEN_INVALID`. | Download a profile from an active provisioning code. | +| `LISTENER_BIND_FAILED` | listener | 41 | No listen address could be bound after all fallbacks (configured address, 0.0.0.0:53, localhost:53, random) were exhausted. `detail.attempts` records each tried address with the UDP/TCP OS error, e.g. `address already in use` (another DNS service owns the port) or `can't assign requested address` (address not on any interface). | Read `detail.attempts`: `address already in use` → find the process owning the port (`sudo lsof -i :53 -nP`); `can't assign requested address` → the configured IP is not present on the device. Then fix the conflict or the listener config. | | `LISTENER_CONFIGURED_ADDR_UNAVAILABLE` | listener | 42 | An explicitly configured listener address could not be bound and configuration checks forbid falling back to another address, or (macOS intercept mode) the required explicit address is unavailable. | The configured `ip:port` in the listener config is wrong for this device or occupied. Verify the address exists on an interface and nothing else binds it; correct the config rather than expecting fallback. | | `SERVICE_INSTALL_FAILED` | service | 51 | The OS service manager refused to install the service (launchd/systemd/SCM registration failed). | Check OS-level constraints: permissions/elevation, MDM policy blocking daemon installation, corrupted previous install. Evidence: result file `message` (service manager error), plus `launchctl print system/ctrld` / `systemctl status ctrld` / SCM state. | | `SERVICE_START_FAILED` | service | 52 | The service installed but the service manager could not start it. | Check the service manager's own log for the start error, then the ctrld home dir `ctrld.log`. Often permissions or a binary quarantined by security tooling. | -| `SERVICE_SELFCHECK_FAILED` | service | 53 | The service started but never became healthy: no fresher failure was reported by the daemon, and the post-install DNS self-check failed. The just-installed service is rolled back (uninstalled). If the daemon itself recorded a more specific failure (e.g. a listener code), that code is reported instead of this one. | Ask for the drained service log printed by `ctrld start` and the result file. If the service was running but unreachable, check host firewall rules intercepting DNS to the listener. | +| `SERVICE_SELFCHECK_FAILED` | service | 53 | The service started but never became healthy: no fresher failure was reported by the daemon, and the post-install DNS self-check failed. On a fresh install or an upgrade, the just-installed service is rolled back (uninstalled). A restart of an already installed service keeps that service installed. If the daemon itself recorded a more specific failure (e.g. a listener code), that code is reported instead of this one. | Ask for the drained service log printed by `ctrld start` and the result file. If the service was running but unreachable, check host firewall rules intercepting DNS to the listener. | +| `UNCLASSIFIED` | service | 59 | This code covers terminal failures on the provisioning boundary that predate a dedicated code. Examples include a config unmarshal, and a file-system or environment failure such as writing the config file, reading `socketDir`, or respawning as a daemon. Other examples are a service-argument update failing mid-upgrade, and a router pre-run or configuration failure. A further example is a DNS-intercept failure the interface-DNS fallback cannot safely take over from. Every one of these used to be a bare crash with nothing to read. Now they all persist a result file. | Read the result file `message`: it names the specific operation that failed and the underlying OS error. When you report it, treat this the same as any other stage-scoped failure. | + +The macOS installer script (`scripts/pkg/postinstall`) emits the package rows +below, not ctrld itself, before ctrld ever runs. They only ever exit 1 (the +script's own exit code), and they never appear in `provision_result.json`. +The same script also reuses `PROVISION_TOKEN_MALFORMED` and +`CUSTOM_HOSTNAME_INVALID` from the table above, for its own pre-flight +checks on the managed-prefs `ProvisionToken`/`CustomHostname` values. It +uses the same rule, and also logs it as `stage=package (exit 1)`. So those +two codes can appear with either stage, depending on which side rejected +the value. + +| `PROFILE_PREFS_MISSING` | package | 1 | The MDM configuration profile (managed prefs domain `com.controld.ctrld`) has no `ProvisionToken`, or the value is empty. This shows up after the postinstall script's wait loop ends: 12 attempts, 10 seconds apart, about 2 minutes. | Scope the `com.controld.ctrld` configuration profile to the device. If the profile is already on the device, set its `ProvisionToken` value. Then reinstall the package, or run `sudo ctrld start --cd-org=` by hand. | +| `INTERCEPT_MODE_INVALID` | package | 1 | On a fresh install, the profile's `InterceptMode` is set, but it is not one of the managed-prefs values (empty, `standard`, `intercept-dns`). A fresh install fails visibly, instead of silently falling back, because a managed deployment must not provision into an unintended mode. On an upgrade, the same bad value only logs a warning and keeps the existing service mode. So a bad profile edit cannot break an already-working fleet. | Fix `InterceptMode` in the profile to one of the allowed values. Then reinstall the package. | +| `SERVICE_RELOAD_FAILED` | package | 1 | An upgrade reused the existing service mode (no managed `InterceptMode` override), and `launchctl load` on the installed plist failed. | Run `sudo launchctl load /Library/LaunchDaemons/ctrld.plist` by hand, and read the error it gives. | +| `TEMP_FILE_UNAVAILABLE` | package | 1 | The script failed to create the private temp file it uses to capture ctrld's output (`/tmp` or `TMPDIR` is full or unwritable). Because ctrld never runs, the script has no diagnostics to add. | Free space in `/tmp` or `TMPDIR`, or fix its permissions. Then reinstall the package. | + +## Customer-visible message shape + +Every failure shows the same two-part shape: the fixed identifier line, then +one free-text line specific to the code. Neither part ever contains the +provision token, the resolver/device ID, or config contents. + +- **Input stage** — names the offending flag or value and the rule it + broke. For example: `--cd-org provisioning code is malformed: must be + 6-64 characters with no whitespace or control characters`. For + `CUSTOM_HOSTNAME_INVALID`, the message names the specific offending + characters and the allowed format. +- **Bootstrap, `TOKEN_*`** — one fixed sentence per code (see the table + above). For example: `the provisioning code has expired; get a new + provisioning code from your administrator`. +- **Bootstrap, `API_UNREACHABLE`/`API_REJECTED`/`API_DEVICE_INVALID`** — + When the failure surfaces during a running install's periodic resolver + refresh, the message is `ControlD API rejected this configuration (HTTP + status )`. When it surfaces during the initial provisioning call, the + message is `provision token exchange failed: ` or `failed to fetch + resolver config: `. Neither ever includes the request or response + body. +- **Listener** — names the address or addresses tried. `detail.attempts` in + the result file carries the OS error for each address and protocol. +- **Service** — For `SERVICE_INSTALL_FAILED` or `SERVICE_START_FAILED`, the + message is ` failed: ` (`` is `Install` or `Start`). + When the daemon itself reported no fresher failure, `SERVICE_SELFCHECK_FAILED` + shows one of three messages, depending on what the self-check saw. When + there is no error and the service is not running, the message is `ctrld + service did not pass its post-start self-check`. When the test query + itself errored, the message is `An error occurred while performing test + query: `. When the service was running but the query silently + failed, the message is `ctrld service was running, but a DNS query could + not be sent to its listener; check firewall rules + blocking/intercepting/redirecting DNS queries`. +- **Package** — the plain-language second line the installer script logs + right after the identifier line. Each package row's *Next action* cell + above quotes this line verbatim, because the script has no separate + result file to carry it. + +## Where each code surfaces + +- **Every ctrld-native code** (every row above except `package`): the CLI + output / persisted `ctrld.log`, `provision_result.json`, and `ctrld diag` + (which reads that same file). +- **Package-stage codes** (`PROFILE_PREFS_MISSING`, `SERVICE_RELOAD_FAILED`, + `TEMP_FILE_UNAVAILABLE`, the package's own `INTERCEPT_MODE_INVALID` check + on the managed-prefs value, and the package pre-flight's reused + `PROVISION_TOKEN_MALFORMED` and `CUSTOM_HOSTNAME_INVALID`): only + `/var/log/install.log`. ctrld never ran, so no result file exists yet. + `sudo ctrld diag` run right after this reports "none recorded" for the last + provisioning result. That is still useful: it shows that the package + script's own gate rejected the profile before ctrld had a chance to run. +- **Any ctrld-native code can also reach `/var/log/install.log`.** The + postinstall script invokes ctrld in two cases: to provision on a fresh + install, or to reapply a managed intercept mode on an upgrade. Either + time, it greps ctrld's own identifier line out of the captured output. + It re-logs that line, and appends `(ctrld exit )`. So a bootstrap or + listener code, not only the three package-only codes, can appear in the + MDM install log. Right after any installer-script failure, it also + appends a fresh `sudo ctrld diag` run, each line prefixed `diag:`. ## Reading the result file @@ -50,12 +144,37 @@ The file sits in the same directory as the persisted internal log (`ctrld.log`) for the user the service runs as. On a healthy install the file is absent. +## Support: `ctrld diag` + +Run `sudo ctrld diag` (or `sudo ctrld diag --json`) to collect the facts +support needs for a provisioning ticket, in one copy-paste-safe command. It +collects the client version and the MDM-managed preferences (macOS only, +the token reported only as present or absent, an empty value counts as +absent). It also collects the last provisioning result, the service state, +and whether the Control D API is reachable. It needs no config. It also +runs without root, but then the last-provisioning-result and the +service-state sections report permission denied, and the text report names +the step that gets the rights it needs. Even when it finds a failure, it +still exits 0: that failure is part of the report, not a command failure. +It never asks for or prints the provisioning token itself. + +The `provision_result.status` field in the JSON report is one of `none` +(no result file), `recorded`, `untrusted` (a file that fails the contract +check), `unreadable` (permission denied), or `corrupt` (a file that could +not be parsed). + ## Rules for maintainers - Codes are append-only once released. Never rename, renumber, or reuse a code or exit number; add a new one and note the deprecation here. +- API rejection reasons are append-only too: `tokenFailureCodeForReason` + maps a fixed set of `error.metadata.reason` strings to their `TOKEN_*` + code. An unrecognized or absent reason falls back to `API_REJECTED` (see + `apiFailureCode`). So a new reason string does nothing until both a + matching code and a row here exist. - Every code added in `cmd/cli/provision_result.go` needs a row here in the same MR. Tests enforce the code/stage/exit maps and that this table has - exactly one row per code. + exactly one `stage != package` row per ctrld code. The `package` rows + belong to `scripts/pkg/postinstall` and are not part of that count. - Detail must stay bounded and free of secrets: the constructor strips the provision token and cd UID and caps sizes; do not bypass it. diff --git a/internal/controld/config.go b/internal/controld/config.go index 4b7ad07..5efbcc4 100644 --- a/internal/controld/config.go +++ b/internal/controld/config.go @@ -38,6 +38,16 @@ const ( sendLogTimeout = 300 * time.Second ) +// Provisioning-token rejection reasons the API sends in error.metadata.reason +// (HTTP 400, code 40003). This list can grow; a value outside it is not an +// error, just one cmd/cli does not classify yet. +const ( + ReasonTokenInvalid = "token_invalid" + ReasonTokenExpired = "token_expired" + ReasonTokenLimitReached = "token_limit_reached" + ReasonTokenDisabled = "token_disabled" +) + // ResolverConfig represents Control D resolver data. type ResolverConfig struct { DOH string `json:"doh"` @@ -58,10 +68,41 @@ type utilityResponse struct { } `json:"body"` } +// errorMetadata carries additive, optional detail on top of Code/Message. +// Older API deployments omit it, so it must decode to its zero value rather +// than fail the whole response. Its custom UnmarshalJSON gives the same +// tolerance to a malformed value: a metadata that is not an object, or a +// Reason that is not a string (a number, an object, or null), degrades to +// the zero value rather than failing the response that contains it. +type errorMetadata struct { + // Reason is a machine-readable rejection reason sent on provisioning-token + // errors (HTTP 400, code 40003): token_invalid, token_expired, + // token_limit_reached, or token_disabled. Empty when absent or malformed; + // callers must treat any other value as unknown rather than reject the + // response. + Reason string `json:"reason"` +} + +func (m *errorMetadata) UnmarshalJSON(data []byte) error { + var raw struct { + Reason json.RawMessage `json:"reason"` + } + // Best-effort: a metadata that is not an object, or a reason that is not + // a string (number, object, null), leaves the zero value instead of + // failing this decode. Code and Message still classify the failure. + if err := json.Unmarshal(data, &raw); err != nil { + *m = errorMetadata{} + return nil + } + _ = json.Unmarshal(raw.Reason, &m.Reason) + return nil +} + type ErrorResponse struct { ErrorField struct { - Message string `json:"message"` - Code int `json:"code"` + Message string `json:"message"` + Code int `json:"code"` + Metadata errorMetadata `json:"metadata"` } `json:"error"` // StatusCode is the HTTP status the API answered with. It is not part of the JSON // body: this type is built for *any* non-200 whose body decodes, so the body alone diff --git a/internal/controld/config_test.go b/internal/controld/config_test.go index 5973de0..16a102f 100644 --- a/internal/controld/config_test.go +++ b/internal/controld/config_test.go @@ -96,3 +96,113 @@ func TestAPIErrorRecordsHTTPStatus(t *testing.T) { } }) } + +// TestAPIErrorDecodesRejectionReason pins the additive metadata.reason field the +// API sends on provisioning-token rejections. cmd/cli maps known reasons to their +// own failure codes, and must fall back cleanly when the field is absent or holds +// a value this build does not recognize yet. +func TestAPIErrorDecodesRejectionReason(t *testing.T) { + tests := []struct { + name string + body string + wantReason string + }{ + { + name: "known reason", + body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":"token_disabled"}}}`, + wantReason: "token_disabled", + }, + { + name: "reason absent", + body: `{"error":{"message":"invalid token","code":40003}}`, + wantReason: "", + }, + { + name: "unknown reason value", + body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":"something_new"}}}`, + wantReason: "something_new", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d := json.NewDecoder(strings.NewReader(tc.body)) + errResp, err := apiErrorFromResponse(http.StatusBadRequest, d) + if err != nil { + t.Fatalf("unexpected decode error: %v", err) + } + if errResp.ErrorField.Metadata.Reason != tc.wantReason { + t.Errorf("reason = %q, want %q", errResp.ErrorField.Metadata.Reason, tc.wantReason) + } + }) + } +} + +// TestAPIErrorToleratesMalformedRejectionReason pins the fix for a decode error +// confined to metadata.reason: a reason sent as the wrong JSON type must not +// discard the rest of the response. Before this fix, apiErrorFromResponse +// returned the raw decode error and nothing else, which cmd/cli's +// apiFailureCode cannot recognize as an *ErrorResponse - it falls back to +// API_UNREACHABLE (a retryable bootstrap failure) instead of the permanent +// rejection the HTTP status and code actually describe. +func TestAPIErrorToleratesMalformedRejectionReason(t *testing.T) { + tests := []struct { + name string + body string + }{ + {name: "reason as a number", body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":12345}}}`}, + {name: "reason as an object", body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":{"inner":"value"}}}}`}, + {name: "reason as null", body: `{"error":{"message":"invalid token","code":40003,"metadata":{"reason":null}}}`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d := json.NewDecoder(strings.NewReader(tc.body)) + errResp, err := apiErrorFromResponse(http.StatusBadRequest, d) + if err != nil { + t.Fatalf("a malformed reason must not fail the whole decode: %v", err) + } + if errResp.ErrorField.Message != "invalid token" { + t.Errorf("message = %q, want it to survive the malformed reason", errResp.ErrorField.Message) + } + if errResp.ErrorField.Code != 40003 { + t.Errorf("code = %d, want it to survive the malformed reason", errResp.ErrorField.Code) + } + if errResp.ErrorField.Metadata.Reason != "" { + t.Errorf("reason = %q, want empty for a malformed value", errResp.ErrorField.Metadata.Reason) + } + }) + } +} + +// TestAPIErrorToleratesMalformedMetadata pins the same tolerance one level +// up: a metadata field that is not a JSON object must not discard the rest +// of the response. Code and Message still classify the failure, and Reason +// stays empty. +func TestAPIErrorToleratesMalformedMetadata(t *testing.T) { + tests := []struct { + name string + body string + }{ + {name: "metadata as a string", body: `{"error":{"message":"invalid token","code":40003,"metadata":"foo"}}`}, + {name: "metadata as a number", body: `{"error":{"message":"invalid token","code":40003,"metadata":7}}`}, + {name: "metadata as an array", body: `{"error":{"message":"invalid token","code":40003,"metadata":["reason"]}}`}, + {name: "metadata as a boolean", body: `{"error":{"message":"invalid token","code":40003,"metadata":true}}`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d := json.NewDecoder(strings.NewReader(tc.body)) + errResp, err := apiErrorFromResponse(http.StatusBadRequest, d) + if err != nil { + t.Fatalf("unexpected decode error: %v", err) + } + if errResp.ErrorField.Code != 40003 { + t.Errorf("code = %d, want 40003", errResp.ErrorField.Code) + } + if errResp.ErrorField.Message != "invalid token" { + t.Errorf("message = %q, want %q", errResp.ErrorField.Message, "invalid token") + } + if errResp.ErrorField.Metadata.Reason != "" { + t.Errorf("reason = %q, want empty for a malformed metadata", errResp.ErrorField.Metadata.Reason) + } + }) + } +} diff --git a/internal/controld/reachability.go b/internal/controld/reachability.go new file mode 100644 index 0000000..6c0a8fc --- /dev/null +++ b/internal/controld/reachability.go @@ -0,0 +1,33 @@ +package controld + +import ( + "context" + "io" + "net/http" +) + +// ProbeReachability makes one lightweight request to the ControlD API host, +// reusing the same transport and IP-fallback logic real provisioning traffic +// takes. Any HTTP response, even an error status, counts as reachable: this +// checks the network path, not whether the endpoint accepts the request. +// +// The caller controls how long to wait via ctx; there is no timeout here +// beyond what ctx enforces. +func ProbeReachability(ctx context.Context, cdDev bool) error { + apiURL := apiURLCom + if cdDev { + apiURL = apiURLDev + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return err + } + client := &http.Client{Transport: apiTransport(cdDev)} + resp, err := doWithFallback(client, req, apiServerIP(cdDev)) + if err != nil { + return err + } + defer resp.Body.Close() + _, _ = io.Copy(io.Discard, resp.Body) + return nil +} diff --git a/internal/controld/reachability_test.go b/internal/controld/reachability_test.go new file mode 100644 index 0000000..9f39ec7 --- /dev/null +++ b/internal/controld/reachability_test.go @@ -0,0 +1,14 @@ +package controld + +import ( + "context" + "testing" +) + +func TestProbeReachabilityRespectsCancelledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := ProbeReachability(ctx, false); err == nil { + t.Error("expected an error for an already-cancelled context") + } +} diff --git a/tasks/plan.md b/tasks/plan.md deleted file mode 100644 index e1303fc..0000000 --- a/tasks/plan.md +++ /dev/null @@ -1,213 +0,0 @@ -# Plan: provisioning failure codes (issue #586) - -Spec: SPEC.md. Baseline: `ac0e6aed` on `v1.0`. -Branches: `issue-586` (off `v1.0`), `issue-586-master` (off `master`). -Two MRs, both referencing #586; the `v1.0` MR carries `Closes #586`. - -## Shared contract (fixed here so parallel tasks cannot diverge) - -| Code | Stage | Exit | -|---|---|---| -| `API_UNREACHABLE` | bootstrap | 30 | -| `API_REJECTED` | bootstrap | 31 | -| `API_DEVICE_INVALID` | bootstrap | 32 | -| `LISTENER_BIND_FAILED` | listener | 41 | -| `LISTENER_CONFIGURED_ADDR_UNAVAILABLE` | listener | 42 | -| `SERVICE_INSTALL_FAILED` | service | 51 | -| `SERVICE_START_FAILED` | service | 52 | -| `SERVICE_SELFCHECK_FAILED` | service | 53 | - -- Result file: `provision_result.json` in the ctrld home dir (same - resolution as the persisted internal log: `absHomeDir` on v1.0, the - `userHomeDir`-based equivalent on master). Atomic write (temp + - rename in the same dir). Cleared when provisioning succeeds. -- Result schema (version 1): `version`, `timestamp` (RFC3339, UTC), - `stage`, `code`, `exit_code`, `message`, optional `detail.attempts[]` - of `{addr, proto, os_error}`; attempts capped at 12 entries, every - string capped at 256 chars. -- Identifier line, exact format (greppable, token-free by - construction): `provisioning failed: stage= code= (exit )`. -- Redaction: results are built through a constructor that takes the - secrets in scope (cd UID, provision token) and strips them from every - field. Messages come from our own summaries plus OS error strings, - never raw config or API bodies. -- Exit seam: `provisionExit = os.Exit` package var so tests can stub - process exit. Emission helper `failProvision(...)` writes the file, - logs the identifier line, calls the notify func, then exits with the - stage code. Nonzero exit is preserved everywhere the daemon exits - nonzero today; the deliberate clean return on permanent API rejection - stays a clean return (result file only). - -## Dependency graph - -``` -A1 (contract module + doc, v1.0) D1 (master port) - ├─► B1 daemon emissions (cli.go) depends on: contract table - ├─► B2 start-side (commands.go+service.go) (from A1) + C1 verified - └─► B3 postinstall (scripts, tests) implementation as reference - └─► C1 v1.0 checkpoint ────────────► D1 ─► E1 final checkpoint -``` - -## Group A — serial, runs inline (1 task) - -### A1. Contract foundation on `issue-586` -Create branch `issue-586` from `v1.0`. New files: -`cmd/cli/provision_result.go`, `cmd/cli/provision_result_test.go`, -`docs/provisioning-failure-codes.md`. - -Module contents: stage type + the 8 code constants + exit-code map; -`ProvisionResult` struct per schema; bounded/redacting constructor; -atomic `writeProvisionResult` / `readProvisionResult` / -`clearProvisionResult`; identifier-line formatter; `provisionExit` -seam; `failProvision` helper. Doc: full table — code, stage, exit, -failure scenario, next safe troubleshooting action / evidence request. - -Tests (RED first): every code maps to exactly one stage and one -in-range exit code (30–39/40–49/50–59); no collision with 0–3 -(`ctrld status`) or 126 (pin); file round-trip; atomic overwrite; -clear; redaction (a result built from inputs containing a fake token -and cd UID serializes without them); attempts/string caps enforced; -identifier line matches the exact format. - -Acceptance: `go build ./...` and `go test ./cmd/cli/` green; doc rows -exactly match the constants. - -## Group B — parallel Workflow fan-out, one subagent per task, worktree isolation, branched from `issue-586` after A1 - -### B1. Daemon emissions in `cli.go` -- Bootstrap branches in `run()` (`cli.go:339-372`): - - permanent rejection (`permanentAPIRejection`): write `API_REJECTED` - result (HTTP status + our own summary, no raw API body), keep the - existing clean return and its comment. - - invalid device (`controld.InvalidConfigCode`): write - `API_DEVICE_INVALID` before `uninstallInvalidCdUID`. - - fatal fetch: write `API_UNREACHABLE`, replace - `cdLogger.Fatal()` with error log + identifier line + - `failProvision` exit 30 (still nonzero for the service manager). -- Listener (`tryUpdateListenerConfig`, `tryUpdateListenerConfigIntercept`): - - record every failed bind attempt `{addr, proto, os_error}` — - capture UDP and TCP errors separately in `tryListen` (keep - `errors.Join` for control flow), cap per contract. - - exhaustion fatal (`cli.go:1639`) and converged-random fatal - (`cli.go:1720`) → `LISTENER_BIND_FAILED` exit 41 with attempts. - - no-fallback-allowed fatal (`cli.go:1652`) and intercept-mode fatals - (`cli.go:1452,1467`) → `LISTENER_CONFIGURED_ADDR_UNAVAILABLE` - exit 42 (fallback-exhausted intercept fatal stays - `LISTENER_BIND_FAILED`). - - all fatals keep calling the notify func first; final message - includes the code string. -- Clear the result file at the point provisioning is known good - (after `updateListenerConfig` succeeds in `run()`). - -Tests (RED first): occupy a UDP+TCP port, drive the listener path to -exhaustion with the exit seam stubbed, assert the result file has -`LISTENER_BIND_FAILED`, the attempted address, both protocols' -`os_error` (`address already in use` class); pure mapping test -API error → code (permanent 4xx → `API_REJECTED`, 40402 → -`API_DEVICE_INVALID`, network error → `API_UNREACHABLE`) following -`cli_preflight_test.go` patterns. - -Acceptance: only `cmd/cli/cli.go` + new/extended tests touched; -`go build ./... && go test ./cmd/cli/` green. - -### B2. `ctrld start` reporting in `commands.go` + `service.go` -- Add `doTasksE` (returns failed task name + error; `doTasks` keeps - its signature and delegates). -- Fresh-install path (`commands.go:618`): failed `Install` task → - `SERVICE_INSTALL_FAILED` (write result, print identifier line, exit - 51); failed `Start` task → `SERVICE_START_FAILED` (exit 52). This - fixes the current fall-through that exits 0 on install failure. -- Existing-service path (`commands.go:528-543`): failure → 52 with the - same reporting (replaces bare `os.Exit(1)`). -- Self-check failure branch (`commands.go:627-664`): keep the log - drain and `uninstall(p, s)`; then read the daemon's result file — - if present and stamped after this start attempt began, report its - stage/code/exit (daemon identity wins: e.g. `LISTENER_BIND_FAILED`); - otherwise write and report `SERVICE_SELFCHECK_FAILED` exit 53. - Extract this into a testable helper (fabricated result files + - stubbed exit seam). -- On successful start (self-check ok), clear any stale result file. - -Tests (RED first): `doTasksE` failure attribution; helper precedence -(fresh daemon result wins; stale/missing falls back to 53); exit-code -selection per failed task. - -Acceptance: only `cmd/cli/commands.go`, `cmd/cli/service.go` + tests -touched; build and package tests green. - -### B3. postinstall MDM surface -- `scripts/pkg/postinstall`: capture `ctrld start` output to a - `mktemp` file (chmod 600) instead of `/dev/null`; keep the plist - check as the success gate; on failure, `grep -m1 '^provisioning - failed: '` from the capture into the install log together with the - exit code; delete the capture file always; never echo any other - output line (token safety preserved by extracting only the - fixed-format line). -- Shell test `test-scripts/darwin/test-postinstall-provision-failure.sh` - (matching existing script conventions): stub `$CTRLD` that prints a - fake token plus a valid identifier line and exits 41; assert the - logged output contains stage/code/exit and not the token; assert - success path unchanged. Runs without root. -- Update `docs/macos-pkg-mdm.md` where it documents the discard - behavior/failure triage, and add the failure-code doc link. - -Acceptance: shell test passes locally (`sh test-scripts/darwin/...`); -only `scripts/pkg/postinstall`, `test-scripts/darwin/`, `docs/` -touched. - -## Checkpoint C1 — serial, after Group B merges - -Merge order: B1, B2, B3 into `issue-586`. Then: `go build ./...`, -`go vet ./...`, `go test ./cmd/cli/...` (and full `./...`), run the B3 -shell test, verify doc table == constants, and verify each spec -acceptance criterion has an implementation + test. Fix-forward any -merge fallout before Group D starts. - -## Group D — serial (1 task, own worktree off `master`) - -### D1. Master port on `issue-586-master` -Create `git worktree` with branch `issue-586-master` from -`origin/master`/`master`. Port with the v1.0 implementation as -reference, adapted to master's structure (zap-shaped logging idiom, -no `commands.go`): - -- `cmd/cli/provision_result.go` + tests: identical contract table. -- Bootstrap: `run()` branches at master `cli.go:340-374` (same - permanent-rejection clean return, invalid-device, fatal fetch). -- Listener: `tryUpdateListenerConfig` fatals at master - `cli.go:1657/1667/1719`; intercept variant at `cli.go:1487/1502`; - per-attempt capture (bind errors currently logged at Debug, - `cli.go:1665`). -- Start side: `commands_service_start.go` — both `doTasks` call sites, - self-check `default:` arm (`os.Exit(1)` ~line 370), same fall-through - audit, same precedence logic; `doTasksE` in `service.go`. -- `docs/provisioning-failure-codes.md`: same table (omit - pkg/postinstall-specific notes; master has no `scripts/pkg`). -- No postinstall work on master. - -Tests mirrored from v1.0 where the structure allows. - -Acceptance: in the master worktree, `go build ./...`, -`go test ./cmd/cli/...` green; constants table semantically identical -to `issue-586`. - -## Checkpoint E1 — serial, final - -- Cross-branch contract equality: compare code constants, exit codes, - identifier-line format, result schema between the two branches. -- Full test suites on both branches. -- Both branches committed (per-task Conventional Commits); no pushes, - no MRs yet — `/draft-review` is the next pipeline step. - -## Execution notes - -- Each parallel group runs as one Workflow fan-out, one subagent per - task, `isolation: 'worktree'` so parallel edits never conflict; - serial tasks (A1, C1, D1, E1) run inline (D1 manages its own - master-based worktree). -- Every subagent follows RED → GREEN → regression → build and commits - in its worktree; the orchestrator merges in dependency order and - runs the full suite before the next group. -- Subagent prompts are self-contained: they carry the contract table - and file anchors from this plan, not references to SPEC.md (worktree - copies may not include untracked files). diff --git a/tasks/todo.md b/tasks/todo.md deleted file mode 100644 index 10a2693..0000000 --- a/tasks/todo.md +++ /dev/null @@ -1,21 +0,0 @@ -# TODO: issue #586 provisioning failure codes - -Groups run in order; tasks inside a parallel group run as one Workflow -fan-out (one subagent per task, worktree isolation). - -## Group A (serial) -- [x] A1: contract module `cmd/cli/provision_result.go` + tests + `docs/provisioning-failure-codes.md` on branch `issue-586` - -## Group B (parallel after A1) -- [x] B1: daemon emissions — bootstrap + listener paths in `cmd/cli/cli.go` + tests -- [x] B2: `ctrld start` reporting — `cmd/cli/commands.go`, `cmd/cli/service.go` (doTasksE, exit-0 fall-through fix, self-check precedence) + tests -- [x] B3: postinstall MDM surface — `scripts/pkg/postinstall`, shell test, `docs/macos-pkg-mdm.md` - -## Checkpoint C1 (serial) -- [x] C1: merge B1→B2→B3 into `issue-586`, full build/vet/test, shell test, doc/constants parity, spec AC audit - -## Group D (serial) -- [x] D1: master port on `issue-586-master` (contract module, cli.go emissions, commands_service_start.go, docs) + tests - -## Checkpoint E1 (serial) -- [x] E1: cross-branch contract equality, full suites on both branches, commits tidy — stop before push/MR (`/draft-review` next) diff --git a/test-scripts/darwin/test-pkg-intercept-mode.sh b/test-scripts/darwin/test-pkg-intercept-mode.sh index d1029cd..f23e930 100755 --- a/test-scripts/darwin/test-pkg-intercept-mode.sh +++ b/test-scripts/darwin/test-pkg-intercept-mode.sh @@ -36,14 +36,6 @@ printf 'launchctl %s\n' "$*" >>"$CALLS" exit 0 EOF -cat >"$bin/mktemp" <<'EOF' -#!/bin/sh -[ "${1:-}" = "-t" ] && [ -n "${2:-}" ] || exit 1 -path="$FAKE_TMPDIR/$2.capture" -: >"$path" -printf '%s\n' "$path" -EOF - cat >"$bin/ctrld" <<'EOF' #!/bin/sh printf 'ctrld %s\n' "$*" >>"$CALLS" @@ -55,7 +47,7 @@ fi exit "${FAKE_CTRLD_EXIT:-0}" EOF -chmod +x "$bin/defaults" "$bin/launchctl" "$bin/mktemp" "$bin/ctrld" +chmod +x "$bin/defaults" "$bin/launchctl" "$bin/ctrld" assert_contains() { expected=$1 @@ -91,6 +83,8 @@ run_case() { calls="$case_dir/calls" output="$case_dir/output" postinstall="$case_dir/postinstall" + case_tmpdir="$case_dir/tmp" + mkdir -p "$case_tmpdir" : >"$calls" if [ "$existing" = "1" ]; then : >"$plist" @@ -105,6 +99,7 @@ run_case() { status=0 PATH="$bin:$PATH" \ + TMPDIR="$case_tmpdir" \ CALLS="$calls" \ FAKE_TOKEN_PRESENT=1 \ FAKE_TOKEN=test-token \ @@ -112,7 +107,6 @@ run_case() { FAKE_MODE="$mode" \ FAKE_CTRLD_EXIT="$ctrld_exit" \ FAKE_PLIST="$plist" \ - FAKE_TMPDIR="$case_dir" \ "$postinstall" >"$output" 2>&1 || status=$? if [ "$status" -ne "$expected_status" ]; then @@ -149,9 +143,9 @@ case_dir=$(run_case upgrade-intercept 1 1 intercept-dns) assert_contains 'ctrld start --intercept-mode dns' "$case_dir/calls" assert_not_contains 'launchctl load' "$case_dir/calls" -case_dir=$(run_case fresh-invalid 0 1 invalid) -assert_contains 'WARNING: unsupported InterceptMode in managed preferences; using standard mode' "$case_dir/output" -assert_not_contains '--intercept-mode' "$case_dir/calls" +case_dir=$(run_case fresh-invalid 0 1 invalid 0 1) +assert_contains 'ERROR: provisioning failed: stage=package code=INTERCEPT_MODE_INVALID (exit 1)' "$case_dir/output" +assert_not_contains 'ctrld start' "$case_dir/calls" case_dir=$(run_case upgrade-invalid 1 1 invalid) assert_contains 'WARNING: unsupported InterceptMode in managed preferences; preserving existing service mode' "$case_dir/output" diff --git a/test-scripts/darwin/test-postinstall-provision-failure.sh b/test-scripts/darwin/test-postinstall-provision-failure.sh index d9f6eed..2cc454e 100755 --- a/test-scripts/darwin/test-postinstall-provision-failure.sh +++ b/test-scripts/darwin/test-postinstall-provision-failure.sh @@ -1,45 +1,168 @@ #!/bin/sh -# Test: the MDM pkg postinstall script surfaces ctrld's provisioning -# failure identifier in its output without leaking the provision token, -# and still reports success once the plist exists. +# Test: the MDM pkg postinstall script validates managed prefs before +# running ctrld, classifies every failure (pre-flight or post-invoke) with +# a stable 'stage=package code=' identifier line, best-effort appends +# 'ctrld diag' output on failure, and never leaks the provision token. # -# Self-contained and root-free: every path postinstall touches is -# redirected into a throwaway temp directory via the -# CTRLD_POSTINSTALL_{PLIST,CTRLD,PREFS} overrides, and 'defaults' is -# stubbed on PATH so the profile-wait loop resolves on its first attempt. +# Self-contained and root-free: each case runs a copy of the real +# postinstall script with its PLIST/CTRLD/PREFS assignments sed-rewritten +# to fixture paths (same seam as test-pkg-intercept-mode.sh), with +# 'defaults', 'launchctl', and 'ctrld' stubbed on PATH. No macOS-only API is +# used, so this runs as plain POSIX sh on Linux too. # -# Out of scope: the upgrade path (plist already exists) calls the real -# launchctl and is not exercised here. +# With POSTINSTALL_TEST_REAL_DEFAULTS=1 on macOS, 'defaults' is not stubbed: +# each case writes its managed-prefs values into a real plist under the +# fixture dir with 'defaults write', and the postinstall copy reads them +# with the real binary. This tests the one boundary a stub cannot: what +# 'defaults read' prints and exits with for an empty string, a boolean, a +# control character, or a missing key. ctrld and launchctl stay stubbed, so +# the run never touches the services of the machine. +# +# The wait loop between profile attempts is shortened to 0 seconds in the +# copy under test. The attempt count and the log lines stay as they are. # # Run: sh test-postinstall-provision-failure.sh -SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) -POSTINSTALL="$SCRIPT_DIR/../../scripts/pkg/postinstall" +set -eu -WORKDIR=$(mktemp -d -t ctrld-postinstall-test) || { - echo "FAIL: could not create test work directory" >&2 - exit 1 -} -trap 'rm -rf "$WORKDIR"' EXIT +repo_root=$(CDPATH= cd -- "$(dirname -- "$0")/../.." && pwd) +postinstall_source="$repo_root/scripts/pkg/postinstall" -FAKE_TOKEN="FAKE-PROVISION-TOKEN-DO-NOT-LEAK-93af0c" -export FAKE_TOKEN +fixture=$(mktemp -d "${TMPDIR:-/tmp}/ctrld-postinstall-test.XXXXXX") +trap 'rm -rf "$fixture"' EXIT HUP INT TERM -STUBBIN="$WORKDIR/stubbin" -mkdir -p "$STUBBIN" +# A well-formed fake token used for cases that must reach ctrld (success, +# ctrld-emitted-identifier passthrough). Obviously fake, but still checked +# for non-leakage like a real one would be. +GOOD_TOKEN="org-v1-FAKE93af0cDONOTLEAK" +# A present-but-malformed value (contains whitespace) for the +# PROVISION_TOKEN_MALFORMED case. +MALFORMED_TOKEN="FAKE TOKEN WITH SPACE DO NOT LEAK" +# A second, differently-shaped org-v1- token: never the value read from +# managed prefs, so it can only be caught by the pattern-based redaction, +# not the exact-match one. +OTHER_ORG_TOKEN="org-v1-DIFFERENTshape001" +export GOOD_TOKEN MALFORMED_TOKEN OTHER_ORG_TOKEN -cat > "$STUBBIN/defaults" <<'STUB' +# Real mktemp, resolved before the fixture bin/ overrides PATH, so the +# mktemp stub can fall through to it when a test case is not forcing a +# failure. +REAL_MKTEMP=$(command -v mktemp) +export REAL_MKTEMP + +bin="$fixture/bin" +mkdir -p "$bin" + +real_defaults=${POSTINSTALL_TEST_REAL_DEFAULTS:-0} +if [ "$real_defaults" = "1" ] && [ "$(uname -s)" != "Darwin" ]; then + echo "POSTINSTALL_TEST_REAL_DEFAULTS=1 needs macOS: the real 'defaults' binary is the point of this mode" >&2 + exit 2 +fi + +# --- Stubs ----------------------------------------------------------- + +if [ "$real_defaults" != "1" ]; then +cat >"$bin/defaults" <<'EOF' #!/bin/sh -# Stand-in for macOS 'defaults read ': answers ProvisionToken -# immediately, like a profile that only sets that one key, so the -# postinstall wait loop never has to sleep. -if [ "$1" = "read" ] && [ "$3" = "ProvisionToken" ]; then - echo "$FAKE_TOKEN" +# Stand-in for macOS 'defaults read '. Every key answers from +# a FAKE_*_PRESENT/FAKE_* pair the test case exports beforehand; a key with +# no *_PRESENT=1 comes back unset, like a profile that never set it. +key=${3:-} +case "$key" in + ProvisionToken) + [ "${FAKE_TOKEN_PRESENT:-0}" = "1" ] || exit 1 + printf '%s\n' "${FAKE_TOKEN_VALUE:-}" + ;; + CustomHostname) + [ "${FAKE_HOST_PRESENT:-0}" = "1" ] || exit 1 + printf '%s\n' "${FAKE_HOST:-}" + ;; + InterceptMode) + [ "${FAKE_MODE_PRESENT:-0}" = "1" ] || exit 1 + printf '%s\n' "${FAKE_MODE:-}" + ;; + UseDevEnvironment) + [ "${FAKE_DEVENV_PRESENT:-0}" = "1" ] || exit 1 + printf '%s\n' "${FAKE_DEVENV:-}" + ;; + *) + exit 1 + ;; +esac +EOF +chmod +x "$bin/defaults" +fi + +# write_real_prefs materializes the FAKE_* managed-prefs values of the +# current case into a real plist at "$1", the path the postinstall copy +# reads as its PREFS domain. A key with no *_PRESENT=1 is not written, like +# a profile that never set it. +write_real_prefs() { + domain=$1 + [ "${FAKE_TOKEN_PRESENT:-0}" = "1" ] && defaults write "$domain" ProvisionToken -string "${FAKE_TOKEN_VALUE:-}" + [ "${FAKE_HOST_PRESENT:-0}" = "1" ] && defaults write "$domain" CustomHostname -string "${FAKE_HOST:-}" + [ "${FAKE_MODE_PRESENT:-0}" = "1" ] && defaults write "$domain" InterceptMode -string "${FAKE_MODE:-}" + [ "${FAKE_DEVENV_PRESENT:-0}" = "1" ] && defaults write "$domain" UseDevEnvironment -string "${FAKE_DEVENV:-}" + return 0 +} + +cat >"$bin/launchctl" <<'EOF' +#!/bin/sh +[ -n "${CALLS:-}" ] && printf 'launchctl %s\n' "$*" >>"$CALLS" +exit "${FAKE_LAUNCHCTL_EXIT:-0}" +EOF +chmod +x "$bin/launchctl" + +cat >"$bin/ctrld" <<'EOF' +#!/bin/sh +[ -n "${CALLS:-}" ] && printf 'ctrld %s\n' "$*" >>"$CALLS" + +if [ "$1" = "diag" ]; then + if [ "${FAKE_DIAG_TRAPS_TERM:-0}" = "1" ]; then + # Ignores the polite signal, so only SIGKILL can end it. Proves + # postinstall's wait stays bounded either way. + trap '' TERM + sleep 30 + exit 0 + fi + if [ "${FAKE_DIAG_WORKS:-0}" = "1" ]; then + echo "resolver: healthy" + echo "listener: 127.0.0.1:53 bound" + [ -n "${FAKE_DIAG_LEAK_LINE:-}" ] && echo "$FAKE_DIAG_LEAK_LINE" + exit 0 + fi + echo "ctrld: unknown command \"diag\"" >&2 + exit 1 +fi + +if [ "${FAKE_CTRLD_EXIT:-0}" = "0" ]; then + : >"$FAKE_PLIST" exit 0 fi -exit 1 -STUB -chmod +x "$STUBBIN/defaults" +# The service manager writes the plist at install time, before the start +# step, so a start failure leaves it behind. Reproduced so the test can +# prove postinstall does not read that leftover as success. +[ "${FAKE_CTRLD_CREATE_PLIST:-0}" = "1" ] && : >"$FAKE_PLIST" +# A real ctrld may echo the raw token on error; reproduced here so the test +# can prove postinstall never lets it reach install.log. +echo "$GOOD_TOKEN" +[ -n "${FAKE_CTRLD_IDENTIFIER:-}" ] && echo "$FAKE_CTRLD_IDENTIFIER" +exit "$FAKE_CTRLD_EXIT" +EOF +chmod +x "$bin/ctrld" + +cat >"$bin/mktemp" <<'EOF' +#!/bin/sh +# Stand-in for mktemp: forces the postinstall script's own mktemp failure +# branch on demand, otherwise falls through to the real mktemp. +if [ "${FAKE_MKTEMP_FAIL:-0}" = "1" ]; then + exit 1 +fi +exec "$REAL_MKTEMP" "$@" +EOF +chmod +x "$bin/mktemp" + +# --- Assertion helpers ------------------------------------------------- FAILURES=0 @@ -71,78 +194,469 @@ assert_not_contains() { esac } -# run_postinstall runs postinstall with the given plist/ctrld overrides and -# a private TMPDIR, so the caller can check that the postinstall's own -# capture file (created inside that TMPDIR via mktemp) does not survive. -run_postinstall() { +assert_not_contains "$(cat "$postinstall_source")" "CTRLD_POSTINSTALL_" "production script must not depend on removed env-var overrides" + +# --- Fixture plumbing --------------------------------------------------- + +# make_postinstall_copy writes a copy of the real postinstall script with +# its PLIST/CTRLD/PREFS assignments rewritten to fixture paths, so the test +# drives the actual production logic instead of a re-implementation. +make_postinstall_copy() { plist_override=$1 ctrld_override=$2 - capture_tmpdir=$3 - output=$(PATH="$STUBBIN:$PATH" \ - TMPDIR="$capture_tmpdir" \ - CTRLD_POSTINSTALL_PLIST="$plist_override" \ - CTRLD_POSTINSTALL_CTRLD="$ctrld_override" \ - CTRLD_POSTINSTALL_PREFS="/does/not/matter" \ - sh "$POSTINSTALL" 2>&1) - exit_code=$? + prefs_override=$3 + dest=$4 + sed \ + -e "s|^PLIST=\"/Library/LaunchDaemons/ctrld.plist\"\$|PLIST=\"$plist_override\"|" \ + -e "s|^CTRLD=\"/usr/local/bin/ctrld\"\$|CTRLD=\"$ctrld_override\"|" \ + -e "s|^PREFS=\"/Library/Managed Preferences/com.controld.ctrld\"\$|PREFS=\"$prefs_override\"|" \ + -e "s|^ sleep 10\$| sleep 0|" \ + "$postinstall_source" >"$dest" + chmod +x "$dest" + # A substitution that stops matching is silent: the copy keeps the real + # system paths and the case runs against the installed machine. Check + # that each redirection landed. + for expected in "PLIST=\"$plist_override\"" "CTRLD=\"$ctrld_override\"" "PREFS=\"$prefs_override\""; do + if ! grep -Fq -- "$expected" "$dest"; then + printf 'FAIL: fixture substitution did not apply (%s); postinstall paths changed?\n' "$expected" >&2 + exit 1 + fi + done + if grep -Eq '^(PLIST|CTRLD|PREFS)="(/Library|/usr/local)' "$dest"; then + printf 'FAIL: a real system path survived substitution\n' >&2 + grep -E '^(PLIST|CTRLD|PREFS)=' "$dest" >&2 + exit 1 + fi } -# --- Failure case: ctrld reports a listener bind failure --------------- +# reset_fakes clears every FAKE_*/CALLS override so one case's env can never +# leak into the next. +reset_fakes() { + unset FAKE_TOKEN_PRESENT FAKE_TOKEN_VALUE FAKE_HOST_PRESENT FAKE_HOST \ + FAKE_MODE_PRESENT FAKE_MODE FAKE_DEVENV_PRESENT FAKE_DEVENV \ + FAKE_CTRLD_EXIT FAKE_CTRLD_IDENTIFIER FAKE_CTRLD_CREATE_PLIST \ + FAKE_DIAG_WORKS FAKE_DIAG_LEAK_LINE FAKE_DIAG_TRAPS_TERM \ + FAKE_LAUNCHCTL_EXIT FAKE_MKTEMP_FAIL CALLS 2>/dev/null || true +} -failure_dir="$WORKDIR/failure" -failure_tmp="$failure_dir/tmp" -mkdir -p "$failure_tmp" -failure_plist="$failure_dir/ctrld.plist" -failure_ctrld="$failure_dir/ctrld" +# run_case sets up a fresh fixture dir (plist, prefs, private TMPDIR, a +# calls log) for "$name", runs a fixture-rewritten postinstall copy with +# every currently-exported FAKE_* var visible to the stubs, and leaves +# $output/$exit_code/$case_dir set for the caller's assertions. +run_case() { + name=$1 + existing_plist=${2:-0} + case_dir="$fixture/$name" + case_tmp="$case_dir/tmp" + mkdir -p "$case_tmp" + plist="$case_dir/ctrld.plist" + prefs="$case_dir/prefs" + calls="$case_dir/calls" + output_file="$case_dir/output" + postinstall="$case_dir/postinstall" + : >"$calls" + [ "$existing_plist" = "1" ] && : >"$plist" + [ "$real_defaults" = "1" ] && write_real_prefs "$prefs" -cat > "$failure_ctrld" <<'STUB' -#!/bin/sh -# Stands in for a ctrld that fails to bind its listener: echoes the raw -# token (as ctrld's own error output may) plus the fixed-format failure -# identifier behind a log-style prefix, then exits with the stage code. -echo "$FAKE_TOKEN" -echo "2024-01-01T00:00:00Z ERR ctrld: provisioning failed: stage=listener code=LISTENER_BIND_FAILED (exit 41)" -exit 41 -STUB -chmod +x "$failure_ctrld" + make_postinstall_copy "$plist" "$bin/ctrld" "$prefs" "$postinstall" -run_postinstall "$failure_plist" "$failure_ctrld" "$failure_tmp" + status=0 + PATH="$bin:$PATH" \ + TMPDIR="$case_tmp" \ + FAKE_PLIST="$plist" \ + CALLS="$calls" \ + "$postinstall" >"$output_file" 2>&1 || status=$? -assert_eq "$exit_code" "1" "failure case: postinstall exit code" -assert_contains "$output" "stage=listener" "failure case: output names the stage" -assert_contains "$output" "LISTENER_BIND_FAILED" "failure case: output names the code" -assert_contains "$output" "41" "failure case: output names the exit code" -assert_not_contains "$output" "$FAKE_TOKEN" "failure case: output must not contain the provision token" + exit_code=$status + output=$(cat "$output_file") + calls_out=$(cat "$calls") +} -leftover=$(ls -A "$failure_tmp" 2>/dev/null) -assert_eq "$leftover" "" "failure case: capture temp file removed" +# --- Case: missing prefs -> PROFILE_PREFS_MISSING ----------------------- +# Runs postinstall's real wait loop to exhaustion (~110s): no seam shortens +# it, since that timing is untouched by this change. -# --- Success case: ctrld provisions and writes the plist ---------------- +reset_fakes +run_case missing-prefs -success_dir="$WORKDIR/success" -success_tmp="$success_dir/tmp" -mkdir -p "$success_tmp" -success_plist="$success_dir/ctrld.plist" -success_ctrld="$success_dir/ctrld" +assert_eq "$exit_code" "1" "missing-prefs: exit code" +assert_contains "$output" "stage=package code=PROFILE_PREFS_MISSING (exit 1)" "missing-prefs: identifier line" +assert_contains "$output" "Scope the com.controld.ctrld configuration profile" "missing-prefs: remediation hint kept" +assert_not_contains "$calls_out" "ctrld start" "missing-prefs: ctrld start never invoked" -cat > "$success_ctrld" < "$success_plist" -exit 0 -STUB -chmod +x "$success_ctrld" +# --- Case: malformed token -> PROVISION_TOKEN_MALFORMED ----------------- -run_postinstall "$success_plist" "$success_ctrld" "$success_tmp" +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$MALFORMED_TOKEN" +run_case malformed-token -assert_eq "$exit_code" "0" "success case: postinstall exit code" -assert_contains "$output" "provisioning complete" "success case: output mentions success" +assert_eq "$exit_code" "1" "malformed-token: exit code" +assert_contains "$output" "stage=package code=PROVISION_TOKEN_MALFORMED (exit 1)" "malformed-token: identifier line" +assert_not_contains "$output" "$MALFORMED_TOKEN" "malformed-token: token value must not leak" +assert_not_contains "$calls_out" "ctrld start" "malformed-token: ctrld start never invoked" + +# --- Case: bad CustomHostname -> CUSTOM_HOSTNAME_INVALID ----------------- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_HOST_PRESENT=1 FAKE_HOST="bad host!" +run_case bad-hostname + +assert_eq "$exit_code" "1" "bad-hostname: exit code" +assert_contains "$output" "stage=package code=CUSTOM_HOSTNAME_INVALID (exit 1)" "bad-hostname: identifier line" +assert_contains "$output" "bad host!" "bad-hostname: message names the hostname" +assert_not_contains "$output" "$GOOD_TOKEN" "bad-hostname: token value must not leak" +assert_not_contains "$calls_out" "ctrld start" "bad-hostname: ctrld start never invoked" + +# --- Case: token pasted into CustomHostname -> message is redacted -------- +# An admin can fill any managed-prefs key with the token by mistake. The +# CUSTOM_HOSTNAME_INVALID message echoes the value, so the log line must +# carry the redaction marker and never the token. + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_HOST_PRESENT=1 FAKE_HOST="${GOOD_TOKEN}!" +run_case token-in-hostname + +assert_eq "$exit_code" "1" "token-in-hostname: exit code" +assert_contains "$output" "stage=package code=CUSTOM_HOSTNAME_INVALID (exit 1)" "token-in-hostname: identifier line" +assert_contains "$output" "[redacted-token]" "token-in-hostname: message carries the redaction marker" +assert_not_contains "$output" "$GOOD_TOKEN" "token-in-hostname: token value must not leak" + +# --- Case: token pasted into UseDevEnvironment -> warning is redacted ------ + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_DEVENV_PRESENT=1 FAKE_DEVENV="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=0 +run_case token-in-dev-env + +assert_eq "$exit_code" "0" "token-in-dev-env: exit code" +assert_contains "$output" "WARNING: UseDevEnvironment value" "token-in-dev-env: still warns on the bad value" +assert_contains "$output" "[redacted-token]" "token-in-dev-env: warning carries the redaction marker" +assert_not_contains "$output" "$GOOD_TOKEN" "token-in-dev-env: token value must not leak" + +# --- Case: CustomHostname structural failures (RFC1123 mirror) ----------- +# Same rule as cmd/cli/hostname.go's validHostname: 3-64 characters, then +# dot-separated labels that start and end on a letter or digit (no +# leading/trailing hyphen or dot, no empty label). None of these contain a +# disallowed character, so only the structural check can catch them. + +bad_hostname_idx=0 +for bad_host in '-' '.abc' 'abc-' 'a'; do + bad_hostname_idx=$((bad_hostname_idx + 1)) + reset_fakes + export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" + export FAKE_HOST_PRESENT=1 FAKE_HOST="$bad_host" + run_case "bad-hostname-struct-$bad_hostname_idx" + assert_eq "$exit_code" "1" "bad-hostname-struct '$bad_host': exit code" + assert_contains "$output" "stage=package code=CUSTOM_HOSTNAME_INVALID (exit 1)" "bad-hostname-struct '$bad_host': identifier line" + assert_not_contains "$calls_out" "ctrld start" "bad-hostname-struct '$bad_host': ctrld start never invoked" +done + +long_host="" +i=0 +while [ "$i" -lt 90 ]; do + long_host="${long_host}a" + i=$((i + 1)) +done + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_HOST_PRESENT=1 FAKE_HOST="$long_host" +run_case bad-hostname-too-long + +assert_eq "$exit_code" "1" "bad-hostname-too-long: exit code" +assert_contains "$output" "stage=package code=CUSTOM_HOSTNAME_INVALID (exit 1)" "bad-hostname-too-long: identifier line" +assert_not_contains "$calls_out" "ctrld start" "bad-hostname-too-long: ctrld start never invoked" + +# --- Case: dotted CustomHostname is valid, passes through ---------------- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_HOST_PRESENT=1 FAKE_HOST="foo.bar" +export FAKE_CTRLD_EXIT=0 +run_case good-hostname-dotted + +assert_eq "$exit_code" "0" "good-hostname-dotted: exit code" +assert_contains "$calls_out" "--custom-hostname=foo.bar" "good-hostname-dotted: passed through to ctrld" + +# --- Case: bad InterceptMode, fresh install -> fails, INTERCEPT_MODE_INVALID + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_MODE_PRESENT=1 FAKE_MODE="bogus" +run_case bad-mode-fresh + +assert_eq "$exit_code" "1" "bad-mode-fresh: exit code" +assert_contains "$output" "stage=package code=INTERCEPT_MODE_INVALID (exit 1)" "bad-mode-fresh: identifier line" +assert_not_contains "$output" "$GOOD_TOKEN" "bad-mode-fresh: token value must not leak" +assert_not_contains "$calls_out" "ctrld start" "bad-mode-fresh: ctrld start never invoked" + +# --- Case: bad InterceptMode, upgrade -> warns, keeps mode, exit 0 ------- + +reset_fakes +export FAKE_MODE_PRESENT=1 FAKE_MODE="bogus" +run_case bad-mode-upgrade 1 + +assert_eq "$exit_code" "0" "bad-mode-upgrade: exit code" +assert_contains "$output" "WARNING: unsupported InterceptMode in managed preferences; preserving existing service mode" "bad-mode-upgrade: warns" +assert_contains "$calls_out" "launchctl load" "bad-mode-upgrade: falls back to launchctl load" +assert_not_contains "$calls_out" "ctrld " "bad-mode-upgrade: ctrld start never invoked" + +# --- Case: upgrade reload fails -> SERVICE_RELOAD_FAILED ----------------- +# No managed InterceptMode override, so postinstall falls back to +# reloading the existing service; launchctl itself fails. + +reset_fakes +export FAKE_LAUNCHCTL_EXIT=1 +run_case reload-fails 1 + +assert_eq "$exit_code" "1" "reload-fails: exit code" +assert_contains "$output" "stage=package code=SERVICE_RELOAD_FAILED (exit 1)" "reload-fails: identifier line" +assert_contains "$output" "sudo launchctl load" "reload-fails: remediation hint kept" +assert_contains "$calls_out" "launchctl load" "reload-fails: launchctl load was attempted" +assert_contains "$calls_out" "ctrld diag" "reload-fails: diag block attempted" + +# --- Case: unrecognized UseDevEnvironment value -> warns, still installs - + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_DEVENV_PRESENT=1 FAKE_DEVENV="banana" +export FAKE_CTRLD_EXIT=0 +run_case bad-dev-env + +assert_eq "$exit_code" "0" "bad-dev-env: exit code" +assert_contains "$output" 'WARNING: UseDevEnvironment value "banana" is not recognized; ignoring' "bad-dev-env: warns on the bad value" +assert_contains "$output" "provisioning complete, service installed" "bad-dev-env: install still proceeds" +assert_not_contains "$calls_out" "--dev" "bad-dev-env: unrecognized value never turns on --dev" +assert_not_contains "$output" "$GOOD_TOKEN" "bad-dev-env: token value must not leak" + +# --- Case: ctrld-emitted identifier passthrough -------------------------- +# The token and hostname both pass postinstall's own shape checks; ctrld +# itself rejects the token and reports its own fixed-format identifier, +# which postinstall must surface unmodified. + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=22 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=input code=PROVISION_TOKEN_MALFORMED (exit 22)" +run_case ctrld-identifier-passthrough + +assert_eq "$exit_code" "1" "identifier-passthrough: exit code" +assert_contains "$output" "provisioning failed: stage=input code=PROVISION_TOKEN_MALFORMED (exit 22)" "identifier-passthrough: surfaces ctrld's own identifier" +assert_contains "$output" "ctrld exit 22" "identifier-passthrough: names ctrld's exit code" +assert_not_contains "$output" "$GOOD_TOKEN" "identifier-passthrough: token value must not leak" + +# --- Case: ctrld-emitted listener-stage identifier, capture cleaned up ---- +# A different stage/code than the input-stage example above (a listener +# bind conflict once ctrld actually ran), and explicitly checks the private +# capture temp file left no trace - the original regression this whole +# suite grew from. + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=41 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=listener code=LISTENER_BIND_FAILED (exit 41)" +run_case ctrld-listener-bind-failure + +assert_eq "$exit_code" "1" "listener-bind-failure: exit code" +assert_contains "$output" "stage=listener" "listener-bind-failure: output names the stage" +assert_contains "$output" "LISTENER_BIND_FAILED" "listener-bind-failure: output names the code" +assert_contains "$output" "ctrld exit 41" "listener-bind-failure: names ctrld's exit code" +assert_not_contains "$output" "$GOOD_TOKEN" "listener-bind-failure: token value must not leak" + +leftover=$(find "$fixture/ctrld-listener-bind-failure/tmp" -type f 2>/dev/null) +assert_eq "$leftover" "" "listener-bind-failure: capture temp file removed" + +# --- Case: service start fails after install wrote the plist ------------- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=52 FAKE_CTRLD_CREATE_PLIST=1 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=service code=SERVICE_START_FAILED (exit 52)" +run_case start-fails-plist-present + +assert_eq "$exit_code" "1" "start-fails-plist-present: exit code" +assert_contains "$output" "stage=service code=SERVICE_START_FAILED (exit 52)" "start-fails-plist-present: identifier line" +assert_not_contains "$output" "provisioning complete" "start-fails-plist-present: leftover plist is not success" +assert_not_contains "$output" "$GOOD_TOKEN" "start-fails-plist-present: token value must not leak" + +# --- Case: empty ProvisionToken -> PROFILE_PREFS_MISSING with its own hint -- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="" +run_case empty-token + +assert_eq "$exit_code" "1" "empty-token: exit code" +assert_contains "$output" "stage=package code=PROFILE_PREFS_MISSING (exit 1)" "empty-token: identifier line" +assert_contains "$output" "ProvisionToken value is empty" "empty-token: names the empty value, not a missing profile" +assert_not_contains "$calls_out" "ctrld start" "empty-token: ctrld start never invoked" + +# --- Case: control character in token arrives escaped, passes preflight ---- +# 'defaults read' prints a non-whitespace control character as escaped text +# (a byte 0x01 becomes the four characters \001), so the preflight sees a +# plain 6-64 character value and hands it to ctrld unchanged. The stub emits +# that escaped text; real-defaults mode stores the raw byte and lets the +# real binary escape it. + +reset_fakes +if [ "$real_defaults" = "1" ]; then + export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$(printf 'org-v1-abc\001def')" +else + export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE='org-v1-abc\001def' +fi +export FAKE_CTRLD_EXIT=0 +run_case control-char-token-escaped + +assert_eq "$exit_code" "0" "control-char-token-escaped: preflight passes the escaped text" +assert_contains "$calls_out" 'ctrld start --cd-org=org-v1-abc\001def' "control-char-token-escaped: ctrld receives the escaped text unchanged" +assert_not_contains "$output" "PROVISION_TOKEN_MALFORMED" "control-char-token-escaped: preflight does not classify escaped text as malformed" + +# --- Case: UseDevEnvironment false -> no warning, no --dev ----------------- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_DEVENV_PRESENT=1 FAKE_DEVENV="0" +export FAKE_CTRLD_EXIT=0 +run_case dev-env-false + +assert_eq "$exit_code" "0" "dev-env-false: exit code" +assert_not_contains "$output" "WARNING: UseDevEnvironment" "dev-env-false: a boolean false is a recognized value" +assert_not_contains "$calls_out" "--dev" "dev-env-false: false never turns on --dev" + +# --- Case: success -------------------------------------------------------- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=0 +export FAKE_DIAG_WORKS=1 +run_case success + +assert_eq "$exit_code" "0" "success: exit code" +assert_contains "$output" "provisioning complete, service installed" "success: reports success" +assert_not_contains "$output" "diag:" "success: no diag block on the success path" +assert_not_contains "$output" "$GOOD_TOKEN" "success: token value must not leak" + +leftover=$(find "$fixture/success/tmp" -type f 2>/dev/null) +assert_eq "$leftover" "" "success: capture temp file removed" + +# --- Case: diag present on failure ---------------------------------------- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=22 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=input code=PROVISION_TOKEN_MALFORMED (exit 22)" +export FAKE_DIAG_WORKS=1 +run_case diag-present + +assert_eq "$exit_code" "1" "diag-present: exit code" +assert_contains "$output" "ctrld postinstall: diag: resolver: healthy" "diag-present: diag output appended" +assert_contains "$output" "ctrld postinstall: diag: listener: 127.0.0.1:53 bound" "diag-present: every diag line prefixed" +assert_not_contains "$output" "$GOOD_TOKEN" "diag-present: token value must not leak" + +leftover=$(find "$fixture/diag-present/tmp" -type f 2>/dev/null) +assert_eq "$leftover" "" "diag-present: capture and diag temp files removed" + +# --- Case: diag output redacts the token, fresh install ------------------ +# diag leaks both the real (managed-prefs) token and an unrelated org-v1- +# shaped string; the first is caught by the exact-match swap, the second +# only by the pattern-based one. The rest of the leaked line must survive. + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=22 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=input code=PROVISION_TOKEN_MALFORMED (exit 22)" +export FAKE_DIAG_WORKS=1 +export FAKE_DIAG_LEAK_LINE="leaked: $GOOD_TOKEN and $OTHER_ORG_TOKEN" +run_case diag-redacts-token + +assert_eq "$exit_code" "1" "diag-redacts-token: exit code" +assert_not_contains "$output" "$GOOD_TOKEN" "diag-redacts-token: real token value must not leak" +assert_not_contains "$output" "$OTHER_ORG_TOKEN" "diag-redacts-token: unrelated org-v1- string must not leak" +assert_contains "$output" "leaked: [redacted-token] and org-v1-[redacted]" "diag-redacts-token: both redacted, rest of the line intact" +assert_contains "$output" "ctrld postinstall: diag: resolver: healthy" "diag-redacts-token: unrelated diag lines untouched" + +# --- Case: diag output redacts the token, upgrade path -------------------- +# Same redaction, but on the upgrade branch that applies a managed +# InterceptMode: it never reads ProvisionToken for its own purposes, so +# this proves redact_token is still populated there. + +reset_fakes +export FAKE_MODE_PRESENT=1 FAKE_MODE="standard" +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=22 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=input code=PROVISION_TOKEN_MALFORMED (exit 22)" +export FAKE_DIAG_WORKS=1 +export FAKE_DIAG_LEAK_LINE="leaked: $GOOD_TOKEN and $OTHER_ORG_TOKEN" +run_case diag-redacts-token-upgrade 1 + +assert_eq "$exit_code" "1" "diag-redacts-token-upgrade: exit code" +assert_contains "$calls_out" "ctrld start --intercept-mode off" "diag-redacts-token-upgrade: applied managed InterceptMode" +assert_not_contains "$output" "$GOOD_TOKEN" "diag-redacts-token-upgrade: real token value must not leak" +assert_not_contains "$output" "$OTHER_ORG_TOKEN" "diag-redacts-token-upgrade: unrelated org-v1- string must not leak" +assert_contains "$output" "leaked: [redacted-token] and org-v1-[redacted]" "diag-redacts-token-upgrade: both redacted, rest of the line intact" + +# --- Case: diag absent/failing -> identifier still logged (fallback) ----- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=1 +run_case diag-absent + +assert_eq "$exit_code" "1" "diag-absent: exit code" +assert_contains "$output" "provisioning failed with no identifier; see /var/log/install.log and run ctrld diag" "diag-absent: falls back when ctrld reports no identifier" +assert_not_contains "$output" "invalid/expired token, or no network" "diag-absent: old speculative wording is gone" +assert_not_contains "$output" "ctrld postinstall: diag:" "diag-absent: no diag block when diag fails" +assert_not_contains "$output" "$GOOD_TOKEN" "diag-absent: token value must not leak" + +leftover=$(find "$fixture/diag-absent/tmp" -type f 2>/dev/null) +assert_eq "$leftover" "" "diag-absent: capture temp file removed" + +# --- Case: diag ignores SIGTERM -> SIGKILL escalation bounds the wait ---- +# Without the escalation this would hang for the diag stub's full 30s +# sleep (or longer, on a real ignore-forever daemon); the grace loop above +# already spends up to 10s, so 20s is a generous bound that still catches +# a regression back to a plain SIGTERM-and-wait. + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_CTRLD_EXIT=22 +export FAKE_CTRLD_IDENTIFIER="provisioning failed: stage=input code=PROVISION_TOKEN_MALFORMED (exit 22)" +export FAKE_DIAG_TRAPS_TERM=1 +start_ts=$(date +%s) +run_case diag-ignores-term +end_ts=$(date +%s) +elapsed=$((end_ts - start_ts)) + +assert_eq "$exit_code" "1" "diag-ignores-term: exit code" +if [ "$elapsed" -gt 20 ]; then + fail "diag-ignores-term: took ${elapsed}s, expected the SIGKILL escalation to keep this well under 30s" +fi +assert_not_contains "$output" "$GOOD_TOKEN" "diag-ignores-term: token value must not leak" + +# --- Case: mktemp failure -> TEMP_FILE_UNAVAILABLE, no diag attempted ---- + +reset_fakes +export FAKE_TOKEN_PRESENT=1 FAKE_TOKEN_VALUE="$GOOD_TOKEN" +export FAKE_MKTEMP_FAIL=1 +run_case tempfile-unavailable + +assert_eq "$exit_code" "1" "tempfile-unavailable: exit code" +assert_contains "$output" "stage=package code=TEMP_FILE_UNAVAILABLE (exit 1)" "tempfile-unavailable: identifier line" +assert_contains "$output" "free space in /tmp or TMPDIR" "tempfile-unavailable: remediation hint" +assert_not_contains "$calls_out" "ctrld " "tempfile-unavailable: ctrld never invoked" +assert_not_contains "$output" "ctrld postinstall: diag:" "tempfile-unavailable: no diag block without a capture file" +assert_not_contains "$output" "$GOOD_TOKEN" "tempfile-unavailable: token value must not leak" if [ "$FAILURES" -gt 0 ]; then echo "$FAILURES assertion(s) failed" >&2 exit 1 fi -echo "OK: postinstall surfaces provisioning failure codes without leaking the token" +if [ "$real_defaults" = "1" ]; then + echo "OK (real defaults): postinstall classifies failures, surfaces diagnostics, and never leaks the token" +else + echo "OK (stubbed defaults): postinstall classifies failures, surfaces diagnostics, and never leaks the token" +fi exit 0