From 59bf1f14af3402a05a4116c8e629efc8325745bd Mon Sep 17 00:00:00 2001 From: datadog-bits <263423550+datadog-bits@users.noreply.github.com> Date: Thu, 19 Mar 2026 09:44:01 +0000 Subject: [PATCH 1/7] Add execution timeout support Co-authored-by: AlexandreYang <49917914+AlexandreYang@users.noreply.github.com> --- README.md | 8 ++++ SHELL_FEATURES.md | 1 + cmd/rshell/main.go | 26 ++++++++++-- cmd/rshell/main_test.go | 8 ++++ interp/api.go | 25 ++++++++++++ interp/timeout_test.go | 87 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 interp/timeout_test.go diff --git a/README.md b/README.md index 57cf27fe..7d7769c1 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ import ( "context" "os" "strings" + "time" "github.com/DataDog/rshell/interp" "mvdan.cc/sh/v3/syntax" @@ -33,6 +34,7 @@ func main() { runner, _ := interp.New( interp.StdIO(nil, os.Stdout, os.Stderr), interp.AllowedCommands([]string{"rshell:echo"}), + interp.MaxExecutionTime(5*time.Second), ) defer runner.Close() @@ -40,6 +42,12 @@ func main() { } ``` +CLI usage also supports a whole-run timeout: + +```bash +rshell --allow-all-commands --timeout 5s -c 'echo "hello from rshell"' +``` + ## Security Model Every access path is default-deny: diff --git a/SHELL_FEATURES.md b/SHELL_FEATURES.md index a95e5ce7..e88bd27f 100644 --- a/SHELL_FEATURES.md +++ b/SHELL_FEATURES.md @@ -98,6 +98,7 @@ Blocked features are rejected before execution with exit code 2. - ✅ AllowedCommands — restricts which commands (builtins or external) may be executed; commands require the `rshell:` namespace prefix (e.g. `rshell:cat`); if not set, no commands are allowed - ✅ AllowAllCommands — permits any command (testing convenience) - ✅ AllowedPaths filesystem sandboxing — restricts all file access to specified directories +- ✅ Whole-run execution timeout — callers can bound a `Run()` call via `context.Context`, `interp.MaxExecutionTime`, or the CLI `--timeout` flag; the deadline applies to the entire script, not each individual command - ❌ External commands — blocked by default; requires an ExecHandler to be configured and the binary to be within AllowedPaths - ❌ Background execution: `cmd &` - ❌ Coprocesses: `coproc` diff --git a/cmd/rshell/main.go b/cmd/rshell/main.go index 7db4ef33..07ae808a 100644 --- a/cmd/rshell/main.go +++ b/cmd/rshell/main.go @@ -14,12 +14,15 @@ import ( "io" "os" "strings" + "time" "github.com/DataDog/rshell/interp" "github.com/spf13/cobra" "mvdan.cc/sh/v3/syntax" ) +const exitCodeTimeout = 124 + func main() { os.Exit(run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) } @@ -30,6 +33,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { allowedPaths string allowedCommands string allowAllCmds bool + timeout time.Duration ) cmd := &cobra.Command{ @@ -48,6 +52,13 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return fmt.Errorf("cannot use -c with file arguments") } + runCtx := cmd.Context() + if timeout > 0 { + var cancel context.CancelFunc + runCtx, cancel = context.WithTimeout(runCtx, timeout) + defer cancel() + } + var paths []string if allowedPaths != "" { paths = strings.Split(allowedPaths, ",") @@ -65,7 +76,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } if commandSet { - return execute(cmd.Context(), command, "", execOpts, stdin, stdout, stderr) + return execute(runCtx, command, "", execOpts, stdin, stdout, stderr) } if len(args) > 0 { @@ -81,7 +92,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if err != nil { return fmt.Errorf("reading %s: %w", file, err) } - if err := execute(cmd.Context(), string(data), file, execOpts, bytes.NewReader(stdinData), stdout, stderr); err != nil { + if err := execute(runCtx, string(data), file, execOpts, bytes.NewReader(stdinData), stdout, stderr); err != nil { return err } } @@ -93,7 +104,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if err != nil { return fmt.Errorf("reading stdin: %w", err) } - return execute(cmd.Context(), string(stdinData), "", execOpts, strings.NewReader(""), stdout, stderr) + return execute(runCtx, string(stdinData), "", execOpts, strings.NewReader(""), stdout, stderr) }, } @@ -107,12 +118,21 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { cmd.Flags().StringVarP(&allowedPaths, "allowed-paths", "p", "", "comma-separated list of directories the shell is allowed to access") cmd.Flags().StringVar(&allowedCommands, "allowed-commands", "", "comma-separated list of namespaced commands (e.g. rshell:cat,rshell:find)") cmd.Flags().BoolVar(&allowAllCmds, "allow-all-commands", false, "allow execution of all commands (builtins and external)") + cmd.Flags().DurationVar(&timeout, "timeout", 0, "maximum execution time for the entire shell run (e.g. 100ms, 5s, 1m)") if err := cmd.Execute(); err != nil { var status interp.ExitStatus if errors.As(err, &status) { return int(status) } + if errors.Is(err, context.DeadlineExceeded) { + if timeout > 0 { + fmt.Fprintf(stderr, "error: execution timed out after %s\n", timeout) + } else { + fmt.Fprintln(stderr, "error: execution timed out") + } + return exitCodeTimeout + } fmt.Fprintf(stderr, "error: %v\n", err) return 1 } diff --git a/cmd/rshell/main_test.go b/cmd/rshell/main_test.go index d475413a..16e95fb9 100644 --- a/cmd/rshell/main_test.go +++ b/cmd/rshell/main_test.go @@ -141,6 +141,7 @@ func TestHelp(t *testing.T) { assert.Contains(t, stdout, "--allowed-paths") assert.Contains(t, stdout, "--allowed-commands") assert.Contains(t, stdout, "--allow-all-commands") + assert.Contains(t, stdout, "--timeout") assert.NotContains(t, stdout, "--command", "-c/--command should be hidden from help") } @@ -242,3 +243,10 @@ func TestCommandLongFormRejected(t *testing.T) { assert.NotEqual(t, 0, code) assert.Contains(t, stderr, "unknown flag: --command") } + +func TestTimeoutFlagTimesOutExecution(t *testing.T) { + code, stdout, stderr := runCLI(t, "--allow-all-commands", "--timeout", "1ns", "-c", `echo hello`) + assert.Equal(t, exitCodeTimeout, code) + assert.Empty(t, stdout) + assert.Contains(t, stderr, "execution timed out") +} diff --git a/interp/api.go b/interp/api.go index 2fd490fe..88cec96b 100644 --- a/interp/api.go +++ b/interp/api.go @@ -58,6 +58,10 @@ type runnerConfig struct { // command. Intended for testing convenience. allowAllCommands bool + // maxExecutionTime bounds the duration of each Run call. Zero disables + // the limit. When non-zero, Run derives a child context with this timeout. + maxExecutionTime time.Duration + // usedNew is set by New() and checked in Reset() to ensure a Runner // was properly constructed rather than zero-initialized. usedNew bool @@ -282,6 +286,22 @@ func StdIO(in io.Reader, out, err io.Writer) RunnerOption { } } +// MaxExecutionTime bounds the total execution time of each [Runner.Run] call. +// +// When d is zero, no timeout is applied. Negative values are rejected. +// +// The timeout is applied per Run call rather than when the Runner is created, +// so reusing a Runner across multiple runs yields a fresh deadline each time. +func MaxExecutionTime(d time.Duration) RunnerOption { + return func(r *Runner) error { + if d < 0 { + return fmt.Errorf("MaxExecutionTime: duration must be >= 0") + } + r.maxExecutionTime = d + return nil + } +} + // Reset returns a runner to its initial state, right before the first call to // Run or Reset. // @@ -367,6 +387,11 @@ func (r *Runner) Run(ctx context.Context, node syntax.Node) (retErr error) { retErr = fmt.Errorf("internal error") } }() + if r.maxExecutionTime > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, r.maxExecutionTime) + defer cancel() + } if !r.didReset { r.Reset() if r.exit.fatalExit { diff --git a/interp/timeout_test.go b/interp/timeout_test.go new file mode 100644 index 00000000..b3e59fc1 --- /dev/null +++ b/interp/timeout_test.go @@ -0,0 +1,87 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026-present Datadog, Inc. + +package interp + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newTimeoutRunner(t *testing.T, opts ...RunnerOption) *Runner { + t.Helper() + allOpts := append([]RunnerOption{AllowAllCommands()}, opts...) + r, err := New(allOpts...) + require.NoError(t, err) + t.Cleanup(func() { _ = r.Close() }) + r.Reset() + return r +} + +func TestMaxExecutionTimeRejectsNegative(t *testing.T) { + _, err := New(MaxExecutionTime(-time.Second)) + require.Error(t, err) + assert.Contains(t, err.Error(), "MaxExecutionTime") +} + +func TestMaxExecutionTimeStopsRun(t *testing.T) { + r := newTimeoutRunner(t, MaxExecutionTime(20*time.Millisecond)) + r.execHandler = func(ctx context.Context, _ []string) error { + <-ctx.Done() + return ctx.Err() + } + + err := r.Run(context.Background(), parseScript(t, "slowcmd")) + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) +} + +func TestMaxExecutionTimeRespectsEarlierParentDeadline(t *testing.T) { + r := newTimeoutRunner(t, MaxExecutionTime(time.Second)) + var got time.Time + r.execHandler = func(ctx context.Context, _ []string) error { + var ok bool + got, ok = ctx.Deadline() + require.True(t, ok, "expected deadline on exec handler context") + return nil + } + + parent, cancel := context.WithTimeout(context.Background(), 25*time.Millisecond) + defer cancel() + parentDeadline, ok := parent.Deadline() + require.True(t, ok) + + err := r.Run(parent, parseScript(t, "slowcmd")) + require.NoError(t, err) + assert.WithinDuration(t, parentDeadline, got, 5*time.Millisecond) +} + +func TestMaxExecutionTimeIsRefreshedPerRun(t *testing.T) { + r := newTimeoutRunner(t, MaxExecutionTime(100*time.Millisecond)) + var deadlines []time.Time + r.execHandler = func(ctx context.Context, _ []string) error { + deadline, ok := ctx.Deadline() + require.True(t, ok, "expected deadline on exec handler context") + deadlines = append(deadlines, deadline) + return nil + } + + prog := parseScript(t, "slowcmd") + + err := r.Run(context.Background(), prog) + require.NoError(t, err) + + time.Sleep(20 * time.Millisecond) + + err = r.Run(context.Background(), prog) + require.NoError(t, err) + + require.Len(t, deadlines, 2) + assert.True(t, deadlines[1].After(deadlines[0]), "expected a fresh deadline on each Run") +} From 3162c4f4d76dbf8a4b3b65fc1fd52459e9fc6cd7 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Thu, 19 Mar 2026 10:52:12 +0100 Subject: [PATCH 2/7] Add time.Duration, context.CancelFunc, context.WithTimeout to interp allowlist The execution timeout feature added in the previous commit uses these symbols in api.go but they were missing from the interp allowed symbols list, causing TestInterpAllowedSymbols and TestVerificationInterpCleanPass to fail in CI. Co-Authored-By: Claude Sonnet 4.6 --- allowedsymbols/symbols_interp.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/allowedsymbols/symbols_interp.go b/allowedsymbols/symbols_interp.go index d86d235c..b688a9a0 100644 --- a/allowedsymbols/symbols_interp.go +++ b/allowedsymbols/symbols_interp.go @@ -18,7 +18,9 @@ package allowedsymbols // The permanently banned packages (reflect, unsafe) apply here too. var interpAllowedSymbols = []string{ "bytes.Buffer", // 🟢 in-memory byte buffer; pure data structure, no I/O. + "context.CancelFunc", // 🟢 function type returned by WithTimeout/WithCancel; pure function type, no side effects. "context.Context", // 🟢 deadline/cancellation plumbing; pure interface, no side effects. + "context.WithTimeout", // 🟢 derives a context with a deadline; needed for execution timeout support. "context.WithValue", // 🟢 derives a context carrying a key-value pair; pure function. "errors.As", // 🟢 error type assertion; pure function, no I/O. "fmt.Errorf", // 🟢 formatted error creation; pure function, no I/O. @@ -58,6 +60,7 @@ var interpAllowedSymbols = []string{ "sync.Mutex", // 🟢 mutual exclusion lock; concurrency primitive, no I/O. "sync.Once", // 🟢 ensures a function runs exactly once; concurrency primitive, no I/O. "sync.WaitGroup", // 🟢 waits for goroutines to finish; concurrency primitive, no I/O. + "time.Duration", // 🟢 numeric duration type; pure type, no side effects. "time.Now", // 🟠 returns current time; read-only, no mutation. "time.Time", // 🟢 time value type; pure data, no side effects. From e09b5771c3453a81fcbac2431b085494d8836480 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Thu, 19 Mar 2026 10:56:40 +0100 Subject: [PATCH 3/7] Address PR feedback: reject negative --timeout and apply timeout to stdin reads - Reject negative --timeout values with an error (consistent with interp.MaxExecutionTime which also rejects negatives) - Replace io.ReadAll(stdin) with readAllContext so the timeout also bounds the blocking stdin read phase, not just script execution - Add TestTimeoutFlagRejectsNegative to cover the new validation Co-Authored-By: Claude Sonnet 4.6 --- cmd/rshell/main.go | 31 +++++++++++++++++++++++++++++-- cmd/rshell/main_test.go | 6 ++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cmd/rshell/main.go b/cmd/rshell/main.go index fbb75445..f31ee968 100644 --- a/cmd/rshell/main.go +++ b/cmd/rshell/main.go @@ -53,6 +53,10 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return fmt.Errorf("cannot use -c with file arguments") } + if timeout < 0 { + return fmt.Errorf("--timeout must be >= 0") + } + runCtx := cmd.Context() if timeout > 0 { var cancel context.CancelFunc @@ -84,7 +88,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { if len(args) > 0 { // Read stdin once so each execute() call gets its own // reader, avoiding a data race on the shared io.Reader. - stdinData, err := io.ReadAll(stdin) + stdinData, err := readAllContext(runCtx, stdin) if err != nil { return fmt.Errorf("reading stdin: %w", err) } @@ -102,7 +106,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } // No -c and no file args: read from stdin. - stdinData, err := io.ReadAll(stdin) + stdinData, err := readAllContext(runCtx, stdin) if err != nil { return fmt.Errorf("reading stdin: %w", err) } @@ -142,6 +146,29 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return 0 } +// readAllContext reads all bytes from r, but returns ctx.Err() immediately if +// the context is cancelled or its deadline expires before the read completes. +// It spawns a goroutine to perform the read; the goroutine may outlive this +// call if the underlying reader blocks (e.g. stdin from a pipe), but it will +// be reclaimed when the process exits. +func readAllContext(ctx context.Context, r io.Reader) ([]byte, error) { + type result struct { + data []byte + err error + } + ch := make(chan result, 1) + go func() { + data, err := io.ReadAll(r) + ch <- result{data, err} + }() + select { + case <-ctx.Done(): + return nil, ctx.Err() + case res := <-ch: + return res.data, res.err + } +} + // rejectLongCommand scans raw CLI args for "--command" or "--command=..." and // returns an error if found. The flag is registered with a long name so that // cobra/pflag help formatting works correctly, but only the -c shorthand is diff --git a/cmd/rshell/main_test.go b/cmd/rshell/main_test.go index 44eb38d6..78b2cc90 100644 --- a/cmd/rshell/main_test.go +++ b/cmd/rshell/main_test.go @@ -251,6 +251,12 @@ func TestTimeoutFlagTimesOutExecution(t *testing.T) { assert.Contains(t, stderr, "execution timed out") } +func TestTimeoutFlagRejectsNegative(t *testing.T) { + code, _, stderr := runCLI(t, "--timeout", "-1s", "-c", `echo hello`) + assert.Equal(t, 1, code) + assert.Contains(t, stderr, "--timeout must be >= 0") +} + func TestProcPathFlagInHelp(t *testing.T) { code, stdout, _ := runCLI(t, "--help") assert.Equal(t, 0, code) From ee693329a242ca472f6a38760116b89a7813b333 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Thu, 19 Mar 2026 11:07:34 +0100 Subject: [PATCH 4/7] Fix TestTimeoutFlagTimesOutExecution on Windows 1ns is too small for Windows' ~15ms timer resolution: the echo command completes before the context cancel goroutine fires, so the test got exit code 0 instead of 124. Replace with a blocking-stdin approach: pass an io.Pipe reader with no -c flag so readAllContext blocks until the 50ms context deadline fires. This is reliable on all platforms because 50ms >> Windows timer jitter. Co-Authored-By: Claude Sonnet 4.6 --- cmd/rshell/main_test.go | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/cmd/rshell/main_test.go b/cmd/rshell/main_test.go index 78b2cc90..fe24bf29 100644 --- a/cmd/rshell/main_test.go +++ b/cmd/rshell/main_test.go @@ -7,6 +7,8 @@ package main import ( "bytes" + "context" + "io" "os" "path/filepath" "runtime" @@ -18,9 +20,14 @@ import ( ) func runCLI(t *testing.T, args ...string) (exitCode int, stdout, stderr string) { + t.Helper() + return runCLIContext(t, context.Background(), args...) +} + +func runCLIContext(t *testing.T, ctx context.Context, args ...string) (exitCode int, stdout, stderr string) { t.Helper() var out, errBuf bytes.Buffer - code := run(args, strings.NewReader(""), &out, &errBuf) + code := run(ctx, args, strings.NewReader(""), &out, &errBuf) return code, out.String(), errBuf.String() } @@ -39,7 +46,7 @@ func TestShortFlag(t *testing.T) { func runCLIWithStdin(t *testing.T, stdin string, args ...string) (exitCode int, stdout, stderr string) { t.Helper() var out, errBuf bytes.Buffer - code := run(args, strings.NewReader(stdin), &out, &errBuf) + code := run(context.Background(), args, strings.NewReader(stdin), &out, &errBuf) return code, out.String(), errBuf.String() } @@ -245,10 +252,22 @@ func TestCommandLongFormRejected(t *testing.T) { } func TestTimeoutFlagTimesOutExecution(t *testing.T) { - code, stdout, stderr := runCLI(t, "--allow-all-commands", "--timeout", "1ns", "-c", `echo hello`) + // Feed a blocking stdin with no -c flag so the timeout fires while readAllContext + // is waiting for EOF. 50ms is well above Windows' ~15ms timer resolution. + pr, pw := io.Pipe() + defer pw.Close() + var out, errBuf bytes.Buffer + code := run(context.Background(), []string{"--timeout", "50ms"}, pr, &out, &errBuf) assert.Equal(t, exitCodeTimeout, code) - assert.Empty(t, stdout) - assert.Contains(t, stderr, "execution timed out") + assert.Contains(t, errBuf.String(), "execution timed out") +} + +func TestCanceledContextExitsWithTimeoutCode(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel before execution starts + code, _, stderr := runCLIContext(t, ctx, "--allow-all-commands", "-c", `echo hello`) + assert.Equal(t, exitCodeTimeout, code) + assert.Contains(t, stderr, "execution canceled") } func TestTimeoutFlagRejectsNegative(t *testing.T) { From 4a4c11894f43894fc3d1e6a96252dd5e0fc66d67 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Thu, 19 Mar 2026 11:23:34 +0100 Subject: [PATCH 5/7] Add ctx parameter to run() and handle context.Canceled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Pass context.Background() through main() into run() so callers (tests, future integrations) can inject a context - Use cmd.ExecuteContext(ctx) instead of cmd.Execute() - Add errors.Is(err, context.Canceled) branch → exit code 124 + "error: execution canceled" message Co-Authored-By: Claude Sonnet 4.6 --- cmd/rshell/main.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/rshell/main.go b/cmd/rshell/main.go index f31ee968..3b3e46c6 100644 --- a/cmd/rshell/main.go +++ b/cmd/rshell/main.go @@ -24,10 +24,10 @@ import ( const exitCodeTimeout = 124 func main() { - os.Exit(run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) + os.Exit(run(context.Background(), os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) } -func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { +func run(ctx context.Context, args []string, stdin io.Reader, stdout, stderr io.Writer) int { var ( command string allowedPaths string @@ -127,7 +127,7 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { cmd.Flags().DurationVar(&timeout, "timeout", 0, "maximum execution time for the entire shell run (e.g. 100ms, 5s, 1m)") cmd.Flags().StringVar(&procPath, "proc-path", "", "path to the proc filesystem used by ps (default \"/proc\")") - if err := cmd.Execute(); err != nil { + if err := cmd.ExecuteContext(ctx); err != nil { var status interp.ExitStatus if errors.As(err, &status) { return int(status) @@ -140,6 +140,10 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } return exitCodeTimeout } + if errors.Is(err, context.Canceled) { + fmt.Fprintln(stderr, "error: execution canceled") + return exitCodeTimeout + } fmt.Fprintf(stderr, "error: %v\n", err) return 1 } From f5a45fe24e234a90df5e74d48120f856a42d1793 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Thu, 19 Mar 2026 11:24:52 +0100 Subject: [PATCH 6/7] Address codex review: add test comment and for-loop timeout test - Add comment to TestMaxExecutionTimeRespectsEarlierParentDeadline explaining why WithinDuration uses the parent deadline (Go's context.WithTimeout picks the earlier of two deadlines). - Add TestMaxExecutionTimeStopsForLoop to exercise the interpreter's own ctx.Err() check inside the for-loop body, not just the execHandler cooperative-cancellation path. Co-Authored-By: Claude Sonnet 4.6 --- interp/timeout_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/interp/timeout_test.go b/interp/timeout_test.go index b3e59fc1..4d49f689 100644 --- a/interp/timeout_test.go +++ b/interp/timeout_test.go @@ -59,9 +59,32 @@ func TestMaxExecutionTimeRespectsEarlierParentDeadline(t *testing.T) { err := r.Run(parent, parseScript(t, "slowcmd")) require.NoError(t, err) + // context.WithTimeout takes the earlier of the two deadlines, so the runner's 1s + // MaxExecutionTime must not override the parent's tighter 25ms deadline. assert.WithinDuration(t, parentDeadline, got, 5*time.Millisecond) } +func TestMaxExecutionTimeStopsForLoop(t *testing.T) { + // Exercises the interpreter's own ctx.Err() check inside the for-loop body + // (runner_exec.go), not just the execHandler cooperative-cancellation path. + // while/until loops are not supported, so we use a for loop with an + // execHandler that sleeps per iteration to make the loop outlast the timeout. + r := newTimeoutRunner(t, MaxExecutionTime(50*time.Millisecond)) + r.execHandler = func(ctx context.Context, _ []string) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(20 * time.Millisecond): + return nil + } + } + + // 10 iterations × 20ms each = 200ms total, well beyond the 50ms timeout. + err := r.Run(context.Background(), parseScript(t, "for x in 1 2 3 4 5 6 7 8 9 10; do cmd; done")) + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) +} + func TestMaxExecutionTimeIsRefreshedPerRun(t *testing.T) { r := newTimeoutRunner(t, MaxExecutionTime(100*time.Millisecond)) var deadlines []time.Time From c73d6f7fe43f9b6489159f40f7dc035b4ee161d6 Mon Sep 17 00:00:00 2001 From: Alexandre Yang Date: Thu, 19 Mar 2026 16:08:57 +0100 Subject: [PATCH 7/7] Use readAllContext for script file reads to honour --timeout Co-Authored-By: Claude Opus 4.6 --- cmd/rshell/main.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/rshell/main.go b/cmd/rshell/main.go index 3b3e46c6..3e20f1d1 100644 --- a/cmd/rshell/main.go +++ b/cmd/rshell/main.go @@ -94,7 +94,12 @@ func run(ctx context.Context, args []string, stdin io.Reader, stdout, stderr io. } for _, file := range args { - data, err := os.ReadFile(file) + f, err := os.Open(file) + if err != nil { + return fmt.Errorf("reading %s: %w", file, err) + } + data, err := readAllContext(runCtx, f) + f.Close() if err != nil { return fmt.Errorf("reading %s: %w", file, err) }