studio: harden lockfile audit followups for #5604#5695
Conversation
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 `::warning::` 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.
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the lockfile supply chain audit script by making unsupported lockfile versions a blocking finding to prevent downgrade attacks. It also hardens the script against GitHub Actions workflow-command injection by escaping user-supplied environment variables in warning messages. Additionally, new regression tests were added to verify these security improvements and ensure the audit runs before package installation in CI workflows. I have no feedback to provide.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70a090fd26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # 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 |
There was a problem hiding this comment.
In .github/workflows/release-desktop.yml (checked the build matrix in this file), the job runs on windows-latest but this new step invokes python3 directly before any interpreter fallback is applied; later in the same job, another step explicitly falls back from python3 to python, which indicates python3 is not assumed everywhere. If the Windows runner lacks a python3 command in Git Bash, this step fails early and blocks the Windows desktop release build even though Python is otherwise available.
Useful? React with 👍 / 👎.
Each PR ran the same staged source files before, which went stale when the upstream PR commits advanced. Refactor to one job per PR with an actions/checkout of that PR's head ref, so cross-OS validation always uses the latest commit: - PR unslothai#5603 sandbox -> studio-sandbox-hardening - PR unslothai#5620 parser parity -> studio-tools-multi-format-v2 - PR unslothai#5696 mtp reload guards -> followup-mtp-reload-guards (unslothai#5582 followup) - PR unslothai#5695 lockfile audit -> followup-lockfile-audit-regressions (unslothai#5604 followup) 4 jobs x 3 OSes = 12 runs; Windows = 4 (below the 5-concurrent cap). cancel-in-progress per (workflow, ref) keeps iteration cheap. All tests stay CPU-only and rely on the CUDA spoof harness in tests/conftest.py + tests/_zoo_aggressive_cuda_spoof.py, so no real GPU is required on any runner.
1 participant
Summary
Followup to #5604. Three regressions surfaced in the reviewer rounds after that PR landed; all addressed here with matching pytest coverage.
unsupported-lockfile-versionis now blocking again. Default mode previously exited 0 on a v1 lockfile because the structural walk only runs on v2/v3, soblocked-known-malicious/known-ioc-stringfindings could not be produced. A checked-inpackage-lock.jsoncould be downgraded to v1 and silently bypass the audit. Restored toBLOCKING_KINDS.UNSLOTH_LOCKFILE_AUDIT_SKIPwarnings now route through_gha_escape(). Both the invalid-skip branch (raw_skip_rawechoed) and the accepted-skip branch (_skipechoed) interpolated user-controlled text into::warning::workflow commands. A value containing\n::error::...produced a second physical workflow-command line. The new_gha_escape()helper added by ci: advisory lockfile supply-chain audit (no install-script changes) #5604 already covers finding annotations; just wiring it in here closes the same surface for the skip path.studio-tauri-smoke.ymlandrelease-desktop.ymlnow run the lockfile audit beforenpm install. Both consumestudio/package-lock.json(added as a default audit target in ci: advisory lockfile supply-chain audit (no install-script changes) #5604) but rannpm installbefore any audit step, defeating the "pre-install" guarantee.release-desktop.ymladditionally hascontents: write, so the gap had real publish-attacker blast radius.Default-mode audit still rc=0 on the three real lockfiles. No behaviour change for users not under attack.
Test plan
python -m pytest tests/security/test_lockfile_supply_chain_audit.py -v— 16/16 pass (4 new):test_unsupported_lockfile_version_blocks_default— v1 lockfile must exit 1 in default modetest_blocking_kinds_contains_unsupported_lockfile_version— module-level pin against future regressionstest_skip_env_warning_escapes_workflow_command_injection— both skip branches must escape\n/%so no injected::-prefixed physical line appearstest_audit_runs_before_npm_install_in_consumer_workflows— regex-based ordering check onrun:lines in both consumer workflowspython scripts/lockfile_supply_chain_audit.pyon real repo lockfiles → rc=0, 0 findingslockfile-audit.yml,studio-tauri-smoke.yml,release-desktop.ymlworkflows green on this branch