fix(terraform): CKV_AWS_358 & CKV_AWS_393 - inspect every value of a multi-value OIDC sub condition

[Dashtid]

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

Description

IAM evaluates the values of a single condition key with a logical OR — "each context key must resolve to true for at least one key value for the desired Allow or Deny effect to be invoked" (IAM User Guide, single-valued context key policy examples). In a GitHub Actions OIDC trust policy the loosest value in the list therefore sets the trust boundary, and a tight value beside it constrains nothing:

condition {
  test     = "StringLike"
  variable = "token.actions.githubusercontent.com:sub"
  values   = ["repo:myOrg/myRepo:ref:refs/heads/MyBranch", "*"]
}

Both AWS GitHub-OIDC checks stop after one value of that list.

CKV_AWS_393 (GithubActionsOIDCTrustPolicyOnRole.py) is the clear case. _evaluate_sub_conditions returns the verdict of the first value it classifies — including PASSED — so every later value is unreachable. assume_role_policy is jsonencode'd and reaches the check as one opaque string, so the author's JSON order survives intact. Measured on main @ d8aec9db through the Terraform Runner:

token.actions.githubusercontent.com:sub values	verdict
["repo:myOrg/myRepo:ref:refs/heads/MyBranch", "*"]	PASSED
["repo:myOrg/myRepo:ref:refs/heads/MyBranch", "workflow:github-actions:*"]	PASSED
["*"]	FAILED
["workflow:github-actions:*"]	FAILED

A bare wildcard in any non-first position is invisible.

CKV_AWS_358 reads condition_value[0] and breaks, but its exposure is narrower and worth stating up front rather than leaving for a reviewer to rediscover: clean_parser_types_lst (checkov/terraform/modules/module_utils.py:224, reached from tf_parser.parse_file) sorts string values lexicographically before any check runs. That incidentally drags "*" and "environment:*" to index 0, so those are caught today. What is missed is any unsafe value that sorts after every safe one — of the six gh_abusable_claims only workflow sorts after repo:, plus a misused repo:<owner-with-no-slash> whose owner sorts high:

values (HCL)	verdict
["repo:myOrg/myRepo:ref:refs/heads/MyBranch", "workflow:github-actions:*"]	PASSED
["repo:myOrg/myRepo:ref:refs/heads/MyBranch", "repo:zzz-org*"]	PASSED
each of those second values alone	FAILED

Narrower than CKV_AWS_393, still a miss. The sort is deliberately left untouched — it is load-bearing across the parser and is not the bug.

AWS's create-time validation does not close this either: IAM verifies only that the sub condition key "is present and that its value is not solely a wildcard character (* and ?) or null" (IAM User Guide, configuring a role for GitHub OIDC identity provider) — a presence-and-shape check that says nothing about the remaining values of a list. A static check is the layer that can catch the poisoned one.

Fix

Classify every string value of a sub condition: FAILED on the first unsafe value, PASSED only once all values of that condition are safe.

Deliberately unchanged:

• single-value semantics;
• the intentional repo:org/* pass (fix(terraform): Added support in restricting to a specific GitHub organization for GithubActionsOIDCTrustPolicy #7221), including the pass-fm-customer regression fixture;
• the existing "first sub condition decides" behaviour when several condition blocks or operators each constrain :sub — IAM ANDs those, so an unrelated loose block does not make a tightly-pinned policy unsafe. A parity fixture covering that shape returns the same verdicts as unfixed main.

Tests

There is no multi-value coverage upstream today. fail-multivalue-wildcard, fail-multivalue-abusable and pass-multivalue-pinned are added to both suites, with labels mirrored across the data and resource variants. Reverting the two check files alone, with the fixtures in place, fails the new assertions — so they are regression guards, not just added coverage.

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

---

Sequencing: this is independent of #7610 and #7627, but it touches the same two AWS test suites as #7610. The fixture additions are append-only, so if #7610 lands first I will rebase this — no need to hold either on the other's account.

It is also the sharper half of the open question left in #7610's body. That one asked whether the org-wide repo:myOrg/* pass is intended long-term; this is the case where the answer does not depend on that judgement at all, since a list can pair a fully-pinned value with one that is unambiguously unsafe. Is there a reason the value loop was scoped to a single element, or would failing closed on any unsafe value in the list be the behaviour you would want here?