From 950752a81bb6afa97aacb9606b2f1cc66fba09ab Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 03:25:49 +0000 Subject: [PATCH 1/3] MIR-871: Interpret app run shell expressions safely Recognize shell syntax in app run commands so variables, pipelines, and redirects execute as users expect without changing ordinary argv semantics. Decline shell expressions on old clusters whose entrypoint exec path cannot preserve a script argument safely. --- blackbox/tasks_test.go | 18 ++++++++ cli/commands/app_run.go | 61 ++++++++++++++++++++++++- cli/commands/app_run_doc.go | 4 ++ cli/commands/app_run_test.go | 86 ++++++++++++++++++++++++++++++++++++ 4 files changed, 168 insertions(+), 1 deletion(-) diff --git a/blackbox/tasks_test.go b/blackbox/tasks_test.go index c3f390e4b..116a60d37 100644 --- a/blackbox/tasks_test.go +++ b/blackbox/tasks_test.go @@ -261,6 +261,24 @@ func TestRunPreservesArgumentBoundaries(t *testing.T) { if !strings.Contains(r.Stdout, "hello world") { t.Fatalf("argument boundaries were lost; want \"hello world\" in output, got:\n%s", r.Stdout) } + + r = m.MustRun("app", "run", "-a", name, "--", "printf", "%s", "hello world", "|", "wc", "-c") + if strings.TrimSpace(r.Stdout) != "11" { + t.Fatalf("shell pipeline did not preserve the spaced argument; want 11, got:\n%s", r.Stdout) + } + + r = m.MustRun("app", "run", "-a", name, "--", "echo", "$MIREN_APP") + if strings.TrimSpace(r.Stdout) != name { + t.Fatalf("shell variable did not expand; want %q, got:\n%s", name, r.Stdout) + } + + r = m.MustRun("app", "run", "-a", name, "--", "printf 'redirected' > /tmp/miren-run-redirect; cat /tmp/miren-run-redirect") + if strings.TrimSpace(r.Stdout) != "redirected" { + t.Fatalf("shell redirection did not write a readable file; got:\n%s", r.Stdout) + } + + r = m.Run("app", "run", "-a", name, "--", "exit 7") + r.RequireExitCode(t, 7) } // [tasks..env] has to reach the container. A task names no service, so diff --git a/cli/commands/app_run.go b/cli/commands/app_run.go index daed4fb7b..f3992a8c0 100644 --- a/cli/commands/app_run.go +++ b/cli/commands/app_run.go @@ -5,6 +5,8 @@ import ( "errors" "fmt" "os" + "path/filepath" + "slices" "strings" "time" @@ -53,6 +55,9 @@ func AppRun(ctx *Context, opts struct { if opts.Task != "" || opts.Detach { return fmt.Errorf("--task and --detach need a newer cluster that supports durable runs; a plain `miren app run` still works against this one") } + if err := legacyRunCommandCheck(opts.Args); err != nil { + return err + } return appRunLegacy(ctx, opts.App, opts.Args) } return err @@ -65,7 +70,7 @@ func AppRun(ctx *Context, opts struct { // separate for whatever is reading them. wantTTY := !opts.Detach && stdinIsTerminal() - created, err := runs.CreateRun(ctx, opts.App, opts.Task, opts.Args, wantTTY) + created, err := runs.CreateRun(ctx, opts.App, opts.Task, runCommand(opts.Args), wantTTY) if err != nil { return err } @@ -90,6 +95,60 @@ func AppRun(ctx *Context, opts struct { return reportRunExit(ctx, runs, runID) } +// runCommand keeps ordinary argv intact, but lets shell expressions be parsed +// by a shell. Quote plain arguments inside an expression so an unrelated space +// in an argument does not turn it into multiple words. +func runCommand(args []string) []string { + if len(args) == 0 { + return args + } + if len(args) == 1 && strings.ContainsAny(args[0], " \t") { + return []string{"/bin/sh", "-c", args[0]} + } + + // An explicit shell already owns parsing its -c argument, including when + // -c is combined with other flags or follows separate options. + if len(args) >= 3 { + switch filepath.Base(args[0]) { + case "sh", "bash", "dash", "ash", "ksh", "zsh": + for _, option := range args[1 : len(args)-1] { + if !strings.HasPrefix(option, "-") || option == "--" { + break + } + if strings.Contains(option[1:], "c") { + return args + } + } + } + } + + const shellSyntax = "$|&;<>()`\\*?[]{}~\n" + usesShell := false + parts := make([]string, len(args)) + for i, arg := range args { + if strings.ContainsAny(arg, shellSyntax) { + usesShell = true + parts[i] = arg + } else { + parts[i] = "'" + strings.ReplaceAll(arg, "'", `'\''`) + "'" + } + } + if !usesShell { + return args + } + return []string{"/bin/sh", "-c", strings.Join(parts, " ")} +} + +func legacyRunCommandCheck(args []string) error { + // Old servers join argv into shell source when the app has an entrypoint. + // There is no way to preserve a -c script argument both there and on + // images without an entrypoint, so do not silently run a different command. + if !slices.Equal(runCommand(args), args) { + return fmt.Errorf("shell expressions in `miren app run` require a newer cluster; this cluster cannot safely preserve the command") + } + return nil +} + // serverPredatesRuns reports whether a runsClient error means the cluster is // too old to offer durable runs, as opposed to being unreachable, wedged, or // refusing the caller. diff --git a/cli/commands/app_run_doc.go b/cli/commands/app_run_doc.go index f81ea67e0..927f09929 100644 --- a/cli/commands/app_run_doc.go +++ b/cli/commands/app_run_doc.go @@ -4,6 +4,10 @@ const appRunDescription = `This command runs a command in a fresh sandbox built With no arguments it opens an interactive shell. With arguments it runs that command. With ` + "`" + `--task` + "`" + ` it runs a task declared in ` + "`" + `app.toml` + "`" + `. +Commands containing shell syntax (such as ` + "`" + `$HOME` + "`" + `, ` + "`" + `|` + "`" + `, or ` + "`" + `>` + "`" + `) run through ` + "`" + `/bin/sh -c` + "`" + ` so expansions, pipelines, and redirects work. Other commands retain their argument boundaries without shell interpretation. + +Quote the expression for your local shell so it reaches Miren intact: ` + "`" + `miren app run -- 'echo $HOME | wc -c'` + "`" + `. Shell expressions require a cluster that supports durable runs; older clusters cannot safely preserve them. + This is useful for: - Debugging application issues in an isolated environment - Running one-off commands with your app's configuration diff --git a/cli/commands/app_run_test.go b/cli/commands/app_run_test.go index 3de6bd659..804eaf40e 100644 --- a/cli/commands/app_run_test.go +++ b/cli/commands/app_run_test.go @@ -2,6 +2,9 @@ package commands import ( "errors" + "os/exec" + "reflect" + "strings" "testing" "time" @@ -11,6 +14,89 @@ import ( "miren.dev/runtime/pkg/ui" ) +func TestRunCommand(t *testing.T) { + tests := []struct { + name string + args []string + want []string + }{ + {"console", nil, nil}, + {"single executable", []string{"date"}, []string{"date"}}, + {"ordinary arguments", []string{"echo", "hello world", "O'Reilly"}, []string{"echo", "hello world", "O'Reilly"}}, + {"variable", []string{"echo", "$HOME"}, []string{"/bin/sh", "-c", "'echo' $HOME"}}, + {"pipeline with spaced argument", []string{"printf", "%s", "hello world", "|", "wc", "-c"}, []string{"/bin/sh", "-c", "'printf' '%s' 'hello world' | 'wc' '-c'"}}, + {"empty argument in pipeline", []string{"printf", "%s", "", "|", "wc", "-c"}, []string{"/bin/sh", "-c", "'printf' '%s' '' | 'wc' '-c'"}}, + {"single shell expression", []string{"echo $HOME | wc -c"}, []string{"/bin/sh", "-c", "echo $HOME | wc -c"}}, + {"single command string", []string{"exit 7"}, []string{"/bin/sh", "-c", "exit 7"}}, + {"redirect", []string{"echo", "hi", ">", "/tmp/result"}, []string{"/bin/sh", "-c", "'echo' 'hi' > '/tmp/result'"}}, + {"explicit shell", []string{"/bin/sh", "-c", "echo $HOME | wc -c"}, []string{"/bin/sh", "-c", "echo $HOME | wc -c"}}, + {"combined shell flags", []string{"sh", "-ec", "exit 7; echo unexpected"}, []string{"sh", "-ec", "exit 7; echo unexpected"}}, + {"login shell flags", []string{"bash", "-lc", "echo $1", "unused", "word"}, []string{"bash", "-lc", "echo $1", "unused", "word"}}, + {"separate shell flags", []string{"bash", "-e", "-c", "echo $1", "unused", "word"}, []string{"bash", "-e", "-c", "echo $1", "unused", "word"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := runCommand(tt.args); !reflect.DeepEqual(got, tt.want) { + t.Fatalf("runCommand(%q) = %q, want %q", tt.args, got, tt.want) + } + }) + } +} + +func TestRunCommandExecutesShellExpression(t *testing.T) { + args := runCommand([]string{"printf", "%s", "hello world", "|", "wc", "-c"}) + out, err := exec.Command(args[0], args[1:]...).Output() + if err != nil { + t.Fatal(err) + } + if got := strings.TrimSpace(string(out)); got != "11" { + t.Fatalf("pipeline output = %q, want 11", got) + } +} + +func TestRunCommandPreservesLiteralArgumentsInPipeline(t *testing.T) { + for _, literal := range []string{"O'Reilly book", "a # b", "a = b", ""} { + t.Run(literal, func(t *testing.T) { + args := runCommand([]string{"printf", "%s", literal, "|", "cat"}) + out, err := exec.Command(args[0], args[1:]...).Output() + if err != nil { + t.Fatal(err) + } + if string(out) != literal { + t.Fatalf("pipeline output = %q, want %q", out, literal) + } + }) + } +} + +func TestRunCommandPreservesExplicitShellExit(t *testing.T) { + for _, args := range [][]string{ + {"sh", "-ec", "exit 7; echo unexpected"}, + {"bash", "-lc", "exit 7; echo unexpected"}, + {"bash", "-e", "-c", "exit 7; echo unexpected"}, + } { + got := runCommand(args) + out, err := exec.Command(got[0], got[1:]...).CombinedOutput() + var exit *exec.ExitError + if !errors.As(err, &exit) || exit.ExitCode() != 7 || len(out) != 0 { + t.Fatalf("%q: exit = %v, output = %q; want code 7 and no output", args, err, out) + } + } + + args := runCommand([]string{"bash", "-lc", `printf '%s' "$1"`, "unused", "two words"}) + out, err := exec.Command(args[0], args[1:]...).Output() + if err != nil || string(out) != "two words" { + t.Fatalf("positional argument output = %q, error = %v; want two words", out, err) + } +} + +func TestLegacyRunCommandCheck(t *testing.T) { + assert.NoError(t, legacyRunCommandCheck([]string{"echo", "plain text"})) + assert.NoError(t, legacyRunCommandCheck(nil)) + assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo", "$HOME"}), "require a newer cluster") + assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo", "hi", "|", "wc"}), "require a newer cluster") +} + // The compatibility fallback hinges on one distinction: a server that answered // and does not offer app-runs (fall back to legacy exec) versus a server that // is unreachable, wedged, or refusing us (surface the real error). Falling back From 0bd89ba425b6df76684a1ff814db560053b1712e Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Thu, 24 Sep 2026 03:45:48 +0000 Subject: [PATCH 2/3] MIR-871: Preserve quoted metacharacters and generated docs Only treat a whole command string or standalone operator tokens as shell syntax. Characters embedded in interpreter arguments, SQL, URLs, and patterns stay literal; commit regenerated app run docs so CI's generation check stays clean. --- blackbox/tasks_test.go | 2 +- cli/commands/app_run.go | 30 +++++++++--------------------- cli/commands/app_run_doc.go | 2 +- cli/commands/app_run_test.go | 13 +++++++++++-- docs/docs/command/app-run.md | 4 ++++ 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/blackbox/tasks_test.go b/blackbox/tasks_test.go index 116a60d37..531e0bef3 100644 --- a/blackbox/tasks_test.go +++ b/blackbox/tasks_test.go @@ -267,7 +267,7 @@ func TestRunPreservesArgumentBoundaries(t *testing.T) { t.Fatalf("shell pipeline did not preserve the spaced argument; want 11, got:\n%s", r.Stdout) } - r = m.MustRun("app", "run", "-a", name, "--", "echo", "$MIREN_APP") + r = m.MustRun("app", "run", "-a", name, "--", "echo $MIREN_APP") if strings.TrimSpace(r.Stdout) != name { t.Fatalf("shell variable did not expand; want %q, got:\n%s", name, r.Stdout) } diff --git a/cli/commands/app_run.go b/cli/commands/app_run.go index f3992a8c0..e3cbcdce7 100644 --- a/cli/commands/app_run.go +++ b/cli/commands/app_run.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "os" - "path/filepath" "slices" "strings" "time" @@ -102,34 +101,23 @@ func runCommand(args []string) []string { if len(args) == 0 { return args } - if len(args) == 1 && strings.ContainsAny(args[0], " \t") { - return []string{"/bin/sh", "-c", args[0]} - } - // An explicit shell already owns parsing its -c argument, including when - // -c is combined with other flags or follows separate options. - if len(args) >= 3 { - switch filepath.Base(args[0]) { - case "sh", "bash", "dash", "ash", "ksh", "zsh": - for _, option := range args[1 : len(args)-1] { - if !strings.HasPrefix(option, "-") || option == "--" { - break - } - if strings.Contains(option[1:], "c") { - return args - } - } - } + const shellSyntax = "$|&;<>()`\\*?[]{}~\n" + // A single command string is shell source. For multiple arguments the + // caller's shell has already stripped its quotes, so only whole operator + // tokens distinguish shell structure from literal data (e.g. SQL or code). + if len(args) == 1 && strings.ContainsAny(args[0], shellSyntax+" \t") { + return []string{"/bin/sh", "-c", args[0]} } - const shellSyntax = "$|&;<>()`\\*?[]{}~\n" usesShell := false parts := make([]string, len(args)) for i, arg := range args { - if strings.ContainsAny(arg, shellSyntax) { + switch arg { + case "|", "||", "&&", ";", "&", ">", ">>", "<", "<<", "2>", "2>>": usesShell = true parts[i] = arg - } else { + default: parts[i] = "'" + strings.ReplaceAll(arg, "'", `'\''`) + "'" } } diff --git a/cli/commands/app_run_doc.go b/cli/commands/app_run_doc.go index 927f09929..92ce173b2 100644 --- a/cli/commands/app_run_doc.go +++ b/cli/commands/app_run_doc.go @@ -4,7 +4,7 @@ const appRunDescription = `This command runs a command in a fresh sandbox built With no arguments it opens an interactive shell. With arguments it runs that command. With ` + "`" + `--task` + "`" + ` it runs a task declared in ` + "`" + `app.toml` + "`" + `. -Commands containing shell syntax (such as ` + "`" + `$HOME` + "`" + `, ` + "`" + `|` + "`" + `, or ` + "`" + `>` + "`" + `) run through ` + "`" + `/bin/sh -c` + "`" + ` so expansions, pipelines, and redirects work. Other commands retain their argument boundaries without shell interpretation. +Pass a single command string to use shell syntax (such as ` + "`" + `$HOME` + "`" + `, ` + "`" + `|` + "`" + `, or ` + "`" + `>` + "`" + `). Separate operator arguments, such as ` + "`" + `|` + "`" + ` between two commands, also invoke a shell. Other arguments retain their boundaries without shell interpretation, even when they contain characters used by a shell. Quote the expression for your local shell so it reaches Miren intact: ` + "`" + `miren app run -- 'echo $HOME | wc -c'` + "`" + `. Shell expressions require a cluster that supports durable runs; older clusters cannot safely preserve them. diff --git a/cli/commands/app_run_test.go b/cli/commands/app_run_test.go index 804eaf40e..beab84631 100644 --- a/cli/commands/app_run_test.go +++ b/cli/commands/app_run_test.go @@ -23,14 +23,21 @@ func TestRunCommand(t *testing.T) { {"console", nil, nil}, {"single executable", []string{"date"}, []string{"date"}}, {"ordinary arguments", []string{"echo", "hello world", "O'Reilly"}, []string{"echo", "hello world", "O'Reilly"}}, - {"variable", []string{"echo", "$HOME"}, []string{"/bin/sh", "-c", "'echo' $HOME"}}, + {"variable argument stays literal", []string{"echo", "$HOME"}, []string{"echo", "$HOME"}}, {"pipeline with spaced argument", []string{"printf", "%s", "hello world", "|", "wc", "-c"}, []string{"/bin/sh", "-c", "'printf' '%s' 'hello world' | 'wc' '-c'"}}, {"empty argument in pipeline", []string{"printf", "%s", "", "|", "wc", "-c"}, []string{"/bin/sh", "-c", "'printf' '%s' '' | 'wc' '-c'"}}, {"single shell expression", []string{"echo $HOME | wc -c"}, []string{"/bin/sh", "-c", "echo $HOME | wc -c"}}, {"single command string", []string{"exit 7"}, []string{"/bin/sh", "-c", "exit 7"}}, {"redirect", []string{"echo", "hi", ">", "/tmp/result"}, []string{"/bin/sh", "-c", "'echo' 'hi' > '/tmp/result'"}}, + {"python code is data", []string{"python", "-c", "print('hi')"}, []string{"python", "-c", "print('hi')"}}, + {"SQL is data", []string{"psql", "-c", "SELECT * FROM users;"}, []string{"psql", "-c", "SELECT * FROM users;"}}, + {"URL is data", []string{"curl", "https://x/?a=1&b=2"}, []string{"curl", "https://x/?a=1&b=2"}}, + {"grep pattern is data", []string{"grep", "-E", "foo|bar", "log"}, []string{"grep", "-E", "foo|bar", "log"}}, + {"substitution is data", []string{"echo", "it's $HOME"}, []string{"echo", "it's $HOME"}}, {"explicit shell", []string{"/bin/sh", "-c", "echo $HOME | wc -c"}, []string{"/bin/sh", "-c", "echo $HOME | wc -c"}}, {"combined shell flags", []string{"sh", "-ec", "exit 7; echo unexpected"}, []string{"sh", "-ec", "exit 7; echo unexpected"}}, + {"shell option value", []string{"sh", "-o", "pipefail", "-c", "echo 'x' | cat"}, []string{"sh", "-o", "pipefail", "-c", "echo 'x' | cat"}}, + {"env shell", []string{"/usr/bin/env", "bash", "-c", "echo $HOME"}, []string{"/usr/bin/env", "bash", "-c", "echo $HOME"}}, {"login shell flags", []string{"bash", "-lc", "echo $1", "unused", "word"}, []string{"bash", "-lc", "echo $1", "unused", "word"}}, {"separate shell flags", []string{"bash", "-e", "-c", "echo $1", "unused", "word"}, []string{"bash", "-e", "-c", "echo $1", "unused", "word"}}, } @@ -93,7 +100,9 @@ func TestRunCommandPreservesExplicitShellExit(t *testing.T) { func TestLegacyRunCommandCheck(t *testing.T) { assert.NoError(t, legacyRunCommandCheck([]string{"echo", "plain text"})) assert.NoError(t, legacyRunCommandCheck(nil)) - assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo", "$HOME"}), "require a newer cluster") + assert.NoError(t, legacyRunCommandCheck([]string{"python", "-c", "print(1)"})) + assert.NoError(t, legacyRunCommandCheck([]string{"psql", "-c", "SELECT * FROM t"})) + assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo $HOME"}), "require a newer cluster") assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo", "hi", "|", "wc"}), "require a newer cluster") } diff --git a/docs/docs/command/app-run.md b/docs/docs/command/app-run.md index 1d915718d..6bf917c0d 100644 --- a/docs/docs/command/app-run.md +++ b/docs/docs/command/app-run.md @@ -12,6 +12,10 @@ This command runs a command in a fresh sandbox built from your app's active vers With no arguments it opens an interactive shell. With arguments it runs that command. With `--task` it runs a task declared in `app.toml`. +Pass a single command string to use shell syntax (such as `$HOME`, `|`, or `>`). Separate operator arguments, such as `|` between two commands, also invoke a shell. Other arguments retain their boundaries without shell interpretation, even when they contain characters used by a shell. + +Quote the expression for your local shell so it reaches Miren intact: `miren app run -- 'echo $HOME | wc -c'`. Shell expressions require a cluster that supports durable runs; older clusters cannot safely preserve them. + This is useful for: - Debugging application issues in an isolated environment - Running one-off commands with your app's configuration From f9e365bf7e651cac0eb6112efa552f629c6fb256 Mon Sep 17 00:00:00 2001 From: Evan Phoenix Date: Fri, 25 Sep 2026 16:33:26 +0000 Subject: [PATCH 3/3] MIR-871: Keep operator argv literal in app run An operator reaching the CLI as a separate argument was escaped or quoted by the caller and must remain data. Restrict automatic shell execution to a single command string and preserve the legacy fallback unchanged. --- blackbox/tasks_test.go | 9 ++++++-- cli/commands/app_run.go | 44 ++++-------------------------------- cli/commands/app_run_doc.go | 4 ++-- cli/commands/app_run_test.go | 24 +++++++------------- docs/docs/command/app-run.md | 4 ++-- 5 files changed, 23 insertions(+), 62 deletions(-) diff --git a/blackbox/tasks_test.go b/blackbox/tasks_test.go index 531e0bef3..3e54206b3 100644 --- a/blackbox/tasks_test.go +++ b/blackbox/tasks_test.go @@ -262,9 +262,14 @@ func TestRunPreservesArgumentBoundaries(t *testing.T) { t.Fatalf("argument boundaries were lost; want \"hello world\" in output, got:\n%s", r.Stdout) } - r = m.MustRun("app", "run", "-a", name, "--", "printf", "%s", "hello world", "|", "wc", "-c") + r = m.MustRun("app", "run", "-a", name, "--", "printf '%s' 'hello world' | wc -c") if strings.TrimSpace(r.Stdout) != "11" { - t.Fatalf("shell pipeline did not preserve the spaced argument; want 11, got:\n%s", r.Stdout) + t.Fatalf("shell pipeline did not run; want 11, got:\n%s", r.Stdout) + } + + r = m.MustRun("app", "run", "-a", name, "--", "expr", "3", ">", "2") + if strings.TrimSpace(r.Stdout) != "1" { + t.Fatalf("quoted comparison was interpreted as redirection; want 1, got:\n%s", r.Stdout) } r = m.MustRun("app", "run", "-a", name, "--", "echo $MIREN_APP") diff --git a/cli/commands/app_run.go b/cli/commands/app_run.go index e3cbcdce7..683790361 100644 --- a/cli/commands/app_run.go +++ b/cli/commands/app_run.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "os" - "slices" "strings" "time" @@ -54,9 +53,6 @@ func AppRun(ctx *Context, opts struct { if opts.Task != "" || opts.Detach { return fmt.Errorf("--task and --detach need a newer cluster that supports durable runs; a plain `miren app run` still works against this one") } - if err := legacyRunCommandCheck(opts.Args); err != nil { - return err - } return appRunLegacy(ctx, opts.App, opts.Args) } return err @@ -94,47 +90,15 @@ func AppRun(ctx *Context, opts struct { return reportRunExit(ctx, runs, runID) } -// runCommand keeps ordinary argv intact, but lets shell expressions be parsed -// by a shell. Quote plain arguments inside an expression so an unrelated space -// in an argument does not turn it into multiple words. +// runCommand treats a single command string as shell source. Multiple args +// remain argv: the caller's shell already removed quoting, so even a standalone +// operator token may be literal data deliberately quoted or escaped locally. func runCommand(args []string) []string { - if len(args) == 0 { - return args - } - const shellSyntax = "$|&;<>()`\\*?[]{}~\n" - // A single command string is shell source. For multiple arguments the - // caller's shell has already stripped its quotes, so only whole operator - // tokens distinguish shell structure from literal data (e.g. SQL or code). if len(args) == 1 && strings.ContainsAny(args[0], shellSyntax+" \t") { return []string{"/bin/sh", "-c", args[0]} } - - usesShell := false - parts := make([]string, len(args)) - for i, arg := range args { - switch arg { - case "|", "||", "&&", ";", "&", ">", ">>", "<", "<<", "2>", "2>>": - usesShell = true - parts[i] = arg - default: - parts[i] = "'" + strings.ReplaceAll(arg, "'", `'\''`) + "'" - } - } - if !usesShell { - return args - } - return []string{"/bin/sh", "-c", strings.Join(parts, " ")} -} - -func legacyRunCommandCheck(args []string) error { - // Old servers join argv into shell source when the app has an entrypoint. - // There is no way to preserve a -c script argument both there and on - // images without an entrypoint, so do not silently run a different command. - if !slices.Equal(runCommand(args), args) { - return fmt.Errorf("shell expressions in `miren app run` require a newer cluster; this cluster cannot safely preserve the command") - } - return nil + return args } // serverPredatesRuns reports whether a runsClient error means the cluster is diff --git a/cli/commands/app_run_doc.go b/cli/commands/app_run_doc.go index 92ce173b2..268a8c8f9 100644 --- a/cli/commands/app_run_doc.go +++ b/cli/commands/app_run_doc.go @@ -4,9 +4,9 @@ const appRunDescription = `This command runs a command in a fresh sandbox built With no arguments it opens an interactive shell. With arguments it runs that command. With ` + "`" + `--task` + "`" + ` it runs a task declared in ` + "`" + `app.toml` + "`" + `. -Pass a single command string to use shell syntax (such as ` + "`" + `$HOME` + "`" + `, ` + "`" + `|` + "`" + `, or ` + "`" + `>` + "`" + `). Separate operator arguments, such as ` + "`" + `|` + "`" + ` between two commands, also invoke a shell. Other arguments retain their boundaries without shell interpretation, even when they contain characters used by a shell. +Pass a single command string to use shell syntax (such as ` + "`" + `$HOME` + "`" + `, ` + "`" + `|` + "`" + `, or ` + "`" + `>` + "`" + `). Multiple arguments retain their boundaries without shell interpretation, including quoted or escaped operator characters. -Quote the expression for your local shell so it reaches Miren intact: ` + "`" + `miren app run -- 'echo $HOME | wc -c'` + "`" + `. Shell expressions require a cluster that supports durable runs; older clusters cannot safely preserve them. +Quote the expression for your local shell so it reaches Miren intact: ` + "`" + `miren app run -- 'echo $HOME | wc -c'` + "`" + `. Older clusters keep their existing command handling and may not interpret a single command string the same way. This is useful for: - Debugging application issues in an isolated environment diff --git a/cli/commands/app_run_test.go b/cli/commands/app_run_test.go index beab84631..80d0e0f22 100644 --- a/cli/commands/app_run_test.go +++ b/cli/commands/app_run_test.go @@ -24,11 +24,12 @@ func TestRunCommand(t *testing.T) { {"single executable", []string{"date"}, []string{"date"}}, {"ordinary arguments", []string{"echo", "hello world", "O'Reilly"}, []string{"echo", "hello world", "O'Reilly"}}, {"variable argument stays literal", []string{"echo", "$HOME"}, []string{"echo", "$HOME"}}, - {"pipeline with spaced argument", []string{"printf", "%s", "hello world", "|", "wc", "-c"}, []string{"/bin/sh", "-c", "'printf' '%s' 'hello world' | 'wc' '-c'"}}, - {"empty argument in pipeline", []string{"printf", "%s", "", "|", "wc", "-c"}, []string{"/bin/sh", "-c", "'printf' '%s' '' | 'wc' '-c'"}}, + {"operator argument stays literal", []string{"grep", "-F", "|", "log"}, []string{"grep", "-F", "|", "log"}}, + {"find exec terminator", []string{"find", ".", "-name", "x", "-exec", "rm", "{}", ";"}, []string{"find", ".", "-name", "x", "-exec", "rm", "{}", ";"}}, + {"expr comparison", []string{"expr", "3", ">", "2"}, []string{"expr", "3", ">", "2"}}, {"single shell expression", []string{"echo $HOME | wc -c"}, []string{"/bin/sh", "-c", "echo $HOME | wc -c"}}, {"single command string", []string{"exit 7"}, []string{"/bin/sh", "-c", "exit 7"}}, - {"redirect", []string{"echo", "hi", ">", "/tmp/result"}, []string{"/bin/sh", "-c", "'echo' 'hi' > '/tmp/result'"}}, + {"redirect is data", []string{"echo", "hi", ">", "/tmp/result"}, []string{"echo", "hi", ">", "/tmp/result"}}, {"python code is data", []string{"python", "-c", "print('hi')"}, []string{"python", "-c", "print('hi')"}}, {"SQL is data", []string{"psql", "-c", "SELECT * FROM users;"}, []string{"psql", "-c", "SELECT * FROM users;"}}, {"URL is data", []string{"curl", "https://x/?a=1&b=2"}, []string{"curl", "https://x/?a=1&b=2"}}, @@ -51,7 +52,7 @@ func TestRunCommand(t *testing.T) { } func TestRunCommandExecutesShellExpression(t *testing.T) { - args := runCommand([]string{"printf", "%s", "hello world", "|", "wc", "-c"}) + args := runCommand([]string{`printf '%s' 'hello world' | wc -c`}) out, err := exec.Command(args[0], args[1:]...).Output() if err != nil { t.Fatal(err) @@ -61,16 +62,16 @@ func TestRunCommandExecutesShellExpression(t *testing.T) { } } -func TestRunCommandPreservesLiteralArgumentsInPipeline(t *testing.T) { +func TestRunCommandPreservesLiteralArguments(t *testing.T) { for _, literal := range []string{"O'Reilly book", "a # b", "a = b", ""} { t.Run(literal, func(t *testing.T) { - args := runCommand([]string{"printf", "%s", literal, "|", "cat"}) + args := runCommand([]string{"printf", "%s", literal}) out, err := exec.Command(args[0], args[1:]...).Output() if err != nil { t.Fatal(err) } if string(out) != literal { - t.Fatalf("pipeline output = %q, want %q", out, literal) + t.Fatalf("command output = %q, want %q", out, literal) } }) } @@ -97,15 +98,6 @@ func TestRunCommandPreservesExplicitShellExit(t *testing.T) { } } -func TestLegacyRunCommandCheck(t *testing.T) { - assert.NoError(t, legacyRunCommandCheck([]string{"echo", "plain text"})) - assert.NoError(t, legacyRunCommandCheck(nil)) - assert.NoError(t, legacyRunCommandCheck([]string{"python", "-c", "print(1)"})) - assert.NoError(t, legacyRunCommandCheck([]string{"psql", "-c", "SELECT * FROM t"})) - assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo $HOME"}), "require a newer cluster") - assert.ErrorContains(t, legacyRunCommandCheck([]string{"echo", "hi", "|", "wc"}), "require a newer cluster") -} - // The compatibility fallback hinges on one distinction: a server that answered // and does not offer app-runs (fall back to legacy exec) versus a server that // is unreachable, wedged, or refusing us (surface the real error). Falling back diff --git a/docs/docs/command/app-run.md b/docs/docs/command/app-run.md index 6bf917c0d..c6e99ef8d 100644 --- a/docs/docs/command/app-run.md +++ b/docs/docs/command/app-run.md @@ -12,9 +12,9 @@ This command runs a command in a fresh sandbox built from your app's active vers With no arguments it opens an interactive shell. With arguments it runs that command. With `--task` it runs a task declared in `app.toml`. -Pass a single command string to use shell syntax (such as `$HOME`, `|`, or `>`). Separate operator arguments, such as `|` between two commands, also invoke a shell. Other arguments retain their boundaries without shell interpretation, even when they contain characters used by a shell. +Pass a single command string to use shell syntax (such as `$HOME`, `|`, or `>`). Multiple arguments retain their boundaries without shell interpretation, including quoted or escaped operator characters. -Quote the expression for your local shell so it reaches Miren intact: `miren app run -- 'echo $HOME | wc -c'`. Shell expressions require a cluster that supports durable runs; older clusters cannot safely preserve them. +Quote the expression for your local shell so it reaches Miren intact: `miren app run -- 'echo $HOME | wc -c'`. Older clusters keep their existing command handling and may not interpret a single command string the same way. This is useful for: - Debugging application issues in an isolated environment