fix(spur-cli): reject unresolved k8s callers - #609
Open
joshkmartinez wants to merge 2 commits into
Open
Conversation
joshkmartinez
marked this pull request as ready for review
August 11, 2026 02:45
joshkmartinez
requested review from
sajmera-pensando and
yansun1996
as code owners
August 11, 2026 02:45
Contributor
There was a problem hiding this comment.
Pull request overview
Updates spur-cli’s k8s subcommands to fail closed when the local username cannot be resolved, preventing ambiguous "unknown" caller identities from being sent to the controller (which rejects them anyway). This aligns the k8s CLI path with the existing caller-resolution behavior used elsewhere in spur-cli.
Changes:
- Route
spur k8s up|down|kubeconfigthrough the shared fail-closed username resolver and error out before dispatching any RPC when resolution fails. - Extend the in-process mock controller harness to implement and record the k8s RPCs needed by CLI tests.
- Add targeted tests covering “no-dispatch on user resolution failure”, “resolved caller is forwarded”, and “unauthenticated commands don’t resolve user”.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/spur-cli/src/k8s.rs | Replaces "unknown" fallback with fail-closed caller resolution for authenticated k8s commands and adds focused tests. |
| crates/spur-cli/src/mock_controller.rs | Implements mock k8s RPC endpoints and captures requests for assertions in CLI tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
111
to
113
| } => { | ||
| let caller = current_user()?; | ||
| cmd_up( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #603.
spur k8s up,down, andkubeconfigusedunknownwhen the local username could not be resolved. The controller rejects that caller, but the CLI still sent an ambiguous identity.These commands now reuse the existing fail-closed username resolver and return its error before dispatching an RPC.
statusand the localinstall-k0scommand do not perform a username lookup.Testing
spur-clitest suite: 360 unit tests and 1 integration testcargo clippy -p spur-cli --all-targets --locked -- -D warningsThe
spur-clichecks required a temporary macOS-only compile shim for the pre-existingnix::unistd::getgroupsfailure on Apple targets. The shim was reverted and is not part of this PR. The repository-wide run then reached the existing Linux-onlyspur-mpi-pmixlinker boundary.