fix(scanners): correct execution-failure signaling and OSV container entrypoint (#125)

* fix(scanners): correct execution-failure signaling and OSV container entrypoint

Five related correctness fixes around scanner execution and status
reporting. Each is a small, targeted change with a regression test;
the linux-default behavior is unchanged on every path that wasn't
broken.

* OSV container exit 127: ``container_entrypoint`` is now the absolute
  ``/osv-scanner`` path. Docker's ``--entrypoint`` override does not
  consult the image's ``$PATH``, and ``ghcr.io/google/osv-scanner``
  declares ``ENTRYPOINT ["/osv-scanner"]``. The bare name resolved
  nowhere.

* lint-yaml false PASS: yamllint exit >= 2 with empty stdout used to
  flow through ``_parse_output('') -> []`` and render ``Status: PASS``,
  hiding bad-config failures. ``YamllintLinter.scan`` now branches on
  the exit code: 0 = clean, 1 = findings, >= 2 with empty findings =
  ``execution_failed`` + reason from stderr.

* Unified failure metadata contract: ``run_subprocess_scan`` now emits
  ``metadata['execution_failed']`` + ``metadata['execution_failure_reason']``
  on every failure path (FileNotFoundError, TimeoutExpired, no-output).
  Previously it used ``metadata['error']`` while the engine's container
  path used ``execution_failed``; the divergence meant local-path
  failures were invisible to the reporter's warning row and to
  ``--fail-on-scanner-error``.

* Reporter degraded status: ``TerminalReporter`` now prints
  ``Status: PASS (degraded - some scanners did not run)`` when the
  threshold check passes but at least one scanner has
  ``execution_failed=True``. Threshold compliance and execution
  success are independent signals; the previous output had a
  ``Warning: ...`` row directly followed by ``Status: PASS`` which
  contradicted itself. ``ScanSummary.passed`` semantics are unchanged.

* Windows chmod skip: ``os.chmod(output_dir, 0o777)`` in
  ``_run_in_container`` is now guarded by ``platform.system() !=
  'Windows'``. NTFS doesn't honor POSIX bits, ``os.chmod`` only
  flips the read-only attribute on Windows, and the macOS uid-mismatch
  failure mode the chmod guards against doesn't apply on Windows
  (Docker Desktop's bind-mount uid mapping is different). Linux/macOS
  keep the chmod - covered by a new test that asserts mode 0o777
  on non-Windows hosts.

Tests:
* New: ``test_yamllint.py`` covers the full exit-code matrix
  (0 / 1 / 1+findings / 2+empty / 99+empty)
* New: OSV ``test_container_entrypoint_uses_absolute_path`` regression
* New: terminal ``test_report_status_pass_degraded_when_any_scanner_failed``,
  ``test_report_status_pass_clean_when_no_failures``,
  ``test_report_status_fail_takes_priority_over_degraded``
* New: engine ``test_chmod_skipped_on_windows`` and
  ``test_chmod_runs_on_non_windows`` - explicit Linux-safety guard
* Updated: ``test_scanner_template`` and ``test_scanner_scan_methods``
  switched from the old ``metadata['error']`` key to
  ``metadata['execution_failed']`` to match the unified contract

Full SDK suite: 1640 passed, 8 skipped.

.ai/errors.yaml: added three new entries (exit-127 entrypoint,
yamllint silent-failure, and reporter degraded-status) so future
agents recognize the patterns.

* fix(scanners): surface parse failures distinctly and use per-scanner failure reasons

Follow-up correctness pass on top of the previous patch. The first
round addressed the silent-failure paths; this round addresses how
those signals are presented and adds a fourth state the user asked
for explicitly.

Adapter exit-code semantics (locked in by tests):
* OSV-scanner: 0 = no vulns, 1 = vulns found (happy path), >= 2 with
  empty output = real runtime failure. Engine parses results.json
  whenever the file exists; exit code is irrelevant.
* yamllint: 0 = clean, 1 = lint violations (happy path), >= 2 with
  empty stdout = real runtime/config failure.

Changes:

* New 'parse_failed' state. The user asked for four distinct
  states: ran cleanly / ran-with-findings / didn't run /
  ran-but-output-unparsable. The fourth state was previously a
  raised exception that propagated up to the engine's exception
  handler and got rolled up as a generic "scanner failed". Now:
  - engine._run_in_container wraps scanner.parse_results() in
    try/except. On exception: metadata['parse_failed']=True and
    metadata['parse_failure_reason']=<exc + clipped output head>.
    Other scanners' results keep flowing.
  - scanner_template.run_subprocess_scan does the same for the
    local-execution path.
  - parse_failed is orthogonal to execution_failed; reporters render
    them in separate warning blocks.

* Reporter no longer emits generic guesses. The previous "uid
  mismatch / crashed / wrong entrypoint" boilerplate was misleading
  for OSV exit-1-with-findings and yamllint binary-not-found cases.
  TerminalReporter now prints the per-scanner reason
  (execution_failure_reason or parse_failure_reason) verbatim, in
  bullet form, with the scanner name. Status label gets a richer
  breakdown: "PASS (degraded - 1 did not run, 1 unparsable)".

* yamllint hardened against FileNotFoundError. is_available() is
  checked before scan(), but the binary can disappear in the gap
  (CI cleanup race, manual uninstall). scan() now catches
  FileNotFoundError and returns a clean execution_failed ScanResult
  instead of letting the exception escape into the engine's
  exception handler (which would render a stack trace).

* --fail-on-scanner-error fires on parse_failed too. Both states
  represent "the scan didn't fully succeed"; from a CI gating
  perspective they're equivalent.

Cross-platform safety:
* No platform-specific branches added or modified. Linux scanner
  flows are unchanged - the new parse-failed wrapping fires only
  when the parser raises (a previously-broken path everywhere).
* Adapter-specific exit-code handling stays at the adapter layer
  (yamllint owns its >= 2 rule, OSV owns its "exit code irrelevant
  to parsing" rule). No global "non-zero means failure" logic was
  added.
* Existing output formats and result schemas stable: parse_failed
  is additive metadata; ScanSummary.passed semantics unchanged
  (still threshold-only).

Tests added (8 new):
* OSV: parse_results succeeds-with-findings on real fixture
  (exit-1 acceptance case)
* OSV: parse_results returns [] on zero-findings fixture without
  raising
* OSV: parse_results raises on malformed JSON (so the engine
  wrapper catches as parse_failed)
* yamllint: FileNotFoundError returns execution_failed, no
  exception escapes
* engine: parse exception in container path -> parse_failed
  (not execution_failed, not raised)
* terminal: warning block shows per-scanner reason, no generic
  text
* terminal: parse_failed renders distinctly from execution_failed
* terminal: degraded status label lists both kinds when both present

Tests updated (1):
* test_scanner_template's test_unexpected_exception_propagates
  renamed/rewritten to test_parse_exception_emits_parse_failed_metadata
  reflecting the new contract.

Full SDK suite: 1648 passed, 8 skipped.

.ai/errors.yaml: degraded-status entry refreshed with new label
shape; new entry added for the parse_failed pattern.

* fix(windows): yamllint AppLocker fallback and UTF-8 encoding for container output

Two Windows-specific correctness fixes. Linux behavior is unchanged.

Bug 1 — yamllint PermissionError on Windows (AppLocker / SRP):

On Windows hosts where Software Restriction Policy or AppLocker
blocks executable launches from user AppData paths, the direct
``yamllint.exe`` invocation raises ``PermissionError [WinError 5]
Access is denied`` mid-scan. The python interpreter itself is
typically whitelisted, so loading the same package via
``python -m yamllint`` works on the same machine and is
argv-compatible (yamllint's __main__ accepts the same flags).

Fix: ``YamllintLinter._run_with_windows_fallback`` wraps the
subprocess.run call. On ``sys.platform == 'win32'``, a
PermissionError/OSError on the direct invocation triggers a retry
with [sys.executable, '-m', 'yamllint'] + cmd[1:]. FileNotFoundError
is re-raised unchanged so 'yamllint not installed' still renders
as a clean execution_failed (the fallback wouldn't help — yamllint
isn't installed at all).

Linux is fully bypassed: AppLocker doesn't exist there, so a
PermissionError on Linux indicates a genuine permission bug
(chmod, mount options) the user needs to see, not a policy case
the fallback compensates for. The Linux invocation path is
byte-identical to before — same single subprocess.run call, same
PATH lookup, same exception handling.

Bug 2 — UnicodeDecodeError reading container output:

Docker container output (and most CLI scanner output) is UTF-8.
``subprocess.run(text=True)`` and ``Path.read_text()`` fall back
to the platform default when ``encoding=`` is omitted — cp1252
on Windows. Any non-ASCII byte (CVE descriptions with accented
characters, file paths with unicode segments, scanner banners
with arrow glyphs) raises UnicodeDecodeError mid-scan.

Fix: explicit ``encoding='utf-8', errors='replace'`` on every
container-output consumer:
* engine._run_in_container's docker subprocess.run
* scanner_template.run_subprocess_scan's subprocess.run
* yamllint's subprocess.run (both direct + Windows fallback)
* All 14 scanner.parse_results .read_text() calls (bandit,
  checkov, clamav, container, gitleaks, grype, opengrep, osv,
  supply_chain, trivy, trivy_iac, zap)
* Engine + scanner_template parse-failed file-head reads

errors='replace' over 'strict' is intentional: a security tool
that shows U+FFFD on undecodable bytes is better than one that
crashes on otherwise-usable scanner output. Linux unaffected
since LANG=C.UTF-8 is the universal default; this only changes
behavior on hosts whose default ISN'T UTF-8.

Tests added (5 new):
* yamllint: PermissionError on Windows triggers python -m
  fallback, second subprocess call uses sys.executable -m yamllint
* yamllint: PermissionError on Linux does NOT fall back, raised
  through to outer handler as execution_failed
* yamllint: subprocess.run gets encoding='utf-8' + errors='replace'
* engine: docker subprocess.run gets encoding='utf-8' + errors='replace'
* OSV: parse_results round-trips non-ASCII UTF-8 content (CVE
  summaries with accents, file paths with unicode) without
  raising — the user's reported decode crash on byte 0x8f at
  position 18203

Full SDK suite: 1653 passed, 8 skipped.

.ai/errors.yaml: two new entries — Windows AppLocker pattern
and the cp1252 UnicodeDecodeError pattern, both with the
exact platform-guard rationale.

---------

Co-authored-by: eFAILution <eFAILution@users.noreply.github.com>

Author: eFAILution

.ai/errors.yaml

@@ -374,3 +374,138 @@ scn_detector_errors:
           how: "Set enable_ai_fallback: true and provide ANTHROPIC_API_KEY"
         - description: "Review manually"
           how: "MANUAL_REVIEW is the expected safe default for unrecognized changes"
+
+  # ==============================================================================
+  # SCANNER EXECUTION FAILURES
+  # ==============================================================================
+
+  - pattern: "exit 127|exec.*not found"
+    category: scanner-execution
+    context: "Container scanner exits 127 immediately (e.g. osv-scanner)"
+
+    root_cause: |
+      Docker's --entrypoint override does NOT consult the image's $PATH
+      when given a bare binary name. Some images declare an absolute
+      entrypoint (e.g. ghcr.io/google/osv-scanner uses
+      ENTRYPOINT ["/osv-scanner"]) and require the absolute path on
+      the override. A bare ``osv-scanner`` resolves nowhere and exits
+      127.
+
+    solution:
+      steps:
+        - "Run ``docker image inspect <image>`` and read ``Config.Entrypoint``"
+        - "Set ``container_entrypoint`` on the scanner class to the absolute path from that field"
+      verification: "Engine strips argv[0] for ENTRYPOINT-based images, so build_args may keep the bare name"
+
+    reference: "argus/scanners/osv.py — see container_entrypoint = '/osv-scanner'"
+
+  - pattern: "yamllint.*PASS.*0 findings|lint-yaml.*passes despite errors"
+    category: scanner-execution
+    context: "Linter exits non-zero but reports zero findings and Status: PASS"
+
+    root_cause: |
+      Tools like yamllint use exit codes to differentiate happy-path
+      lint failures (exit 1, with parseable findings on stdout) from
+      runtime errors (exit ≥ 2, empty stdout). Naive callers that map
+      empty stdout to ``[]`` lose the runtime-error signal entirely
+      and surface ``Status: PASS`` even when the tool failed to run.
+
+    solution:
+      steps:
+        - "Distinguish exit codes: 0 = clean, 1 = findings, ≥2 = real failure"
+        - "When exit ≥ 2 with empty findings, set metadata['execution_failed']=True and metadata['execution_failure_reason']=<stderr summary>"
+        - "Use the same shape the engine container path emits — the terminal reporter and --fail-on-scanner-error key off these exact field names"
+
+    reference: "argus/linters/yamllint.py — scan() exit-code branching"
+
+  - pattern: "scanner produced no output|execution_failed"
+    category: scanner-execution
+    context: "Reporter shows Warning row but Status: PASS contradicts it"
+
+    root_cause: |
+      Threshold compliance (``ScanSummary.passed``) and execution
+      success are independent signals. A scanner that fails to run
+      produces zero findings, which alone passes any threshold. The
+      reporter must label PASS as ``PASS (degraded — some scanners
+      did not run)`` whenever any scanner has
+      metadata['execution_failed']=True, otherwise the Warning above
+      and the Status below contradict each other.
+
+    solution:
+      steps:
+        - "Reporter checks for any result.metadata.get('execution_failed')"
+        - "If passed and any failed: print 'Status: PASS (degraded — N did not run, M unparsable)'"
+        - "Add --fail-on-scanner-error in CI for hard-fail behavior"
+
+    reference: "argus/reporters/terminal.py::_print_status"
+
+  - pattern: "0 findings.*known to be vulnerable|JSONDecodeError.*results.json"
+    category: scanner-execution
+    context: "Scanner produced output but parser couldn't interpret it (third state)"
+
+    root_cause: |
+      Distinct from execution-failure: the scanner ran, exited, and
+      wrote a results file — we just couldn't parse what came out
+      (schema drift, truncated JSON, mixed text+JSON). Previously
+      this surfaced as a stack trace from the engine's exception
+      handler and got rolled up as a generic "scanner failed".
+      Reporters and CI gates now have a fourth state for it:
+      ``parse_failed`` + ``parse_failure_reason`` (carries the
+      exception type and a clipped output head).
+
+    solution:
+      steps:
+        - "Engine's container path wraps scanner.parse_results in try/except"
+        - "On any Exception, set metadata['parse_failed']=True and metadata['parse_failure_reason']"
+        - "scanner_template.run_subprocess_scan does the same for local execution"
+        - "TerminalReporter renders parse_failed in its own warning block"
+        - "--fail-on-scanner-error fires on parse_failed too"
+
+    reference: "argus/core/engine.py::_run_in_container — try/except around parse_results"
+
+  # ==============================================================================
+  # WINDOWS-SPECIFIC SCANNER ERRORS
+  # ==============================================================================
+
+  - pattern: "PermissionError.*WinError 5.*Access is denied|yamllint.*Access is denied"
+    category: scanner-execution
+    context: "yamllint launches with PermissionError on Windows hosts with AppLocker / SRP"
+
+    root_cause: |
+      AppLocker or Software Restriction Policy on Windows blocks
+      executable launches from user AppData paths (where pip --user
+      and virtualenv typically install scripts). The python
+      interpreter itself is whitelisted, so loading the same
+      package via ``python -m yamllint`` works on the same
+      machine.
+
+    solution:
+      steps:
+        - "YamllintLinter._run_with_windows_fallback wraps subprocess.run"
+        - "On sys.platform == 'win32', PermissionError/OSError triggers a retry with [sys.executable, '-m', 'yamllint'] + cmd[1:]"
+        - "FileNotFoundError still propagates so 'yamllint not installed' renders cleanly"
+        - "Linux/macOS bypass the fallback — Linux PermissionError is a genuine bug, not a policy case"
+
+    reference: "argus/linters/yamllint.py::_run_with_windows_fallback"
+
+  - pattern: "UnicodeDecodeError.*charmap codec.*can't decode byte"
+    category: scanner-execution
+    context: "Scanner result decode fails on Windows with cp1252 default encoding"
+
+    root_cause: |
+      Docker container output (and most CLI tool output) is UTF-8.
+      ``subprocess.run(text=True)`` and ``Path.read_text()`` fall
+      back to the platform default encoding when ``encoding=`` is
+      omitted — cp1252 on Windows. Any non-ASCII byte (CVE
+      descriptions, accented file paths, scanner banners) raises
+      UnicodeDecodeError mid-scan.
+
+    solution:
+      steps:
+        - "Engine docker subprocess: encoding='utf-8', errors='replace'"
+        - "scanner_template subprocess: same"
+        - "All scanner.parse_results read_text() calls: same"
+        - "Yamllint subprocess + Windows fallback: same"
+      note: "errors='replace' is preferred over 'strict' — a security tool showing � is better than crashing on otherwise-usable output"
+
+    reference: "argus/core/engine.py and every argus/scanners/*.py read_text()"

argus/cli.py

@@ -1693,20 +1693,37 @@ def _cmd_source_scan(args: argparse.Namespace) -> int:
         r.scanner for r in summary.results
         if r.metadata.get("execution_failed")
     ]
+    scanner_parse_failures = [
+        r.scanner for r in summary.results
+        if r.metadata.get("parse_failed")
+    ]
     if not summary.passed:
         exit_code = EXIT_FINDINGS
     elif sbom_batch_failures:
         exit_code = EXIT_ERROR
     elif (
         getattr(args, "fail_on_scanner_error", False)
-        and scanner_execution_failures
+        and (scanner_execution_failures or scanner_parse_failures)
     ):
-        log.error(
-            "Exiting non-zero: %d scanner(s) produced no output (%s) and "
-            "--fail-on-scanner-error is set.",
-            len(scanner_execution_failures),
-            ", ".join(scanner_execution_failures),
-        )
+        # Both states represent "the scan didn't fully succeed":
+        # execution_failed = couldn't run; parse_failed = ran but
+        # output unintelligible. From a CI gating perspective they're
+        # equivalent — the user asked for a hard fail when scanners
+        # don't deliver clean results.
+        if scanner_execution_failures:
+            log.error(
+                "Exiting non-zero: %d scanner(s) did not run cleanly "
+                "(%s) and --fail-on-scanner-error is set.",
+                len(scanner_execution_failures),
+                ", ".join(scanner_execution_failures),
+            )
+        if scanner_parse_failures:
+            log.error(
+                "Exiting non-zero: %d scanner(s) produced unparsable "
+                "output (%s) and --fail-on-scanner-error is set.",
+                len(scanner_parse_failures),
+                ", ".join(scanner_parse_failures),
+            )
         exit_code = EXIT_ERROR
     else:
         exit_code = EXIT_SUCCESS

argus/core/engine.py

@@ -2,6 +2,7 @@
 
 import logging
 import os
+import platform
 import shutil
 import subprocess
 import tempfile
@@ -674,7 +675,15 @@ def _run_in_container(
             #  - holds only one scan's transient output (no secrets;
             #    findings travel through ``parse_results`` and end up
             #    in the user-specified output_dir, never here).
-            os.chmod(output_dir, 0o777)
+            #
+            # Skip on Windows: NTFS doesn't honor POSIX bits, ``os.chmod``
+            # only flips the read-only attribute, and Docker Desktop on
+            # Windows handles uid mapping for bind mounts differently
+            # (it doesn't suffer from the macOS uid-mismatch failure mode
+            # this guard exists for). Calling ``chmod 0o777`` there is
+            # at best a no-op and at worst confusing in stack traces.
+            if platform.system() != "Windows":
+                os.chmod(output_dir, 0o777)
 
             docker_cmd = [
                 self._runtime, "run", "--rm",
@@ -738,10 +747,21 @@ def _run_in_container(
             )
 
             start = time.monotonic()
+            # Docker container output is always UTF-8. Without
+            # ``encoding='utf-8'``, ``text=True`` falls back to the
+            # platform default — cp1252 on Windows — which raises
+            # ``UnicodeDecodeError`` on any non-ASCII byte the
+            # scanner emits (CVE descriptions, file paths with
+            # non-ASCII characters, etc.). ``errors='replace'`` is
+            # a safe fallback over ``strict``: a security tool
+            # showing ``�`` is better than crashing the whole
+            # scan on output we'd otherwise be able to use.
             proc = subprocess.run(
                 docker_cmd,
                 capture_output=True,
                 text=True,
+                encoding="utf-8",
+                errors="replace",
             )
             elapsed = int((time.monotonic() - start) * 1000)
 
@@ -854,35 +874,65 @@ def _run_in_container(
                         f"no output files and no stdout (exit={proc.returncode})"
                     )
             if result_files and hasattr(scanner, "parse_results"):
-                parsed = scanner.parse_results(result_files[0])
-                # parse_results may return either a list of Findings,
-                # a ``(list, int)`` tuple (legacy passed_count channel,
-                # used by linters), or a ``(list, dict)`` tuple (extra
-                # metadata merged into ScanResult.metadata — used by
-                # Grype to flag "source.target=unknown" which means
-                # "couldn't identify packages" rather than "nothing
-                # vulnerable").
-                if isinstance(parsed, tuple):
-                    findings, extra = parsed
-                    if isinstance(extra, int):
-                        metadata_extra["passed_count"] = extra
-                    elif isinstance(extra, dict):
-                        metadata_extra.update(extra)
-                        # Warn at the engine layer too so the signal is
-                        # visible even when a reporter doesn't render
-                        # per-scanner metadata.
-                        if "warning" in extra:
-                            logger.warning(
-                                "Scanner '%s': %s",
-                                scanner.name, extra["warning"],
-                            )
+                try:
+                    parsed = scanner.parse_results(result_files[0])
+                except Exception as exc:
+                    # Scanner produced output but the parser couldn't
+                    # interpret it (e.g. osv-scanner v2 rev'd its
+                    # schema, truncated output, mixed text+JSON). This
+                    # is a third state distinct from "execution failed"
+                    # and "ran clean" — we surface it as
+                    # ``parse_failed`` so the reporter can show "OSV
+                    # produced 12KB of output we couldn't parse" rather
+                    # than the misleading "no output produced". The
+                    # parser bug doesn't crash the rest of the scan;
+                    # other scanners' results are still useful.
+                    head = ""
+                    try:
+                        head = result_files[0].read_text(
+                            encoding="utf-8", errors="replace",
+                        )[:200]
+                    except OSError:
+                        head = "<unreadable>"
+                    metadata_extra["parse_failed"] = True
+                    metadata_extra["parse_failure_reason"] = (
+                        f"{type(exc).__name__}: {exc}. "
+                        f"output head: {head!r}"
+                    )
+                    logger.warning(
+                        "Scanner '%s' produced output but parse failed: %s",
+                        scanner.name, exc,
+                    )
+                    findings = []
                 else:
-                    findings = parsed
-                logger.debug(
-                    "Parsed %d finding(s) from %s",
-                    len(findings),
-                    result_files[0].name,
-                )
+                    # parse_results may return either a list of Findings,
+                    # a ``(list, int)`` tuple (legacy passed_count channel,
+                    # used by linters), or a ``(list, dict)`` tuple (extra
+                    # metadata merged into ScanResult.metadata — used by
+                    # Grype to flag "source.target=unknown" which means
+                    # "couldn't identify packages" rather than "nothing
+                    # vulnerable").
+                    if isinstance(parsed, tuple):
+                        findings, extra = parsed
+                        if isinstance(extra, int):
+                            metadata_extra["passed_count"] = extra
+                        elif isinstance(extra, dict):
+                            metadata_extra.update(extra)
+                            # Warn at the engine layer too so the signal is
+                            # visible even when a reporter doesn't render
+                            # per-scanner metadata.
+                            if "warning" in extra:
+                                logger.warning(
+                                    "Scanner '%s': %s",
+                                    scanner.name, extra["warning"],
+                                )
+                    else:
+                        findings = parsed
+                    logger.debug(
+                        "Parsed %d finding(s) from %s",
+                        len(findings),
+                        result_files[0].name,
+                    )
 
             return ScanResult(
                 scanner=scanner.name,