Skip to content

fix: parse quoted gcp auth-provider command args#1990

Open
immanuwell wants to merge 1 commit into
kube-rs:mainfrom
immanuwell:fix-gcp-auth-provider-cmd-args
Open

fix: parse quoted gcp auth-provider command args#1990
immanuwell wants to merge 1 commit into
kube-rs:mainfrom
immanuwell:fix-gcp-auth-provider-cmd-args

Conversation

@immanuwell

@immanuwell immanuwell commented May 25, 2026

Copy link
Copy Markdown

Motivation

cmd-args in the legacy gcp auth-provider path is split on plain spaces right now. That breaks valid quoted args, so configs like sh -c 'printf %s "my token"' get mangled and auth blows up. Kinda rough, and pretty easy to hit if a kubeconfig shells through a wrapper.

Repro:

  1. Build an auth-provider config with cmd-path: sh
  2. Set cmd-args: -c 'printf %s "my token"'
  3. Call Auth::try_from(...)

Current main:
AuthExecRun ... Syntax error: Unterminated quoted string

Solution

Parse cmd-args and cmd-drop-env with shell-words instead of split(' ').
Added a regression test for quoted args, and kept the existing refresh coverage on the same path. small fix

Signed-off-by: immanuwell <pchpr.00@list.ru>
@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 76.7%. Comparing base (1f5a46b) to head (12f9d88).
⚠️ Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
kube-client/src/client/auth/mod.rs 93.4% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1990     +/-   ##
=======================================
+ Coverage   76.7%   76.7%   +0.1%     
=======================================
  Files         89      89             
  Lines       8747    8755      +8     
=======================================
+ Hits        6703    6713     +10     
+ Misses      2044    2042      -2     
Files with missing lines Coverage Δ
kube-client/src/client/auth/mod.rs 60.3% <93.4%> (+2.5%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@doxxx93

doxxx93 commented May 27, 2026

Copy link
Copy Markdown
Member

Thanks for the fix. little bit worried about the new crate but, it looks stable.

does this change leave any existing tests or comments stale? Since cmd-args is no longer split on spaces, it might be worth a quick pass to make sure nothing still references the old space-splitting behavior.

For example, exec_token_refresh_does_not_block_runtime still has a comment saying the provider "splits cmd-args on spaces so we can't inline this via sh -c", which the new exec_auth_command_parses_quoted_cmd_args test now contradicts.

let drop_env = shell_words::split(&drop_env)
.map_err(|e| Error::AuthExec(format!("invalid cmd-drop-env: {e}")))?;
let params = shell_words::split(&params)
.map_err(|e| Error::AuthExec(format!("invalid cmd-args: {e}")))?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would puts a new crate dep in direct control of shell out parameters and as such would be a valuable library to take over.

it's also by a quite inactive github user.

so i'm not a huge fan of this addition for a legacy provider (even though the upstream source code is very sensible right now).

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.

3 participants