Repository navigation
fix(exec): use is_elevated() instead of bare-name check after resolve_program - #165
Merged
Merged
Conversation
…_program
resolve_program() returns a full absolute path (e.g. /usr/bin/sudo), not a
bare command name. The original check
if command.eq("doas") || command.eq("sudo") || command.eq("run0") { ... }
was introduced in #159 without updating for the new full-path return value.
The comparison always evaluates to false, so self.elevate() (which calls
`sudo --validate` with inherited stdin to warm the credential cache) is never
called for privileged steps.
Trigger: a privileged: true non-streaming command.run step runs with expired
sudo credentials. Because elevate() is not called first, sudo runs with stdin
piped (not inherited, Command::output() default), cannot prompt, and fails
with "sudo: a password is required" rather than prompting the user.
Fix: replace the bare-name check with self.is_elevated(), which is the exact
semantic that was intended (privileged=true && not root) and covers all three
providers (sudo, doas, run0) correctly regardless of their resolved path.
Regression test: resolve_program_returns_full_path_not_bare_name asserts
the return value is an absolute path so this bug cannot silently re-appear.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMRWKLTCW3D6caXjkjvCiL
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.
Bug
resolve_program()(added in #159) returns a full absolute path like/usr/bin/sudo, not a bare command name. The elevation gate that follows it checks:Since
"/usr/bin/sudo" != "sudo", this condition is alwaysfalse.self.elevate()— which callssudo --validatewith inherited stdin to warm the credential cache — is never called for any privileged step.Impact
Trigger: a
privileged: truenon-streamingcommand.runstep runs when sudo credentials are expired.Before #159:
elevate()calledsudo --validatewithStdio::inherit()first, letting the user enter their password. The subsequent.output()call (stdin not inherited) succeeded because the cache was warm.After #159 (broken):
elevate()skipped entirely. The.output()call runssudodirectly without inherited stdin; sudo cannot prompt → fails with"sudo: a password is required".Streaming mode is less affected (stdin is inherited for the actual run), but the credential pre-check is still missing.
Root Cause
resolve_programwas inserted betweenelevate_if_required()and the bare-name check without updating the check to account for the full-path return value.Fix
Replace the bare-name string comparison with
self.is_elevated(), which is the exact predicate that was intended (privileged=true && not root) and works correctly regardless of how the provider binary is resolved.Validation
resolve_program_returns_full_path_not_bare_nameasserts the return value is an absolute path, locking in the assumption that makes the bare-name check wrong.elevate_*unit tests that checkelevate_if_required()fail in this root environment becauseelevation_needed(true, "root") = false— pre-existing, unrelated to this change, confirmed by running them onmainbefore the fix).Generated by Claude Code