Skip to content

✨ checks/dangerous_workflow: detect committer.{name,email} as untrusted input - #5250

Open
Shxnque wants to merge 1 commit into
ossf:mainfrom
Shxnque:feat/dangerous-workflow-committer-untrusted
Open

Shxnque wants to merge 1 commit into
ossf:mainfrom
Shxnque:feat/dangerous-workflow-committer-untrusted

Conversation

@Shxnque

@Shxnque Shxnque commented Sep 26, 2026

Copy link
Copy Markdown

What

Adds head_commit.committer.{email,name} and commits.*.committer.{email,name} to untrustedContextPattern in checks/raw/dangerous_workflow.go, mirroring the existing author.* handling.

Why

The Dangerous-Workflow check already treats head_commit/commits[*].author.{name,email} as attacker-controllable Script-Injection inputs, but not the committer equivalents. A commit's committer name and email are attacker-settable git metadata in exactly the same way as the author fields (git commit --amend --author/GIT_COMMITTER_*), and are surfaced on push (head_commit, commits[*]) and via workflow_run.head_commit. Interpolating them into a run: step is the same injection class the check exists to catch.

This is the committer gap raised by @pnacht in #3915:

Another set of potentially risky variables is github.event.{head_commit/commits[*]}.committer.{name/email}. Scorecard detects the data for the author, but not the committer.

It is complementary to #5226 (which adds the fork.forkee.*, pull_request.head.repo.{description,homepage} and workflow_run.* fields) — there is no overlap; that PR does not touch committer.

Scope discipline

Minimal, parity-only change. It follows the issue's own scoping guidance (prioritise fields an unprivileged external actor controls) and deliberately does not add write-access-only fields (e.g. milestone.title, project_card.note).

Tests

Added to TestUntrustedContextVariables:

  • head_commit.committer.{name,email} → detected (true)
  • commits[2].committer.{name,email} → detected (true)
  • head_commit.committer.date → not matched (false) — keeps the pattern precise (a date is not injectable free-text).

Validated as a pass/reject matrix against containsUntrustedContextPattern (all pass; existing author.* and trusted-field cases unchanged). gofmt clean. Full suite via CI.

Refs #3915.

@Shxnque
Shxnque requested a review from a team as a code owner September 26, 2026 16:12
@Shxnque
Shxnque requested review from justaugustus and spencerschrock and removed request for a team September 26, 2026 16:12
@Shxnque Shxnque changed the title checks/dangerous_workflow: detect committer.{name,email} as untrusted input ✨ checks/dangerous_workflow: detect committer.{name,email} as untrusted input Sep 30, 2026
@Shxnque

Shxnque commented Sep 30, 2026

Copy link
Copy Markdown
Author

CI is green now — the earlier Verify PR contents failure was just the missing title type-prefix (added ✨, non-breaking feature). No code changes; DCO + Kusari pass.

Recap of the change for review: dangerous_workflow already flags github.event.*.author.{name,email} as attacker-controllable, but not the committer.{name,email} parity fields, which are equally untrusted git metadata surfaced on push/head_commit/commits[]/workflow_run.head_commit. This adds the committer.name/committer.email detections (with committer.date deliberately NOT matched, to avoid false positives) plus a pass/reject test. Complementary to #5226 (which covers the fork/repo/workflow_run field additions), not overlapping. Happy to adjust naming/scoping if you prefer.

… input

The untrusted-context pattern flags head_commit/commits[*].author.{name,email}
as attacker-controllable script-injection inputs, but not the committer
equivalents. A commit's committer name and email are attacker-settable git
metadata in exactly the same way as the author fields, and are surfaced on
push (head_commit, commits[*]) and via workflow_run.head_commit — so they carry
the same Script Injection risk in a run: step.

Adds head_commit.committer.{email,name} and commits.*.committer.{email,name} to
untrustedContextPattern, mirroring the existing author handling, plus table
tests (4 positive + a committer.date negative to keep the match precise).

Addresses a remaining gap in ossf#3915 (raised by @pnacht); complementary to ossf#5226,
which covers the fork/repo/workflow_run fields and does not touch committer.

Signed-off-by: Shxnque <shinque03@gmail.com>
@Shxnque
Shxnque force-pushed the feat/dangerous-workflow-committer-untrusted branch from b240124 to 26d4094 Compare October 2, 2026 15:18

This branch has not been deployed

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant