fix(spur-cli): reject unresolved usernames - #569
Conversation
Signed-off-by: Phlimosx <190250254+01xjw@users.noreply.github.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #569 +/- ##
=======================================
Coverage 76.03% 76.03%
=======================================
Files 166 166
Lines 63002 62988 -14
=======================================
- Hits 47898 47890 -8
+ Misses 15104 15098 -6 🚀 New features to boost your workflow:
|
yansun1996
left a comment
There was a problem hiding this comment.
Approving. One optional follow-up worth tracking separately: k8s.rs effective_user() (line 112) still uses the old "unknown" sentinel and feeds caller into the k0s/kubeconfig RPCs — the last one left in spur-cli. Not a bypass (is_k0s_admin rejects "unknown"), so fine outside this PR.
There was a problem hiding this comment.
Pull request overview
This PR hardens spur-cli’s authenticated job operations by making local username resolution fail closed, preventing jobs from being recorded or accessed under inconsistent “fallback” owner values when the OS cannot resolve the current UID.
Changes:
- Change
interactive::current_user()to returnResult<String>and propagate failures through multiple CLI request paths. - Remove
"unknown"fallback usernames in several commands (submit/control/exec/attach/output streaming), replacing them with error propagation. - Reuse the
JobSpec.usercaptured at submission time for later cancellation insrun/sallocto avoid re-looking up the username.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/spur-cli/src/interactive.rs | Make username resolution fail closed (Result) and map lookup failures into RPC status where needed. |
| crates/spur-cli/src/srun.rs | Propagate current_user()? into job spec/step/output streaming; reuse submitted owner for cancellations; adjust output streaming to return Result<bool>. |
| crates/spur-cli/src/salloc.rs | Use current_user()? in job spec and reuse submitted owner for cancellation paths. |
| crates/spur-cli/src/sbatch.rs | Use current_user()? instead of a fallback username when building the submitted JobSpec. |
| crates/spur-cli/src/scancel.rs | Use current_user()? when --user is not provided instead of falling back to "unknown". |
| crates/spur-cli/src/scontrol.rs | Use current_user()? for authenticated control-plane requests (cancel/suspend/resume/reservation updates). |
| crates/spur-cli/src/sattach.rs | Use current_user()? for output streaming authentication. |
| crates/spur-cli/src/exec.rs | Use current_user()? for authenticated exec requests. |
| crates/spur-cli/src/scrontab.rs | Make the effective user lookup fail closed by returning Result<String> and propagating errors into user-facing messages. |
Suppressed comments (1)
crates/spur-cli/src/scrontab.rs:74
list_crontabcallseffective_user()?separately in each branch, which repeats the OS username lookup and can produce inconsistent output if resolution changes. Resolve once at the top and reuse the string for all messages.
fn list_crontab(path: &PathBuf) -> Result<()> {
if path.exists() {
let content = std::fs::read_to_string(path)?;
if content.trim().is_empty()
|| content
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Local username sent with authenticated job requests. | ||
| /// | ||
| /// Refuse to continue when the operating system cannot resolve the caller: | ||
| /// recording a sentinel can either collide with a real account or disagree | ||
| /// with a later exec/attach request. | ||
| pub fn current_user() -> Result<String> { | ||
| whoami::username().context("failed to determine current username") | ||
| } |
| let mut stream = match agent | ||
| .stream_job_output(StreamJobOutputRequest { | ||
| job_id, | ||
| stream: "stdout".into(), | ||
| user: crate::interactive::current_user(), | ||
| user: crate::interactive::current_user()?, |
Summary
Resultfromcurrent_user().JobSpecfor later cancellation instead of looking it up again.Closes #567.
Validation
cargo test -p spur-cli --lockedcargo fmt --all --checkcargo clippy -p spur-cli --all-targets --locked -- -D warningsfailed to determine current usernameand never emits a sentinel.Disclosure
This change was prepared with assistance from the radeon-issue automation and independently checked by a separately configured validation model. Maintainer review is still required.