studio: harden lockfile supply-chain audit followups for #5604

Three regressions surfaced after #5604 landed. All addressed here with
matching pytest coverage.

1. Default mode no longer blocks on `unsupported-lockfile-version`. A
   lockfile downgraded to v1 (or bumped to an unsupported future
   version) silently passes default CI because the auditor's structural
   walk only runs on v2/v3, so `blocked-known-malicious` /
   `known-ioc-string` findings cannot be produced. v1 downgrade is a
   documented supply-chain attack shape; restoring it to BLOCKING_KINDS.

2. `UNSLOTH_LOCKFILE_AUDIT_SKIP` warnings interpolated the raw env var
   value into `:⚠️:` workflow commands without `_gha_escape()`,
   so a value containing `\n::error::...` was emitted as a second
   physical workflow-command line (GH Actions parses workflow commands
   per physical line). Routing both branches through `_gha_escape()`
   collapses any control char onto a single annotation line.

3. The two workflows that consume `studio/package-lock.json` --
   `studio-tauri-smoke.yml` and `release-desktop.yml` -- did not invoke
   `scripts/lockfile_supply_chain_audit.py` before their `npm install`
   steps. Lifecycle scripts in a compromised lockfile would run before
   the audit could refuse the lockfile, defeating the script's
   "pre-install" guarantee. `release-desktop.yml` additionally has
   `contents: write`, so the gap had real publish-attacker blast
   radius. Both workflows now run the audit before any `npm install`
   or `npm ci`.

Tests
-----
- test_unsupported_lockfile_version_blocks_default: v1 lockfile must
  exit 1 in default mode.
- test_blocking_kinds_contains_unsupported_lockfile_version: direct
  module-level pin against future regressions.
- test_skip_env_warning_escapes_workflow_command_injection: both
  skip-env branches must escape `\n` / `%` so no injected physical
  workflow-command line is produced.
- test_audit_runs_before_npm_install_in_consumer_workflows: regex
  ordering check on the `run:` lines in the two consumer workflows so
  a future reorder cannot silently bring back the pre-install gap.

16/16 pytest pass; default-mode audit still rc=0 on real lockfiles.

.github/workflows/release-desktop.yml

@@ -361,13 +361,23 @@ jobs:
         with:
           node-version: 24
 
+      - name: Lockfile supply-chain audit (pre-install scan)
+        shell: bash
+        # Must run BEFORE any `npm install`. This job has
+        # ``contents: write`` (release upload), so a compromised
+        # lockfile that runs a malicious postinstall script during the
+        # Tauri CLI install or frontend install below could publish
+        # attacker-controlled binaries through the release. The audit
+        # refuses the run before any npm lifecycle script executes.
+        run: python3 scripts/lockfile_supply_chain_audit.py
+
       - name: Install pinned Tauri CLI
         # Lifecycle scripts (esbuild native-binary postinstall, etc.) are
         # required for `vite build`. The pre-install lockfile structural
-        # audit (lockfile_supply_chain_audit.py) is the practical defence
-        # against the npm postinstall-dropper class -- it fires BEFORE any
-        # tarball runs, on the injection pattern itself rather than an
-        # advisory-DB lookup.
+        # audit (lockfile_supply_chain_audit.py) above is the practical
+        # defence against the npm postinstall-dropper class -- it fires
+        # BEFORE any tarball runs, on the injection pattern itself
+        # rather than an advisory-DB lookup.
         run: npm install --save-dev --prefix studio @tauri-apps/cli@2.10.1 --no-fund --no-audit
 
       - name: Verify pinned Tauri CLI

.github/workflows/studio-tauri-smoke.yml

@@ -60,13 +60,20 @@ jobs:
         with:
           workspaces: studio/src-tauri -> target
 
+      - name: Lockfile supply-chain audit (pre-install scan)
+        # Must run BEFORE any `npm install` (including the Tauri CLI
+        # install below). Lifecycle scripts in a compromised lockfile
+        # would otherwise execute inside this runner before the audit
+        # gets a chance to refuse the lockfile.
+        run: python3 scripts/lockfile_supply_chain_audit.py
+
       - name: Install pinned Tauri CLI (matches release-desktop.yml)
         # Lifecycle scripts (esbuild native-binary postinstall, etc.) are
         # required for `vite build`. The pre-install lockfile structural
-        # audit (lockfile_supply_chain_audit.py) is the practical defence
-        # against the npm postinstall-dropper class -- it fires BEFORE any
-        # tarball runs, on the injection pattern itself rather than an
-        # advisory-DB lookup.
+        # audit (lockfile_supply_chain_audit.py) above is the practical
+        # defence against the npm postinstall-dropper class -- it fires
+        # BEFORE any tarball runs, on the injection pattern itself
+        # rather than an advisory-DB lookup.
         run: npm install --save-dev --prefix studio @tauri-apps/cli@2.10.1 --no-fund --no-audit
 
       - name: Verify pinned Tauri CLI version
@@ -75,9 +82,6 @@ jobs:
           echo "$out"
           [ "$out" = "tauri-cli 2.10.1" ] || { echo "::error::expected tauri-cli 2.10.1, got $out"; exit 1; }
 
-      - name: Lockfile supply-chain audit (pre-install scan)
-        run: python3 scripts/lockfile_supply_chain_audit.py
-
       - name: Frontend build (npm ci, vite)
         working-directory: studio/frontend
         # Lifecycle scripts (esbuild native-binary postinstall, etc.) are

scripts/lockfile_supply_chain_audit.py

@@ -730,6 +730,14 @@ BLOCKING_KINDS: frozenset[str] = frozenset(
         "missing-lockfile",
         "unreadable-lockfile",
         "missing-toml-parser",
+        # An unsupported lockfileVersion means the audit could not walk
+        # the dependency tree at all (the structural rules below only
+        # apply to npm v2/v3). Treating it as advisory would let a v1
+        # downgrade -- a known supply-chain attack shape -- silently
+        # pass CI: the scanner reports the kind, exits 0, and no
+        # blocking finding is raised. Keep this blocking so a checked-in
+        # lockfile cannot be downgraded out of audit coverage.
+        "unsupported-lockfile-version",
     }
 )
 
@@ -797,17 +805,27 @@ def main(argv: list[str] | None = None) -> int:
     if _skip_raw is not None:
         _skip = _skip_raw.strip()
         _invalid_tokens = {"", "1", "0", "true", "false", "yes", "no", "on", "off"}
+        # Both branches echo the user-supplied env var inside a
+        # ``::warning::`` GH Actions workflow command. The raw value can
+        # contain ``%``, ``\r``, ``\n`` or even another ``::error::``
+        # line (workflow-command injection); _gha_escape collapses each
+        # message onto a single annotation line per the GH workflow-
+        # commands spec.
         if _skip.lower() in _invalid_tokens or len(_skip) < 5:
             print(
-                "::warning::Lockfile audit skip REQUIRES a justification "
-                f"value (>=5 chars, not '{_skip_raw}'). Proceeding with "
-                "audit. Use e.g. UNSLOTH_LOCKFILE_AUDIT_SKIP=ticket-1234.",
+                "::warning::"
+                + _gha_escape(
+                    "Lockfile audit skip REQUIRES a justification "
+                    f"value (>=5 chars, not '{_skip_raw}'). Proceeding with "
+                    "audit. Use e.g. UNSLOTH_LOCKFILE_AUDIT_SKIP=ticket-1234."
+                ),
                 file = sys.stderr,
                 flush = True,
             )
         else:
             print(
-                f"::warning::Lockfile audit skipped: reason='{_skip}'",
+                "::warning::"
+                + _gha_escape(f"Lockfile audit skipped: reason='{_skip}'"),
                 file = sys.stderr,
                 flush = True,
             )

tests/security/test_lockfile_supply_chain_audit.py

@@ -382,3 +382,173 @@ def test_skip_env_var_with_short_value_rejected(tmp_path):
         assert "[lockfile-audit] npm:" in c, (
             f"value {bad_val!r} should have fallen through to run audit; " f"got:\n{c}"
         )
+
+
+# ---------------------------------------------------------------------------
+# Followup regression tests for #5604:
+#   - unsupported lockfile versions must block in default mode (v1 downgrade
+#     would otherwise pass with rc=0 because the structural walk only runs
+#     on v2/v3)
+#   - the ``UNSLOTH_LOCKFILE_AUDIT_SKIP`` warning must be routed through
+#     ``_gha_escape()`` so an attacker-controlled value cannot inject a
+#     second workflow-command line via embedded ``\n::error::...``
+#   - the audit script must be invoked BEFORE ``npm install`` in any
+#     workflow that consumes the audited lockfiles
+# ---------------------------------------------------------------------------
+
+
+def test_unsupported_lockfile_version_blocks_default(tmp_path):
+    """A v1 lockfile (or any non-v2/v3 version) means the structural
+    dependency walk never runs, so ``blocked-known-malicious`` /
+    ``known-ioc-string`` findings cannot be produced. Treating that as
+    advisory lets an attacker downgrade a checked-in lockfile to v1
+    and silently exit CI with rc=0. Default mode must refuse.
+    """
+    p = tmp_path / "package-lock.json"
+    p.write_text(
+        '{\n'
+        '  "name": "test",\n'
+        '  "version": "1.0.0",\n'
+        '  "lockfileVersion": 1,\n'
+        '  "dependencies": {"react": {"version": "18.2.0"}}\n'
+        '}\n'
+    )
+    proc = _run_auditor(root = tmp_path, npm_lockfiles = [p])
+    combined = proc.stdout + proc.stderr
+    assert proc.returncode == 1, (
+        f"v1 lockfile must block default mode (was advisory pre-followup); "
+        f"rc={proc.returncode}\n--- stdout ---\n{proc.stdout}\n"
+        f"--- stderr ---\n{proc.stderr}"
+    )
+    assert "unsupported-lockfile-version" in combined, combined
+
+
+def test_blocking_kinds_contains_unsupported_lockfile_version():
+    """Direct module-level assertion: if anyone moves
+    ``unsupported-lockfile-version`` back out of BLOCKING_KINDS this
+    test trips immediately, before they re-introduce the downgrade
+    bypass."""
+    assert "unsupported-lockfile-version" in lsa.BLOCKING_KINDS
+
+
+def test_skip_env_warning_escapes_workflow_command_injection(tmp_path):
+    """An attacker controlling ``UNSLOTH_LOCKFILE_AUDIT_SKIP`` could
+    embed a literal ``\\n::error::...`` and split the warning into a
+    second workflow-command annotation. Both the invalid-skip branch
+    (raw value echoed) and the accepted-skip branch (stripped value
+    echoed) must escape the value via ``_gha_escape()`` so the message
+    is collapsed onto one annotation line -- meaning no stderr line
+    OTHER than the audit's own ``::warning::`` may begin with ``::``.
+    """
+    fixture = FIXTURES / "clean_lockfile.json"
+
+    def _physical_lines_starting_with_double_colon(stderr: str) -> list[str]:
+        # GH Actions parses workflow commands per physical line. Only
+        # lines that START with `::` after any leading whitespace count
+        # as a new annotation. Any such line BEYOND the first warning
+        # is an injected command.
+        return [ln for ln in stderr.splitlines() if ln.lstrip().startswith("::")]
+
+    # Branch A -- invalid skip value (rejected, audit falls through).
+    # Use a value that survives the validation check (not a boolean
+    # token, >=5 chars) BUT contains injection chars. The accepted
+    # branch is the easier-to-trip target; the rejected branch is
+    # exercised in test_skip_env_var_with_short_value_rejected.
+    injected_bad = "%inject\n::error::bad"  # contains %, \n, and ::
+    env_a = {**os.environ, "UNSLOTH_LOCKFILE_AUDIT_SKIP": injected_bad}
+    proc_a = subprocess.run(
+        [
+            sys.executable,
+            str(SCRIPT),
+            "--root",
+            str(tmp_path),
+            "--npm-lockfile",
+            str(fixture),
+        ],
+        capture_output = True,
+        text = True,
+        timeout = 30,
+        env = env_a,
+    )
+    # The stripped value is "%inject\n::error::bad" (len 21) and is not
+    # a booleanish token -> accepted-skip path; rc 0, audit skipped.
+    assert proc_a.returncode == 0
+    assert "%0A" in proc_a.stderr and "%25" in proc_a.stderr, (
+        "skip value containing \\n and %% must be %0A / %25 escaped; "
+        f"stderr was:\n{proc_a.stderr}"
+    )
+    cmd_lines_a = _physical_lines_starting_with_double_colon(proc_a.stderr)
+    assert len(cmd_lines_a) == 1 and cmd_lines_a[0].startswith("::warning::"), (
+        "exactly one ::-prefixed physical line expected (the audit's own "
+        f"::warning::); injection split the message into: {cmd_lines_a}"
+    )
+
+    # Branch B -- short skip value with embedded injection.
+    # The PRE-strip raw value is also interpolated in the rejected
+    # branch's warning, so it MUST be escaped too.
+    injected_short = "1\n::error::short-bad"
+    # Critically, this value strips to "1\n::error::short-bad" which is
+    # NOT a booleanish token (the literal newline + tail prevents the
+    # ``_skip.lower() in _invalid_tokens`` match), so the audit ends up
+    # routing it through the ACCEPTED branch, not the rejected one.
+    # That is itself a hardening property worth pinning: an attacker
+    # cannot bypass the length check via embedded control chars without
+    # the warning being escaped on the way out.
+    env_b = {**os.environ, "UNSLOTH_LOCKFILE_AUDIT_SKIP": injected_short}
+    proc_b = subprocess.run(
+        [
+            sys.executable,
+            str(SCRIPT),
+            "--root",
+            str(tmp_path),
+            "--npm-lockfile",
+            str(fixture),
+        ],
+        capture_output = True,
+        text = True,
+        timeout = 30,
+        env = env_b,
+    )
+    assert "%0A" in proc_b.stderr, (
+        f"value with embedded \\n must be %0A-escaped; stderr was:\n{proc_b.stderr}"
+    )
+    cmd_lines_b = _physical_lines_starting_with_double_colon(proc_b.stderr)
+    assert all(ln.startswith("::warning::") for ln in cmd_lines_b), (
+        f"injection split the message into a non-::warning:: physical "
+        f"line: {cmd_lines_b}"
+    )
+
+
+def test_audit_runs_before_npm_install_in_consumer_workflows():
+    """Any GH Actions workflow that consumes one of the audited
+    lockfiles via ``npm install`` / ``npm ci`` must run the
+    lockfile_supply_chain_audit step BEFORE that install, otherwise a
+    compromised lockfile's lifecycle scripts execute before the audit
+    can refuse the run. Static text check so a future edit cannot
+    silently reintroduce the asymmetric ordering."""
+    import re
+
+    workflows_dir = REPO_ROOT / ".github" / "workflows"
+    # Match `run:` lines invoking the audit / npm install. We look at
+    # ``run:`` lines specifically so the prose comment block above
+    # each step (which mentions both audit and `npm install`) does
+    # not bias the ordering check.
+    audit_re = re.compile(r"^\s*run:\s*python3\s+scripts/lockfile_supply_chain_audit\.py", re.MULTILINE)
+    install_re = re.compile(r"^\s*run:\s*(?:.*&&\s*)?npm\s+(?:install|ci)\b", re.MULTILINE)
+    for wf_name in ("studio-tauri-smoke.yml", "release-desktop.yml"):
+        wf = workflows_dir / wf_name
+        assert wf.is_file(), f"missing workflow: {wf}"
+        text = wf.read_text(encoding = "utf-8")
+        audit_match = audit_re.search(text)
+        assert audit_match, (
+            f"{wf_name}: must invoke lockfile_supply_chain_audit.py "
+            f"via a ``run: python3 scripts/...`` line (none found)"
+        )
+        for install_match in install_re.finditer(text):
+            assert audit_match.start() < install_match.start(), (
+                f"{wf_name}: lockfile audit (offset {audit_match.start()}) "
+                f"must come BEFORE every ``npm install`` / ``npm ci`` run "
+                f"line (offending offset {install_match.start()}); a "
+                f"compromised lockfile's lifecycle scripts would otherwise "
+                f"execute before the audit can refuse the lockfile"
+            )