Skip to content

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

Merged
egibs merged 1 commit into
chainguard-dev:mainfrom
egibs:fix/diff-report-empty-entries
Oct 27, 2026
Merged

egibs merged 1 commit into
chainguard-dev:mainfrom
egibs:fix/diff-report-empty-entries

Conversation

@egibs

@egibs egibs commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Summary

This started with a crash. mal diff --report dereferenced nil when a report held an empty entry or behavior, and the CLI's deferred os.Exit in main hid it: the process exited 0 with no output. Fixing that led to an audit of the report loader, diff, renderers, CLI, scan pipeline, archive extraction, OCI handling, and detection engine. This PR fixes what the audit found. A test that failed before its fix confirms each finding.

Behavior changes

CLI

  • A panic now prints its trace and exits 2. Before, the deferred os.Exit turned it into exit 0.
  • --exit-first-hit and --exit-first-miss exit 1 and still render the report. Before, they exited 0, and JSON and YAML wrote nothing.
  • A missing scan path, or no path, exits 22; an unreadable one exits 5. A dangling symlink, as a -compat package holds, is still skipped.
  • Unknown commands, bad flags, and --sensitivity outside 1 to 5 exit 22. Before, an unknown command made urfave/cli exit 3, the profiler-error code, after -o was truncated.
  • --format interactive with -o is rejected; --stats writes to -o; a second Ctrl-C ends the process at once.

Detection

  • Scan mode no longer hides HIGH findings when a file's top match is overridden, ignored by rule, or ignored by tag. Overrides apply per rule before behaviors dedupe, so an override no longer hides sibling rules.
  • --ignore-self skips only the running binary, matched by SHA-256. Before, any file named mal with three package paths in it was skipped.
  • --quantity-increases-risk counts reported HIGH behaviors, not raw matches.
  • Evidence keeps escaped bytes (\x7fELF) instead of pattern IDs, capped at 1,000 strings per behavior; MatchStringsOmitted counts the rest.

Scanning

  • One file that vanishes or cannot be read no longer aborts the scan.
  • .git is scanned except .git/objects, so hooks are no longer invisible.
  • --processes reads /proc/PID/exe, so deleted binaries are scanned and an argv[0] decoy is not. Interpreter scripts are scanned too.
  • A FIFO named like an archive no longer blocks the scan forever. Pseudo filesystems are skipped on Linux.
  • Scan paths share one queue, the walk streams into it, and --max-files bounds walked files.

Archives

  • The format comes from content, not the file name: an .apk, .vsix, or zip with any name gets extracted.
  • Debian control members, maintainer scripts included, extract to control/.
  • A bad entry is skipped instead of aborting every later one.
  • Duplicate and case-colliding names get sibling names; zip symlink entries become files holding their bytes.
  • Byte caps cover the whole nested tree, and decoder windows are capped at 128 MiB.
  • zstd, bzip2, LZMA, and xz zip methods decode, as do bzip2 and lzma RPM payloads.
  • Comments and other framing bytes go to a malcontent-metadata.bin sidecar.

OCI images

  • Layers apply as containerd applies them: a whiteout hides only lower layers, and a hardlink keeps its bytes when an upper layer replaces its target. ko's absolute entry names no longer fail the image.
  • Every platform of an index is scanned by default, each in its own directory; --platform narrows the set.
  • Entrypoint, Cmd, and Env are scanned as malcontent-image-config.sh.
  • Credentials refuse plain HTTP unless --oci-allow-insecure-auth is set, and registry hosts are normalized.
  • --oci-pull-timeout-seconds is a response-header timeout. The new --oci-pull-deadline-seconds, default 3600, bounds the whole pull. --oci-retry-max-attempts is the real total.

Diff

  • --file-risk-change and --file-risk-increase work at the default sensitivity.
  • A directory diffed against an archive compares every file, not the first.
  • A report diff fails on ambiguous keys instead of pairing at random.
  • Trees from different roots pair when they share no key.

Rendering

  • Report and file text is escaped in every terminal renderer and in Markdown. JSON writes DEL and C1 as \u escapes.
  • The interactive viewer no longer hides results that contain "skipped", and its search no longer panics.
  • JSON and YAML keep files whose keys collide after sanitizing.
  • Diff namespace lines no longer show risk changes computed from partial data.

Report loading

  • report.Load rejects damaged reports: empty entries or behaviors, non-finite floats, unknown risk scores, and diff reports.
  • YAML loads in linear time; 80,000 entries took 11 s before.

API changes

  • DiffReport fields are *malcontent.OrderedFileReports, replacing wk8/go-ordered-map.
  • report.Generate and report.GenerateRules drop their unused logger and risk parameters.
  • Removed report.ExtractImageURI and archive.GzMIME, which had no callers left.
  • Added Behavior.MatchStringsOmitted, file.InspectContents, file.MapContents, file.Inode, archive.ErrUnsupportedArchive, programkind.FileType.Archive, and the OCIPlatforms, OCIAllowInsecureAuth, and OCIPullDeadlineSeconds config fields.
  • ScanResult's YAML tag is files, matching the renderer (fix: load YAML reports in diff --report #1769 review).

Dependencies

  • crypto/sha256 replaces minio/sha256-simd, whose last commit was 2023-02-22. The stdlib takes the same SHA-NI and ARMv8 SHA2 paths and was equal or faster in a benchmark on Apple silicon: 913 to 1077 MB/s against 786 to 802 at 64 bytes, and level at 4 KiB and 1 MiB.
  • go.yaml.in/yaml/v3 v3.0.5 replaces gopkg.in/yaml.v3, archived 2025-04-01.
  • OrderedFileReports replaces wk8/go-ordered-map/v2, which has not merged a change since 2025-04-01 and ties YAML encoding to gopkg.in/yaml.v3. JSON and YAML output is unchanged byte for byte.
  • go.mod loses ten modules in all.
  • Kept cosnicolaou/pbzip2 and klauspost/compress. On 32 MiB corpora, pbzip2 decoded about 10 times faster than compress/bzip2, and klauspost gzip 1.5 to 1.8 times faster than compress/gzip. The three panic calls in pbzip2 v1.0.6 guard invariants no input reaches; pkg/archive/decoders.go cites the checks.

Rules

  • Three false-positive overrides now also cover a HIGH sibling rule that the old order hid: trino_upx_override lowers upx_elf, linux_s390x and linux_sparc64 lower possible_dropper, and security_solution_plugin lowers http_url_with_powershell. Applying overrides per rule exposed these siblings on known-clean samples.
  • fchmod is medium. Its tag was the typo notamble, meant as notable.
  • net/ip/ip-spoof.yara is now spoof-tool.yara, and net/proxy/tunnel.yara is now tunnel-create.yara. Their behavior IDs collided with sibling files, so dedup dropped one of each pair.
  • MIME filetypes in four rules and one override became extensions, so those rules fire again; a corpus test rejects unknown values.
  • 75 stale JPCERT copies are gone; 28 were older than their top-level versions. update.sh now clears each feed before copying.
  • yara-x warnings are logged at debug level. A test fails on new unsatisfiable_expr or unintended_pattern_in_set warnings; 35 current third-party rules are allowlisted for triage.

Test data

  • The tests/ goldens were regenerated with mal refresh in cgr.dev/chainguard/wolfi-base (amd64, Go 1.27.3). Wolfi ships yara-x 1.22.1, and the Makefile pins 1.22.0.
  • The pkg/action/testdata goldens were regenerated with a new -update flag.

Known limits

  • Diffing a single-arch image against a multi-arch one shows every file as replaced, since each platform extracts to its own directory.
  • Versioned directories, such as python3.11/ and python3.12/, pair only when the two trees share no other key.

Verification

  • make verify: go vet, gosec, gofumpt, golangci-lint v2.15.0, yara-x fmt, and go test -race ./pkg/... ./internal/.... Run on macOS with GOFLAGS=-p=1.
  • go fix -omitzero=false -diff ./... and the modernize analyzer: no changes.
  • The go-standards semgrep rules over the 168 changed Go files leave one finding, a map[string]bool whose false value means "listed for removal".
  • The Linux-only paths (/proc, cgroups, pseudo filesystems) were tested in cgr.dev/chainguard/go.

@egibs
egibs requested a review from mattlorimor October 27, 2026 19:28
Comment thread pkg/archive/oci_pull_test.go Dismissed
mal diff --report dereferenced nil on an empty report entry or behavior,
and the deferred os.Exit in main hid the panic behind exit 0. Fixing it
led to an audit of the report loader, diff, renderers, CLI, scan
pipeline, archive extraction, OCI handling, and detection engine.

- CLI: panics exit 2 with a trace; matched exit conditions exit 1 and
  render the report; missing scan paths and usage errors exit 22.
- Detection: scan mode applies its threshold after overrides and
  ignores; overrides apply per rule before dedup; --ignore-self matches
  the running binary by SHA-256; evidence keeps escaped bytes.
- Scanning: per-file errors no longer abort a scan; .git is scanned
  except objects; --processes reads /proc/PID/exe; FIFOs no longer
  block the scan.
- Archives: format comes from content; deb control members extract;
  bad entries are skipped; caps cover the nested tree.
- OCI: layers apply as containerd applies them; every platform of an
  index is scanned; the image config is scanned; credentials refuse
  plain HTTP.
- Diff and rendering: risk filters work at the default sensitivity;
  report text is escaped in every renderer; report.Load rejects damaged
  reports and loads YAML in linear time.
- Dependencies: crypto/sha256 replaces minio/sha256-simd,
  go.yaml.in/yaml/v3 replaces gopkg.in/yaml.v3, and OrderedFileReports
  replaces wk8/go-ordered-map.
@egibs
egibs force-pushed the fix/diff-report-empty-entries branch from fec3bbf to 4eb03e8 Compare October 27, 2026 19:43
@egibs
egibs enabled auto-merge (squash) October 27, 2026 19:44
@egibs
egibs merged commit 797caf2 into chainguard-dev:main Oct 27, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants