Skip to content

refactor: remove unused code/deps, configure knip#7117

Open
serhalp wants to merge 11 commits into
mainfrom
serhalp/refactor/remove-unused-and-configure-knip
Open

refactor: remove unused code/deps, configure knip#7117
serhalp wants to merge 11 commits into
mainfrom
serhalp/refactor/remove-unused-and-configure-knip

Conversation

@serhalp

@serhalp serhalp commented Jun 26, 2026

Copy link
Copy Markdown
Member

Summary

This PR adds https://knip.dev/, a fantastic tool that reliably flags unused code and dependencies, and configures it to run in CI.

It also removes what it flagged as unused.

Note that I already did a manual pass last year: #6097.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added automated unused-code checks in pull requests.
    • Added a new guide for cleaning up unused files, dependencies, and exports.
  • Bug Fixes

    • Updated build and validation rules to better support the newer bundle format.
    • Fixed test server startup and test setup path resolution.
  • Chores

    • Simplified several package configurations and CI installs.
    • Removed outdated helpers and exports, and refreshed project metadata.

Walkthrough

This PR integrates knip for unused-code detection across the monorepo. It adds a knip.jsonc root configuration with per-workspace entry/ignore rules, a knip npm script and devDependency, a new GitHub Actions workflow that runs knip on pull requests, and a skill guide for AI agents. As a result of the initial knip run, a broad set of dead code is removed: unused exports and types across build, config, edge-bundler, and zip-it-and-ship-it packages; the BundleFormat.JS enum member and related schema/test fixtures; and per-package typescript devDependencies now hoisted to the root. The conventional-commit CI workflow is also updated to use npm ci.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • pieh
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description has a summary, but it omits the required issue reference, motivation, and checklist sections from the template. Add the Fixes #issue reference, a short motivation paragraph, and the checklist items required by the template.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: removing unused code/deps and configuring knip.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch serhalp/refactor/remove-unused-and-configure-knip

Comment @coderabbitai help to get the list of available commands.

"@vitest/ui": "^3.0.0",
"execa": "^8.0.0",
"memfs": "^4.0.0",
"typescript": "^5.0.0",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

typescript was assumed to be hoisted in the root but was declared instead as a dev dep in many (but not all) individual packages. I centralized it as a dev dep in the root.

"execa": "^8.0.0",
"memfs": "^4.0.0",
"typescript": "^5.0.0",
"ts-node": "^10.9.2",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ts-node was missing from this package; it just happened to work due to hoisting

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

⚠️ Dependency Count

This PR adds 20 new dependencies (1556 → 1576), which exceeds the threshold of 10.

📊 Dependency Size Changes

Warning

This PR adds 19.9 MB of new dependencies, which exceeds the threshold of 100 kB.

📦 Package 📏 Size
@napi-rs/wasm-runtime@1.1.6 6.3 MB
@oxc-parser/binding-linux-x64-gnu@0.137.0 2.4 MB
@oxc-resolver/binding-linux-x64-gnu@11.21.3 2.4 MB
@emnapi/core@1.11.1 2 MB
@emnapi/core@1.11.0 2 MB
knip@6.21.0 1.7 MB
oxc-parser@0.137.0 1.4 MB
@tybys/wasm-util@0.10.3 825.1 kB
@emnapi/runtime@1.11.1 433.5 kB
@emnapi/runtime@1.11.0 433.5 kB
@emnapi/wasi-threads@1.2.2 225.1 kB
unbash@4.0.1 180.9 kB
import-meta-resolve@4.2.0 -79.2 kB
@commitlint/rules@19.8.1 -70.6 kB
oxc-resolver@11.21.3 60.2 kB
lodash.mergewith@4.6.2 -54.2 kB
jiti@2.6.1 → jiti@2.7.0 50.5 kB
@oxc-project/types@0.137.0 44.5 kB
@commitlint/load@19.8.1 -36.3 kB
@commitlint/cli@19.8.1 -32.5 kB
conventional-commits-parser@5.0.0 -29.9 kB
tar@7.5.16 → tar@7.5.19 23.2 kB
tinyexec@1.2.4 -22.9 kB
formatly@0.3.0 21.6 kB
split2@4.2.0 -17.4 kB
@commitlint/resolve-extends@19.8.1 -17 kB
@commitlint/lint@19.8.1 -14 kB
cosmiconfig-typescript-loader@6.3.0 -13.9 kB
@commitlint/read@19.8.1 -13.2 kB
ini@4.1.1 -12.7 kB
dargs@8.1.0 -12.3 kB
@commitlint/config-validator@19.8.1 -12.1 kB
@commitlint/ensure@19.8.1 -12 kB
@commitlint/format@19.8.1 -10.5 kB
strip-json-comments@5.0.3 8.2 kB
global-directory@4.0.1 -8.1 kB
git-raw-commits@4.0.0 -7.9 kB
fd-package-json@2.0.0 7.7 kB
@commitlint/is-ignored@19.8.1 -6.9 kB
text-extensions@2.4.0 -6.3 kB
@commitlint/parse@19.8.1 -4.6 kB
@commitlint/execute-rule@19.8.1 -3.9 kB
@commitlint/top-level@19.8.1 -3.7 kB
is-text-path@2.0.0 -2.9 kB
@types/is-ci@3.0.4 -2.9 kB
@commitlint/to-lines@19.8.1 -2.7 kB
@commitlint/message@19.8.1 -2.7 kB
js-yaml@3.14.2 → js-yaml@3.15.0 1.2 kB
p-map@7.0.4 → p-map@7.0.5 490 B
nock@14.0.15 → nock@14.0.16 393 B
@typescript-eslint/types@8.62.0 → @typescript-eslint/types@8.62.1 0 B
@typescript-eslint/visitor-keys@8.62.0 → @typescript-eslint/visitor-keys@8.62.1 0 B
@typescript-eslint/project-service@8.62.0 → @typescript-eslint/project-service@8.62.1 0 B
@typescript-eslint/tsconfig-utils@8.62.0 → @typescript-eslint/tsconfig-utils@8.62.1 0 B
@typescript-eslint/typescript-estree@8.62.0 → @typescript-eslint/typescript-estree@8.62.1 0 B
lodash.isplainobject@4.0.6 Unknown
lodash.kebabcase@4.1.1 Unknown
lodash.snakecase@4.1.1 Unknown
lodash.startcase@4.4.0 Unknown
lodash.uniq@4.5.0 Unknown
lodash.upperfirst@4.3.1 Unknown

Total size change: 19.9 MB

"@types/lodash-es": "^4.17.6",
"@types/node": "^22.12.0",
"ava": "^5.0.0",
"c8": "^10.0.0",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

c8 wasn't used in this package

Comment thread package.json
Comment on lines -49 to -50
"@commitlint/cli": "^19.0.0",
"@commitlint/config-conventional": "^19.0.0",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We removed husky in #6464 and orphaned this

Comment thread package.json
"@commitlint/config-conventional": "^19.0.0",
"@eslint/compat": "^1.2.9",
"@eslint/js": "^9.28.0",
"@typescript-eslint/eslint-plugin": "~8.49.0",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not needed when using typescript-eslint

@serhalp serhalp force-pushed the serhalp/refactor/remove-unused-and-configure-knip branch from d601446 to dd29887 Compare June 26, 2026 22:56
@github-actions

Copy link
Copy Markdown
Contributor

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

@serhalp serhalp force-pushed the serhalp/refactor/remove-unused-and-configure-knip branch from b3bd4e2 to 8b3c43a Compare June 29, 2026 20:46
serhalp added 9 commits June 30, 2026 07:15
- c8 wasn't used in that package
- typescript was assumed to be hoisted in the root but was declared instead as a dev dep in many
  (but not all) individual packages
- ts-node was missing from one package, just happened to work due to hoisting
This was left over from the deletion in a00e0ea.
@serhalp serhalp force-pushed the serhalp/refactor/remove-unused-and-configure-knip branch from 8b3c43a to 7b78ce3 Compare June 30, 2026 11:15
@@ -1,6 +1,5 @@
export enum BundleFormat {
ESZIP2 = 'eszip2',
JS = 'js',

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Leftover deletion from 2022 removal in a00e0ea

@serhalp serhalp force-pushed the serhalp/refactor/remove-unused-and-configure-knip branch from 54f81b5 to f496251 Compare June 30, 2026 11:24
@serhalp serhalp marked this pull request as ready for review June 30, 2026 13:11
@serhalp serhalp requested a review from a team as a code owner June 30, 2026 13:11

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🧹 Nitpick comments (1)
packages/edge-bundler/node/validation/manifest/index.test.ts (1)

14-15: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a regression case for the removed js format.

The valid fixture now covers eszip2, but the invalid-format test still uses arbitrary foo; it would not fail if js were accidentally allowed again.

Suggested test addition
 describe('bundle', () => {
+  test('should throw on removed js format', () => {
+    const manifest = getBaseManifest()
+    manifest.bundles[0].asset = 'function.js'
+    manifest.bundles[0].format = 'js'
+
+    expect(() => validateManifest(manifest)).toThrowErrorMatchingSnapshot()
+  })
+
   test('should throw on additional property in bundle', () => {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/edge-bundler/node/validation/manifest/index.test.ts` around lines 14
- 15, The manifest validation tests need a regression case for the removed js
format, since the current invalid-format coverage only checks an arbitrary value
and would miss js being reallowed. Update the manifest validation test in
index.test.ts to explicitly assert that js is rejected, alongside the existing
eszip2 fixture coverage, using the manifest validation path exercised by the
current test cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/knip/SKILL.md:
- Around line 14-18: The knip skill guide is using a bare npx-based availability
check even though the repo already provides a lockfile-backed knip dependency
and npm script. Update the setup and command examples in SKILL.md to use the
local script path via npm run knip -- ... (or the repo’s equivalent script), and
remove the npx knip --version check so the documented workflow stays pinned to
the repo-local installation.

In @.github/workflows/conventional-commit.yml:
- Around line 10-12: The conventional-commit workflow currently runs npm ci
before ensuring a specific Node version is installed, so the job depends on
whatever Node/npm is preinstalled on the runner. Update the job to add
actions/setup-node before the Install Dependencies step, using the same Node
version/pinning approach as the other workflows in this repo, and keep the
existing actions/checkout step followed by setup-node and then npm ci.

In @.github/workflows/knip.yml:
- Around line 14-16: The Knip workflow’s checkout step is leaving credentials
persisted by default, which keeps the GitHub token in local git config during
pull_request builds. Update the existing actions/checkout step in the knip job
to disable credential persistence by setting persist-credentials to false. Keep
the change scoped to the checkout configuration so the rest of the workflow
behavior stays the same.

---

Nitpick comments:
In `@packages/edge-bundler/node/validation/manifest/index.test.ts`:
- Around line 14-15: The manifest validation tests need a regression case for
the removed js format, since the current invalid-format coverage only checks an
arbitrary value and would miss js being reallowed. Update the manifest
validation test in index.test.ts to explicitly assert that js is rejected,
alongside the existing eszip2 fixture coverage, using the manifest validation
path exercised by the current test cases.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d74fc97e-8f6d-4382-ad83-23d7b5a543c0

📥 Commits

Reviewing files that changed from the base of the PR and between 677f1bd and f496251.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • packages/edge-bundler/node/validation/manifest/__snapshots__/index.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (40)
  • .agents/skills/knip/SKILL.md
  • .claude/skills/knip
  • .github/workflows/conventional-commit.yml
  • .github/workflows/knip.yml
  • knip.jsonc
  • package.json
  • packages/build-info/package.json
  • packages/build-info/playwright.config.ts
  • packages/build-info/vitest.config.ts
  • packages/build/src/log/logger.ts
  • packages/build/src/plugins_core/frameworks_api/skew_protection.ts
  • packages/build/tests/unit/validate_edge_manifest/tests.js
  • packages/cache-utils/package.json
  • packages/config/package.json
  • packages/config/src/api/site_info.ts
  • packages/config/src/log/messages.js
  • packages/edge-bundler/node/bundle.ts
  • packages/edge-bundler/node/utils/sha256.ts
  • packages/edge-bundler/node/validation/manifest/index.test.ts
  • packages/edge-bundler/node/validation/manifest/schema.ts
  • packages/edge-bundler/package.json
  • packages/edge-bundler/shared/consts.ts
  • packages/functions-utils/package.json
  • packages/git-utils/package.json
  • packages/headers-parser/package.json
  • packages/js-client/package.json
  • packages/js-client/tsconfig.json
  • packages/nock-udp/package.json
  • packages/opentelemetry-sdk-setup/package.json
  • packages/opentelemetry-utils/package.json
  • packages/redirect-parser/package.json
  • packages/run-utils/package.json
  • packages/testing/package.json
  • packages/zip-it-and-ship-it/package.json
  • packages/zip-it-and-ship-it/src/config.ts
  • packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts
  • packages/zip-it-and-ship-it/src/runtimes/node/parser/index.ts
  • packages/zip-it-and-ship-it/src/runtimes/node/utils/entry_file.ts
  • packages/zip-it-and-ship-it/src/utils/timer.ts
  • skills-lock.json
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • netlify/blueprints (manual)
💤 Files with no reviewable changes (22)
  • packages/edge-bundler/node/bundle.ts
  • packages/edge-bundler/shared/consts.ts
  • packages/zip-it-and-ship-it/src/runtimes/node/utils/entry_file.ts
  • packages/headers-parser/package.json
  • packages/config/src/api/site_info.ts
  • packages/config/src/log/messages.js
  • packages/run-utils/package.json
  • packages/redirect-parser/package.json
  • packages/cache-utils/package.json
  • packages/zip-it-and-ship-it/src/runtimes/node/in_source_config/index.ts
  • packages/zip-it-and-ship-it/src/utils/timer.ts
  • packages/opentelemetry-utils/package.json
  • packages/edge-bundler/node/utils/sha256.ts
  • packages/git-utils/package.json
  • packages/build/src/plugins_core/frameworks_api/skew_protection.ts
  • packages/build/src/log/logger.ts
  • packages/nock-udp/package.json
  • packages/functions-utils/package.json
  • packages/zip-it-and-ship-it/src/config.ts
  • packages/edge-bundler/package.json
  • packages/zip-it-and-ship-it/package.json
  • packages/opentelemetry-sdk-setup/package.json

Comment on lines +14 to +18
1. Check if knip is available:
- Run `npx knip --version` to test
- If it fails or is very slow, check if `knip` is in package.json devDependencies
- If not installed locally, install with `npm install -D knip` (or pnpm/yarn/bun equivalent based on lockfile
present)

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.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'FILE LIST (skill/doc related)\n'
git ls-files '.agents/skills/knip/SKILL.md' 'package.json' 'pnpm-lock.yaml' 'yarn.lock' 'package-lock.json' 'bun.lockb' | sed 's#^`#-` #'

printf '\nSKILL FILE SNIPPET\n'
cat -n .agents/skills/knip/SKILL.md | sed -n '1,220p'

printf '\nPACKAGE SCRIPTS / KNIP REFERENCES\n'
python3 - <<'PY'
import json, pathlib, sys
p = pathlib.Path('package.json')
if p.exists():
    data = json.loads(p.read_text())
    print('package.json scripts:')
    for k,v in (data.get('scripts') or {}).items():
        if 'knip' in k.lower() or 'knip' in str(v).lower():
            print(f'  {k}: {v}')
    print('\npackage.json knip refs:')
    for field in ['devDependencies','dependencies','optionalDependencies','peerDependencies']:
        deps = data.get(field) or {}
        if 'knip' in deps:
            print(f'  {field}.knip: {deps["knip"]}')
else:
    print('no package.json')
PY

printf '\nSEARCH KNIP REFERENCES\n'
rg -n --hidden --glob '!**/.git/**' '\bknip\b|npx knip|npm run knip' .agents package.json . 2>/dev/null | sed -n '1,220p'

Repository: netlify/build

Length of output: 14071


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'FILE LIST (skill/doc related)\n'
git ls-files '.agents/skills/knip/SKILL.md' 'package.json' 'pnpm-lock.yaml' 'yarn.lock' 'package-lock.json' 'bun.lockb' | sed 's#^`#-` #'

printf '\nSKILL FILE SNIPPET\n'
cat -n .agents/skills/knip/SKILL.md | sed -n '1,220p'

printf '\nPACKAGE SCRIPTS / KNIP REFERENCES\n'
python3 - <<'PY'
import json, pathlib
p = pathlib.Path('package.json')
if p.exists():
    data = json.loads(p.read_text())
    print('package.json scripts:')
    for k,v in (data.get('scripts') or {}).items():
        if 'knip' in k.lower() or 'knip' in str(v).lower():
            print(f'  {k}: {v}')
    print('\npackage.json knip refs:')
    for field in ['devDependencies','dependencies','optionalDependencies','peerDependencies']:
        deps = data.get(field) or {}
        if 'knip' in deps:
            print(f'  {field}.knip: {deps["knip"]}')
else:
    print('no package.json')
PY

printf '\nSEARCH KNIP REFERENCES\n'
rg -n --hidden --glob '!**/.git/**' '\bknip\b|npx knip|npm run knip' .agents package.json . 2>/dev/null | sed -n '1,220p'

Repository: netlify/build

Length of output: 14071


Use the repo-local knip script instead of bare npx

.agents/skills/knip/SKILL.md already has knip pinned in devDependencies and exposed as an npm script, so the setup and command examples should stay on the lockfile-backed path (npm run knip -- ...) and avoid npx knip --version as the availability check.

🧰 Tools
🪛 SkillSpector (2.3.7)

[warning] 15: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 37: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 92: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 122: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 125: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 143: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 146: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 149: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 152: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))


[warning] 155: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.

Remediation: Pin the version: npx @scope/server@1.2.3

(MCP Rug Pull (RP1))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/knip/SKILL.md around lines 14 - 18, The knip skill guide is
using a bare npx-based availability check even though the repo already provides
a lockfile-backed knip dependency and npm script. Update the setup and command
examples in SKILL.md to use the local script path via npm run knip -- ... (or
the repo’s equivalent script), and remove the npx knip --version check so the
documented workflow stays pinned to the repo-local installation.

Source: Linters/SAST tools

Comment on lines 10 to +12
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
- name: Install Dependencies
run: npm install @commitlint/config-conventional
run: npm ci

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.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== conventional-commit workflow ==\n'
sed -n '1,120p' .github/workflows/conventional-commit.yml

printf '\n== workflows mentioning setup-node ==\n'
rg -n "actions/setup-node|node-version|npm ci|cache: 'npm'|cache: npm" .github/workflows -S || true

printf '\n== package manager / engines clues ==\n'
rg -n "\"packageManager\"|\"engines\"|node-version|volta|nvm|asdf" package.json .nvmrc .node-version .tool-versions .github/workflows -S || true

Repository: netlify/build

Length of output: 4824


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== workflow files ==\n'
find .github/workflows -maxdepth 1 -type f | sort

printf '\n== relevant package metadata ==\n'
for f in package.json .nvmrc .node-version .tool-versions; do
  if [ -f "$f" ]; then
    echo "--- $f"
    sed -n '1,120p' "$f"
  fi
done

Repository: netlify/build

Length of output: 2500


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== workflow runtime references in repository ==\n'
rg -n "ubuntu-latest|setup-node@|node-version:|npm ci|npm install|corepack|pnpm|yarn" .github/workflows -S || true

Repository: netlify/build

Length of output: 3725


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Focus on the reviewed workflow and any repo-standard Node setup in CI.
printf '\n== conventional-commit workflow ==\n'
cat -n .github/workflows/conventional-commit.yml

printf '\n== any workflow using setup-node ==\n'
rg -n "actions/setup-node" .github/workflows -S || true

printf '\n== repo node version markers ==\n'
for f in package.json .nvmrc .node-version .tool-versions; do
  [ -f "$f" ] && { echo "--- $f"; cat "$f"; }
done

Repository: netlify/build

Length of output: 3431


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== conventional-commit.yml =='
sed -n '1,80p' .github/workflows/conventional-commit.yml

echo
echo '== setup-node occurrences =='
rg -n "actions/setup-node|node-version|cache: 'npm'|cache: npm" .github/workflows -S || true

echo
echo '== Node version markers =='
for f in package.json .nvmrc .node-version .tool-versions; do
  if [ -f "$f" ]; then
    echo "--- $f"
    sed -n '1,60p' "$f"
  fi
done

Repository: netlify/build

Length of output: 4143


Pin Node before npm ci here. This job depends on the runner’s preinstalled Node/npm; add actions/setup-node first to keep the check reproducible and match the other workflows in this repo.

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 10-10: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/conventional-commit.yml around lines 10 - 12, The
conventional-commit workflow currently runs npm ci before ensuring a specific
Node version is installed, so the job depends on whatever Node/npm is
preinstalled on the runner. Update the job to add actions/setup-node before the
Install Dependencies step, using the same Node version/pinning approach as the
other workflows in this repo, and keep the existing actions/checkout step
followed by setup-node and then npm ci.

Comment on lines +14 to +16
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0

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.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== workflow file ==\n'
cat -n .github/workflows/knip.yml

printf '\n== locate pull_request workflow + checkout usage ==\n'
rg -n --context 3 'pull_request|actions/checkout|persist-credentials|npm ci|npm run build|npm run knip' .github/workflows/knip.yml

Repository: netlify/build

Length of output: 1644


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== actions/checkout default docs in repo (if any) ==\n'
rg -n --context 2 'persist-credentials' .github README.md docs . 2>/dev/null || true

Repository: netlify/build

Length of output: 205


🏁 Script executed:

#!/bin/bash
set -euo pipefail

cat -n .github/workflows/knip.yml
printf '\n---\n'
rg -n --context 3 'pull_request|actions/checkout|persist-credentials|npm ci|npm run build|npm run knip' .github/workflows/knip.yml

Repository: netlify/build

Length of output: 1575


🌐 Web query:

actions/checkout persist-credentials default true documentation

💡 Result:

The persist-credentials input for the actions/checkout GitHub Action has a default value of true [1][2][3]. When set to true, this input configures the GitHub Actions authentication token (or SSH key) into the local git configuration [1][4]. This allows your workflow scripts to run authenticated git commands (such as git push or git fetch) directly within the workspace [1][5]. The action automatically removes these credentials during the post-job cleanup phase [1][2]. For improved security, modern versions of the action store these credentials in a separate file under $RUNNER_TEMP rather than directly within the.git/config file, which helps prevent them from being inadvertently included in artifacts [1][2]. If you wish to disable this behavior for security reasons or if it is not required for your workflow, you can explicitly set the input to false in your workflow file [1][5]: - uses: actions/checkout@v4 with: persist-credentials: false

Citations:


Disable checkout credential persistence in the Knip workflow.

This job runs repo-controlled install/build steps on pull_request, so leaving actions/checkout at its default credential persistence keeps the token available in local git config. Set persist-credentials: false.

Suggested fix
       - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
         with:
           fetch-depth: 0
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0
- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
fetch-depth: 0
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 14-16: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/knip.yml around lines 14 - 16, The Knip workflow’s
checkout step is leaving credentials persisted by default, which keeps the
GitHub token in local git config during pull_request builds. Update the existing
actions/checkout step in the knip job to disable credential persistence by
setting persist-credentials to false. Keep the change scoped to the checkout
configuration so the rest of the workflow behavior stays the same.

Source: Linters/SAST tools

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant