fix: lock promptfoo transitive dependencies to prevent CI breakage#1115
fix: lock promptfoo transitive dependencies to prevent CI breakage#1115
Conversation
npx -y promptfoo@0.121.3 resolves transitive deps fresh on every run, so a bad patch release (like @asamuzakjp/css-color@5.1.5 introducing top-level await) can break the Skill Trigger Eval CI job unpredictably. Add package.json + package-lock.json in evals/promptfoo/ to lock the full dependency tree. The eval script now runs npm ci before invoking promptfoo, and the dependency audit scans the npm lockfile alongside deno.lock for vulnerabilities. Closes #1110 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CI Security Review
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — The only workflow change replaces a grep-based version extraction with jq -r '.dependencies.promptfoo' evals/promptfoo/package.json in the deps-audit job's outdated-npm-dependencies check (ci.yml:110). This is security-neutral: no new actions, no permission changes, no secret handling changes, no prompt modifications, and no untrusted data interpolation. The remaining file changes (.gitignore, deno.json excludes, package.json/package-lock.json, and TypeScript scripts) do not affect workflow security.
Deno requires all package.json files under the workspace root to be listed as workspace members, even if excluded from lint/fmt/test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The lockfile is generated on macOS but CI runs on Linux. npm ci fails when platform-specific optional deps (like @img/sharp-linux-*) are missing from the lockfile. npm install respects the lockfile for version resolution while handling cross-platform optional deps gracefully. Also adds --ignore-scripts since native addon compilation (sharp, better-sqlite3) is not needed — only promptfoo's JS files are used. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CI Security Review
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — The only workflow change is in ci.yml:110-113, replacing a grep extraction of the pinned promptfoo version with a jq read from evals/promptfoo/package.json. This is security-neutral: no new triggers, no new secrets, no new expression interpolations, no changes to LLM prompts or tool scoping, and no supply chain changes. The non-workflow files (scripts, lockfiles, gitignore, deno.json) do not affect CI security posture.
promptfoo requires better-sqlite3's native binding at runtime for database migrations. --ignore-scripts prevented it from compiling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add --package-lock=false so local runs and CI don't produce unwanted diffs to package-lock.json when npm versions or platforms differ. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
CI Security Review
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — The only workflow change is in .github/workflows/ci.yml lines 108-116, replacing a grep-based version extraction with jq on package.json and updating the warning message text. This is a security-neutral refactor with no new expressions, triggers, actions, secrets usage, or tool scoping changes. All other changed files (deno.json, .gitignore, package.json, package-lock.json, scripts) are non-workflow files with no CI security impact.
Pre-existing workflow security posture (not introduced by this PR) remains unchanged: permissions are job-scoped, LLM prompts include security preambles, Bash tools are tightly scoped, fork PRs are excluded from auto-merge, and third-party actions use SHA pins.
There was a problem hiding this comment.
CI Security Review
Critical / High
None.
Medium
None.
Low
None.
Verdict
PASS — The only workflow change is in the deps-audit job (ci.yml:110-113), switching from grep-based version extraction to jq-based extraction from a committed package.json. No GitHub event fields are interpolated, no permissions changed, no action pins modified, and no LLM prompts altered. The move from npx -y promptfoo@0.121.3 to npm install with a committed package-lock.json is a minor supply-chain improvement (integrity hashes pin the dependency tree).
There was a problem hiding this comment.
Code Review
Blocking Issues
None.
Suggestions
-
Hardcoded lockfile path in audit script (
scripts/audit_deps.ts:339): ThenpmLockfilesarray is hardcoded to["evals/promptfoo/package-lock.json"]. If more npm lockfiles are added in the future, consider discovering them via glob. Not urgent — a single entry is fine for now and easy to extend later. -
Minor: npm lockfile chain tracing (
scripts/audit_deps.ts:368-372): The comment notes that npm lockfile packages are reported without dependency chains (only deno.lock packages get chain tracing). This is a reasonable tradeoff for now, but could be enhanced later to also trace chains within the npm lockfile's dependency tree.
Overall this is a clean, well-documented fix for a real CI stability problem. The cross-platform rationale for npm install --package-lock=false over npm ci is clearly explained in the code comments. Types are strict, promises are all awaited, no libswamp boundary violations, and no security concerns.
Summary
package.json+package-lock.jsoninevals/promptfoo/to lock the full promptfoo dependency tree, preventing transitive dependency breakage like the@asamuzakjp/css-color@5.1.5incident (#1110 comment)npm cibefore invoking promptfoo instead ofnpx -y promptfoo@0.121.3scripts/audit_deps.tsto scan the npm lockfile for vulnerabilities alongsidedeno.lockpackage.jsonviajqTest Plan
deno check,deno lint,deno fmt --checkall passnpm ci && npx promptfoo --versionworks correctly inevals/promptfoo/deno.lock(99) andpackage-lock.json(984)css-color@5.1.5reproduces theERR_REQUIRE_ASYNC_MODULEerror and5.1.6(locked in our lockfile) does notCloses #1110