fix(cli,supply-chain): parse package.json as JSON, and send fatal errors to stderr

Two correctness fixes, both found while driving the CLI from automation.

package.json was scanned line by line. A manifest written on a single line —
valid JSON, and what several generators emit — never entered the dependency
section, so it produced *no* dependencies at all and the file passed silently.
That is not noise, it is blindness: the scanner reports nothing and the caller
cannot tell the difference from a clean manifest.

It is now parsed as JSON. Version extraction is unchanged, including the caret
handling, which is a separate question (#302): only the parsing changes. Line
numbers survive the switch — the entry is located from the section header
onwards, so a name that also appears in "scripts" does not steal the position —
and a manifest that does not parse still falls back to the previous scan rather
than going blind.

Fatal errors were printed with the default Rich console, which writes to stdout.
Anything driving the CLI from a script separates the two streams, so the only
diagnosis available was discarded: a scan that failed left an empty error log
and nothing to act on. Concretely, "Error: unsupported baseline version 1" —
which is exactly the message a user needs after upgrading — arrived on stdout.
Errors now go to a stderr console.

Tests: one-line manifest, compact manifest, line numbers preserved, a name
shadowed by "scripts", invalid JSON falling back, a non-object manifest, and
non-string specs ignored. Full suite: 1567 passed, 14 skipped, 6 xfailed.

Signed-off-by: Mark2Mac <Mark2Mac@users.noreply.github.com>

Author: Mark2Mac

src/skillspector/cli.py

@@ -71,6 +71,10 @@ def _ensure_utf8_streams() -> None:
 )
 
 console = Console()
+# Fatal errors go to stderr. Anything driving the CLI from a script separates the two streams,
+# and with the message on stdout the only diagnosis available was thrown away: a failed scan
+# left an empty error log and the caller had nothing to act on.
+err_console = Console(stderr=True)
 
 
 class FormatChoice(StrEnum):
@@ -378,13 +382,13 @@ def scan(
     except typer.Exit:
         raise
     except (FileNotFoundError, ValueError) as e:
-        console.print(f"[red]Error:[/red] {e}")
+        err_console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(code=2) from e
     except Exception as e:
         if verbose:
             console.print_exception()
         else:
-            console.print(f"[red]Error:[/red] {e}")
+            err_console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(code=2) from e
     finally:
         if result is not None:
@@ -444,7 +448,7 @@ def _scan_multi_skill(
             severity = result.get("risk_severity") or "LOW"
             console.print(f"         Score: {score}/100 ({severity})\n")
         except Exception as e:
-            console.print(f"         [red]Error:[/red] {e}\n")
+            err_console.print(f"         [red]Error:[/red] {e}\n")
             execution_failed = True
             results.append({"skill_name": skill.name, "error": str(e)})
 
@@ -558,7 +562,7 @@ def mcp(
 
         run_mcp(transport=transport.value, host=host, port=port)
     except ModuleNotFoundError as e:
-        console.print(f"[red]Error:[/red] {e}")
+        err_console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(code=2) from e
 
 
@@ -631,13 +635,13 @@ def baseline(
     except typer.Exit:
         raise
     except (FileNotFoundError, ValueError) as e:
-        console.print(f"[red]Error:[/red] {e}")
+        err_console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(code=2) from e
     except Exception as e:
         if verbose:
             console.print_exception()
         else:
-            console.print(f"[red]Error:[/red] {e}")
+            err_console.print(f"[red]Error:[/red] {e}")
         raise typer.Exit(code=2) from e
     finally:
         if result is not None:

src/skillspector/nodes/analyzers/static_patterns_supply_chain.py

@@ -27,6 +27,7 @@
 
 from __future__ import annotations
 
+import json
 import re
 import sys
 import tomllib
@@ -544,8 +545,23 @@ def _extract_packages_from_requirements(content: str) -> list[tuple[str, str | N
     return results
 
 
-def _extract_packages_from_package_json(content: str) -> list[tuple[str, str | None, int]]:
-    """Extract (package_name, version_or_None, line_number) from package.json content."""
+_NPM_DEPENDENCY_SECTIONS = ("dependencies", "devDependencies", "peerDependencies")
+
+
+def _package_json_line(content: str, section: str, name: str) -> int:
+    """Best-effort line for a dependency entry, so findings keep pointing somewhere useful.
+
+    Parsing JSON loses positions, and the search starts at the section header so a name that
+    also appears in ``scripts`` does not win.
+    """
+    header = re.search(rf'"{re.escape(section)}"\s*:', content)
+    start = header.end() if header else 0
+    entry = re.compile(rf'"{re.escape(name)}"\s*:').search(content, start)
+    return get_line_number(content, entry.start()) if entry else 1
+
+
+def _extract_packages_from_package_json_scan(content: str) -> list[tuple[str, str | None, int]]:
+    """Line-oriented fallback, used only when the manifest is not valid JSON."""
     results: list[tuple[str, str | None, int]] = []
     in_deps = False
     for i, line in enumerate(content.splitlines(), 1):
@@ -559,9 +575,34 @@ def _extract_packages_from_package_json(content: str) -> list[tuple[str, str | N
         if in_deps:
             m = re.match(r'"([^"]+)"\s*:\s*"([^"]*)"', stripped)
             if m:
-                name = m.group(1)
-                version = _pinned_npm_version(m.group(2))
-                results.append((name, version, i))
+                results.append((m.group(1), _pinned_npm_version(m.group(2)), i))
+    return results
+
+
+def _extract_packages_from_package_json(content: str) -> list[tuple[str, str | None, int]]:
+    """Extract (package_name, version_or_None, line_number) from package.json content.
+
+    package.json is JSON, so it is parsed as JSON. Scanning it line by line made the result
+    depend on formatting: a manifest written on a single line — which is valid, and what many
+    generators emit — never entered the dependency section at all and yielded *no* dependencies,
+    silently. The line-oriented scan remains as a fallback for manifests that do not parse.
+    """
+    try:
+        data = json.loads(content)
+    except (ValueError, TypeError):
+        return _extract_packages_from_package_json_scan(content)
+    if not isinstance(data, dict):
+        return []
+    results: list[tuple[str, str | None, int]] = []
+    for section in _NPM_DEPENDENCY_SECTIONS:
+        deps = data.get(section)
+        if not isinstance(deps, dict):
+            continue
+        for name, spec in deps.items():
+            if not isinstance(name, str) or not isinstance(spec, str):
+                continue
+            line = _package_json_line(content, section, name)
+            results.append((name, _pinned_npm_version(spec), line))
     return results
 
 

tests/unit/test_patterns_new.py

@@ -1532,6 +1532,60 @@ def test_extract_packages_package_json_caret_is_not_a_pin(self) -> None:
         assert versions["shell-quote"] is None
         assert versions["semver"] is None
         assert versions["glob"] is None
+    def test_package_json_on_a_single_line_is_not_invisible(self) -> None:
+        # Regression: the line-oriented scan never entered the dependency section, so a valid
+        # one-line manifest yielded no dependencies at all — silently.
+        content = '{"name":"x","dependencies":{"express":"^4.18.0","lodash":"4.17.21"}}'
+        names = {p[0] for p in sc_mod._extract_packages_from_package_json(content)}
+        assert names == {"express", "lodash"}
+
+    def test_package_json_compact_keeps_versions(self) -> None:
+        # Version resolution is not this PR's subject: it stays whatever the shared predicate
+        # decides (#319). Only the parsing of the manifest changes, and a compact manifest must
+        # resolve exactly like the indented one.
+        content = '{"dependencies":{"lodash":"4.17.21","semver":"^7.5.0"}}'
+        versions = {p[0]: p[1] for p in sc_mod._extract_packages_from_package_json(content)}
+        assert versions["lodash"] == "4.17.21"
+        assert versions["semver"] is None
+
+    def test_package_json_line_numbers_survive_parsing(self) -> None:
+        content = (
+            "{\n"
+            '  "name": "x",\n'
+            '  "dependencies": {\n'
+            '    "express": "4.18.0"\n'
+            "  }\n"
+            "}\n"
+        )
+        lines = {p[0]: p[2] for p in sc_mod._extract_packages_from_package_json(content)}
+        assert lines["express"] == 4
+
+    def test_package_json_line_prefers_the_dependency_over_a_script(self) -> None:
+        # A name that also appears in "scripts" must not steal the line number.
+        content = (
+            "{\n"
+            '  "scripts": { "express": "node server.js" },\n'
+            '  "dependencies": {\n'
+            '    "express": "4.18.0"\n'
+            "  }\n"
+            "}\n"
+        )
+        lines = {p[0]: p[2] for p in sc_mod._extract_packages_from_package_json(content)}
+        assert lines["express"] == 4
+
+    def test_package_json_invalid_falls_back_to_the_scan(self) -> None:
+        # A manifest that does not parse keeps the previous behaviour instead of going blind.
+        content = '{\n  "dependencies": {\n    "express": "4.18.0",\n'  # truncated
+        names = {p[0] for p in sc_mod._extract_packages_from_package_json(content)}
+        assert "express" in names
+
+    def test_package_json_non_object_is_empty(self) -> None:
+        assert sc_mod._extract_packages_from_package_json("[1, 2, 3]") == []
+
+    def test_package_json_ignores_non_string_specs(self) -> None:
+        content = '{"dependencies":{"ok":"1.0.0","broken":{"version":"1.0.0"},"n":42}}'
+        names = {p[0] for p in sc_mod._extract_packages_from_package_json(content)}
+        assert names == {"ok"}
 
     def test_extract_packages_package_json(self) -> None:
         content = (