encode commands in base64 to reduce quote bugs#477
Open
Conversation
📝 WalkthroughWalkthroughUpdates CHANGELOG.md and changes command execution in two modules to Base64-encode composed shell payloads and pipe decoded bytes into the shell; privilege-escalation selection/logging now uses Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as "Caller"
participant Args as "arguments"
participant Escal as "PrivilegeEscalation Shell\n(when provided)"
participant Shell as "Shell (sh)"
participant Bash as "bash (decoder target)"
Caller->>Args: provide command_string, keep_stdin_open, privilege_escalation_command
alt privilege_escalation_command present
Args->>Escal: build wrapper
alt keep_stdin_open
Escal->>Escal: execute direct command_string
else keep stdin closed
Escal->>Bash: run "echo <base64> | base64 -d | bash"
end
else no escalation
Args->>Shell: build wrapper
alt keep_stdin_open
Shell->>Shell: execute direct command_string
else keep stdin closed
Shell->>Bash: run "echo <base64> | base64 -d | bash"
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
github-actions
Bot
added
rust
mrshmllow
force-pushed
the
marshmallow/p-putkoutmxzzm
branch
from
e21c62f to
9120399
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/commands/pty/mod.rs`:
- Around line 336-341: The PTY path hardcodes "sudo -u root --" when
arguments.is_elevated() is true; change it to use the configured privilege
escalation string from arguments.privilege_escalation_command instead. Update
the branch that calls command.arg(...) inside the arguments.is_elevated()
condition to build the escalation prefix from
arguments.privilege_escalation_command (preserving the trailing "-- bash -c
'echo {encoded} | base64 -d | bash'" behavior), so interactive PTY runs use the
same configured escalation command as noninterative code paths (refer to
arguments.is_elevated(), arguments.privilege_escalation_command, and the
command.arg(...) invocation).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82ac4df2-e7cd-4ecd-bcd1-c2f0b2ed9801
📒 Files selected for processing (3)
CHANGELOG.mdcrates/core/src/commands/noninteractive.rscrates/core/src/commands/pty/mod.rs
mrshmllow
force-pushed
the
marshmallow/p-putkoutmxzzm
branch
from
e965d13 to
b8d2f76
Compare
mrshmllow
force-pushed
the
marshmallow/p-putkoutmxzzm
branch
from
b8d2f76 to
33283bf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 1-10: The file CHANGELOG.md was accidentally replaced with CI job
output; restore it to be a proper changelog entry that documents the PR’s change
(the base64 encoding change for shell commands to reduce quote-related bugs).
Replace the CI lines with a concise release note that includes: a short summary
sentence mentioning "base64 encoding for shell commands", the rationale (avoids
quote/escaping issues), the user-facing impact or migration steps (how to
decode/run encoded commands or any flag/API change), and reference affected
areas/components (the shell command construction logic and any CLI/runner that
sends shell commands). Ensure the entry is dated/versioned consistently with
existing changelog style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5812472c-004d-4dd4-9d2e-c7f27eb1669a
📒 Files selected for processing (3)
CHANGELOG.mdcrates/core/src/commands/noninteractive.rscrates/core/src/commands/pty/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/commands/pty/mod.rs`:
- Around line 337-340: The privilege-escalation command is being applied twice:
once by prefixing the whole invocation from
arguments.privilege_escalation_command and again inside the shell string passed
to command.arg (you can see both references to {escalation_command} surrounding
the inner bash). Remove the inner/second escalation invocation so that
command.arg uses a single shell string like "sh -c 'echo {encoded} | base64 -d |
bash'" and rely on the outer prefix from arguments.privilege_escalation_command
(or conversely remove the outer prefix and keep a single inner use) to ensure
escalation is invoked only once; update the command.arg construction accordingly
(affecting the command.arg call that references {encoded} and
{escalation_command}).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4741588c-0871-4da3-94a0-f60e8ffa5dc9
📒 Files selected for processing (3)
CHANGELOG.mdcrates/core/src/commands/noninteractive.rscrates/core/src/commands/pty/mod.rs
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
mrshmllow
force-pushed
the
marshmallow/p-putkoutmxzzm
branch
from
d2714bf to
dd88214
Compare
mrshmllow
force-pushed
the
marshmallow/p-putkoutmxzzm
branch
from
dd88214 to
1ca1213
Compare
mrshmllow
force-pushed
the
marshmallow/p-putkoutmxzzm
branch
from
1ca1213 to
a175346
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/commands/pty/mod.rs`:
- Around line 337-343: The elevated keep_stdin_open branch currently passes
command_string through "sh -c" which breaks because command_string contains
bash-only IO_SUBS and may lose quotes; change that branch (the block that checks
arguments.privilege_escalation_command && arguments.keep_stdin_open) to use the
same Base64+bash pattern as the other elevated branch: base64-encode
command_string (the existing encoded variable) and pass an argument like
"{escalation_command} sh -c 'echo {encoded} | base64 -d | bash'" so the decoded
payload runs under bash and avoids shell quoting/compatibility issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ffec0e4c-ab03-4822-b60b-355834b47f08
📒 Files selected for processing (2)
crates/core/src/commands/noninteractive.rscrates/core/src/commands/pty/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/core/src/commands/noninteractive.rs
Comment on lines
+337
to
+343
| if let Some(escalation_command) = &arguments.privilege_escalation_command { | ||
| if arguments.keep_stdin_open { | ||
| command.arg(format!("{escalation_command} sh -c '{command_string}'")); | ||
| } else { | ||
| command.arg(format!( | ||
| "{escalation_command} sh -c 'echo {encoded} | base64 -d | bash'" | ||
| )); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the specific lines mentioned
cd crates/core/src/commands/pty && wc -l mod.rsRepository: forallsys/wire
Length of output: 69
🏁 Script executed:
# Read the context around lines 337-343 to understand the full scope
head -n 360 crates/core/src/commands/pty/mod.rs | tail -n 50Repository: forallsys/wire
Length of output: 1466
🏁 Script executed:
# Search for IO_SUBS definition and usage
rg "IO_SUBS" crates/core/src/commands/pty/Repository: forallsys/wire
Length of output: 272
🏁 Script executed:
# Search for command_string definition to understand what it contains
rg -B5 -A5 "let command_string" crates/core/src/commands/pty/mod.rsRepository: forallsys/wire
Length of output: 605
🏁 Script executed:
# Search for encoded definition to understand the Base64 payload
rg -B5 -A5 "let encoded" crates/core/src/commands/pty/mod.rsRepository: forallsys/wire
Length of output: 381
Use bash with Base64 encoding in the elevated keep_stdin_open path.
Lines 338–339 pass command_string through sh -c, but command_string always includes IO_SUBS, which contains bash-only process substitution (>(...)). On systems where /bin/sh is not bash, this will fail. Additionally, the approach is vulnerable to quote loss if command_string contains embedded single quotes.
The non-elevated keep_stdin_open path works because it runs the command directly without invoking a shell. The elevated non-keep_stdin_open path works because it uses the Base64 payload and explicitly pipes to bash. Apply the same Base64 + bash pattern to the elevated keep_stdin_open path:
Suggested fix
if let Some(escalation_command) = &arguments.privilege_escalation_command {
if arguments.keep_stdin_open {
- command.arg(format!("{escalation_command} sh -c '{command_string}'"));
+ command.arg(format!(
+ r#"{escalation_command} env WIRE_PTY_COMMAND_B64={encoded} bash -c 'eval "$(printf %s "$WIRE_PTY_COMMAND_B64" | base64 -d)"'"#
+ ));
} else {
command.arg(format!(
"{escalation_command} sh -c 'echo {encoded} | base64 -d | bash'"
));
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/core/src/commands/pty/mod.rs` around lines 337 - 343, The elevated
keep_stdin_open branch currently passes command_string through "sh -c" which
breaks because command_string contains bash-only IO_SUBS and may lose quotes;
change that branch (the block that checks arguments.privilege_escalation_command
&& arguments.keep_stdin_open) to use the same Base64+bash pattern as the other
elevated branch: base64-encode command_string (the existing encoded variable)
and pass an argument like "{escalation_command} sh -c 'echo {encoded} | base64
-d | bash'" so the decoded payload runs under bash and avoids shell
quoting/compatibility issues.
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
1 participant
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.
Summary by CodeRabbit
Bug Fixes
Documentation