Skip to content

ci: pin every Studio install path to its lockfile#5479

Closed
danielhanchen wants to merge 21 commits into
mainfrom
ci/frozen-lockfile-installs
Closed

ci: pin every Studio install path to its lockfile#5479
danielhanchen wants to merge 21 commits into
mainfrom
ci/frozen-lockfile-installs

Conversation

@danielhanchen

@danielhanchen danielhanchen commented May 16, 2026

Copy link
Copy Markdown
Member

Summary

Closes a supply-chain risk that ran through every install path Unsloth Studio uses. Every script that builds or installs the frontend called naked bun install and npm install. With caret ranges in studio/frontend/package.json (the default for most deps), those commands resolve a fresh minor or patch from the registry if one is available, even though package-lock.json pins specific versions. An attacker who hijacks any transitive dep and publishes a malicious patch release would have it pulled into the next release build, the next desktop signed artifact, or the next end-user install with no visible signal.

The fix is the standard hardening: switch every install path to lockfile-strict mode.

File Before After
build.sh:39 bun install bun install --frozen-lockfile
build.sh:47 npm install (fallback) npm ci (fallback)
studio/setup.sh:346 bun install bun install --frozen-lockfile
studio/setup.sh:384 npm install --no-fund --no-audit --loglevel=error (fallback) npm ci --no-fund --no-audit --loglevel=error (fallback)
studio/setup.sh:420 npm install --no-fund --no-audit --loglevel=error (OXC validator) npm ci --no-fund --no-audit --loglevel=error (OXC validator)
studio/setup.ps1:1338 bun install (Windows installer) bun install --frozen-lockfile
studio/setup.ps1:1352 bun install (Windows cache-clear retry) bun install --frozen-lockfile
studio/setup.ps1:1371 npm install (Windows fallback) npm ci
studio/setup.ps1:1414 npm install (Windows OXC validator) npm ci
.github/workflows/release-desktop.yml:452 npm install --no-fund --no-audit npm ci --no-fund --no-audit

Both --frozen-lockfile (bun) and npm ci install only what the lockfile pins, verify cryptographic hashes, and abort with a clear error if package.json and the lockfile have drifted. This matches what the CI workflows already do (studio-frontend-ci.yml, studio-tauri-smoke.yml, wheel-smoke.yml, security-audit.yml all use npm ci).

Why this matters

A worked example using a recent incident: the May 2026 TanStack Mini Shai-Hulud attack (GHSA-g7cv-rxg3-hmpx) published malware as new patch versions of 42 @tanstack/* packages. If a transitive consumer had "@tanstack/router-core": "^1.169.2" in their dep tree and ran naked npm install during a release build before the bad versions were yanked, npm would resolve to the malicious 1.169.5 or 1.169.8. npm ci against a clean lockfile pinned to 1.169.2 would skip the compromised versions entirely.

The same pattern applies to any transitive dep with a caret range, which is the vast majority of npm packages.

The release-desktop.yml site is the most important one: that workflow builds the signed .app, .dmg, .AppImage, and .msi artifacts that ship to end users. Before this PR, a single hijacked transitive could land in a release artifact between the lockfile commit and the workflow run. Now the install is identical to what the lockfile pins.

What this does not change

  • Windows workflows still use npm install -g 'npm@^11' to bootstrap npm itself. That is a global install of the package manager and has no lockfile, so --frozen-lockfile does not apply.
  • The pinned Tauri CLI install in release-desktop.yml (npm install --save-dev --prefix studio @tauri-apps/cli@2.10.1) stays as npm install because that line intentionally adds a specific package to package.json, not syncs from the lockfile.
  • The fallback ordering (try bun first, fall back to npm) is preserved.
  • The custom binary-integrity check in studio/setup.sh and studio/setup.ps1 (which verifies that tsc and vite bins materialise after bun finishes, to catch bun's known corrupt-cache mode) is preserved.

Test plan

  • git diff shows only the install-command swaps and explanatory comments.
  • npm ci --no-fund --no-audit --dry-run exits 0 against the current studio/frontend/package-lock.json (1042 packages resolved, no drift between manifest and lockfile).
  • npm ci --no-fund --no-audit --dry-run exits 0 against studio/backend/core/data_recipe/oxc-validator/package-lock.json (lockfile and manifest agree).
  • CI: studio-frontend-ci green (already uses npm ci).
  • CI: studio-tauri-smoke green.
  • CI: release-desktop workflow runs cleanly with npm ci (dispatch from main after merge).
  • Local ./build.sh succeeds end to end with the new commands (someone with bun installed should confirm the bun path too).
  • Local ./install.sh --local --no-torch reaches the Studio install path and completes.

Credit

Reported in code review of PR #5477 (drop unused frontend deps); separating the fix into its own PR so it can land independently and be backported easily.

build.sh and studio/setup.sh both call naked `bun install` and
`npm install`. With caret ranges in package.json (the default for
most deps), those commands resolve a fresh minor/patch from the
registry if one exists, even though the lockfile pins specific
versions. An attacker who hijacks any transitive dep and publishes
a malicious patch release can have it pulled into the release build
or end-user install without anyone noticing.

Both paths now use lockfile-strict mode:
bun install -> bun install --frozen-lockfile
npm install -> npm ci

These install exactly what the committed lockfile pins, verify
cryptographic hashes, and fail fast on any drift between package.json
and the lockfile. The CI workflows that build the frontend already
use `npm ci`; this aligns the local build and end-user setup paths
with the same guarantee.

Verified `npm ci --no-fund --no-audit --dry-run` exits 0 against
the current studio/frontend lockfile (1042 packages, no drift).
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 152fe8d581

ℹ️ 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".

Comment thread studio/setup.sh Outdated
# it. Combined with the npm-ci fallback below, this keeps the
# release-build dep set identical to what the committed lockfile
# captures.
bun install --frozen-lockfile >"$_log" 2>&1 || _exit_code=$?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add a Bun lockfile before freezing Bun installs

In a normal checkout of this repo there is studio/frontend/package-lock.json but no bun.lock; with that state, bun install --frozen-lockfile cannot install because Bun first needs to migrate the npm lockfile into its own lockfile and --frozen-lockfile rejects that write. On machines with Bun installed this path now always fails, and setup.sh then misclassifies it as a possible corrupt cache, clears the user's Bun cache, retries the same guaranteed-failing command, and only then falls back to npm. Either commit a Bun lockfile for this path or skip Bun when only the npm lockfile is present.

Useful? React with 👍 / 👎.

Followup to 152fe8d. Three more sites still called naked
`bun install` / `npm install`, which honour caret ranges in
package.json and can pull a fresh minor/patch of a transitive
dep from the registry on the next run.

studio/setup.ps1 (4 sites): the Windows end-user installer.
bun install -> bun install --frozen-lockfile (both initial
and the cache-clear retry); the npm fallback and the OXC
validator npm install both -> npm ci. Error messages updated
to reference the new command.

studio/setup.sh: the OXC validator runtime install for the
Unix path was still naked `npm install`. Now `npm ci`.

github/workflows/release-desktop.yml: the desktop release
build's frontend install was still naked `npm install`. Now
`npm ci` so a published .app/.dmg/.AppImage/.msi can never
have shipped with a registry-resolved transitive that drifted
from the committed lockfile.

The pinned Tauri CLI install in the same workflow stays as
`npm install --save-dev @tauri-apps/cli@2.10.1` because that
line is intentionally adding a specific package to package.json,
not syncing from the lockfile.

Verified `npm ci --no-fund --no-audit --dry-run` exits 0 against
both the studio/frontend and studio/backend/core/data_recipe/
oxc-validator lockfiles.
@danielhanchen danielhanchen changed the title ci: pin install to lockfile in build.sh and studio/setup.sh ci: pin every Studio install path to its lockfile May 16, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7bb1eb6a16

ℹ️ 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".

Comment thread studio/setup.sh Outdated
run_quiet_no_exit "npm install (oxc validator runtime)" npm install --no-fund --no-audit --loglevel=error
# npm ci pins the oxc validator install to its committed lockfile so a
# hijacked transitive cannot land via caret-range resolution.
run_quiet_no_exit "npm ci (oxc validator runtime)" npm ci --no-fund --no-audit --loglevel=error

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep npm install until OXC has a lockfile

In the OXC validator directory I found only package.json and no package-lock.json/npm-shrinkwrap.json, so this new npm ci path now exits before installing anything whenever setup reaches this block. I confirmed the CLI behavior by running npm ci there; npm reports that npm ci can only install with an existing package-lock or shrinkwrap. This blocks the Unix setup path here, and the same replacement in studio/setup.ps1 blocks the Windows path as well unless a lockfile is committed for studio/backend/core/data_recipe/oxc-validator.

Useful? React with 👍 / 👎.

Followup to 7bb1eb6 (npm install -> npm ci for the oxc-validator
runtime install in studio/setup.sh, studio/setup.ps1). That commit
worked locally because the lockfile already existed there from an
earlier `npm install`, but `npm ci` failed in CI because the
lockfile was never committed:

npm error EUSAGE
npm error The `npm ci` command can only install with an existing
npm error package-lock.json or npm-shrinkwrap.json with lockfileVersion >= 1.

Root cause: the project-root .gitignore has a bare `package-lock.json`
entry left over from a Python-template gitignore. The frontend
lockfile was force-added past it; the oxc-validator lockfile never
was. So a fresh actions/checkout did not have it.

Fix:
Force-commit studio/backend/core/data_recipe/oxc-validator/package-lock.json
(5 packages, lockfileVersion 3, integrity-pinned).
Replace the bare gitignore rule with explicit `!` exceptions for
the two committed npm-project lockfiles, with a comment explaining
why stray lockfiles in random Python subtrees are still ignored.

The pyproject.toml package-data glob `backend/core/data_recipe/oxc-validator/*.json`
already pulls the lockfile into the pip-installed wheel; the only
gap was that fresh git checkouts (CI) didn't have it.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa93a49ae3

ℹ️ 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".

@@ -0,0 +1,799 @@
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wire the OXC lockfile into npm security scans

Adding this second npm lockfile makes oxc-parser/oxlint part of the setup-time install surface, but the existing security workflow still only watches and scans studio/frontend/package-lock.json: the PR path filter omits this file, lockfile_supply_chain_audit.py defaults to only the frontend lockfile, OSV is invoked only with the frontend lockfile, and scan_npm_packages.py is run with its frontend default. As a result, a PR that changes only the OXC lockfile can skip the npm lockfile/tarball checks that are supposed to run before any install uses the pinned dependencies.

Useful? React with 👍 / 👎.

Collapse the worked-example narrative to one-line WHYs. Code is
unchanged.
Two issues codex flagged:

1. bun.lock gate (studio/frontend/.gitignore line 14 ignores bun.lock,
so it is never committed). bun install --frozen-lockfile cannot
migrate from package-lock.json, so without a bun.lock the bun path
always fails. setup.sh then misclassifies that as a corrupt cache,
clears the user's bun cache, and re-runs the same guaranteed-failing
command before falling back to npm. build.sh, studio/setup.sh, and
studio/setup.ps1 now only enter the bun path when bun.lock is
present; otherwise we go straight to npm ci.

2. OXC validator lockfile was outside the npm supply-chain scan
surface. lockfile_supply_chain_audit.py default, npm audit, OSV,
scan_npm_packages.py invocation, and the diff-for-new-install-scripts
step all now cover both lockfiles. security-audit.yml pull_request
paths filter triggers on changes to either. wheel-smoke checks the
built wheel ships the OXC lockfile too.

Verified:
python3 scripts/lockfile_supply_chain_audit.py
> OK: 0 findings across 2 npm + 1 cargo lockfile(s)
python3 scripts/scan_npm_packages.py --lockfile oxc-validator/...
> OK
@danielhanchen

Copy link
Copy Markdown
Member Author

Addressing the codex review:

Source Issue Resolution
codex P1 (resolved) OXC validator has no committed lockfile, so npm ci would exit before installing fa93a49 — committed studio/backend/core/data_recipe/oxc-validator/package-lock.json (force-added past the bare package-lock.json rule in the repo-root .gitignore via explicit ! exceptions for the two real npm projects)
codex P2 bun install --frozen-lockfile cannot migrate from package-lock.json; without a committed bun.lock the bun path always fails, then the corrupt-cache retry clears the user's bun cache and re-runs the same guaranteed-failing command 0ddfc10 — gated the bun path on [ -f bun.lock ] in build.sh and studio/setup.sh, and Test-Path "bun.lock" in studio/setup.ps1. Without a committed bun.lock we go straight to npm ci. If a maintainer later commits a bun.lock, the bun fast path re-engages automatically with --frozen-lockfile.
codex P2 OXC lockfile outside the npm supply-chain scan surface 0ddfc10 — added the OXC lockfile to lockfile_supply_chain_audit.py DEFAULT_NPM_LOCKFILES (audit now reports 2 npm + 1 cargo), a second npm audit step in security-audit.yml, a second OSV --lockfile= flag, a second scan_npm_packages.py --lockfile=... invocation, a second check_new_install_scripts.py diff run for PR-triggered events, the PR paths filter, and the wheel-smoke lockfile shipped check

Verified locally:

  • python3 scripts/lockfile_supply_chain_audit.pyOK: 0 findings across 2 npm + 1 cargo lockfile(s)
  • python3 scripts/scan_npm_packages.py --lockfile studio/backend/core/data_recipe/oxc-validator/package-lock.json → OK
  • npm ci --no-fund --no-audit --dry-run clean against both lockfiles

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ddfc102c1

ℹ️ 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".

Comment on lines +941 to +944
LOG2=logs-scan-npm-oxc.txt
python3 scripts/scan_npm_packages.py \
--lockfile studio/backend/core/data_recipe/oxc-validator/package-lock.json \
2>&1 | tee "$LOG2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Continue to the OXC scan after frontend findings

GitHub documents default bash run steps as fail-fast (set -e, with -o pipefail for bash), and this step also enables pipefail explicitly. If the frontend tarball scan exits 1/2 for a HIGH finding or scanner error, the step exits at the previous pipeline before reaching this newly added OXC scan, so the OXC lockfile is not scanned in exactly those runs. Capture the first status, run the OXC scan, then fail after both results are collected.

Useful? React with 👍 / 👎.

@danielhanchen

Copy link
Copy Markdown
Member Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the build and setup scripts to enforce strict lockfile usage by switching to "bun install --frozen-lockfile" and "npm ci". It also introduces a new lockfile for the oxc-validator component, updates the supply chain audit script to include it, and modifies .gitignore to track these specific lockfiles. The review feedback correctly identifies that the scripts should check for both "bun.lock" and "bun.lockb" to maintain compatibility with the binary lockfile format used by Bun versions prior to 1.2, preventing unnecessary fallbacks to npm.

Comment thread build.sh
_install_ok=false
if command -v bun &>/dev/null; then
if bun install; then
if [ -f bun.lock ] && command -v bun &>/dev/null; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check for bun.lock should also include bun.lockb. Bun versions prior to 1.2 use the binary bun.lockb format by default. If a project is using the binary format, this check will fail, and the script will fall back to the slower npm ci path even if Bun is available and a lockfile is present.

Suggested change
if [ -f bun.lock ] && command -v bun &>/dev/null; then
if ([ -f bun.lock ] || [ -f bun.lockb ]) && command -v bun &>/dev/null; then

Comment thread studio/setup.ps1 Outdated
# Only use bun when a committed bun.lock is present. bun install
# --frozen-lockfile cannot migrate from package-lock.json, so without
# bun.lock the bun path would always fail.
$UseBun = ($null -ne (Get-Command bun -ErrorAction SilentlyContinue)) -and (Test-Path "bun.lock")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The check for bun.lock should be expanded to include bun.lockb to support Bun's default binary lockfile format (standard in versions < 1.2). Without this, users with existing bun.lockb files will be forced into the npm ci fallback path.

    $UseBun = ($null -ne (Get-Command bun -ErrorAction SilentlyContinue)) -and ((Test-Path "bun.lock") -or (Test-Path "bun.lockb"))

Comment thread studio/setup.sh
# only enter the bun path when a committed bun.lock exists. Without it,
# bun would fail every time and the corrupt-cache retry would clear the
# user's bun cache for nothing.
if [ -f bun.lock ] && command -v bun &>/dev/null; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To ensure compatibility with all Bun versions, the script should check for both bun.lock (text format, Bun 1.2+) and bun.lockb (binary format, Bun < 1.2). Currently, projects using the binary format will skip the Bun installation path entirely.

Suggested change
if [ -f bun.lock ] && command -v bun &>/dev/null; then
if ([ -f bun.lock ] || [ -f bun.lockb ]) && command -v bun &>/dev/null; then

Reviewer found a real bug in 0ddfc10: the new two-scan step ran under
`set -o pipefail` (default `set -e` from the step shell), so a HIGH or
CRITICAL on the frontend lockfile would abort the step before the OXC
scan ran. Both reports are most useful exactly when one has already
failed.

Capture each rc via PIPESTATUS, run both scans unconditionally, write
both into the step summary, and only then propagate the worst rc.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 507be43849

ℹ️ 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".

Comment thread studio/setup.sh
# only enter the bun path when a committed bun.lock exists. Without it,
# bun would fail every time and the corrupt-cache retry would clear the
# user's bun cache for nothing.
if [ -f bun.lock ] && command -v bun &>/dev/null; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require bun.lock to be tracked before using Bun

When a checkout already has a local bun.lock generated by the previous bun install path, this test treats that untracked file as the authoritative lockfile. If it is stale but still satisfies package.json, bun install --frozen-lockfile succeeds and the script skips npm ci, so the install no longer matches the committed package-lock.json this change is trying to pin to; the same presence-only check was added in build.sh and setup.ps1. Check that bun.lock is actually tracked/packaged, or ignore local Bun lockfiles and fall back to npm.

Useful? React with 👍 / 👎.

@danielhanchen

Copy link
Copy Markdown
Member Author

Addressing the 10x reviewer run (commit 507be43).

Only the P2 finding is acted on per the user's earlier direction to focus on P1s/P2s. The three P3 comment-accuracy items (the "bun cannot migrate from package-lock.json under --frozen-lockfile" wording) are technically correct that local Bun 1.3.11 does migrate — the defensible policy is still "skip Bun when no bun.lock is committed so a release install never silently generates one", but the comment phrasing could be reworked in a follow-up.

# Pri Finding Fix
1 P2 OXC tarball scan was skipped when the frontend scan exited nonzero. bash -e + set -o pipefail aborts the step on the first failed pipeline, so a HIGH/CRITICAL on the frontend lockfile would skip the new OXC scan entirely — exactly when both reports are most useful. 507be43set +e, capture each scan's rc via PIPESTATUS, run both scans + write both summaries, then propagate the worst rc at the end

Smoke-tested the new pipeline locally: a failing first scan no longer aborts the step before the OXC scan runs, both step-summary sections render, and the final exit returns the worst of the two rcs.

Two blockers from the parallel Opus review batch:

1. The Tauri CLI install in release-desktop.yml was the last unfrozen
install path: `npm install --save-dev --prefix studio
@tauri-apps/cli@2.10.1 --no-fund --no-audit` pins the top-level
version but leaves the transitive closure floating, defeats the
pre-install lockfile audit (no lockfile to scan), and skips
integrity verification. Committed a minimal studio/package.json
(devDep @tauri-apps/cli@2.10.1) plus the resolved
studio/package-lock.json (12 packages: CLI + 11 platform-native
binaries, all with integrity hashes, lockfileVersion 3). Switched
the step to `npm ci --prefix studio` and added a pre-install
lockfile_supply_chain_audit.py step ahead of it so any tarball
postinstall is gated by the structural scan. Allowlisted
studio/package-lock.json .gitignore and added it to the
audit script's default scan set.

2. The bun branch was dead code in build.sh, studio/setup.sh, and
studio/setup.ps1: nowhere in the repo is a bun.lock committed,
and `bun install --frozen-lockfile` cannot migrate from
package-lock.json. With no lockfile, every entry to the bun
path either silently regenerates a bun.lock (under permissive
install modes -- a fresh attack surface) or fails outright (under
frozen-lockfile). Removed `npm install -g bun` bootstrap, the
`_try_bun_install` helper + cache-retry, every `if bun.lock &&
command -v bun` guard, and the now-unreachable
"fall back to npm" messaging. All three scripts now have a
single `npm ci` path. bun.lock skip entries in lint-ci.yml +
wheel-smoke.yml are kept as forward-compat sanity checks --
they assert bun.lock is NOT shipped / scanned, which is
stronger after this commit, not weaker.

Smoke-tested locally:
`npm ci --prefix studio` resolves 3 packages (CLI + 2 linux native
binaries), `npx --prefix studio tauri --version` prints
`tauri-cli 2.10.1` exactly.
`python3 scripts/lockfile_supply_chain_audit.py` scans 3 npm + 1
cargo lockfiles, 0 findings.
`bash -n build.sh`, `bash -n studio/setup.sh`, and a pwsh
scriptblock parse of studio/setup.ps1 all succeed.
@danielhanchen

Copy link
Copy Markdown
Member Author

Addressing the 4x Opus reviewer batch (commit 970aaf8).

Per the earlier direction, P1s + P2s only.

# Pri Finding Fix
1 P1 The Tauri CLI install in release-desktop.yml was the last unfrozen install path: npm install --save-dev --prefix studio @tauri-apps/cli@2.10.1 pins the top-level version but leaves the transitive closure floating, defeats lockfile_supply_chain_audit.py (no lockfile to scan), and skips integrity verification. Committed a minimal studio/package.json (devDep @tauri-apps/cli@2.10.1) plus the resolved studio/package-lock.json (12 packages: CLI + 11 platform-native binaries, all with integrity hashes, lockfileVersion: 3). Workflow step switched to npm ci --prefix studio. Added a pre-install lockfile_supply_chain_audit.py step ahead of it so any tarball postinstall is gated by the structural scan. studio/package-lock.json is now allowlisted in .gitignore and is in the audit script's default scan set.
2 P2 The bun branch was dead code in build.sh, studio/setup.sh, and studio/setup.ps1: there is no bun.lock anywhere in the repo, and bun install --frozen-lockfile cannot migrate from package-lock.json. With no lockfile committed, the bun path either silently regenerates a bun.lock (a fresh attack surface for the next release install) or fails outright under --frozen-lockfile. Removed the npm install -g bun bootstrap, the _try_bun_install helper + cache-retry, every if bun.lock && command -v bun guard, the corresponding PowerShell $UseBun block, and the now-unreachable "fall back to npm" messaging. All three scripts have a single npm ci path. The bun.lock skip entries in lint-ci.yml + wheel-smoke.yml are kept — they assert bun.lock is NOT shipped or scanned, which is stronger after this commit.

Smoke-tested locally:

  • npm ci --prefix studio resolves 3 packages on Linux (CLI + 2 platform-native binaries that match linux-x64-gnu); npx --prefix studio tauri --version prints exactly tauri-cli 2.10.1.
  • python3 scripts/lockfile_supply_chain_audit.py now scans 3 npm + 1 cargo lockfiles, 0 findings.
  • bash -n build.sh, bash -n studio/setup.sh, and a pwsh scriptblock parse of studio/setup.ps1 all succeed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 970aaf8b28

ℹ️ 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".

Comment on lines +56 to +57
- 'studio/backend/core/data_recipe/oxc-validator/package.json'
- 'studio/backend/core/data_recipe/oxc-validator/package-lock.json'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add the Tauri CLI lockfile to npm audit triggers

This paths block now watches the new OXC npm project but still omits studio/package.json and studio/package-lock.json, even though the desktop release workflow now installs that project with npm ci --prefix studio. A PR that changes only the Tauri CLI holder or its lockfile will not trigger security-audit.yml, so the lockfile structural audit, OSV scan, npm tarball scan, and install-script diff can all be skipped for the newly pinned release-time install surface.

Useful? React with 👍 / 👎.

@danielhanchen danielhanchen added the auto-reviewing Auto-review in progress label May 16, 2026
Brings the parallel CI paths into line with the lockfile-pinned
release path and tightens the supply-chain audit surface:

studio-tauri-smoke.yml: run lockfile_supply_chain_audit.py before
the Tauri CLI install, and install via `npm ci --prefix studio`
against the committed studio/package-lock.json (was a mutable
`npm install --save-dev` post-audit). This relocates the existing
pre-install lockfile supply-chain audit step; the step's name and
command are preserved verbatim so its purpose is unchanged, only
its position relative to the install. The earlier security
rationale about lifecycle scripts and the postinstall-dropper
class is preserved on the Frontend build step where it actually
applies (vite/esbuild lifecycle scripts run on the frontend
install); the Tauri CLI install step gets a new rationale tied to
`npm ci` semantics.

security-audit.yml:
  * add studio/package.json and studio/package-lock.json to the PR
    path filter so a Tauri CLI lockfile change cannot bypass the
    workflow,
  * extend OSV-Scanner, scan_npm_packages.py (with LOG3 and exit-
    code propagation), and the install-script diff to cover
    studio/package-lock.json,
  * add an npm audit step for the Tauri CLI holder project,
  * extend the npm-provenance-and-install-scripts job with
    --ignore-scripts installs + npm audit signatures for the
    oxc-validator and Tauri CLI holder projects; the existing
    frontend audit-signatures step is renamed to "(Studio
    frontend, informational)" purely for disambiguation against
    the two new sibling steps, with its log path rerouted through
    $GITHUB_WORKSPACE so a single artifact upload can collect all
    three logs,
  * update the lockfile-audit step summary to list the Tauri CLI
    holder lockfile,
  * fix the stale "Initially non-blocking" comment on the now-
    blocking npm scan-packages step.

build.sh and studio/setup.ps1 (oxc): pass --no-fund --no-audit to
npm ci for parity with the other call sites.

studio/setup.sh and studio/setup.ps1: restore the bun.lock
exclusion in the frontend staleness check so a leftover local
bun.lock from the migration does not trigger a spurious rebuild.

scripts/lockfile_supply_chain_audit.py: emit a HIGH-severity
missing-lockfile Finding when a requested lockfile does not
exist, so a deleted default cannot silently pass the audit. Uses
the script's own Finding accumulator pattern (sibling
scripts/scan_npm_packages.py implements the same intent via an
rc=2 hard-fail, its single-lockfile-per-invocation idiom; this
script aggregates multiple lockfiles so Finding is the natural
channel).

scripts/check_frontend_dep_removal.py: add studio/package.json
and studio/package-lock.json to EXPECTED_NOISE_FILES; the new
Tauri CLI holder manifests must not count as frontend dep usage.
When a caller passes an explicit --npm-lockfile or --cargo-lockfile,
they are scoping the scan to the paths they listed; the script was
still silently grafting the other ecosystem's defaults on top, which
meant `--npm-lockfile X` would also audit DEFAULT_CARGO_LOCKFILES.
With the missing-lockfile Finding now emitted, that surfaced as a
false positive whenever a caller explicitly scoped only one
ecosystem. Default fallback is now reserved for the no-args CI
invocation, where every default path is expected to exist.
Collapse 4-6 line "why this matters" blocks to 1-2 lines stating the
single load-bearing fact in lockfile_supply_chain_audit.py (missing-
lockfile finding rationale, CLI default-scoping rationale) and in
studio-tauri-smoke.yml (pre-install audit ordering, npm ci semantics,
frontend lifecycle-script context). No behaviour change.
@danielhanchen danielhanchen force-pushed the ci/frozen-lockfile-installs branch from 970aaf8 to 7238504 Compare May 16, 2026 13:49

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7238504b47

ℹ️ 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".

Comment thread pyproject.toml Outdated
"unsloth[intelgputorch271]"
]
intelgputorch291 = [
intelgputorch210 = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore the removed Intel GPU extras

For users installing Intel/XPU-specific extras such as unsloth[intel-gpu-torch291] or unsloth[intel-gpu-torch2110], this hunk removes those optional-dependency keys entirely from the package metadata; the table now jumps from intel-gpu-torch290 to intelgputorch210. Missing extras are not treated as aliases to adjacent versions by packaging tools, so those install commands no longer pull the pinned torch/triton wheels they previously selected and instead lose the requested accelerator stack. This is unrelated to the lockfile hardening, so keep the published extras or replace them with explicit aliases.

Useful? React with 👍 / 👎.

@danielhanchen danielhanchen removed the auto-reviewing Auto-review in progress label May 16, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

Auto-review verdict: Approved

Converts Studio's frontend, OXC, and desktop Tauri install paths to lockfile-strict npm ci with three committed lockfiles and extends the security-audit pipeline (paths, OSV, scan_npm_packages, install-script diff, npm audit, audit signatures) to cover all three; legitimately closes the npm postinstall-dropper class of attack on the signed-desktop release path.

Reason: All P1/P2 review findings fixed; supply-chain hardening surface now symmetric across release, smoke, and audit paths.

@danielhanchen danielhanchen added the auto-approved Auto-review approved the PR label May 16, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c244a9d0f5

ℹ️ 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".

run: npm install --save-dev --prefix studio @tauri-apps/cli@2.10.1 --no-fund --no-audit
# `npm ci` resolves @tauri-apps/cli from studio/package-lock.json
# (transitives fully pinned, integrity hashes verified).
run: npm ci --prefix studio --no-fund --no-audit

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include Tauri lockfile changes in smoke trigger

This workflow now installs and verifies the Tauri CLI from studio/package-lock.json, but the pull_request.paths block above still only watches studio/frontend/**, studio/src-tauri/**, and unsloth_cli/**. A PR that changes only studio/package.json or studio/package-lock.json can therefore alter or break the CLI used by this smoke build without running the workflow that would catch it; add those two files to this workflow's path filter.

Useful? React with 👍 / 👎.

@danielhanchen danielhanchen marked this pull request as draft May 18, 2026 11:20
The merge of main into ci/frozen-lockfile-installs in 888347d resolved
pyproject.toml against the pre-merge branch state, silently dropping the
intel-gpu xpu extras that landed on main in f7069b2 and were pinned in
e20bbef (#5499) after this branch was created. None of the lockfile-pin
feature commits touch pyproject.toml; this restore reverts only the
unintended deletion and keeps the supply-chain hardening intact.

Restored extras:
  intelgputorch271 + intel-gpu-torch271
  intelgputorch291 + intel-gpu-torch291
  intelgputorch2110 + intel-gpu-torch2110
  intelgputorch2120 + intel-gpu-torch2120

Restored the triton-xpu 3.6.0 pin section inside intelgputorch210.
danielhanchen and others added 7 commits May 19, 2026 01:42
Opus reviewer 3 caught a crash path: when a lockfile is unreadable
(chmod 000, permission denied, is-a-directory, broken pipe, etc.)
the audit script bubbles up a raw Python traceback and exits 1 with
no Finding emitted. In CI, a transient permission or encoding error
is indistinguishable from a malicious lockfile -- the maintainer reads
the log and sees noise, not a structured diagnosis.

Wraps both path.read_text() calls (npm and cargo branches) in a
try/except OSError that appends a new Finding kind
"unreadable-lockfile" with the original errno detail, mirroring how
missing-lockfile is handled today. Backward compatible (additive);
all 24 existing audit fixtures still pass.

Verified by fixture: chmod 000 on a lockfile -> exit 1 with
[unreadable-lockfile] finding, no traceback.
Adds bun.lock alongside the existing package-lock.json for:
  studio/                                              (Tauri CLI holder, 13 pkgs)
  studio/backend/core/data_recipe/oxc-validator/       (49 pkgs)
  studio/frontend/                                     (1090 pkgs)

Generated with `bun install --lockfile-only --save-text-lockfile`
against Bun 1.3.11. Each lockfile carries sha512 integrity entries
and full platform-conditional optionalDependencies coverage for
linux/darwin/win32 x64/arm64/musl variants.

Updates .gitignore to mirror the existing package-lock.json opt-in
pattern: a top-level `bun.lock` ignore with negations for the three
surfaces, plus removal of the legacy `bun.lock` entry in
studio/frontend/.gitignore.

This unlocks `bun install --frozen-lockfile` as the fast path in
setup.sh / setup.ps1 / build.sh (follow-up commit).
PR 5479 dropped Bun support because no bun.lock was committed. With
the bun.lock files now committed for all three Studio install
surfaces (previous commit), Bun --frozen-lockfile becomes a
viable fast path again -- ~5-10x faster than npm ci in practice
(measured locally: frontend 1.4s vs 8s, oxc 35ms vs 371ms, studio
14ms vs 444ms).

Install logic per surface (studio/setup.sh, studio/setup.ps1,
build.sh):

  1. If bun.lock exists AND `bun` is on PATH:
       try `bun install --frozen-lockfile`
       verify critical binaries are present (tsc + vite for frontend,
       oxc-parser for oxc validator) -- workaround for the known
       bun-cache-corruption bug where install can exit 0 but leave
       binaries missing
       on validation failure: rm -rf node_modules + clear bun cache,
       fall through to npm ci
  2. Else (no bun.lock, no bun, or bun failed):
       `npm ci` against the committed package-lock.json

Both paths run lockfile-strict, so the install is byte-reproducible
from whichever lockfile the chosen package manager understands. The
build always runs through Node (`npm run build`) -- avoids bun
runtime quirks on some platforms.

Bun auto-install via `npm install -g bun` is NOT restored: a user
who wants the speed-up installs Bun themselves, and the npm ci path
remains the default-available install route. This matches the
upstream Studio docs' install instructions.
Adds audit_bun_lockfile() that parses Bun's text lockfile format
(bun.lock, lockfileVersion 1) and applies the same supply-chain
checks already used on npm and cargo lockfiles:

  - non-registry-resolved-url   (git+/file:/tarball sources)
  - missing-integrity-hash      (no sha-prefixed tail on registry entry)
  - blocked-known-malicious     (BLOCKED_NPM_VERSIONS hit)
  - known-ioc-string            (IOC substring in raw body)
  - missing-lockfile            (path doesn't exist)
  - unreadable-lockfile         (chmod 000 / OSError)
  - malformed-lockfile          (JSONC parse failure)
  - unsupported-lockfile-version (anything != 1)

bun.lock is JSONC (valid JSON with trailing commas allowed); the
parser strips trailing commas via a single regex before json.loads.
Each package entry is a 4-element array:
  ["name@version", "<registry-url-or-empty>", {metadata}, "sha512-..."]
Default scope adds the three bun.lock paths committed alongside
the three package-lock.json paths. New repeatable --bun-lockfile
flag mirrors --npm-lockfile and --cargo-lockfile.

Verified locally:
  positive (3 npm + 3 bun + 1 cargo, real PR lockfiles): exit 0
  unreadable bun.lock (chmod 000): exit 1, [unreadable-lockfile]
  malformed bun.lock: exit 1, [malformed-lockfile]
  missing bun.lock:  exit 1, [missing-lockfile]
@danielhanchen

Copy link
Copy Markdown
Member Author

Superseded by #5616: clean re-derivation off origin/main with strict scope (no bun.lock, no unrelated reverts, no policy flips). The bun-side changes from this PR have been deferred per the maintainer decision to not commit bun.lock yet — when bun.lock eventually lands, the install scripts in #5616 auto-tighten because every bun call is gated on [ -f bun.lock ] && command -v bun with --frozen-lockfile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-approved Auto-review approved the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants