Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 118 additions & 0 deletions .github/workflows/security.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
name: Security

on:
pull_request:
types: [opened, synchronize, reopened]
push:
branches: [main, dev, stg]

permissions:
contents: read
security-events: write # CodeQL needs this to upload SARIF results
Comment on lines +9 to +11

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Scope security-events: write to the CodeQL job only.

security-events: write is only required by the codeql job to upload SARIF results, yet it's granted workflow-wide, giving audit, dependency-review, lint-security, and secret-scan unnecessary write access. Static analysis flags this as excessive-permissions.

As per path instructions, "Prefer least-privilege permissions: blocks."

Proposed fix
 permissions:
   contents: read
-  security-events: write  # CodeQL needs this to upload SARIF results
 
 jobs:
   ...
   codeql:
     name: CodeQL
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      security-events: write  # needed to upload SARIF results
     steps:
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 11-11: overly broad permissions (excessive-permissions): security-events: write is overly broad at the workflow level

(excessive-permissions)

🤖 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 9 - 11, Scope the workflow
permissions to least privilege by removing the workflow-wide security-events
write grant from the top-level permissions block in security.yml and applying it
only to the codeql job that uploads SARIF results. Keep contents: read at the
workflow level, then add a job-level permissions block for codeql with
security-events: write so audit, dependency-review, lint-security, and
secret-scan do not inherit unnecessary write access.

Sources: Path instructions, Linters/SAST tools


jobs:

# ── 1. Dependency audit ────────────────────────────────────────────────────
# Blocks on HIGH/CRITICAL in production dependencies (what ships to users).
# Dev-only vulns (vitest, esbuild) are reported but do not fail the build.
audit:
name: Dependency Audit
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Pin third-party actions to full commit SHAs.

All actions (actions/checkout@v5, actions/setup-node@v5, actions/dependency-review-action@v4, github/codeql-action/init@v3, github/codeql-action/analyze@v3, gitleaks/gitleaks-action@v2) are referenced by tag rather than a pinned 40-character commit SHA. Tags are mutable and can be repointed, allowing a compromised upstream release to run in this pipeline with security-events: write and repo checkout access.

As per path instructions, "All third-party actions must be pinned to a full 40-char commit SHA."

Example pin for checkout
-      - uses: actions/checkout@v5
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v5.0.0

Also applies to: 45-45, 46-46, 61-61, 63-63, 68-68, 76-76, 88-88, 90-90, 110-115

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 22-22: 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/security.yml at line 22, The workflow still references
third-party actions by mutable tags instead of full commit SHAs. Update every
action use in the security workflow, including actions/checkout,
actions/setup-node, actions/dependency-review-action, github/codeql-action/init,
github/codeql-action/analyze, and gitleaks/gitleaks-action, so each is pinned to
a 40-character commit SHA rather than a version tag. Ensure the corresponding
action entries in the workflow are replaced consistently wherever these symbols
appear.

Source: Path instructions


- uses: actions/setup-node@v5
with:
node-version: 22
cache: 'npm'

- run: npm ci

- name: Audit production dependencies (blocking)
run: npm audit --omit=dev --audit-level=high

- name: Audit all dependencies (informational)
run: npm audit --audit-level=high || true

# ── 2. Dependency review on PRs ───────────────────────────────────────────
# Blocks PRs that introduce new vulnerable packages.
# Uses GITHUB_TOKEN automatically — no external API key needed.
dependency-review:
name: Dependency Review
runs-on: ubuntu-latest
if: github.event_name == 'pull_request'
steps:
- uses: actions/checkout@v5
- uses: actions/dependency-review-action@v4
with:
fail-on-severity: high

# ── 3. CodeQL static analysis ─────────────────────────────────────────────
# Catches classes of bugs the linter/typechecker miss:
# CWE-22 path traversal (F-003, F-009)
# CWE-918 SSRF (F-002)
# CWE-73 external file path control (F-008)
# CWE-116 improper output encoding (F-004, F-006)
# Free for public repos. Uses GITHUB_TOKEN — no external key.
codeql:
name: CodeQL
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5

- uses: actions/setup-node@v5
with:
node-version: 22

- name: Initialize CodeQL
uses: github/codeql-action/init@v3
with:
languages: javascript-typescript
queries: security-extended

- run: npm ci && npm run build

- name: Analyze
uses: github/codeql-action/analyze@v3
with:
category: '/language:javascript-typescript'

# ── 4. ESLint with security rules ─────────────────────────────────────────
# Runs eslint-plugin-security against src/ using eslint.security.config.mjs.
# Separate from the main lint job so security findings surface distinctly.
# No API key — pure local analysis.
lint-security:
name: ESLint Security
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5

- uses: actions/setup-node@v5
with:
node-version: 22
cache: 'npm'

- run: npm ci

- name: Install security plugin
run: npm install --no-save eslint-plugin-security

- name: Run security lint
run: npx eslint src/ --config eslint.security.config.mjs --format stylish

# ── 5. Secret scanning ────────────────────────────────────────────────────
# Scans git history for accidentally committed secrets (API keys, tokens).
# gitleaks is open source, no account or API key required.
secret-scan:
name: Secret Scan
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5
with:
fetch-depth: 0 # full history needed for git log scan

- name: Scan for secrets with gitleaks
uses: gitleaks/gitleaks-action@v2
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
# GITLEAKS_LICENSE not set — free mode scans public repos without limit
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ test/dev-e2e/.env.local
# Per-session Claude Code worktrees (parallel-session scratch space). Other
# `.claude/` content (e.g., `skills/`) stays tracked.
.claude/worktrees/
sandbox/
2 changes: 1 addition & 1 deletion eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import globals from 'globals';

export default tseslint.config(
{
ignores: ['dist/**', 'coverage/**', 'node_modules/**', 'perf/**', '.claude/worktrees/**'],
ignores: ['dist/**', 'coverage/**', 'node_modules/**', 'perf/**', '.claude/worktrees/**', 'sandbox/**'],
},
js.configs.recommended,
...tseslint.configs.recommended,
Expand Down
39 changes: 39 additions & 0 deletions eslint.security.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/**
* Security-focused ESLint config used by CI security job only.
* Run: npx eslint src/ --config eslint.security.config.mjs
*
* Requires eslint-plugin-security to be installed:
* npm install --no-save eslint-plugin-security
*/
import security from 'eslint-plugin-security';
import tseslint from 'typescript-eslint';
import globals from 'globals';

export default tseslint.config(
{
ignores: ['dist/**', 'coverage/**', 'node_modules/**', 'sandbox/**'],
},
security.configs.recommended,
{
languageOptions: {
ecmaVersion: 2022,
sourceType: 'module',
globals: { ...globals.node },
},
rules: {
// Block non-literal paths in fs calls — catches CWE-22, CWE-73
'security/detect-non-literal-fs-filename': 'error',
// Warn on object injection via bracket notation with user input
'security/detect-object-injection': 'warn',
// Warn on timing-unsafe comparisons (token equality checks)
'security/detect-possible-timing-attacks': 'warn',
// Error on non-literal RegExp (ReDoS)
'security/detect-non-literal-regexp': 'warn',
// Error on child_process with non-literal args
'security/detect-child-process': 'error',
// Disable rules that generate too much noise for a CLI codebase
'security/detect-non-literal-require': 'off',
'security/detect-unsafe-regex': 'warn',
Comment on lines +30 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Comment/severity mismatches.

The comment on Line 30 says "Error on non-literal RegExp (ReDoS)" but the rule is set to 'warn' on Line 31. Similarly, the Line 34 comment "Disable rules that generate too much noise" precedes detect-unsafe-regex: 'warn' (Line 36), which is enabled, not disabled — only detect-non-literal-require is actually off. These stale comments could mislead future maintainers about actual enforcement.

Proposed fix
-      // Error on non-literal RegExp (ReDoS)
+      // Warn on non-literal RegExp (ReDoS)
       'security/detect-non-literal-regexp': 'warn',
       // Error on child_process with non-literal args
       'security/detect-child-process': 'error',
-      // Disable rules that generate too much noise for a CLI codebase
+      // Disable/reduce noisy rules for a CLI codebase
       'security/detect-non-literal-require': 'off',
       'security/detect-unsafe-regex': 'warn',
📝 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
// Error on non-literal RegExp (ReDoS)
'security/detect-non-literal-regexp': 'warn',
// Error on child_process with non-literal args
'security/detect-child-process': 'error',
// Disable rules that generate too much noise for a CLI codebase
'security/detect-non-literal-require': 'off',
'security/detect-unsafe-regex': 'warn',
// Warn on non-literal RegExp (ReDoS)
'security/detect-non-literal-regexp': 'warn',
// Error on child_process with non-literal args
'security/detect-child-process': 'error',
// Disable/reduce noisy rules for a CLI codebase
'security/detect-non-literal-require': 'off',
'security/detect-unsafe-regex': 'warn',
🤖 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 `@eslint.security.config.mjs` around lines 30 - 36, Update the stale rule
comments in eslint.security.config.mjs so they match the actual severities in
the security rule block: adjust the note above
security/detect-non-literal-regexp to reflect that it is set to warn, and revise
the “Disable rules that generate too much noise” comment near
security/detect-non-literal-require/security/detect-unsafe-regex so it no longer
implies both are off when only detect-non-literal-require is disabled.

},
},
);