diff --git a/cmd/executors/npx.go b/cmd/executors/npx.go index d687b92..b088220 100644 --- a/cmd/executors/npx.go +++ b/cmd/executors/npx.go @@ -20,7 +20,7 @@ func NewNpxCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executeNpxFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/executors/pnpx.go b/cmd/executors/pnpx.go index 372f1e4..08d4a1c 100644 --- a/cmd/executors/pnpx.go +++ b/cmd/executors/pnpx.go @@ -20,7 +20,7 @@ func NewPnpxCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executePnpxFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/npm/bun.go b/cmd/npm/bun.go index 68ac92d..f909849 100644 --- a/cmd/npm/bun.go +++ b/cmd/npm/bun.go @@ -20,7 +20,7 @@ func NewBunCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executeBunFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/npm/npm.go b/cmd/npm/npm.go index 07a4b01..abdd71b 100644 --- a/cmd/npm/npm.go +++ b/cmd/npm/npm.go @@ -20,7 +20,7 @@ func NewNpmCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executeNpmFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/npm/pnpm.go b/cmd/npm/pnpm.go index 9370289..fda9a36 100644 --- a/cmd/npm/pnpm.go +++ b/cmd/npm/pnpm.go @@ -20,7 +20,7 @@ func NewPnpmCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executePnpmFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/npm/yarn.go b/cmd/npm/yarn.go index a4440cc..7072983 100644 --- a/cmd/npm/yarn.go +++ b/cmd/npm/yarn.go @@ -20,7 +20,7 @@ func NewYarnCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executeYarnFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil }, diff --git a/cmd/pypi/pip.go b/cmd/pypi/pip.go index 53c2016..73b1346 100644 --- a/cmd/pypi/pip.go +++ b/cmd/pypi/pip.go @@ -20,7 +20,7 @@ func NewPipCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executePipFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/pypi/pip3.go b/cmd/pypi/pip3.go index 898a4b2..3e35af6 100644 --- a/cmd/pypi/pip3.go +++ b/cmd/pypi/pip3.go @@ -20,7 +20,7 @@ func NewPip3Command() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executePip3Flow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/pypi/poetry.go b/cmd/pypi/poetry.go index bcae4dc..b8878dd 100644 --- a/cmd/pypi/poetry.go +++ b/cmd/pypi/poetry.go @@ -20,7 +20,7 @@ func NewPoetryCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executePoetryFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/cmd/pypi/uv.go b/cmd/pypi/uv.go index 706e7d3..e3b1f89 100644 --- a/cmd/pypi/uv.go +++ b/cmd/pypi/uv.go @@ -20,7 +20,7 @@ func NewUvCommand() *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { err := executeUvFlow(cmd.Context(), args) if err != nil { - ui.ErrorExit(err) + ui.ExitFromCommandError(err) } return nil diff --git a/internal/flows/proxy_flow.go b/internal/flows/proxy_flow.go index 78ae992..3089134 100644 --- a/internal/flows/proxy_flow.go +++ b/internal/flows/proxy_flow.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "os/exec" "path/filepath" "time" @@ -290,16 +289,16 @@ func (f *proxyFlow) Run(ctx context.Context, args []string, parsedCmd *packagema return handleExecutionResultError(executionError) } -// handleExecutionResultError handles the error from the execution result. +// handleExecutionResultError returns the execution error so RunE can route it +// through ui.ExitFromCommandError, the single exit point. A transparent +// *runner.ChildExitError survives the %w wrap (errors.As unwraps it) and is +// passed through with the child's exit code; everything else keeps the visible +// PMG error framing. func handleExecutionResultError(err error) error { if err == nil { return nil } - if exitErr, ok := err.(*exec.ExitError); ok { - os.Exit(exitErr.ExitCode()) - } - return fmt.Errorf("failed to execute command: %w", err) } diff --git a/internal/proc/signal_other.go b/internal/proc/signal_other.go new file mode 100644 index 0000000..10d6f40 --- /dev/null +++ b/internal/proc/signal_other.go @@ -0,0 +1,10 @@ +//go:build windows + +package proc + +// SignalInfo on non-unix platforms cannot decode signal termination from the +// process status, so it always reports not-signaled and the raw exit code is +// used verbatim by callers. +func SignalInfo(_ any) (signum int, signaled bool) { + return 0, false +} diff --git a/internal/proc/signal_unix.go b/internal/proc/signal_unix.go new file mode 100644 index 0000000..05a8460 --- /dev/null +++ b/internal/proc/signal_unix.go @@ -0,0 +1,19 @@ +//go:build !windows + +// Package proc decodes process termination status. It is a leaf utility shared +// by the execution layer (runner) and the PTY session, so neither has to +// reach through wrapper layers to learn how a child process ended. +package proc + +import "syscall" + +// SignalInfo reports whether a process was terminated by a signal, given the +// platform-specific status from (*exec.ExitError).Sys() / (*ptyx.ExitError).Sys(). +// signum is the signal number; callers form the conventional exit code as 128+signum. +func SignalInfo(sys any) (signum int, signaled bool) { + ws, ok := sys.(syscall.WaitStatus) + if !ok || !ws.Signaled() { + return 0, false + } + return int(ws.Signal()), true +} diff --git a/internal/proc/signal_unix_test.go b/internal/proc/signal_unix_test.go new file mode 100644 index 0000000..2240ce3 --- /dev/null +++ b/internal/proc/signal_unix_test.go @@ -0,0 +1,58 @@ +//go:build !windows + +package proc + +import ( + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSignalInfo(t *testing.T) { + tests := []struct { + name string + sys any + wantSignum int + wantSignaled bool + }{ + { + name: "SIGINT terminated process", + sys: syscall.WaitStatus(int(syscall.SIGINT)), + wantSignum: int(syscall.SIGINT), + wantSignaled: true, + }, + { + name: "SIGTERM terminated process", + sys: syscall.WaitStatus(int(syscall.SIGTERM)), + wantSignum: int(syscall.SIGTERM), + wantSignaled: true, + }, + { + name: "normal exit code 2 is not signaled", + sys: syscall.WaitStatus(2 << 8), + wantSignum: 0, + wantSignaled: false, + }, + { + name: "nil status is not signaled", + sys: nil, + wantSignum: 0, + wantSignaled: false, + }, + { + name: "non-WaitStatus value is not signaled", + sys: "not a wait status", + wantSignum: 0, + wantSignaled: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + signum, signaled := SignalInfo(tt.sys) + assert.Equal(t, tt.wantSignaled, signaled) + assert.Equal(t, tt.wantSignum, signum) + }) + } +} diff --git a/internal/pty/exiterror_test.go b/internal/pty/exiterror_test.go new file mode 100644 index 0000000..8ae8eb6 --- /dev/null +++ b/internal/pty/exiterror_test.go @@ -0,0 +1,34 @@ +//go:build !windows + +package pty + +import ( + "errors" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewExitError(t *testing.T) { + underlying := errors.New("boom") + + t.Run("normal non-zero exit carries the raw code", func(t *testing.T) { + e := newExitError(2, syscall.WaitStatus(2<<8), underlying) + assert.Equal(t, 2, e.Code) + assert.False(t, e.Signaled) + assert.ErrorIs(t, e, underlying) + }) + + t.Run("signal termination resolves to 128+signum and marks signaled", func(t *testing.T) { + e := newExitError(-1, syscall.WaitStatus(int(syscall.SIGINT)), underlying) + assert.Equal(t, 128+int(syscall.SIGINT), e.Code) + assert.True(t, e.Signaled) + }) + + t.Run("no wait status falls back to the provided code", func(t *testing.T) { + e := newExitError(-1, nil, underlying) + assert.Equal(t, -1, e.Code) + assert.False(t, e.Signaled) + }) +} diff --git a/internal/pty/session.go b/internal/pty/session.go index 04f25d7..6c73db1 100644 --- a/internal/pty/session.go +++ b/internal/pty/session.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/safedep/dry/log" + "github.com/safedep/pmg/internal/proc" "github.com/safedep/ptyx" "golang.org/x/term" ) @@ -152,9 +153,9 @@ func (s *session) Wait() error { err := s.spawn.Wait() if err != nil { if exitErr, ok := err.(*ptyx.ExitError); ok { - return &ExitError{Code: exitErr.ExitCode, Err: err} + return newExitError(exitErr.ExitCode, exitErr.Sys(), err) } - return &ExitError{Code: -1, Err: err} + return newExitError(-1, nil, err) } return nil } @@ -178,8 +179,19 @@ func (s *session) Close() error { // ExitError is returned when the child process exits with non-zero code type ExitError struct { - Code int - Err error // Underlying error from ptyx + Code int + Signaled bool // true if the child was terminated by a signal (Ctrl+C, SIGTERM, …) + Err error // Underlying error from ptyx +} + +// newExitError resolves the child's termination into an ExitError. When the +// process was signal-terminated, Code is the conventional 128+signum and +// Signaled is set; otherwise the raw exit code is preserved. +func newExitError(code int, sys any, err error) *ExitError { + if signum, signaled := proc.SignalInfo(sys); signaled { + return &ExitError{Code: 128 + signum, Signaled: true, Err: err} + } + return &ExitError{Code: code, Err: err} } func (e *ExitError) Error() string { diff --git a/internal/runner/childexit.go b/internal/runner/childexit.go new file mode 100644 index 0000000..95025ec --- /dev/null +++ b/internal/runner/childexit.go @@ -0,0 +1,81 @@ +package runner + +import ( + "errors" + "fmt" + "os/exec" + + "github.com/safedep/dry/usefulerror" + "github.com/safedep/pmg/errcodes" + "github.com/safedep/pmg/internal/proc" + "github.com/safedep/pmg/internal/pty" +) + +// ChildExitError marks a transparent passthrough: the wrapped package manager +// exited on its own. PMG attributes nothing to itself and mirrors the exit code, +// so the package manager's own output stands and PMG stays invisible. +type ChildExitError struct { + Code int // child's exit code (or 128+signum for signal termination) + Signaled bool // true if terminated by a signal (Ctrl+C, SIGTERM, …) + PMName string // package manager name, for the dim one-liner +} + +func (e *ChildExitError) Error() string { + return fmt.Sprintf("%s exited with code %d", e.PMName, e.Code) +} +func (e *ChildExitError) ExitCode() int { return e.Code } +func (e *ChildExitError) Transparent() bool { return true } +func (e *ChildExitError) IsSignaled() bool { return e.Signaled } + +// classify turns a package-manager execution error into either a transparent +// child exit or a visible PMG error, and is the only place the fork lives. It is +// pure: sandbox denials are persisted separately by the caller and never make a +// child's own non-zero exit loud — a restrictive policy routinely denies benign +// operations and causation cannot be inferred from a denial. Only a failure on +// PMG's side of the boundary (the tool never produced an exit status) is loud. +func classify(err error, pmName string) error { + if err == nil { + return nil + } + + code, signaled, resolved := extractExit(err) + return decideExit(err, code, signaled, resolved, pmName) +} + +// extractExit pulls the exit code and signal status from a process error. +// resolved is false when the child never produced a real exit status (e.g. the +// binary failed to launch), which must surface as a visible PMG error. +func extractExit(err error) (code int, signaled bool, resolved bool) { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + if signum, sig := proc.SignalInfo(exitErr.Sys()); sig { + return 128 + signum, true, true + } + code = exitErr.ExitCode() + return code, false, code >= 0 + } + + var ptyErr *pty.ExitError + if errors.As(err, &ptyErr) { + return ptyErr.Code, ptyErr.Signaled, ptyErr.Signaled || ptyErr.Code >= 0 + } + + return -1, false, false +} + +func decideExit(err error, code int, signaled, resolved bool, pmName string) error { + if !resolved { + return visibleExecError(err) + } + return &ChildExitError{Code: code, Signaled: signaled, PMName: pmName} +} + +// visibleExecError is the loud error for a genuine PMG-side failure: the package +// manager never produced an exit status (e.g. the binary could not be launched). +func visibleExecError(err error) error { + return usefulerror.NewUsefulError(). + WithCode(errcodes.PackageManagerExecutionFailed). + WithHumanError("Failed to execute package manager command"). + WithHelp("Check the package manager command and its arguments"). + Wrap(err) +} diff --git a/internal/runner/childexit_test.go b/internal/runner/childexit_test.go new file mode 100644 index 0000000..a1e1b64 --- /dev/null +++ b/internal/runner/childexit_test.go @@ -0,0 +1,103 @@ +package runner + +import ( + "errors" + "os/exec" + "testing" + + "github.com/safedep/dry/usefulerror" + "github.com/safedep/pmg/errcodes" + "github.com/safedep/pmg/internal/pty" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExtractExit(t *testing.T) { + t.Run("direct non-zero exit resolves the real code", func(t *testing.T) { + err := exec.Command("sh", "-c", "exit 2").Run() + require.Error(t, err) + + code, signaled, resolved := extractExit(err) + assert.Equal(t, 2, code) + assert.False(t, signaled) + assert.True(t, resolved) + }) + + t.Run("direct signal termination resolves to 128+signum", func(t *testing.T) { + err := exec.Command("sh", "-c", "kill -INT $$").Run() + require.Error(t, err) + + code, signaled, resolved := extractExit(err) + assert.Equal(t, 130, code) // 128 + SIGINT(2) + assert.True(t, signaled) + assert.True(t, resolved) + }) + + t.Run("pty exit error reads its fields directly", func(t *testing.T) { + code, signaled, resolved := extractExit(&pty.ExitError{Code: 1}) + assert.Equal(t, 1, code) + assert.False(t, signaled) + assert.True(t, resolved) + + code, signaled, resolved = extractExit(&pty.ExitError{Code: 143, Signaled: true}) + assert.Equal(t, 143, code) + assert.True(t, signaled) + assert.True(t, resolved) + }) + + t.Run("non-exit error is unresolved", func(t *testing.T) { + _, _, resolved := extractExit(errors.New("failed to launch binary")) + assert.False(t, resolved) + }) +} + +func TestDecideExit(t *testing.T) { + runErr := errors.New("npm failed") + + t.Run("plain child exit becomes a transparent ChildExitError", func(t *testing.T) { + err := decideExit(runErr, 1, false, true, "npm") + + var ce *ChildExitError + require.True(t, errors.As(err, &ce)) + assert.Equal(t, 1, ce.ExitCode()) + assert.True(t, ce.Transparent()) + assert.False(t, ce.IsSignaled()) + assert.Equal(t, "npm", ce.PMName) + }) + + t.Run("signaled child exit is transparent and signaled", func(t *testing.T) { + err := decideExit(runErr, 130, true, true, "npm") + + var ce *ChildExitError + require.True(t, errors.As(err, &ce)) + assert.True(t, ce.IsSignaled()) + assert.Equal(t, 130, ce.ExitCode()) + }) + + t.Run("unresolved exit is a visible launch failure", func(t *testing.T) { + err := decideExit(runErr, -1, false, false, "npm") + + usefulErr, ok := usefulerror.AsUsefulError(err) + require.True(t, ok) + assert.Equal(t, errcodes.PackageManagerExecutionFailed, usefulErr.Code()) + assert.Equal(t, "Failed to execute package manager command", usefulErr.HumanError()) + + var ce *ChildExitError + assert.False(t, errors.As(err, &ce)) + }) +} + +// classify is pure: a child that produced its own exit status is always a +// transparent passthrough. Sandbox denials are persisted by the caller and +// never reach classify, so they cannot make the exit loud (issue #309). +func TestClassifyTransparentChildExit(t *testing.T) { + childErr := exec.Command("sh", "-c", "exit 1").Run() + require.Error(t, childErr) + + err := classify(childErr, "npm") + + var ce *ChildExitError + require.True(t, errors.As(err, &ce)) + assert.Equal(t, 1, ce.ExitCode()) + assert.Equal(t, "npm", ce.PMName) +} diff --git a/internal/runner/execute.go b/internal/runner/execute.go index aa5240d..340af75 100644 --- a/internal/runner/execute.go +++ b/internal/runner/execute.go @@ -115,13 +115,13 @@ func ExecuteWithOptions(ctx context.Context, pc *packagemanager.ParsedCommand, o switch mode { case ExecutionModePTY: - return runPTY(ctx, cmd, cmd.Env, result, opts.PreparePTYSession) + return runPTY(ctx, cmd, cmd.Env, result, opts.PackageManagerName, opts.PreparePTYSession) default: - return runDirect(cmd, result) + return runDirect(cmd, result, opts.PackageManagerName) } } -func runDirect(cmd *exec.Cmd, result *sandbox.ExecutionResult) error { +func runDirect(cmd *exec.Cmd, result *sandbox.ExecutionResult, pmName string) error { if !result.ShouldRun() { return nil } @@ -129,7 +129,8 @@ func runDirect(cmd *exec.Cmd, result *sandbox.ExecutionResult) error { log.Debugf("Running command with args: %s: %v", cmd.Path, cmd.Args[1:]) if err := cmd.Run(); err != nil { - return wrapCommandExecutionError(err, result) + executor.ObserveViolations(result, err) + return classify(err, pmName) } log.Debugf("Command completed successfully") @@ -141,6 +142,7 @@ func runPTY( cmd *exec.Cmd, env []string, result *sandbox.ExecutionResult, + pmName string, beforeWait func(*PTYRuntime) error, ) error { if !result.ShouldRun() { @@ -241,7 +243,8 @@ func runPTY( } if sessionError != nil { - return wrapCommandExecutionError(sessionError, result) + executor.ObserveViolations(result, sessionError) + return classify(sessionError, pmName) } return nil @@ -309,15 +312,3 @@ func mergeEnv(base, overrides []string) []string { return env } - -func wrapCommandExecutionError(err error, result *sandbox.ExecutionResult) error { - if exitErr, ok := err.(*exec.ExitError); ok { - return executor.WrapCommandExecutionError(err, result, exitErr.ExitCode()) - } - - if sessionError, ok := err.(*pty.ExitError); ok { - return executor.WrapCommandExecutionError(sessionError, result, sessionError.Code) - } - - return executor.WrapCommandExecutionError(err, result, -1) -} diff --git a/internal/ui/error.go b/internal/ui/error.go index 462ffa2..304c928 100644 --- a/internal/ui/error.go +++ b/internal/ui/error.go @@ -37,29 +37,28 @@ func ErrorExitWithCode(err error, code int) { os.Exit(code) } -// printMinimalError prints error in minimal two-line format: +// Error output goes to stderr so the wrapped package manager's stdout passes +// through clean (e.g. `pmg npm view --json` stays parseable). func printMinimalError(code, message, hint string) { - fmt.Printf("%s %s\n", Colors.ErrorCode(" %s ", code), Colors.Red(message)) + fmt.Fprintf(os.Stderr, "%s %s\n", Colors.ErrorCode(" %s ", code), Colors.Red(message)) if hint != "" && hint != "No additional help is available for this error." { - fmt.Printf(" %s %s\n", Colors.Dim("→"), Colors.Dim(hint)) + fmt.Fprintf(os.Stderr, " %s %s\n", Colors.Dim("→"), Colors.Dim(hint)) } } -// printVerboseError prints detailed error for debugging (--verbose mode) -// Includes additional help and original error chain for troubleshooting func printVerboseError(code, message, hint, additionalHelp, originalError string) { - fmt.Printf("%s %s\n", Colors.ErrorCode(" %s ", code), Colors.Red(message)) + fmt.Fprintf(os.Stderr, "%s %s\n", Colors.ErrorCode(" %s ", code), Colors.Red(message)) if hint != "" && hint != "No additional help is available for this error." { - fmt.Printf(" %s %s\n", Colors.Dim("→"), Colors.Dim(hint)) + fmt.Fprintf(os.Stderr, " %s %s\n", Colors.Dim("→"), Colors.Dim(hint)) } if additionalHelp != "" && additionalHelp != "No additional help is available for this error." { - fmt.Printf(" %s %s\n", Colors.Dim("→"), Colors.Dim(additionalHelp)) + fmt.Fprintf(os.Stderr, " %s %s\n", Colors.Dim("→"), Colors.Dim(additionalHelp)) } if originalError != "" && originalError != message { - fmt.Printf(" %s %s\n", Colors.Dim("┄"), Colors.Dim(originalError)) + fmt.Fprintf(os.Stderr, " %s %s\n", Colors.Dim("┄"), Colors.Dim(originalError)) } } diff --git a/internal/ui/exit.go b/internal/ui/exit.go new file mode 100644 index 0000000..785cbd2 --- /dev/null +++ b/internal/ui/exit.go @@ -0,0 +1,58 @@ +package ui + +import ( + "errors" + "fmt" + "os" +) + +// transparentExit is satisfied by *runner.ChildExitError without importing it. +type transparentExit interface { + error + Transparent() bool + ExitCode() int + IsSignaled() bool +} + +type exitDecision struct { + transparent bool + notice bool + code int + message string +} + +// classifyExit is the pure decision behind ExitFromCommandError. The notice is +// suppressed for signal exits (the user initiated the interrupt) and in silent +// mode. +func classifyExit(err error) exitDecision { + var te transparentExit + if !errors.As(err, &te) || !te.Transparent() { + return exitDecision{} + } + + d := exitDecision{transparent: true, code: te.ExitCode()} + if !te.IsSignaled() && verbosityLevel != VerbosityLevelSilent { + d.notice = true + d.message = "↳ pmg: " + te.Error() + } + return d +} + +// ExitFromCommandError is the single exit point for package-manager commands. A +// child that exited on its own is mirrored transparently; everything else keeps +// the visible PMG error framing. +func ExitFromCommandError(err error) { + if err == nil { + return + } + + if d := classifyExit(err); d.transparent { + ClearStatus() + if d.notice { + fmt.Fprintln(os.Stderr, Colors.Dim(d.message)) + } + os.Exit(d.code) + } + + ErrorExit(err) +} diff --git a/internal/ui/exit_test.go b/internal/ui/exit_test.go new file mode 100644 index 0000000..490a29e --- /dev/null +++ b/internal/ui/exit_test.go @@ -0,0 +1,81 @@ +package ui + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// fakeChildExit satisfies the transparentExit interface, standing in for +// *runner.ChildExitError without a cross-package import. +type fakeChildExit struct { + code int + signaled bool + pmName string +} + +func (e *fakeChildExit) Error() string { return fmt.Sprintf("%s exited with code %d", e.pmName, e.code) } +func (e *fakeChildExit) ExitCode() int { return e.code } +func (e *fakeChildExit) Transparent() bool { return true } +func (e *fakeChildExit) IsSignaled() bool { return e.signaled } + +func withVerbosity(t *testing.T, level VerbosityLevel) { + t.Helper() + prev := verbosityLevel + verbosityLevel = level + t.Cleanup(func() { verbosityLevel = prev }) +} + +func TestClassifyExit(t *testing.T) { + t.Run("genuine non-zero child exit prints a dim notice and mirrors the code", func(t *testing.T) { + withVerbosity(t, VerbosityLevelNormal) + + d := classifyExit(&fakeChildExit{code: 1, pmName: "npm"}) + + assert.True(t, d.transparent) + assert.Equal(t, 1, d.code) + assert.True(t, d.notice) + assert.Equal(t, "↳ pmg: npm exited with code 1", d.message) + }) + + t.Run("transparent exit is detected even when wrapped", func(t *testing.T) { + withVerbosity(t, VerbosityLevelNormal) + + wrapped := fmt.Errorf("failed to execute command: %w", &fakeChildExit{code: 2, pmName: "pnpm"}) + d := classifyExit(wrapped) + + assert.True(t, d.transparent) + assert.Equal(t, 2, d.code) + assert.True(t, d.notice) + }) + + t.Run("signal termination is silent but still mirrors the code", func(t *testing.T) { + withVerbosity(t, VerbosityLevelNormal) + + d := classifyExit(&fakeChildExit{code: 130, signaled: true, pmName: "npm"}) + + assert.True(t, d.transparent) + assert.Equal(t, 130, d.code) + assert.False(t, d.notice) + assert.Empty(t, d.message) + }) + + t.Run("silent mode suppresses the notice", func(t *testing.T) { + withVerbosity(t, VerbosityLevelSilent) + + d := classifyExit(&fakeChildExit{code: 1, pmName: "npm"}) + + assert.True(t, d.transparent) + assert.Equal(t, 1, d.code) + assert.False(t, d.notice) + }) + + t.Run("non-transparent error falls through to the loud path", func(t *testing.T) { + d := classifyExit(errors.New("some PMG failure")) + + assert.False(t, d.transparent) + assert.False(t, d.notice) + }) +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go index 14ce50e..70c2496 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -184,7 +184,7 @@ func PromptSecret(label string) (string, error) { func Fatalf(msg string, args ...interface{}) { ClearStatus() - fmt.Println(Colors.Red(fmt.Sprintf(msg, args...))) + fmt.Fprintln(os.Stderr, Colors.Red(fmt.Sprintf(msg, args...))) os.Exit(1) } diff --git a/sandbox/executor/diagnostics.go b/sandbox/executor/diagnostics.go index 6d366b2..cc7af2b 100644 --- a/sandbox/executor/diagnostics.go +++ b/sandbox/executor/diagnostics.go @@ -1,54 +1,22 @@ package executor import ( - "fmt" - "github.com/safedep/dry/log" - "github.com/safedep/dry/usefulerror" "github.com/safedep/pmg/config" - "github.com/safedep/pmg/errcodes" "github.com/safedep/pmg/sandbox" ) -// WrapCommandExecutionError converts a package manager execution error into a -// user-facing error. It never attributes the failure to the sandbox: causation -// cannot be inferred from EPERM/EACCES returns alone, and a security tool -// should not make best-effort claims. Any observed sandbox denials are -// persisted to the violation cache for forensic review via -// `pmg sandbox violations list` and `pmg sandbox explain`. -func WrapCommandExecutionError(err error, result *sandbox.ExecutionResult, exitCode int) error { - if err == nil { - return nil - } - - observed := observeAndPersistViolations(result, err) - - humanError := "Failed to execute package manager command" - if exitCode >= 0 { - humanError = fmt.Sprintf("Package manager command exited with code: %d", exitCode) - } - - help := "Check the package manager command and its arguments" - builder := usefulerror.NewUsefulError(). - WithCode(errcodes.PackageManagerExecutionFailed). - WithHumanError(humanError). - WithHelp(help) - - if observed > 0 { - builder = builder.WithAdditionalHelp(fmt.Sprintf( - "Sandbox observed %d denied operation(s) during this run. Run `pmg sandbox violations list` to investigate.", - observed, - )) - } - - return builder.Wrap(err) -} - -// observeAndPersistViolations collects any sandbox violation report associated -// with the run and writes it to the violation cache. Returns the number of -// violations observed. Failures are logged and swallowed; observability MUST +// ObserveViolations collects any sandbox violation report associated with the +// run, persists it to the violation cache for forensic review (via +// `pmg sandbox violations list` / `pmg sandbox explain`), and returns the number +// of violations observed. Failures are logged and swallowed; observability MUST // NOT affect command exit. -func observeAndPersistViolations(result *sandbox.ExecutionResult, runErr error) int { +// +// This is the sandbox package's only stake in command-failure handling. It +// deliberately does not classify or shape the failure: causation cannot be +// inferred from EPERM/EACCES returns alone, so attribution is left to the +// execution layer (see internal/runner classify). +func ObserveViolations(result *sandbox.ExecutionResult, runErr error) int { if result == nil { return 0 } diff --git a/sandbox/executor/diagnostics_test.go b/sandbox/executor/diagnostics_test.go index b515edf..a51dc91 100644 --- a/sandbox/executor/diagnostics_test.go +++ b/sandbox/executor/diagnostics_test.go @@ -6,11 +6,8 @@ import ( "os/exec" "testing" - "github.com/safedep/dry/usefulerror" - "github.com/safedep/pmg/errcodes" "github.com/safedep/pmg/sandbox" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) type fakeViolationSandbox struct { @@ -37,10 +34,7 @@ func (f *fakeViolationSandbox) BestEffortViolation(error) (*sandbox.ViolationRep return f.report, nil } -// WrapCommandExecutionError must never claim the sandbox blocked a command. -// Even when violations were observed, the user-facing error stays the package -// manager's native exit; a neutral breadcrumb points at the forensic command. -func TestWrapCommandExecutionErrorDoesNotAttributeFailureToSandbox(t *testing.T) { +func TestObserveViolationsCountsObservedViolations(t *testing.T) { result := sandbox.NewExecutionResult(sandbox.WithExecutionResultSandbox(&fakeViolationSandbox{ report: &sandbox.ViolationReport{ SandboxName: sandbox.DriverSeatbelt, @@ -58,29 +52,17 @@ func TestWrapCommandExecutionErrorDoesNotAttributeFailureToSandbox(t *testing.T) }, })) - err := WrapCommandExecutionError(errors.New("npm failed"), result, 1) - - usefulErr, ok := usefulerror.AsUsefulError(err) - require.True(t, ok) - assert.Equal(t, errcodes.PackageManagerExecutionFailed, usefulErr.Code()) - assert.Equal(t, "Package manager command exited with code: 1", usefulErr.HumanError()) - assert.NotContains(t, usefulErr.Help(), "./.env") - assert.Contains(t, usefulErr.AdditionalHelp(), "pmg sandbox violations list") + assert.Equal(t, 1, ObserveViolations(result, errors.New("npm failed"))) } -func TestWrapCommandExecutionErrorOmitsBreadcrumbWhenNoViolations(t *testing.T) { +func TestObserveViolationsReturnsZeroWhenNoReport(t *testing.T) { result := sandbox.NewExecutionResult(sandbox.WithExecutionResultSandbox(&fakeViolationSandbox{ report: nil, })) - err := WrapCommandExecutionError(errors.New("npm failed"), result, 1) - - usefulErr, ok := usefulerror.AsUsefulError(err) - require.True(t, ok) - assert.Equal(t, errcodes.PackageManagerExecutionFailed, usefulErr.Code()) - assert.NotContains(t, usefulErr.AdditionalHelp(), "pmg sandbox violations list") + assert.Equal(t, 0, ObserveViolations(result, errors.New("npm failed"))) } -func TestWrapCommandExecutionErrorReturnsNilOnNilError(t *testing.T) { - assert.NoError(t, WrapCommandExecutionError(nil, nil, 0)) +func TestObserveViolationsReturnsZeroOnNilResult(t *testing.T) { + assert.Equal(t, 0, ObserveViolations(nil, errors.New("npm failed"))) }