Chore/security ci pipeline#220
Open
OkeyAmy wants to merge 5 commits into
Open
Conversation
… gitignore Adds a 5-job GitHub Actions security pipeline (dependency audit, dependency review, CodeQL, ESLint security rules, secret scanning via gitleaks). No external API keys required — all jobs use GITHUB_TOKEN or npm public registry. Adds eslint.security.config.mjs for targeted security linting of src/ using eslint-plugin-security. Updates main eslint config and .gitignore to exclude the local sandbox directory.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds a GitHub Actions security workflow for dependency auditing, dependency review, CodeQL, ESLint security checks, and secret scanning. Adds a dedicated security ESLint config, installs the security plugin, and excludes ChangesSecurity scanning setup
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.github/workflows/security.yml:
- Line 22: The checkout steps in the workflow are leaving GitHub credentials
persisted by default, which is unnecessary here and can expose the token. Update
each actions/checkout usage in the workflow to explicitly disable credential
persistence by setting persist-credentials to false, including the checkout
step(s) referenced by the security.yml job definitions.
- Line 22: The workflow still uses mutable action tags, so update each
third-party action reference in security.yml to a full 40-character commit SHA
while keeping the existing version comment for readability. Apply this to all
affected action entries, including the ones in the
checkout/setup-node/dependency-review, CodeQL init/analyze, and gitleaks steps,
so the workflow pins exact revisions instead of floating tags.
- Around line 97-98: The workflow currently installs eslint-plugin-security ad
hoc in CI, which bypasses the lockfile and uses an unpinned package. Remove the
standalone install step from the security workflow and add
eslint-plugin-security as a pinned devDependency in package.json so npm ci
installs it through package-lock integrity checks; use the existing workflow job
and package manager setup to locate where the plugin is consumed.
- Around line 9-11: The workflow-level permissions are too broad because
security-events: write is granted to every job, even though only the codeql job
needs it to upload SARIF results. Move the security-events: write permission
from the top-level permissions block into a job-level permissions block on
codeql, and keep audit, lint-security, and secret-scan limited to the minimal
read access they actually need.
In `@eslint.security.config.mjs`:
- Around line 5-8: The security ESLint config currently imports
eslint-plugin-security directly, but the dependency is not declared and is being
installed ad hoc, so make the config reproducible by adding a pinned
eslint-plugin-security entry to package.json devDependencies and removing the
workflow’s no-save install step. Update the setup that loads
eslint.security.config.mjs so it relies on the declared dependency, and keep the
import in that config file unchanged aside from ensuring the package is
available through normal dependency installation.
- Around line 30-31: The comment for security/detect-non-literal-regexp does not
match its configured severity. Update the comment near the security rule entry
in eslint.security.config.mjs so it accurately reflects that the rule is set to
warn, not error, or change the rule severity if error is the intended behavior;
keep the comment and the rule value consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 04904fef-3139-4fd1-a8a0-8eb591625b90
📒 Files selected for processing (4)
.github/workflows/security.yml.gitignoreeslint.config.mjseslint.security.config.mjs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/security.yml (1)
108-121: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSet
GITLEAKS_LICENSEfor this organization-owned repo.gitleaks-actionv2 expects a license for org accounts, so leaving it unset will break the scan here; use the license secret or switch to the standalonegitleaksCLI if you want to avoid the wrapper’s licensing.🤖 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/security.yml around lines 108 - 121, The secret scan workflow is using gitleaks-action without the required GITLEAKS_LICENSE for an organization-owned repo, which will cause the scan to fail. Update the secret-scan job in the security workflow to provide the license from a GitHub secret, or replace the gitleaks/gitleaks-action step with the standalone gitleaks CLI if you want to avoid the wrapper licensing requirement.
♻️ Duplicate comments (2)
.github/workflows/security.yml (2)
21-21: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSet
persist-credentials: falseon all checkout steps.Still not set anywhere in the file, as previously flagged by static analysis. None of these jobs push back to the repo, so persisted credentials are unnecessary risk exposure.
🔒️ Proposed fix (apply to each checkout step)
- - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 + with: + persist-credentials: falseAlso applies to: 44-44, 63-63, 90-90, 112-114
🤖 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/security.yml at line 21, The workflow’s checkout steps are still using the default persisted Git credentials, which should be disabled across all jobs. Update each actions/checkout usage in the security workflow to set persist-credentials to false, including the checkout steps associated with the referenced jobs, so the repository is not left with unnecessary write-capable credentials.Source: Linters/SAST tools
99-100: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAd-hoc install of
eslint-plugin-securitystill bypasses the lockfile.
npm install --no-save eslint-plugin-securityremains unpinned and outsidepackage-lock.json, a supply-chain risk previously flagged. Add it as a pinned devDependency instead sonpm ciinstalls it with lockfile integrity checks.🤖 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/security.yml around lines 99 - 100, The workflow still installs eslint-plugin-security ad hoc, bypassing lockfile integrity. Move the dependency into the project’s pinned devDependencies and regenerate the lockfile so npm ci installs it via package-lock.json instead of using the Install security plugin step in the workflow. Use the existing workflow and package manifest entries to locate where the plugin installation is defined and replace the unpinned npm install --no-save usage with a lockfile-backed dependency.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In @.github/workflows/security.yml:
- Around line 108-121: The secret scan workflow is using gitleaks-action without
the required GITLEAKS_LICENSE for an organization-owned repo, which will cause
the scan to fail. Update the secret-scan job in the security workflow to provide
the license from a GitHub secret, or replace the gitleaks/gitleaks-action step
with the standalone gitleaks CLI if you want to avoid the wrapper licensing
requirement.
---
Duplicate comments:
In @.github/workflows/security.yml:
- Line 21: The workflow’s checkout steps are still using the default persisted
Git credentials, which should be disabled across all jobs. Update each
actions/checkout usage in the security workflow to set persist-credentials to
false, including the checkout steps associated with the referenced jobs, so the
repository is not left with unnecessary write-capable credentials.
- Around line 99-100: The workflow still installs eslint-plugin-security ad hoc,
bypassing lockfile integrity. Move the dependency into the project’s pinned
devDependencies and regenerate the lockfile so npm ci installs it via
package-lock.json instead of using the Install security plugin step in the
workflow. Use the existing workflow and package manifest entries to locate where
the plugin installation is defined and replace the unpinned npm install
--no-save usage with a lockfile-backed dependency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 06fc778d-1743-4cfb-bcba-bf1b5f760785
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.github/workflows/security.ymleslint.security.config.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- eslint.security.config.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a dedicated security CI workflow that runs on every PR and push to
main,dev,stg.Changes
.github/workflows/security.yml— new security pipeline (5 jobs, zero external API keys)eslint.security.config.mjs— security-focused ESLint config usingeslint-plugin-securityeslint.config.mjs— addsandbox/**to ignore listSecurity jobs
npm audit --omit=devsecurity-extended)eslint-plugin-securityNotes
GITHUB_TOKEN(automatic) or run fully offline.eslint.security.config.mjsruns separately from the main lint so security findings surface as their own CI check.Contact: @okey_amy
Summary by CodeRabbit
sandbox/from routine linting and checks.