- Notifications
You must be signed in to change notification settings - Fork 57
[StepSecurity] Apply security best practices #1191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[StepSecurity] Apply security best practices #1191
Conversation
Signed-off-by: StepSecurity Bot <bot@stepsecurity.io>
|
|
WalkthroughAdds and updates CI/security configurations: expands Dependabot to daily for more paths, introduces new CodeQL, Dependency Review, and Scorecards workflows, adds runner hardening and pins actions in several workflows, and adds a pre-commit configuration with security and lint hooks. Changes
Sequence Diagram(s)sequenceDiagram autonumber actor Dev as Developer/PR participant GH as GitHub Actions participant CQL as CodeQL Action participant CS as Code Scanning Dev->>GH: Push / PR to master GH->>GH: Harden runner (audit egress) GH->>CQL: Initialize (JS/TS matrix) CQL->>CQL: Autobuild CQL->>CQL: Analyze CQL-->>CS: Upload SARIF results CS-->>Dev: Security alerts (if any) sequenceDiagram autonumber actor Dev as Pull Request participant GH as GitHub Actions participant DR as Dependency Review Action Dev->>GH: PR opened/synchronized GH->>GH: Harden runner (audit egress) GH->>DR: Run dependency review DR-->>GH: Report vulnerable changes GH-->>Dev: PR check status (pass/fail) sequenceDiagram autonumber participant CRON as Schedule/Push participant GH as GitHub Actions participant SC as OSSF Scorecard participant ART as Artifact Storage participant CS as Code Scanning CRON->>GH: Trigger workflow GH->>GH: Harden runner (audit egress) GH->>SC: Run scorecard analysis SC-->>ART: Upload results.sarif SC-->>CS: Publish SARIF to code scanning Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews: pre_merge_checks: custom_checks: - name: "Undocumented Breaking Changes" mode: "warning" instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/codesee-arch-diagram.yml (1)
7-8: Avoid pull_request_target with third‑party actions.Using pull_request_target runs untrusted code with elevated token scope. Switch to pull_request and restrict permissions.
Apply:
-on: +on: push: branches: - master - pull_request_target: + pull_request: types: [opened, synchronize, reopened] @@ -permissions: read-all +permissions: + contents: read @@ codesee: runs-on: ubuntu-latest continue-on-error: true name: Analyze the repo with CodeSee steps:Also applies to: 20-26
.github/workflows/nodejs.yml (1)
28-34: Don’t downgrade npm on Node 22/24.Installing npm@9 can break with Node 22/24; use the bundled npm or pin a compatible major.
Apply:
- npm install -g npm@9 npm ciIf you must pin: npm@10 for Node 20/22; check Node 24’s bundled npm before pinning.
🧹 Nitpick comments (14)
.pre-commit-config.yaml (4)
6-9: ShellCheck hook may require Docker or a local shellcheck binary; consider shellcheck-py for portability.The jumanjihouse hook commonly uses Docker/system deps; this breaks on contributors without Docker or shellcheck installed.
Switch to shellcheck-py for consistent installs via Python wheels:
- - repo: https://github.com/jumanjihouse/pre-commit-hooks - rev: 3.0.0 - hooks: - - id: shellcheck + - repo: https://github.com/shellcheck-py/shellcheck-py + rev: v0.10.0.1 # pin exact + hooks: + - id: shellcheck + args: ["-x"] # follow sourced files + files: "^(scripts|ci)/.*\\.sh$"
2-5: Tune gitleaks to scan staged changes and reduce noise; add redact and exit‑code policy.Default gitleaks can be slow/noisy on large repos. Use protect, staged, and redact to fit pre-commit usage.
- - repo: https://github.com/gitleaks/gitleaks - rev: v8.16.3 - hooks: - - id: gitleaks + - repo: https://github.com/gitleaks/gitleaks + rev: v8.16.3 + hooks: + - id: gitleaks + name: gitleaks (staged, redact) + args: ["protect", "--staged", "--redact", "--no-git", "--verbose"] + exclude: "(^|/)(node_modules|dist|build|coverage|.git)/"If you prefer full-repo scans in CI instead, keep pre-commit lightweight and add a dedicated workflow job with gitleaks detect.
14-18: Add a few low-cost hygiene hooks (YAML/JSON/merge conflicts/large files).These catch common issues early and align with the PR’s security/quality posture.
- repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.4.0 hooks: - id: end-of-file-fixer - id: trailing-whitespace + - id: check-merge-conflict + - id: check-yaml + - id: check-json + - id: check-case-conflict + - id: check-added-large-files + args: ["--maxkb=1024"]
1-19: Optional ergonomics: set defaults and CI integration.
- default_stages: commit speeds up common runs.
- pre-commit.ci: enforces hooks on PRs and can auto‑fix.
repos: +# Optional: CI and defaults +ci: + autofix_commit_msg: "chore(pre-commit): auto-fixes from pre-commit.ci" + autoupdate_schedule: monthly + +default_stages: [commit] +default_language_version: + node: "lts/*" + python: "3.11".github/workflows/dependency-review.yml (1)
19-23: Move from audit to block after burn‑in.Once outbound domains are known, flip to block and allowlist GitHub endpoints.
Proposed:
- - name: Harden the runner (Audit all outbound calls) + - name: Harden the runner (Block non-allowed egress) uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1 with: - egress-policy: audit + egress-policy: block + allowed-endpoints: > + api.github.com + github.com + objects.githubusercontent.com + raw.githubusercontent.com.github/workflows/codesee-arch-diagram.yml (1)
20-23: Plan to tighten egress to block.Same hardening note as other workflows—flip to block with allowlist after initial runs.
.github/workflows/nodejs.yml (2)
22-24: Upgrade pinned actions to v4 for parity and support.checkout/setup-node are on older v3 pins here while other workflows use v4.
Apply:
- - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0 + - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 @@ - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@3235b876344d2a9aa001b8d1453c930bba69e610 # v3.9.1 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
3-3: Consider testing PRs too.Add pull_request so external contributions run CI.
Apply:
-on: [push] +on: [push, pull_request].github/workflows/codeql.yml (2)
42-49: Fetch full history for better analysis.CodeQL benefits from full git history. Set fetch-depth: 0.
Apply:
- name: Checkout repository - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + fetch-depth: 0
43-47: Plan to move egress policy from audit to block.After a few runs, enable block with an allowlist.
.github/workflows/scorecards.yml (1)
17-19: Remove top-level read‑all.You already set precise job permissions; drop read-all to avoid accidental privilege creep.
Apply:
-# Declare default permissions as read only. -permissions: read-all +permissions: {}.github/workflows/release-drafter.yml (2)
11-34: Scope GITHUB_TOKEN explicitly.Add job‑level permissions (contents: write) so the token isn’t broader than needed.
Apply:
jobs: update_release_draft: runs-on: ubuntu-latest + permissions: + contents: write + actions: read + pull-requests: read steps:Optional: move harden‑runner to egress block with an allowlist after burn‑in.
16-20: Harden‑runner egress: consider block mode later.Same recommendation as other workflows.
.github/dependabot.yml (1)
15-29: Paths verified — reduce Dependabot cadence and group updatespackage.json present in /dist, /example/nextjs, /example/react (verified). Daily cadence may create noise — switch to weekly and add grouping/labels.
- package-ecosystem: npm directory: /dist schedule: - interval: daily + interval: weekly @@ - package-ecosystem: npm directory: /example/nextjs schedule: - interval: daily + interval: weekly @@ - package-ecosystem: npm directory: /example/react schedule: - interval: daily + interval: weekly + groups: + minor-and-patch: + update-types: [minor, patch] + labels: + - dependencies
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/dependabot.yml(1 hunks).github/workflows/codeql.yml(1 hunks).github/workflows/codesee-arch-diagram.yml(1 hunks).github/workflows/dependency-review.yml(1 hunks).github/workflows/nodejs.yml(2 hunks).github/workflows/release-drafter.yml(1 hunks).github/workflows/scorecards.yml(1 hunks).pre-commit-config.yaml(1 hunks)
🔇 Additional comments (6)
.pre-commit-config.yaml (2)
1-19: Good addition: baseline pre-commit safeguards now exist.This is a solid starting point that aligns with the PR’s security goals.
1-1: Pin pre-commit hooks to commit SHAs (don’t rely on floating tags).Use SHA revs in .pre-commit-config.yaml — tags can be moved; if you keep tags, add a regular cadence to bump them.
Verified latest releases:
- gitleaks/gitleaks — v8.28.0 (release commit 4fb4382)
- shellcheck-py/shellcheck-py — 0.11.0.1 (PyPI, Aug 9, 2025)
- pre-commit/mirrors-eslint — v9.30.0 (tag commit 1447d4adf25d...)
- pre-commit/pre-commit-hooks — 6.0.0 (PyPI, Aug 9, 2025)
File: .pre-commit-config.yaml
.github/workflows/nodejs.yml (1)
5-7: LGTM on least‑privilege token.contents: read is appropriate for CI.
.github/workflows/codeql.yml (1)
23-34: Permissions look right.contents: read at workflow level and job-scoped security-events: write is the correct split.
.github/dependabot.yml (1)
1-9: Retain root weekly npm updates.Root config is fine as-is.
.github/workflows/scorecards.yml (1)
37-45: Good hardening and safe checkout.Harden‑runner plus persist-credentials: false is correct here.
| - name: Harden the runner (Audit all outbound calls) | ||
| uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1 | ||
| with: | ||
| egress-policy: audit | ||
| | ||
| - name: 'Checkout Repository' | ||
| uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 | ||
| - name: 'Dependency Review' | ||
| uses: actions/dependency-review-action@595b5aeba73380359d98a5e087f648dbb0edce1b # v4.7.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce failure threshold for vulnerable deps.
Set fail-on-severity so the check actually blocks on risky PRs (defaults don’t fail builds).
Apply:
- - name: 'Dependency Review' - uses: actions/dependency-review-action@595b5aeba73380359d98a5e087f648dbb0edce1b # v4.7.3 + - name: 'Dependency Review' + uses: actions/dependency-review-action@595b5aeba73380359d98a5e087f648dbb0edce1b # v4.7.3 + with: + fail-on-severity: high + warn-on-openssf-score: 5Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/dependency-review.yml around lines 19 to 27, the Dependency Review step is missing a fail-on-severity input so it won't block PRs with high-severity vulnerabilities; add a with block for the actions/dependency-review-action step and set fail-on-severity to the desired threshold (for example "high" or "moderate" depending on policy) so the action returns non-zero on vulnerabilities at or above that level; ensure the key is named fail-on-severity and the value is the chosen severity string. | steps: | ||
| - uses: actions/checkout@v3 | ||
| - name: Harden the runner (Audit all outbound calls) | ||
| uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1 | ||
| with: | ||
| egress-policy: audit | ||
| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Windows runner compatibility for harden‑runner.
Confirm StepSecurity hardening supports Windows; if not, gate by OS.
Proposed:
- - name: Harden the runner (Audit all outbound calls) + - name: Harden the runner (Audit all outbound calls) + if: runner.os != 'Windows' uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1 with: egress-policy: audit🌐 Web query:
Does step-security/harden-runner fully support Windows runners for egress control? 💡 Result:
Short answer: No — Harden-Runner does not fully support Windows runners for egress control. It only supports Linux (Ubuntu) for GitHub-hosted and self-hosted/ARC environments; Windows and macOS jobs/runners are listed as unsupported. [1][2]
Sources:
- step-security/harden-runner README and environment/limitations notes. [1]
- StepSecurity documentation (Community/installation and limitations). [2]
Gate harden-runner to Linux-only (Windows/macOS unsupported)
harden-runner only supports Linux (Ubuntu); do not run it on Windows or macOS.
File: .github/workflows/nodejs.yml Lines: 16-21
- - name: Harden the runner (Audit all outbound calls) + - name: Harden the runner (Audit all outbound calls) + if: runner.os == 'Linux' uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1 with: egress-policy: audit🤖 Prompt for AI Agents
.github/workflows/nodejs.yml lines 16-21: the harden-runner step is Linux-only but currently runs on all OS runners; add an execution guard so the step only runs on Linux by adding an if condition (if: runner.os == 'Linux') to the step, ensuring the step is skipped on Windows and macOS while leaving the uses and with keys unchanged. | - repo: https://github.com/pre-commit/mirrors-eslint | ||
| rev: v8.38.0 | ||
| hooks: | ||
| - id: eslint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
ESLint hook likely to fail without project plugins; prefer local runner or add dependencies.
The mirrors-eslint hook doesn’t know your project’s plugin set/rules. It often breaks on React/TypeScript repos unless you enumerate plugins via additional_dependencies, or delegate to your package.json scripts.
Option A (prefer): use the repo’s ESLint via npm/yarn/pnpm.
- - repo: https://github.com/pre-commit/mirrors-eslint - rev: v8.38.0 - hooks: - - id: eslint + - repo: local + hooks: + - id: eslint + name: eslint (repo local) + entry: npm run -s lint -- # or: npx --yes eslint + language: system + pass_filenames: true + types_or: [javascript, jsx, ts, tsx] + files: "^(src|scripts)/" + exclude: "(^|/)(dist|build|coverage|node_modules)/"Option B: stay on mirrors-eslint but include plugins matching your .eslintrc and package.json.
- repo: https://github.com/pre-commit/mirrors-eslint rev: v8.38.0 hooks: - id: eslint + types_or: [javascript, jsx, ts, tsx] + additional_dependencies: + - eslint@8.38.0 + - eslint-plugin-react@<pin> + - eslint-plugin-react-hooks@<pin> + - @typescript-eslint/parser@<pin> + - @typescript-eslint/eslint-plugin@<pin> + - eslint-config-prettier@<pin>Run this to detect required ESLint plugins from your config and package.json:
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "ESLint config files (if any):" fd -H '(.eslintrc|.eslintrc.(js|cjs|json|yml|yaml)|eslint.config.(js|cjs|mjs|ts))' -u echo -e "\nDeclared eslint deps in package.json:" jq -r '[.dependencies,.devDependencies]|add|to_entries[]|select(.key|test("^(eslint(|-plugin-| -config-)|@typescript-eslint)"))|.key+"@"+.value' package.json 2>/dev/null || trueLength of output: 489
ESLint hook: add project plugins to pre-commit or run the local lint script
Repo contains .eslintrc.json and these eslint deps in package.json — mirrors-eslint without them will break; either switch to a local runner (e.g. npm run lint) or add the listed packages as additional_dependencies for the mirrors-eslint hook.
Suggested update (Option B — add plugins to .pre-commit-config.yaml):
- repo: https://github.com/pre-commit/mirrors-eslint rev: v8.38.0 hooks: - id: eslint types_or: [javascript, jsx, ts, tsx] additional_dependencies: - eslint@^8.36.0 - @typescript-eslint/parser@^5.56.0 - @typescript-eslint/eslint-plugin@^5.56.0 - eslint-config-airbnb@^19.0.4 - eslint-config-prettier@^8.8.0 - eslint-plugin-flowtype@^8.0.3 - eslint-plugin-import@^2.27.5 - eslint-plugin-jest@^26.9.0 - eslint-plugin-jsx-a11y@^6.7.1 - eslint-plugin-prettier@^4.2.1 - eslint-plugin-react@^7.32.2 - eslint-plugin-react-hooks@^4.6.0 - eslint-watch@^8.0.0🤖 Prompt for AI Agents
.pre-commit-config.yaml lines 10-13 — the mirrors-eslint hook is declared without your project's ESLint plugins/parsers so the pre-commit runner will fail; fix by either switching the hook to run your local lint script (e.g. replace the hook with a local hook invoking npm run lint) or add the project's ESLint packages as additional_dependencies on the mirrors-eslint hook; update the hook to include types_or for javascript/jsx/ts/tsx and add additional_dependencies entries for eslint, @typescript-eslint/parser, @typescript-eslint/eslint-plugin, eslint-config-airbnb, eslint-config-prettier, eslint-plugin-flowtype, eslint-plugin-import, eslint-plugin-jest, eslint-plugin-jsx-a11y, eslint-plugin-prettier, eslint-plugin-react, eslint-plugin-react-hooks, and any other plugins/versions that match package.json so the pre-commit mirror has the same environment as your project. 


Summary
This pull request is created by StepSecurity at the request of @chintan9. Please merge the Pull Request to incorporate the requested changes. Please tag @chintan9 on your message if you have any questions related to the PR.
Security Fixes
Least Privileged GitHub Actions Token Permissions
The GITHUB_TOKEN is an automatically generated secret to make authenticated calls to the GitHub API. GitHub recommends setting minimum token permissions for the GITHUB_TOKEN.
Pinned Dependencies
GitHub Action tags and Docker tags are mutable. This poses a security risk. GitHub's Security Hardening guide recommends pinning actions to full length commit.
Harden Runner
Harden-Runner is an open-source security agent for the GitHub-hosted runner to prevent software supply chain attacks. It prevents exfiltration of credentials, detects tampering of source code during build, and enables running jobs without
sudoaccess. See how popular open-source projects use Harden-Runner here.Harden runner usage
You can find link to view insights and policy recommendation in the build log
Please refer to documentation to find more details.
Keeping your actions up to date with Dependabot
With Dependabot version updates, when Dependabot identifies an outdated dependency, it raises a pull request to update the manifest to the latest version of the dependency. This is recommended by GitHub as well as The Open Source Security Foundation (OpenSSF).
Detect Vulnerabilities with SAST Workflow
Static Code Analysis (also known as Source Code Analysis) is usually performed as part of a Code Review (also known as clear-box testing) and is carried out at the Implementation phase of a Security Development Lifecycle (SDL). Static Code Analysis commonly refers to the running of Static Code Analysis tools that attempt to highlight possible vulnerabilities within ‘static’ (non-running) source code by using techniques such as Taint Analysis and Data Flow Analysis.
Add Dependency Review Workflow
The Dependency Review Workflow enforces dependency reviews on your pull requests. The action scans for vulnerable versions of dependencies introduced by package version changes in pull requests, and warns you about the associated security vulnerabilities. This gives you better visibility of what's changing in a pull request, and helps prevent vulnerabilities being added to your repository.
Add OpenSSF Scorecard Workflow
OpenSSF Scorecard is an automated tool that assesses a number of important heuristics ("checks") associated with software security and assigns each check a score of 0-10. You can use these scores to understand specific areas to improve in order to strengthen the security posture of your project.
Scorecard workflow also allows maintainers to display a Scorecard badge on their repository to show off their hard work.
Maintain Code Quality with Pre-Commit
Pre-commit is a framework for managing and maintaining multi-language pre-commit hooks. Hooks can be any scripts, code, or binaries that run at any stage of the git workflow. Pre-commit hooks are useful for enforcing code quality, code formatting, and detecting security vulnerabilities.
Feedback
For bug reports, feature requests, and general feedback; please email support@stepsecurity.io. To create such PRs, please visit https://app.stepsecurity.io/securerepo.
Signed-off-by: StepSecurity Bot bot@stepsecurity.io
Summary by CodeRabbit