Skip to content

fix(cli): kates ctx use, delete, export, import and current exit 1 when they fail - #380

Merged
bmscomp merged 1 commit into
mainfrom
fix/audit-11-ctx-exit
Oct 18, 2026
Merged

bmscomp merged 1 commit into
mainfrom
fix/audit-11-ctx-exit

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 17, 2026

Copy link
Copy Markdown
Owner

Why

Five kates ctx commands print an error when they fail and then exit 0. A script cannot tell that they failed.

Take a CI step or runbook like kates ctx import --file ci-contexts.yaml && kates ctx use lab && kates test create --type STRESS .... Suppose the file has no lab context, because it was renamed or mistyped. ctx use prints Context 'lab' not found and exits 0. The && chain goes on, and the test runs against whichever context was current. On an engineer's machine that may be prod. The same goes for disruption run in place of test create.

Checked in cli/cmd/config.go on main:

  • ctxUseCmd (:151), ctxDeleteCmd (:179), ctxCurrentCmd (:215), ctxExportCmd (:320) and ctxImportCmd (:375) use Run. Each failure is output.Error(msg); return, and Run has no error to return. The failures are:
    • ctx use and ctx delete: a missing context, or a failed save (:165/:168, :199/:202).
    • ctx current: an unknown --context (:225). With no active context it only warns (:218).
    • ctx export --name: a missing context (:330), or a failed marshal (:357). kates ctx export --name prod > prod.yaml leaves an empty file and exits 0.
    • ctx import: a file it cannot read (:384), invalid YAML (:390), or a failed save (:405).
  • ctx set's comment (:85-87) names this exact bug, but only ctx set had been moved to RunE.
  • No test caught it. TestCtxCommands (root_test.go:429-448) called .Run, then returned nil whatever happened.

The plan also found two places the fix as written missed:

  • ctx_export_test.go:22-34: runCtxCommandLine calls cmd.Run. Once export and import use RunE, Run is nil, so every export and import test there would panic.
  • docs/book/10-cli-reference.md:2490 lists these commands under "Commands that exit 0 on failure".

Changes

cli/cmd/config.go

  • ctx use, ctx delete, ctx current, ctx export and ctx import use RunE. Each output.Error(msg); return is now return cmdErr(msg), and each success path ends in return nil.
  • The messages are the same. cmdErr prints the message with output.Error and returns a silentErr (helpers.go:30-33). Execute turns that into exit 1 and does not print it a second time (root.go:591-598).
  • ctx current with no active context returns cmdErr too, so its message now prints as an error, not as a warning (see Decisions).
  • ctxUseCmd has a 3-line comment that names the && chain case, the way ctx set's comment does.
  • ctx show has no failure path and stays Run.

cli/cmd tests

  • ctx_export_test.go: runCtxCommandLine calls RunE when the command has one. It falls back to Run for ctx show, and returns the error. All 12 callers pass that error back through commandOutput.
  • root_test.go: TestCtxCommands and the ports import test call RunE (see Tests).
  • context_test.go: new TestCtxFailuresExitOne.

Docs

  • Book 10, "Commands that exit 0 on failure": the ctx commands are gone from the first item. That item now lists only a mistyped subcommand under a command group (kates ctx list prints the group's help), which this PR does not change.

eval/mcp

  • kates_api.py: read_context's comment says ctx export exits 1 for a missing context, and that older CLIs exited 0 with nothing on stdout. The check already fails on either, so the code is unchanged.
  • testdata/fake_kates.py: the fake CLI exits 1 for a missing context, as the real one now does.

Behaviour change. A script that ran one of these commands and ignored a failure now stops at it. The repo's own callers are safe:

  • Makefile:737 and :741 run kates ctx use local … || true.
  • scripts/install-kates.sh:187 only prints the command as a hint.
  • Nothing else in scripts/ or .github/ calls these commands.

Tests

Added or changed:

  • TestCtxFailuresExitOne (cli/cmd/context_test.go). It uses runKatesCLI, which runs the real command tree in a subprocess and returns the exit code. Each of these exits 1, and none prints contexts: YAML:

    • ctx use nope
    • ctx delete nope
    • ctx export --name nope
    • ctx import --file <missing path>
    • ctx import --file <file with invalid YAML>
    • --context nope ctx current

    The control case, ctx use lab, exits 0. This is the && chain from Why.

  • TestCtxCommands (cli/cmd/root_test.go): each row calls RunE and returns its error. A new wantErr field is set on "use of a missing context changes nothing" and "delete of a missing context changes nothing". The loop checks (err != nil) != tt.wantErr rather than failing on any error.

  • TestCtxImport_ClearsKeySource (root_test.go:393): calls RunE and fails on an error.

Run locally, all passing:

  • git diff --name-only origin/main...HEAD: 1 commit.
    • Changed: cli/cmd/{config.go,context_test.go,ctx_export_test.go,root_test.go}, docs/book/10-cli-reference.md, eval/mcp/kates_api.py and eval/mcp/testdata/fake_kates.py.
    • That triggers ci.yml code, ci-docker, integration and codeql (cli/**), and book.yml and docs.yml (docs/book/**).
    • No CI path filter covers eval/mcp, so its tests were run as an extra check.
  • cd cli && go test ./...: every package ok, cli/cmd in 31.8 s.
  • cd cli && go vet ./...: no output.
  • cd cli && golangci-lint run --new-from-rev=origin/main ./...: 0 issues.
  • cd cli && govulncheck ./...: no vulnerabilities found.
  • scripts/check-cli-style.sh and scripts/check-cli-compat.sh: OK.
  • scripts/check-book-style.sh: OK. All 38 pages are within the baseline ratchet.
  • scripts/gen-version-matrix.sh --check and scripts/gen-chart-table.sh --check: in sync.
  • python3 scripts/check-book-contrast.py: all 170 colour pairs meet WCAG AA.
  • scripts/check-mermaid.sh --quiet: 156 diagrams, 0 invalid.
  • scripts/prepare-book-render.sh && quarto render docs/book --to html, on a scratch copy of docs/ and scripts/: the render log has no Unable to resolve or WARN line.
  • python3 scripts/check-book-fragments.py on that render: all 7,458 fragment links in 38 pages land on an id.
  • Extra, because no CI job covers eval/mcp:
    • python3 -m unittest discover eval/mcp: 240 tests OK. 4 are skipped because they need KATES_BIN.
    • The same, with KATES_BIN set to a kates built from this branch: 240 tests OK, 0 skipped. That includes the cross-check against the real CLI, which covers the new ctx export exit code.

The backend is unchanged. The full backend suite (./mvnw test) ran on main and passed, and gives the same result here, so it was not run again on this branch.

Not run, because nothing they check changed:

  • helm lint, helm template, scripts/check-versions.sh and yamllint: no charts/** or version pins changed.
  • make check-help: the Makefile did not change.
  • actionlint: .github/** did not change.
  • shellcheck: no shell script changed.

Run only in CI:

  • The *IT integration tests and the Kind integration tests (integration.yml, triggered by cli/**).
  • The Docker image builds (ci-docker.yml, triggered by cli/**).
  • CodeQL Go analysis (codeql.yml, triggered by cli/**/*.go).
  • The Trivy and security scans.
  • The book PDF render and its index.log font check (book.yml).
  • docs.yml Markdown Link Check, lychee --offline --include-fragments. lychee is not installed locally.
  • ci.yml Backend Build & Test (mvnw verify) on the code filter. It was skipped locally because kates/ is unchanged.
  • The native image build is not triggered, because kates/** is unchanged.

Decisions

  • Split from finding 14. This is a separate PR. The two share no file and no book chapter, and they are two concerns with two squash messages.
  • ctx current with no active context also exits 1. It used to only warn. kubectl config current-context also exits 1 when no context is set. The case is rare, because a fresh config has the built-in default context.
  • ctx import of a file with no contexts stays as it is. It still reports Imported 0 context(s) and exits 0. The finding does not ask for a change there.

Deliberately not done

  • No separate TestCtxExport_MissingNameFails. The plan marked it optional. TestCtxFailuresExitOne already checks that ctx export --name nope exits 1 and prints no contexts: YAML.
  • A mistyped subcommand under a command group still exits 0. For example, kates ctx list prints the group's help. It stays in book 10's list. So do kates status, kates ports and kates snapshot create, which belong to other findings.
  • No new comments except the one on ctxUseCmd and the updated doc comment on runCtxCommandLine.

Landing

Lands after #373 (plan PR 4), #375 (plan PR 6) and #378 (plan PR 9). Each one edits docs/book/10-cli-reference.md, and so does this PR. No other file is shared.

Recipe once those have merged:

  1. Rebase onto main.
  2. Build a diff line map of each file that moved, from this branch's base to the merged file.
  3. Re-point the moved caveat refs from that map. The plan found no mcpAnchoredRef or path:line comment that cites a file this PR edits. A grep of cli/cmd and eval/mcp agrees, so the map should leave nothing to re-point.
  4. Run cd cli && go test ./cmd -run TestMCPCaveat.

…en they fail

In a step like "kates ctx import --file ci-contexts.yaml && kates ctx use lab
&& kates test create --type STRESS ...", a file with no lab context made ctx
use print "Context 'lab' not found" and exit 0, so the test went to whichever
context was current. ctx delete and ctx export --name did the same for a
missing context (export > prod.yaml left an empty file), ctx import for a
file it could not read, parse or save, and ctx current for an unknown
--context or no active context. Only ctx set had been moved to RunE.

These commands now use RunE and return cmdErr, which prints the same message
and exits 1. TestCtxFailuresExitOne runs each failing case through the real
command tree and checks the exit code; TestCtxCommands expects an error from
the missing-context rows, and the ctx export tests' helper calls RunE and
returns its error to commandOutput. The book no longer lists these commands
under the ones that exit 0 on failure, and the MCP eval harness's comment and
its fake CLI say ctx export exits 1 for a missing context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant