fix(exec): reject empty command before executor setup#467
Conversation
Greptile SummaryThe PR moves empty exec-command validation ahead of orchestrator setup.
Confidence Score: 4/5The PR is not yet safe to merge because a whitespace-only CLI executable still bypasses the new early validation and reaches executor setup. The guard checks src/cli/exec/mod.rs Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/cli/exec/mod.rs:153
**Whitespace-only executable bypasses validation**
When a user invokes `codspeed exec " "`, Clap supplies a non-empty whitespace-only first token, so `exe.is_empty()` returns false and executor setup proceeds. The token is then sent to exec-harness as the program name and fails during process execution instead of being rejected up front.
Reviews (2): Last reviewed commit: "fix(exec): reject empty command before e..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| TargetCommand::Exec { exec } => { | ||
| let command = shell_words::split(exec) | ||
| .with_context(|| format!("Failed to parse command: {exec}"))?; | ||
| if command.is_empty() { |
There was a problem hiding this comment.
When a config uses a quoted empty command such as exec: "''", shell_words::split produces a one-element vector containing an empty string, so this zero-length-vector check passes and the invalid executable reaches executor and harness setup, causing the same delayed process failure this change is intended to prevent. The CLI check has the same gap for codspeed exec ''.
Knowledge Base Used: CLI Commands
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/exec/multi_targets.rs
Line: 56
Comment:
**Empty executable token bypasses validation**
When a config uses a quoted empty command such as `exec: "''"`, `shell_words::split` produces a one-element vector containing an empty string, so this zero-length-vector check passes and the invalid executable reaches executor and harness setup, causing the same delayed process failure this change is intended to prevent. The CLI check has the same gap for `codspeed exec ''`.
**Knowledge Base Used:** [CLI Commands](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/cli-commands.md)
How can I resolve this? If you propose a fix, please make it concise.
Merging this PR will not alter performance
|
`codspeed exec` with no command, and `codspeed run` with an empty or
whitespace-only `exec` target, built a BenchmarkTarget with an empty
command that only failed deep inside exec-harness ("Empty command in
stdin input"), surfaced as the cryptic "failed to execute memory
tracker process: exit status: 1" after a full memtrack probe attach.
Enforce a non-empty command at each origin: mark the exec positional
clap-required, and bail in build_benchmark_targets when a config exec
command parses to zero words.
80723f5 to
c4f262e
Compare
| let executor::BenchmarkTarget::Exec { command, name, .. } = target else { | ||
| continue; | ||
| }; | ||
| if command.first().is_none_or(|exe| exe.is_empty()) { |
There was a problem hiding this comment.
When a user invokes codspeed exec " ", Clap supplies a non-empty whitespace-only first token, so exe.is_empty() returns false and executor setup proceeds. The token is then sent to exec-harness as the program name and fails during process execution instead of being rejected up front.
Knowledge Base Used: CLI Commands
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/exec/mod.rs
Line: 153
Comment:
**Whitespace-only executable bypasses validation**
When a user invokes `codspeed exec " "`, Clap supplies a non-empty whitespace-only first token, so `exe.is_empty()` returns false and executor setup proceeds. The token is then sent to exec-harness as the program name and fails during process execution instead of being rejected up front.
**Knowledge Base Used:** [CLI Commands](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/cli-commands.md)
How can I resolve this? If you propose a fix, please make it concise.
1 participant
codspeed execwith no command — andcodspeed runwith an empty or whitespace-onlyexecconfig target — built aBenchmarkTarget::Execwith an empty command. That empty target survived the full orchestrator (memtrack install, eBPF/libc uprobe attach, exec-harness spawn) before exec-harness rejected it withEmpty command in stdin input, surfaced at the top level as the crypticfailed to execute memory tracker process: exit status: 1after a ~2.3s probe detach.The command is only ever supplied at two origins, so each is now validated up front — before any setup work:
codspeed exec): the positionalcommandis now clap-required, so a missing command fails immediately at arg-parse with a usage message.codspeed run):build_benchmark_targetsbails when a configexeccommand parses to zero words, naming the offending target.Refs COD-3225