Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions interp/allowed_paths_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,19 @@ func TestRunRecoversPanic(t *testing.T) {
assert.Contains(t, err.Error(), "deliberate test panic")
}

func TestRunZeroValueRunnerReturnsError(t *testing.T) {
// A zero-value Runner (not created via New) should return an explicit
// error from Run instead of panicking.
var r Runner
parser := syntax.NewParser()
prog, err := parser.Parse(strings.NewReader("echo hi"), "")
require.NoError(t, err)

err = r.Run(context.Background(), prog)
require.Error(t, err)
assert.Contains(t, err.Error(), "use interp.New to construct a Runner")
}

func TestAllowedPathsExecDefaultBlocksAll(t *testing.T) {
dir := t.TempDir()
// No AllowedPaths option — default blocks all exec
Expand Down
6 changes: 5 additions & 1 deletion interp/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ func StdIO(in io.Reader, out, err io.Writer) RunnerOption {
// current directory.
func (r *Runner) Reset() {
if !r.usedNew {
panic("use interp.New to construct a Runner")
r.exit.fatal(fmt.Errorf("use interp.New to construct a Runner"))
return
}
if !r.didReset {
r.origDir = r.Dir
Expand Down Expand Up @@ -333,6 +334,9 @@ func (r *Runner) Run(ctx context.Context, node syntax.Node) (retErr error) {
}()
if !r.didReset {
r.Reset()
if r.exit.fatalExit {
return r.exit.err
}
}
r.fillExpandConfig(ctx)
if err := validateNode(node); err != nil {
Expand Down
4 changes: 3 additions & 1 deletion interp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (
)

// HandlerCtx returns HandlerContext value stored in ctx.
// It panics if ctx has no HandlerContext stored.
// It panics if ctx has no HandlerContext stored; this indicates a
// programming error by the caller (e.g. passing a context that did not
// originate from the interpreter).
func HandlerCtx(ctx context.Context) HandlerContext {
hc, ok := ctx.Value(handlerCtxKey{}).(HandlerContext)
if !ok {
Expand Down
131 changes: 131 additions & 0 deletions interp/internal_errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package interp

import (
"context"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"mvdan.cc/sh/v3/expand"
"mvdan.cc/sh/v3/syntax"
)

// newResetRunner creates a Runner via New and calls Reset so that internal
// state (writeEnv, etc.) is fully initialised for unit-level tests.
func newResetRunner(t *testing.T) *Runner {
t.Helper()
r, err := New()
require.NoError(t, err)
t.Cleanup(func() { r.Close() })
r.Reset()
return r
}

func TestInternalErrorf(t *testing.T) {
r := newResetRunner(t)

r.internalErrorf("something went wrong: %s", "details")

require.True(t, r.exit.fatalExit)
assert.Contains(t, r.exit.err.Error(), "internal error: something went wrong: details")
}

func TestInternalErrorfIdempotent(t *testing.T) {
r := newResetRunner(t)

r.internalErrorf("first error")
r.internalErrorf("second error")

assert.Contains(t, r.exit.err.Error(), "first error",
"exit.fatal keeps the first error and ignores subsequent calls")
}

func TestLookupVarEmptyName(t *testing.T) {
r := newResetRunner(t)

vr := r.lookupVar("")

assert.Equal(t, expand.Variable{}, vr, "should return zero Variable")
require.True(t, r.exit.fatalExit)
assert.Contains(t, r.exit.err.Error(), "variable name must not be empty")
}

func TestSetVarWithIndexNonNilIndex(t *testing.T) {
r := newResetRunner(t)

// Any non-nil ArithmExpr triggers the invariant check.
idx := &syntax.Word{}
r.setVarWithIndex(expand.Variable{}, "X", idx, expand.Variable{})

require.True(t, r.exit.fatalExit)
assert.Contains(t, r.exit.err.Error(), "index should have been rejected by AST validation")
}

func TestAssignValAppend(t *testing.T) {
r := newResetRunner(t)

as := &syntax.Assign{Append: true, Name: &syntax.Lit{Value: "X"}}
vr := r.assignVal(expand.Variable{}, as, "")

assert.Equal(t, expand.Variable{}, vr, "should return zero Variable")
require.True(t, r.exit.fatalExit)
assert.Contains(t, r.exit.err.Error(), "append should have been rejected by AST validation")
}

func TestAssignValArray(t *testing.T) {
r := newResetRunner(t)

as := &syntax.Assign{
Name: &syntax.Lit{Value: "X"},
Array: &syntax.ArrayExpr{},
}
vr := r.assignVal(expand.Variable{}, as, "")

assert.Equal(t, expand.Variable{}, vr, "should return zero Variable")
require.True(t, r.exit.fatalExit)
assert.Contains(t, r.exit.err.Error(), "array assignment should have been rejected by AST validation")
}

func TestResetZeroValueRunnerSetsFatal(t *testing.T) {
var r Runner
r.Reset()

require.True(t, r.exit.fatalExit)
assert.Contains(t, r.exit.err.Error(), "use interp.New to construct a Runner")
}

func TestRunZeroValueRunnerMultipleCalls(t *testing.T) {
// Calling Run repeatedly on a zero-value Runner should consistently
// return an explicit error, not panic.
var r Runner
parser := syntax.NewParser()
prog, err := parser.Parse(strings.NewReader("echo hi"), "")
require.NoError(t, err)

for i := 0; i < 3; i++ {
err = r.Run(context.Background(), prog)
require.Error(t, err)
assert.Contains(t, err.Error(), "use interp.New to construct a Runner")
}
}

func TestInternalErrorStopsExecution(t *testing.T) {
// After an internal error, the runner's stop() check should halt
// further statement execution, surfacing the error via Run.
r := newResetRunner(t)

// Inject a fatal internal error before running a program.
r.internalErrorf("forced failure")

parser := syntax.NewParser()
prog, err := parser.Parse(strings.NewReader("echo should_not_run"), "")
require.NoError(t, err)

// Run resets exit, but the next stmt() call will see exiting=true.
// We verify the pattern by calling stmts directly on a prepared runner.
r.fillExpandConfig(context.Background())
r.stmts(context.Background(), prog.Stmts)

assert.True(t, r.exit.fatalExit, "fatal flag should remain set")
}
19 changes: 15 additions & 4 deletions interp/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,17 @@ func (o *overlayEnviron) Each(f func(name string, vr expand.Variable) bool) {
}
}

// internalErrorf records an internal assertion failure as a fatal error.
// Use this for conditions that should be unreachable (e.g. invariants
// enforced by AST validation).
func (r *Runner) internalErrorf(format string, a ...any) {
r.exit.fatal(fmt.Errorf("internal error: "+format, a...))
}

func (r *Runner) lookupVar(name string) expand.Variable {
if name == "" {
panic("variable name must not be empty")
r.internalErrorf("variable name must not be empty")
return expand.Variable{}
}
// Only $? is supported as a special variable in safe-shell.
if name == "?" {
Expand Down Expand Up @@ -125,7 +133,8 @@ func (r *Runner) setVar(name string, vr expand.Variable) {
// blocked by the AST validator, so we only handle simple string assignment.
func (r *Runner) setVarWithIndex(prev expand.Variable, name string, index syntax.ArithmExpr, vr expand.Variable) {
if index != nil {
panic("setVarWithIndex: index should have been rejected by AST validation")
r.internalErrorf("setVarWithIndex: index should have been rejected by AST validation")
return
}
prev.Set = true
if name2, var2 := prev.Resolve(r.writeEnv); name2 != "" {
Expand All @@ -142,10 +151,12 @@ func (r *Runner) setVarWithIndex(prev expand.Variable, name string, index syntax
func (r *Runner) assignVal(prev expand.Variable, as *syntax.Assign, _ string) expand.Variable {
prev.Set = true
if as.Append {
panic("assignVal: append should have been rejected by AST validation")
r.internalErrorf("assignVal: append should have been rejected by AST validation")
return expand.Variable{}
}
if as.Array != nil {
panic("assignVal: array assignment should have been rejected by AST validation")
r.internalErrorf("assignVal: array assignment should have been rejected by AST validation")
return expand.Variable{}
}
if as.Value != nil {
prev.Kind = expand.String
Expand Down