Skip to content

fix(terraform): CKV_AZURE_249 should flag pull_request OIDC subjects - #7627

Open
Dashtid wants to merge 1 commit into
bridgecrewio:mainfrom
Dashtid:fix/ckv-azure-249-pull-request
Open

fix(terraform): CKV_AZURE_249 should flag pull_request OIDC subjects#7627
Dashtid wants to merge 1 commit into
bridgecrewio:mainfrom
Dashtid:fix/ckv-azure-249-pull-request

Conversation

@Dashtid

@Dashtid Dashtid commented Jul 27, 2026

Copy link
Copy Markdown

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Description

CKV_AZURE_249 currently passes a federated identity credential whose subject is repo:<org>/<repo>:pull_request.

That subject matches tokens minted by pull_request-triggered workflow runs — runs that execute a proposed change rather than the protected default branch. GitHub's security-hardening guidance treats the pull_request subject as a distinct, broader trust surface for exactly this reason: the code that runs during a pull request has not necessarily been reviewed or merged, and on public repositories it can originate from a fork.

Since an Azure FIC subject is an exact string match, a credential written this way trusts every pull-request run of that repository. The check inspects the subject closely enough to reject wildcards and abusable leading claims, so this seemed like a gap in the same family rather than an intentional allowance.

Change

One additional comparison in scan_resource_conf, after the existing repo-format check:

# Fourth check -> pull_request event
if (
    split_condition[0] == "repo"
    and len(split_condition) > 2
    and split_condition[2] == self.GH_PULL_REQUEST_EVENT
):
    return CheckResult.FAILED

It matches only the event segment (index 2) of a repo: subject, so it is deliberately narrow:

Subject Before After
repo:myOrg/myRepo:pull_request PASSED FAILED
repo:myOrg/myRepo:environment:pull_request PASSED PASSED (environment named pull_request)
repo:myOrg/pull_request:ref:refs/heads/x PASSED PASSED (repo named pull_request)
repo:myOrg/myRepo:ref:refs/heads/main PASSED PASSED
repo:myOrg/myRepo:environment:Production PASSED PASSED
repo:*, *, workflow:... FAILED FAILED

No other check or shared helper is touched, so CKV_AWS_358 / CKV_AWS_393 / CKV_GCP_125 are unaffected (their suites pass unchanged).

Tests

Two fixtures: fail_pull_request (the flagged subject) and pass_environment_named_pull_request, which pins the precision boundary so a later broadened match would fail the suite.

References:

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my feature, policy, or fix is effective and works
  • New and existing tests pass locally with my changes

Two things I noticed while working in this file and left alone, in case they are of interest:

  1. validate_subject_claim appears to be unused — scan_resource_conf re-implements the same sequence inline, and I could not find a caller. I mirrored the new condition into both so they do not drift further, but would removing it (or having scan_resource_conf call it) be preferable?

  2. The check registers only azuread_application_federated_identity_credential. The AzureAD provider added azuread_application_flexible_federated_identity_credential in 3.7, where matching moves to a claims_matching_expression and the matches operator honours * — so claims['sub'] matches 'repo:org/*' there really does admit the whole org, unlike a classic FIC subject where * is an inert literal. Is flexible FIC out of scope by design, or would a follow-up covering that resource be welcome?


Note on overlap: #7610 (immutable OIDC subject IDs) adds a fixture to the same two Azure test files, so whichever of the two lands first will leave the other with a small fixture-level conflict. The changes are independent — that one widens gh_repo_regex, this one adds an event-segment comparison — and I am happy to rebase whichever comes second.

A federated identity credential whose subject is
"repo:<org>/<repo>:pull_request" trusts tokens minted by
pull_request-triggered workflow runs, which execute proposed changes
rather than the protected default branch (and on public repositories may
originate from a fork). CKV_AZURE_249 passed such subjects.

Add one comparison after the existing repo-format check, matching only
the event segment of a repo: subject so that an environment, ref, or
repository merely NAMED "pull_request" is unaffected. No shared helper is
touched, so CKV_AWS_358 / CKV_AWS_393 / CKV_GCP_125 are unchanged.

Fixtures cover the flagged subject and the environment-named-pull_request
precision boundary.
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