Skip to content

fix: harden scanning, extraction, diffing, and rendering #1779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
egibs merged 1 commit into from
Oct 27, 2026
Merged

fix: harden scanning, extraction, diffing, and rendering #1779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
16 changes: 12 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ malcontent is at its best analyzing programs that run on Linux. Still, it also p
GLOBAL OPTIONS:
--all Ignore nothing within a provided scan path
--exit-extraction Exit when encountering file extraction errors
--exit-first-miss Exit with error if scan source has no matching capabilities
--exit-first-hit Exit with error if scan source has matching capabilities
--exit-first-miss Exit with status 1 if scan source has no matching capabilities
--exit-first-hit Exit with status 1 if scan source has matching capabilities
--format string Output format (interactive, json, markdown, simple, strings, terminal, yaml) (default: "auto")
--ignore-rules string YARA rule names to ignore (comma-separated; supports filepath.Match globs, e.g. "py_lib_alias_val,py_lib_*")
--ignore-self Ignore the malcontent binary
Expand All @@ -72,8 +72,11 @@ GLOBAL OPTIONS:
--min-file-risk string Only show results for files which meet the given risk level (any, low, medium, high, critical) (default: "low")
--min-level int Obsoleted by --min-risk (default: -1)
--min-risk string Only show results which meet the given risk level (any, low, medium, high, critical) (default: "low")
--oci-allow-insecure-auth Allow --oci-auth credentials to be sent to a registry over plain HTTP
--oci-auth Authenticate OCI pulls with MALCONTENT_REGISTRY_USER/PASS, scoped to the registry in MALCONTENT_REGISTRY_HOST
--oci-pull-deadline-seconds int Maximum seconds for a whole OCI image pull and extraction (0 for no deadline) (default: 3600)
--output string, -o string Write output to specified file instead of stdout
--platform string OCI image index platforms to scan as comma-separated os/arch[/variant] (default: all)
--profile, -p Generate profile and trace files
--quantity-increases-risk Increase file risk score based on behavior quantity
--stats, -s Show scan statistics
Expand All @@ -84,8 +87,13 @@ GLOBAL OPTIONS:
```

> Malcontent defaults to anonymous OCI pulls. Authentication is opt-in via `--oci-auth`, which reads Basic credentials from the `MALCONTENT_REGISTRY_USER` and `MALCONTENT_REGISTRY_PASS` environment variables. The ambient Docker keychain (`~/.docker/config.json`) is intentionally never used.
> Credentials are scoped to a single registry: they are sent only when the image's registry host matches `MALCONTENT_REGISTRY_HOST` exactly, and malcontent falls back to anonymous pulls for any other registry, so private credentials are never presented to an unexpected (potentially malicious) registry.
> Registry hosts are normalized by `google/go-containerregistry`, so set `MALCONTENT_REGISTRY_HOST=index.docker.io` (not `docker.io`) when authenticating to Docker Hub.
> Credentials are scoped to a single registry: they are sent only when the image's registry host matches `MALCONTENT_REGISTRY_HOST`, and malcontent falls back to anonymous pulls (with a warning) for any other registry, so private credentials are never presented to an unexpected (potentially malicious) registry.
> Hosts are compared without regard to case or a `:443` suffix, and `docker.io`, `index.docker.io`, and `registry-1.docker.io` all name Docker Hub.
> `google/go-containerregistry` falls back to plain HTTP for `localhost` and private IP registries; malcontent refuses to send credentials over it unless `--oci-allow-insecure-auth` is set.

> An image's filesystem is extracted as a container runtime assembles it from the layers, so files that an upper layer deletes are not scanned.
> Every platform of a multi-platform image is scanned, each under a directory named for its platform such as `linux-arm64-v8`; `--platform` narrows the selection, and a single selected platform is scanned at the image root.
> The image config's `Env`, `Entrypoint`, and `Cmd` are scanned as a shell script named `malcontent-image-config.sh` at the image root (`malcontent-image-config.N.sh` if the image has a file of that name).

## Modes

Expand Down
236 changes: 223 additions & 13 deletions cmd/mal/cli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,22 @@ import (
// errRender is the failure fakeRenderer returns from Full when asked to.
var errRender = errors.New("render failed")

// fakeRenderer counts Full calls and returns fullErr from them.
// fakeRenderer counts Full calls, records the state of the context the last
// one was given, and returns fullErr from them.
type fakeRenderer struct {
name string
fullErr error
full int
name string
fullErr error
full int
fullCtxErr error
}

func (r *fakeRenderer) Scanning(context.Context, string) {}

func (r *fakeRenderer) File(context.Context, *malcontent.FileReport) error { return nil }

func (r *fakeRenderer) Full(context.Context, *malcontent.Config, *malcontent.Report) error {
func (r *fakeRenderer) Full(ctx context.Context, _ *malcontent.Config, _ *malcontent.Report) error {
r.full++
r.fullCtxErr = ctx.Err()
return r.fullErr
}

Expand Down Expand Up @@ -176,7 +179,8 @@ func TestExitCode(t *testing.T) {
{"input or output failure", ExitInputOutput, failure, ExitInputOutput},
{"profiler failure", ExitProfilerError, failure, ExitProfilerError},
{"invalid rules", ExitInvalidRules, failure, ExitInvalidRules},
{"matched exit condition", ExitActionFailed, matched, ExitOK},
{"matched exit condition", ExitActionFailed, matched, ExitMatchedCondition},
{"exit condition met during an interrupt", ExitActionFailed, errors.Join(matched, errInterrupted), ExitActionFailed},
}

for _, tt := range tests {
Expand All @@ -189,6 +193,86 @@ func TestExitCode(t *testing.T) {
}
}

func TestExecuteExitStatus(t *testing.T) {
// Not parallel: flag parsing writes package-level variables, os.Stderr is
// redirected, and cli.OsExiter is process-wide.
orig := cli.OsExiter
t.Cleanup(func() { cli.OsExiter = orig })
cli.OsExiter = func(code int) { t.Errorf("urfave/cli exited the process with status %d", code) }
target := t.TempDir()

tests := []struct {
name string
args []string
canceled bool
wantCode int
wantMsg string
}{
{"no command shows help", nil, false, ExitOK, ""},
{"help needs no setup", []string{"help", "scan"}, false, ExitOK, ""},
{"unknown command", []string{"scna", target}, false, ExitInvalidArgument, `unknown command "scna"`},
{"unknown global flag", []string{"--bogus", "scan", target}, false, ExitInvalidArgument, "flag provided but not defined: -bogus"},
{"unknown command flag", []string{"scan", "--bogus", target}, false, ExitInvalidArgument, "flag provided but not defined: -bogus"},
{"malformed flag value", []string{"--jobs", "many", "scan", target}, false, ExitInvalidArgument, "invalid value"},
{"help topic for no command", []string{"help", "scna"}, false, ExitInvalidArgument, `no help topic for "scna"`},
{"diff sensitivity below range", []string{"diff", "--sensitivity", "0", target, target}, false, ExitInvalidArgument, "--sensitivity must be 1 through 5, got 0"},
{"diff sensitivity above range", []string{"diff", "--sensitivity", "6", target, target}, false, ExitInvalidArgument, "--sensitivity must be 1 through 5, got 6"},
{"interrupted run that otherwise succeeds", []string{"--version"}, true, ExitActionFailed, "interrupted"},
{"interactive viewer cannot draw to an output file", []string{"--format", "interactive", "scan", target}, false, ExitInvalidArgument, "--format interactive cannot write to --output"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// None of these runs a command, so none may truncate the output.
dir := t.TempDir()
if err := file.WriteFileIn(dir, "report.json", []byte("previous report"), 0o600); err != nil {
t.Fatalf("write report: %v", err)
}
st := newTestState(t)
args := slices.Concat([]string{"mal", "--output", filepath.Join(dir, "report.json")}, tt.args)

var code int
stderr := malCaptureStderr(t, func() { code = st.execute(testContext(t, st, tt.canceled), args) })
if code != tt.wantCode {
t.Errorf("execute(%v) exit status: got = %d, want = %d", args, code, tt.wantCode)
}
switch {
case tt.wantMsg == "" && strings.Contains(stderr, "💣"):
t.Errorf("execute(%v) stderr: got = %q, want no error", args, stderr)
case !strings.Contains(stderr, tt.wantMsg):
t.Errorf("execute(%v) stderr: got = %q, want an error containing %q", args, stderr, tt.wantMsg)
}
if data, err := file.ReadFileIn(dir, "report.json"); err != nil || string(data) != "previous report" {
t.Errorf("output file after run: got = %q (%v), want = %q", data, err, "previous report")
}
})
}
}

func TestCheckSensitivity(t *testing.T) {
t.Parallel()
tests := []struct {
name string
sensitivity int
wantErr bool
}{
{"lowest", 1, false},
{"highest", 5, false},
{"zero", 0, true},
{"negative", -1, true},
{"above highest", 6, true},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
if err := checkSensitivity(tt.sensitivity); (err != nil) != tt.wantErr {
t.Errorf("checkSensitivity(%d): got error = %v, want error = %v", tt.sensitivity, err, tt.wantErr)
}
})
}
}

func TestResolveFormat(t *testing.T) {
t.Parallel()
tests := []struct {
Expand Down Expand Up @@ -551,6 +635,49 @@ func TestBeforeReportsProfilerFailure(t *testing.T) {
}
}

func TestAfterClosesOutput(t *testing.T) {
t.Parallel()
tests := []struct {
name string
output bool
closed bool // closing an already closed file fails, as a failed flush would
code int
wantErr bool
wantCode int
}{
{"closes the output", true, false, ExitOK, false, ExitOK},
{"failed close fails a successful run", true, true, ExitOK, true, ExitInputOutput},
{"failed close keeps an earlier failure code", true, true, ExitRenderFailed, true, ExitRenderFailed},
{"no output to close", false, false, ExitOK, false, ExitOK},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
st := newTestState(t)
st.returnCode = tt.code
out := st.outFile
if tt.closed {
_ = out.Close()
}
if !tt.output {
st.outFile = nil
}

err := st.after(t.Context(), nil)
if (err != nil) != tt.wantErr {
t.Errorf("after error: got = %v, want error = %v", err, tt.wantErr)
}
if st.returnCode != tt.wantCode {
t.Errorf("recorded code: got = %d, want = %d", st.returnCode, tt.wantCode)
}
if _, err := out.Write([]byte("x")); tt.output && !errors.Is(err, os.ErrClosed) {
t.Errorf("output after run: got write error = %v, want = %v", err, os.ErrClosed)
}
})
}
}

func TestAnalyzeRendersRequestedReport(t *testing.T) {
// Not parallel: flag parsing writes package-level variables. The Before
// stage compiles the full rule set (served from the user cache when warm),
Expand Down Expand Up @@ -602,6 +729,7 @@ func TestAnalyzeRendersRequestedReport(t *testing.T) {
func TestAnalyzeCommand(t *testing.T) {
t.Parallel()
script := writeScript(t, "hello.sh")
missing := filepath.Join(t.TempDir(), "missing")

tests := []struct {
name string
Expand All @@ -616,6 +744,8 @@ func TestAnalyzeCommand(t *testing.T) {
{"render failure", []string{"analyze", script}, false, errRender, ExitRenderFailed, errRender, 1},
{"scan failure", []string{"analyze", script}, true, nil, ExitActionFailed, context.Canceled, 0},
{"process lookup failure", []string{"analyze", "--processes"}, true, nil, ExitActionFailed, context.Canceled, 0},
{"no path given", []string{"analyze"}, false, nil, ExitInvalidArgument, errNoScanPath, 0},
{"missing scan path", []string{"analyze", missing}, false, nil, ExitInvalidArgument, fs.ErrNotExist, 0},
}

for _, tt := range tests {
Expand All @@ -641,6 +771,7 @@ func TestAnalyzeCommand(t *testing.T) {
func TestScanCommand(t *testing.T) {
t.Parallel()
script := writeScript(t, "hello.sh")
missing := filepath.Join(t.TempDir(), "missing")

tests := []struct {
name string
Expand All @@ -656,8 +787,10 @@ func TestScanCommand(t *testing.T) {
{"scan and render succeed", "Simple", []string{"scan", script}, false, nil, ExitOK, nil, "", 1},
{"render failure", "Simple", []string{"scan", script}, false, errRender, ExitRenderFailed, errRender, "", 1},
{"scan failure", "Simple", []string{"scan", script}, true, nil, ExitActionFailed, context.Canceled, "scan: ", 0},
{"interactive renderer shows a failed scan", "Interactive", []string{"scan", script}, true, nil, ExitOK, nil, "", 1},
{"interactive renderer shows a failed scan, then reports it", "Interactive", []string{"scan", script}, true, nil, ExitActionFailed, context.Canceled, "scan: ", 1},
{"process lookup failure", "Simple", []string{"scan", "--processes"}, true, nil, ExitActionFailed, context.Canceled, "process paths: ", 0},
{"no path given", "Simple", []string{"scan"}, false, nil, ExitInvalidArgument, errNoScanPath, "", 0},
{"missing scan path", "Simple", []string{"scan", missing}, false, nil, ExitInvalidArgument, fs.ErrNotExist, "", 0},
}

for _, tt := range tests {
Expand Down Expand Up @@ -686,6 +819,76 @@ func TestScanCommand(t *testing.T) {
}
}

func TestMetExitConditionRendersReport(t *testing.T) {
t.Parallel()
r := &fakeRenderer{name: "JSON"}
st := scanReadyState(t, r)
// The test rule never matches, so the script meets --exit-first-miss. The
// scan command skips files below high risk, so only analyze can meet it.
st.mc.ExitFirstMiss = true
script := writeScript(t, "hello.sh")

err := targetCommand("analyze", st.analyze).Run(testContext(t, st, false), []string{"analyze", script})
if !conditionMet(err) {
t.Fatalf("analyze error: got = %v, want a met exit condition", err)
}
if r.full != 1 {
t.Errorf("Full calls: got = %d, want = 1", r.full)
}
if got := exitCode(st.returnCode, err); got != ExitMatchedCondition {
t.Errorf("exit code: got = %d, want = %d", got, ExitMatchedCondition)
}
}

// stoppableRenderer is a fakeRenderer that counts Stop calls, as a renderer
// that can end its display without the reader does.
type stoppableRenderer struct {
fakeRenderer
stops int
}

func (r *stoppableRenderer) Stop() { r.stops++ }

func TestReleaseTerminal(t *testing.T) {
t.Parallel()
tests := []struct {
name string
renderer string
stoppable bool
wantFull int
wantStop int
}{
{"display that can stop is stopped without the reader", "Interactive", true, 0, 1},
{"other renderers are left alone", "Terminal", false, 0, 0},
{"no renderer before setup", "", false, 0, 0},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
r := &stoppableRenderer{name: tt.renderer}
st := newTestState(t)
switch {
case tt.stoppable:
st.renderer = r
case tt.renderer != "":
st.renderer = &r.fakeRenderer
}

st.releaseTerminal()
if r.full != tt.wantFull {
t.Errorf("Full calls: got = %d, want = %d", r.full, tt.wantFull)
}
if r.stops != tt.wantStop {
t.Errorf("Stop calls: got = %d, want = %d", r.stops, tt.wantStop)
}
if r.fullCtxErr != nil {
t.Errorf("Full context: got error = %v, want a live context", r.fullCtxErr)
}
})
}
}

func TestDiffCommand(t *testing.T) {
// Not parallel: the diff flags write package-level variables.
src := writeScript(t, "before.sh")
Expand All @@ -703,11 +906,12 @@ func TestDiffCommand(t *testing.T) {
oci bool
report bool
}{
{"default sensitivity applies no risk filter", []string{"diff", src}, nil, ExitActionFailed, "requires 2 paths", false, false, 5, false, false},
{"sensitivity one shows only risk changes", []string{"diff", "--sensitivity", "1", src}, nil, ExitActionFailed, "requires 2 paths", true, false, 1, false, false},
{"file risk change flag", []string{"diff", "--file-risk-change", src}, nil, ExitActionFailed, "requires 2 paths", true, false, 5, false, false},
{"file risk increase flag", []string{"diff", "--file-risk-increase", src}, nil, ExitActionFailed, "requires 2 paths", false, true, 5, false, false},
{"image and report flags", []string{"diff", "--image", "--report", src}, nil, ExitActionFailed, "requires 2 paths", false, false, 5, true, true},
{"default sensitivity applies no risk filter", []string{"diff", src}, nil, ExitInvalidArgument, "requires 2 paths", false, false, 5, false, false},
{"sensitivity one shows only risk changes", []string{"diff", "--sensitivity", "1", src}, nil, ExitInvalidArgument, "requires 2 paths", true, false, 1, false, false},
{"file risk change flag", []string{"diff", "--file-risk-change", src}, nil, ExitInvalidArgument, "requires 2 paths", true, false, 5, false, false},
{"file risk increase flag", []string{"diff", "--file-risk-increase", src}, nil, ExitInvalidArgument, "requires 2 paths", false, true, 5, false, false},
{"image and report flags", []string{"diff", "--image", "--report", src}, nil, ExitInvalidArgument, "requires 2 paths", false, false, 5, true, true},
{"three paths", []string{"diff", src, dest, src}, nil, ExitInvalidArgument, "requires 2 paths", false, false, 5, false, false},
{"render failure", []string{"diff", src, dest}, errRender, ExitRenderFailed, errRender.Error(), false, false, 5, false, false},
{"diff and render succeed", []string{"diff", src, dest}, nil, ExitOK, "", false, false, 5, false, false},
}
Expand Down Expand Up @@ -770,11 +974,16 @@ func TestAwaitShutdown(t *testing.T) {
exited := make(chan int, 1)
logger := clog.New(slog.NewTextHandler(io.Discard, nil))

awaitShutdown(sigCh, func() { canceled = true }, logger, time.Millisecond, func(code int) { exited <- code })
var stopped []chan<- os.Signal
awaitShutdown(sigCh, func() { canceled = true }, logger, time.Millisecond, func(code int) { exited <- code }, func(c chan<- os.Signal) { stopped = append(stopped, c) })

if !canceled {
t.Error("cancel after signal: got = not called, want = called")
}
// Stopping the relay leaves a second signal to its default action.
if len(stopped) != 1 || stopped[0] != chan<- os.Signal(sigCh) {
t.Errorf("signal relay stops: got = %v, want = one stop of the signal channel", stopped)
}
if code := <-exited; code != 1 {
t.Errorf("forced exit code: got = %d, want = 1", code)
}
Expand Down Expand Up @@ -816,6 +1025,7 @@ func TestShowError(t *testing.T) {
}{
{"failure", errors.New("scan failed"), "💣 scan failed\n"},
{"matched exit condition", fmt.Errorf("/tmp/x %w", action.ErrMatchedCondition), "👋 /tmp/x matched exit criteria\n"},
{"exit condition met during an interrupt", errors.Join(fmt.Errorf("/tmp/x %w", action.ErrMatchedCondition), errInterrupted), "💣 /tmp/x matched exit criteria\ninterrupted\n"},
}

for _, tt := range tests {
Expand Down
Loading
Loading