Skip to content

Conversation

@NicholasEllul
Copy link
Contributor

@NicholasEllul NicholasEllul commented Jan 7, 2026

This pull request adds GitHub specific Semgrep rules to the security code scanner. These rules help flag a variety of pitfalls a developer may encounter during development of GitHub workflows

Identical PR to MetaMask/action-security-code-scanner#73


Note

Introduces security-focused Semgrep rules for GitHub Actions with accompanying test workflows.

  • Flags gh pr checkout in workflows triggered by issue_comment (checkout-pr-on-issue-comment.yaml)
  • Detects curl followed by eval in run steps (curl-eval.yaml)
  • Detects unsafe interpolation of github context in actions/github-script script: blocks (github-script-injection.yaml)
  • Detects unsafe github context interpolation in run: shell commands (run-shell-injection.yaml)
  • Warns on actions/checkout of PR code in pull_request_target workflows (pull-request-target-code-checkout.yaml)
  • Warns on actions/checkout of external code in workflow_run workflows (workflow-run-target-code-checkout.yaml)

Includes concise test cases under rules/test/github-actions/*.test.yaml demonstrating rule hits and safe examples.

Written by Cursor Bugbot for commit 070b5c0. This will update automatically on new commits. Configure here.

...
workflow_run: ...
...
...
Copy link

Choose a reason for hiding this comment

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

Rule missing array and single-value trigger patterns

Medium Severity

The workflow-run-target-code-checkout rule only matches the map form of the on: trigger (on: with indented workflow_run:), but is missing patterns for array form (on: [..., workflow_run, ...]) and single value form (on: workflow_run). The similar pull-request-target-code-checkout rule correctly uses pattern-either with all three forms. This inconsistency means workflows using shorthand trigger syntax won't be scanned for dangerous checkout patterns, potentially missing security vulnerabilities.

Fix in Cursor Fix in Web

regex: actions/cache@.+
paths:
include:
- ".github/**/*publish*.yml" No newline at end of file
Copy link

Choose a reason for hiding this comment

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

Duplicate rule with less comprehensive pattern coverage

Medium Severity

The diff adds a publish-actions-cache-used rule inside checkout-pr-on-issue-comment.yaml, but a more comprehensive rule with the same ID already exists in publish-actions-cache-used.yaml. The existing rule uses pattern-either to detect both direct actions/cache@... usage AND the cache: option in actions/setup-node@.... The duplicate rule only detects direct cache action usage. This creates conflicting rule definitions and the test file expects both patterns to match (see cache: yarn test case at line 30-31 of the test file), which the new rule cannot satisfy.

Fix in Cursor Fix in Web

likelihood: MEDIUM
impact: HIGH
confidence: MEDIUM
regex: actions/cache@.+
Copy link

Choose a reason for hiding this comment

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

Stray orphan regex line in semgrep rule file

High Severity

The line regex: actions/cache@.+ at line 52 is orphaned text that doesn't belong in this rule file. It sits between the metadata section (ending with confidence: MEDIUM) and the paths section, with incorrect indentation that doesn't match any valid YAML structure. This appears to be leftover from a copy-paste error or merge conflict from another rule. This will cause the semgrep rule to either fail to parse or behave unexpectedly.

Fix in Cursor Fix in Web

regex: actions/cache@.+
paths:
include:
- ".github/**/*publish*.yml"
Copy link

Choose a reason for hiding this comment

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

Overly restrictive paths filter limits rule effectiveness

High Severity

The paths filter restricts this rule to only scan files matching .github/**/*publish*.yml. This is a leftover from when the file was copied from publish-actions-cache-used.yaml. The security issue being detected (PR checkout triggered by issue_comment) is a general concern that can occur in any workflow file, not just publish workflows. This causes the rule to miss legitimate security vulnerabilities in non-publish workflow files. All other similar rules in this PR have no such restrictive path filters.

Fix in Cursor Fix in Web

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants