Skip to content

Commit a819e58

Browse files
committed
fix(product-run): execute direct run commands
1 parent 27d8069 commit a819e58

9 files changed

Lines changed: 178 additions & 39 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/app/peritus-cli/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ peritus-codec = { version = "=0.0.0", path = "../../foundation/peritus-codec" }
1717
peritus-launcher = { version = "=0.0.0", path = "../peritus-launcher" }
1818
peritus-product-runner = { version = "=0.0.0", path = "../peritus-product-runner" }
1919
peritus-run-settlement = { version = "=0.0.0", path = "../../orchestration/peritus-run-settlement" }
20+
peritus-tools-shell = { version = "=0.0.0", path = "../../tools/peritus-tools-shell" }
2021
peritus-types = { version = "=0.0.0", path = "../../foundation/peritus-types" }
2122
serde_json.workspace = true
2223
sha2.workspace = true

crates/app/peritus-cli/src/product_run.rs

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use peritus_product_runner::ProductRunner;
1111
use peritus_run_settlement::{
1212
CandidateCheckpoint, CandidateStage, EvidenceStatus, QualificationEvidence, RunSettlement,
1313
};
14+
use peritus_tools_shell::ExecInput;
1415
use peritus_types::{RunId, SessionId};
1516

1617
use crate::{
@@ -364,16 +365,32 @@ const fn evidence_name(value: &EvidenceStatus<QualificationEvidence>) -> &'stati
364365
}
365366
}
366367

367-
#[cfg(unix)]
368368
fn run_command(root: &str, instruction: &str) -> Result<std::process::ExitStatus, CliError> {
369-
Command::new("sh").args(["-lc", instruction]).current_dir(Path::new(root)).status().map_err(
370-
|error| CliError::local_io("run candidate", Some(Path::new(root).to_owned()), error),
371-
)
369+
let input = ExecInput::from_command_line(instruction).map_err(|error| {
370+
CliError::usage(format!("candidate run instruction is not executable: {}", error.detail()))
371+
})?;
372+
Command::new(input.executable())
373+
.args(input.arguments())
374+
.current_dir(Path::new(root))
375+
.status()
376+
.map_err(|error| {
377+
CliError::local_io("run candidate", Some(Path::new(root).to_owned()), error)
378+
})
372379
}
373380

374-
#[cfg(windows)]
375-
fn run_command(root: &str, instruction: &str) -> Result<std::process::ExitStatus, CliError> {
376-
Command::new("cmd").args(["/C", instruction]).current_dir(Path::new(root)).status().map_err(
377-
|error| CliError::local_io("run candidate", Some(Path::new(root).to_owned()), error),
378-
)
381+
#[cfg(test)]
382+
mod tests {
383+
use super::*;
384+
385+
#[test]
386+
fn candidate_command_executes_directly_and_rejects_shell_text() {
387+
assert!(
388+
run_command(env!("CARGO_MANIFEST_DIR"), "rustc --version")
389+
.expect("direct command")
390+
.success()
391+
);
392+
assert!(
393+
run_command(env!("CARGO_MANIFEST_DIR"), "rustc --version && rustc --version",).is_err()
394+
);
395+
}
379396
}

crates/app/peritus-product-runner/src/turn.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ pub async fn complete_developer_turn(
114114
) else {
115115
continue;
116116
};
117-
let terminal = match parse_grounded_terminal(&tools, &result) {
117+
let terminal = match parse_grounded_terminal(&tools, &result).and_then(|terminal| {
118+
terminal::validate_run_instructions(input.delivery_scope, terminal)
119+
}) {
118120
Ok(terminal) => terminal,
119121
Err(error) => {
120122
let current = WorkspaceCheckpoint::capture(&input.workspace_root)?;
@@ -330,7 +332,7 @@ fn writer_system(
330332
) -> String {
331333
let delivery = match delivery_scope {
332334
super::ProductDeliveryScope::WorkspaceChanges => {
333-
"This run accepts exact workspace changes. Label build, test, lint, and other inspection commands with purpose `verification`; external effects are not an alternate completion path. The workspace_list result gives the exact workspace_root and declares workspace tool paths to be relative to it. When the task names an absolute path below that exact root, remove the root prefix once; never repeat the root directory inside itself."
335+
"This run accepts exact workspace changes. Label build, test, lint, and other inspection commands with purpose `verification`; external effects are not an alternate completion path. The workspace_list result gives the exact workspace_root and declares workspace tool paths to be relative to it. When the task names an absolute path below that exact root, remove the root prefix once; never repeat the root directory inside itself. For `run_instructions`, return exactly one direct command such as `cargo run --quiet`: use whitespace-separated executable and arguments only, with no prose, Markdown, quotes, environment assignments, redirections, expansions, or shell operators."
334336
}
335337
super::ProductDeliveryScope::AuthorizedExternalEffects => {
336338
if effect_requirement.is_required() {

crates/app/peritus-product-runner/src/turn/terminal.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22
33
use serde::Deserialize;
44

5-
use crate::{ProductRunnerError, ProductRunnerErrorKind};
5+
use peritus_tools_shell::ExecInput;
6+
7+
use crate::{ProductDeliveryScope, ProductRunnerError, ProductRunnerErrorKind};
68

79
#[derive(Deserialize)]
810
#[serde(deny_unknown_fields)]
@@ -44,6 +46,24 @@ pub(super) fn parse(value: &str) -> Result<TerminalTurn, ProductRunnerError> {
4446
}
4547
}
4648

49+
pub(super) fn validate_run_instructions(
50+
scope: ProductDeliveryScope,
51+
terminal: TerminalTurn,
52+
) -> Result<TerminalTurn, ProductRunnerError> {
53+
if let (ProductDeliveryScope::WorkspaceChanges, TerminalTurn::Complete((_, command))) =
54+
(scope, &terminal)
55+
{
56+
ExecInput::from_command_line(command).map_err(|error| {
57+
ProductRunnerError::new(
58+
ProductRunnerErrorKind::InvalidModelOutput,
59+
"validate candidate run command",
60+
error.detail(),
61+
)
62+
})?;
63+
}
64+
Ok(terminal)
65+
}
66+
4767
fn invalid(detail: &'static str) -> ProductRunnerError {
4868
ProductRunnerError::new(
4969
ProductRunnerErrorKind::InvalidModelOutput,

crates/app/peritus-product-runner/src/turn/tests.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,46 @@ fn writer_batches_tools_and_respects_artifact_workspaces() {
165165
assert!(prompt.contains("missing controlling fact"));
166166
assert!(prompt.contains("preserve the complete selected source value"));
167167
assert!(prompt.contains("apply only explicitly named transformations"));
168+
assert!(prompt.contains("return exactly one direct command"));
169+
assert!(prompt.contains("no prose, Markdown, quotes"));
170+
}
171+
172+
#[test]
173+
fn workspace_completion_requires_a_direct_run_command() {
174+
let invalid = terminal::parse(
175+
r#"{"kind":"complete","summary":"done","run_instructions":"From the root, run `cargo run`."}"#,
176+
)
177+
.expect("terminal shape");
178+
let Err(error) =
179+
terminal::validate_run_instructions(ProductDeliveryScope::WorkspaceChanges, invalid)
180+
else {
181+
panic!("prose must not qualify as a run command");
182+
};
183+
assert_eq!(error.kind(), ProductRunnerErrorKind::InvalidModelOutput);
184+
assert_eq!(error.operation(), "validate candidate run command");
185+
186+
let direct = terminal::parse(
187+
r#"{"kind":"complete","summary":"done","run_instructions":"cargo run --quiet"}"#,
188+
)
189+
.expect("terminal shape");
190+
assert!(
191+
terminal::validate_run_instructions(ProductDeliveryScope::WorkspaceChanges, direct).is_ok()
192+
);
193+
}
194+
195+
#[test]
196+
fn external_effect_completion_may_retain_concise_steps() {
197+
let instructions = terminal::parse(
198+
r#"{"kind":"complete","summary":"done","run_instructions":"Inspect the configured service."}"#,
199+
)
200+
.expect("terminal shape");
201+
assert!(
202+
terminal::validate_run_instructions(
203+
ProductDeliveryScope::AuthorizedExternalEffects,
204+
instructions,
205+
)
206+
.is_ok()
207+
);
168208
}
169209

170210
#[test]

crates/app/peritus-tui/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ peritus-codec = { version = "=0.0.0", path = "../../foundation/peritus-codec" }
1818
peritus-protocol = { version = "=0.0.0", path = "../../foundation/peritus-protocol" }
1919
peritus-product-runner = { version = "=0.0.0", path = "../peritus-product-runner" }
2020
peritus-run-settlement = { version = "=0.0.0", path = "../../orchestration/peritus-run-settlement" }
21+
peritus-tools-shell = { version = "=0.0.0", path = "../../tools/peritus-tools-shell" }
2122
peritus-types = { version = "=0.0.0", path = "../../foundation/peritus-types" }
2223
ratatui.workspace = true
2324
sha2.workspace = true

crates/app/peritus-tui/src/runtime/candidate.rs

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use std::{path::PathBuf, process::Command};
44

55
use peritus_product_runner::ProductRunner;
6+
use peritus_tools_shell::ExecInput;
67
use peritus_types::Sha256Digest;
78

89
/// Runs the candidate while the full-screen terminal is suspended.
@@ -31,26 +32,21 @@ fn execute_blocking(
3132
.to_owned(),
3233
);
3334
}
34-
let mut command = shell_command(instruction);
35+
let mut command = direct_command(instruction)?;
3536
let status = command
3637
.current_dir(workspace)
3738
.status()
3839
.map_err(|error| format!("could not start candidate command: {error}"))?;
3940
if status.success() { Ok(()) } else { Err(format!("candidate command exited with {status}")) }
4041
}
4142

42-
#[cfg(unix)]
43-
fn shell_command(instruction: &str) -> Command {
44-
let mut command = Command::new("sh");
45-
command.args(["-lc", instruction]);
46-
command
47-
}
48-
49-
#[cfg(windows)]
50-
fn shell_command(instruction: &str) -> Command {
51-
let mut command = Command::new("cmd");
52-
command.args(["/D", "/S", "/C", instruction]);
53-
command
43+
fn direct_command(instruction: &str) -> Result<Command, String> {
44+
let input = ExecInput::from_command_line(instruction).map_err(|error| {
45+
format!("candidate run instruction is not executable: {}", error.detail())
46+
})?;
47+
let mut command = Command::new(input.executable());
48+
command.args(input.arguments());
49+
Ok(command)
5450
}
5551

5652
#[cfg(test)]
@@ -75,6 +71,15 @@ mod tests {
7571
.await
7672
.is_err()
7773
);
74+
assert!(
75+
execute(
76+
workspace.path().to_path_buf(),
77+
"rustc --version && rustc --version".to_owned(),
78+
digest,
79+
)
80+
.await
81+
.is_err()
82+
);
7883
std::fs::write(workspace.path().join("changed.txt"), "changed").expect("candidate change");
7984
let error = execute(workspace.path().to_path_buf(), success_command().to_owned(), digest)
8085
.await
@@ -88,23 +93,11 @@ mod tests {
8893
assert!(status.success());
8994
}
9095

91-
#[cfg(unix)]
92-
const fn success_command() -> &'static str {
93-
"true"
94-
}
95-
96-
#[cfg(unix)]
97-
const fn failure_command() -> &'static str {
98-
"false"
99-
}
100-
101-
#[cfg(windows)]
10296
const fn success_command() -> &'static str {
103-
"exit /b 0"
97+
"rustc --version"
10498
}
10599

106-
#[cfg(windows)]
107100
const fn failure_command() -> &'static str {
108-
"exit /b 1"
101+
"rustc --definitely-invalid-peritus-option"
109102
}
110103
}

crates/tools/peritus-tools-shell/src/input.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,31 @@ impl ExecInput {
3232
Ok(Self { executable, arguments })
3333
}
3434

35+
/// Parses one deliberately restricted direct-execution command into literal argv.
36+
///
37+
/// This format is intended for persisted user-facing run instructions. It accepts
38+
/// whitespace-separated argv only: quoting, command separators, expansions, redirections,
39+
/// and multiple lines are rejected rather than interpreted.
40+
///
41+
/// # Errors
42+
/// Returns a typed failure when the value is empty, contains command-language or markup
43+
/// syntax, starts with an environment assignment, or violates the structured-argv contract.
44+
pub fn from_command_line(value: &str) -> Result<Self, ShellError> {
45+
let value = value.trim();
46+
if value.is_empty() || value.chars().any(char::is_control) {
47+
return Err(invalid_direct_command());
48+
}
49+
if value.chars().any(is_command_language_character) {
50+
return Err(invalid_direct_command());
51+
}
52+
let mut words = value.split_ascii_whitespace();
53+
let executable = words.next().ok_or_else(invalid_direct_command)?;
54+
if executable.contains('=') {
55+
return Err(invalid_direct_command());
56+
}
57+
Self::new(executable, words.map(str::to_owned).collect())
58+
}
59+
3560
/// Decodes already schema-validated protocol arguments defensively.
3661
///
3762
/// # Errors
@@ -59,6 +84,20 @@ impl ExecInput {
5984
}
6085
}
6186

87+
fn invalid_direct_command() -> ShellError {
88+
ShellError::new(
89+
ShellErrorKind::InvalidInput,
90+
"run instructions must be one direct command with whitespace-separated arguments and no quoting, expansion, redirection, markup, or shell operators",
91+
)
92+
}
93+
94+
const fn is_command_language_character(character: char) -> bool {
95+
matches!(
96+
character,
97+
'\'' | '"' | '`' | '$' | '|' | '&' | ';' | '<' | '>' | '(' | ')' | '{' | '}' | '#'
98+
)
99+
}
100+
62101
/// Explicit interpreter and script input accepted only by `shell.script`.
63102
#[derive(Clone, Debug, Eq, PartialEq)]
64103
pub struct ScriptInput {
@@ -214,6 +253,30 @@ mod tests {
214253
assert_eq!(command.arguments()[1], "$(touch escaped)");
215254
}
216255

256+
#[test]
257+
fn direct_command_line_becomes_literal_argv() {
258+
let input = ExecInput::from_command_line("cargo run --quiet --features tui,serde")
259+
.expect("direct command");
260+
assert_eq!(input.executable(), "cargo");
261+
assert_eq!(input.arguments(), ["run", "--quiet", "--features", "tui,serde"]);
262+
}
263+
264+
#[test]
265+
fn direct_command_line_rejects_shell_syntax_and_markup() {
266+
for value in [
267+
"cargo test && cargo run",
268+
"cargo run > output.txt",
269+
"cargo run `whoami`",
270+
"From the root, run `cargo run`.",
271+
"MODE=release cargo run",
272+
"cargo run\ncargo test",
273+
"sh -c echo",
274+
] {
275+
let error = ExecInput::from_command_line(value).expect_err("restricted command");
276+
assert_eq!(error.kind(), ShellErrorKind::InvalidInput, "{value}");
277+
}
278+
}
279+
217280
#[test]
218281
fn script_is_one_literal_argument() {
219282
let input = ScriptInput::new(

0 commit comments

Comments
 (0)