Skip to content

Commit e21072e

Browse files
committed
Enforce optional required_claims on Buildkite OIDC mappings
Pipelines that run fork-PR builds can now be constrained via required_claims in ci_providers.yml. The relay validates each listed claim against the token before granting identity, rejecting builds whose claims don't match (e.g. fork branches). Pipelines without required_claims are assumed to not expose OIDC to forks.
1 parent b12f691 commit e21072e

3 files changed

Lines changed: 166 additions & 28 deletions

File tree

aws/lambda/cross_repo_ci_relay/config/ci_providers.yml

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,38 @@
33
# CI platforms that don't carry a native "repository" OIDC claim need an
44
# explicit mapping from their pipeline identity to a GitHub owner/repo.
55
#
6-
# Format: one top-level key per CI provider. Each provider section maps
7-
# the provider's pipeline identifier to the GitHub repo it represents.
8-
#
96
# IMPORTANT: Buildkite mappings use immutable organization_id/pipeline_id
107
# (UUIDs) rather than slugs. Slugs are renameable and a released slug
118
# can be claimed by a different organization; IDs are permanent.
129
# Find IDs via: buildkite-agent oidc request-token --audience <aud>
1310
#
11+
# SECURITY: By default, any build in a mapped pipeline can authenticate.
12+
# If a pipeline runs fork-PR builds, use required_claims to restrict
13+
# which builds are authorized (e.g. only builds on specific branches).
14+
# Pipelines that do NOT expose OIDC to fork builds are safe without
15+
# required_claims.
16+
#
1417
# The Lambda fetches this file at runtime (via CI_PROVIDERS_URL) and
1518
# caches it in Redis, so changes here take effect without redeployment.
1619

1720
buildkite:
18-
# organization_id/pipeline_id: owner/repo # slug (for readability)
19-
# 018e4f2a-1b2c-3d4e/018e5a6b-7c8d-9e0f: vllm-project/vllm # vllm/ci
21+
# Simple format (pipeline does not run fork builds):
22+
# organization_id/pipeline_id: owner/repo
23+
#
24+
# Constrained format (pipeline may run fork builds):
25+
# organization_id/pipeline_id:
26+
# repo: owner/repo
27+
# required_claims:
28+
# build_branch: [main, nightly] # only these branches can auth
29+
# cluster_id: specific-cluster-uuid
30+
31+
# Example entries:
32+
# 018e4f2a-1b2c-3d4e/018e5a6b-7c8d-9e0f: vllm-project/vllm
33+
#
34+
# 018e4f2a-1b2c-3d4e/018e9a0b-1c2d-3e4f:
35+
# repo: vllm-project/vllm
36+
# required_claims:
37+
# build_branch: [main, release]
2038

2139
# gitlab:
2240
# group/project: owner/repo

aws/lambda/cross_repo_ci_relay/tests/test_jwt_helper.py

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,57 @@ def test_buildkite_uses_correct_audience(self):
185185

186186
self.assertEqual(self.mock_decode.call_args.kwargs["audience"], AUDIENCE)
187187

188+
def test_required_claims_pass_when_matching(self):
189+
load_ci_provider_mappings(
190+
{
191+
"buildkite": {
192+
"org-uuid-123/pipe-uuid-456": {
193+
"repo": "myorg/myrepo",
194+
"required_claims": {"build_branch": ["main", "nightly"]},
195+
}
196+
}
197+
}
198+
)
199+
self.mock_decode.return_value = _fake_buildkite_claims(build_branch="main")
200+
claims = verify_oidc_token("bk.oidc.token")
201+
self.assertEqual(claims["repository"], "myorg/myrepo")
202+
203+
def test_required_claims_reject_disallowed_branch(self):
204+
load_ci_provider_mappings(
205+
{
206+
"buildkite": {
207+
"org-uuid-123/pipe-uuid-456": {
208+
"repo": "myorg/myrepo",
209+
"required_claims": {"build_branch": ["main", "nightly"]},
210+
}
211+
}
212+
}
213+
)
214+
self.mock_decode.return_value = _fake_buildkite_claims(
215+
build_branch="fork-pr-branch"
216+
)
217+
with self.assertRaises(HTTPException) as ctx:
218+
verify_oidc_token("bk.oidc.token")
219+
self.assertEqual(ctx.exception.status_code, 403)
220+
self.assertIn("build_branch", ctx.exception.detail)
221+
222+
def test_required_claims_reject_missing_claim(self):
223+
load_ci_provider_mappings(
224+
{
225+
"buildkite": {
226+
"org-uuid-123/pipe-uuid-456": {
227+
"repo": "myorg/myrepo",
228+
"required_claims": {"cluster_id": ["cluster-abc"]},
229+
}
230+
}
231+
}
232+
)
233+
self.mock_decode.return_value = _fake_buildkite_claims()
234+
with self.assertRaises(HTTPException) as ctx:
235+
verify_oidc_token("bk.oidc.token")
236+
self.assertEqual(ctx.exception.status_code, 403)
237+
self.assertIn("cluster_id", ctx.exception.detail)
238+
188239

189240
class TestLoadCIProviderMappings(unittest.TestCase):
190241
"""Tests for loading CI provider repo mappings from ci_providers.yml."""
@@ -205,17 +256,40 @@ def test_loads_valid_buildkite_entries(self):
205256
}
206257
load_ci_provider_mappings(raw)
207258
self.assertEqual(
208-
BUILDKITE_REPO_MAP[("org-id-1", "pipe-id-1")], "vllm-project/vllm"
259+
BUILDKITE_REPO_MAP[("org-id-1", "pipe-id-1")]["repo"],
260+
"vllm-project/vllm",
261+
)
262+
self.assertEqual(
263+
BUILDKITE_REPO_MAP[("org-id-2", "pipe-id-2")]["repo"], "acme/repo"
209264
)
210-
self.assertEqual(BUILDKITE_REPO_MAP[("org-id-2", "pipe-id-2")], "acme/repo")
265+
266+
def test_loads_constrained_entries(self):
267+
raw = {
268+
"buildkite": {
269+
"org-id/pipe-id": {
270+
"repo": "myorg/myrepo",
271+
"required_claims": {
272+
"build_branch": ["main", "nightly"],
273+
"cluster_id": "cluster-uuid",
274+
},
275+
}
276+
}
277+
}
278+
load_ci_provider_mappings(raw)
279+
entry = BUILDKITE_REPO_MAP[("org-id", "pipe-id")]
280+
self.assertEqual(entry["repo"], "myorg/myrepo")
281+
self.assertEqual(
282+
entry["required_claims"]["build_branch"], ["main", "nightly"]
283+
)
284+
self.assertEqual(entry["required_claims"]["cluster_id"], ["cluster-uuid"])
211285

212286
def test_empty_config_clears_map(self):
213-
BUILDKITE_REPO_MAP[("old", "entry")] = "old/repo"
287+
BUILDKITE_REPO_MAP[("old", "entry")] = {"repo": "old/repo", "required_claims": {}}
214288
load_ci_provider_mappings({})
215289
self.assertEqual(len(BUILDKITE_REPO_MAP), 0)
216290

217291
def test_missing_buildkite_section_clears_map(self):
218-
BUILDKITE_REPO_MAP[("old", "entry")] = "old/repo"
292+
BUILDKITE_REPO_MAP[("old", "entry")] = {"repo": "old/repo", "required_claims": {}}
219293
load_ci_provider_mappings({"gitlab": {"group/proj": "org/repo"}})
220294
self.assertEqual(len(BUILDKITE_REPO_MAP), 0)
221295

@@ -225,7 +299,9 @@ def test_skips_invalid_entries(self):
225299
}
226300
load_ci_provider_mappings(raw)
227301
self.assertNotIn(("noslash", ""), BUILDKITE_REPO_MAP)
228-
self.assertEqual(BUILDKITE_REPO_MAP[("ok-id", "pipe-id")], "ok/repo")
302+
self.assertEqual(
303+
BUILDKITE_REPO_MAP[("ok-id", "pipe-id")]["repo"], "ok/repo"
304+
)
229305

230306

231307
class TestUnsupportedIssuer(unittest.TestCase):

aws/lambda/cross_repo_ci_relay/utils/jwt_helper.py

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,67 @@
3535
}
3636

3737
# Runtime-populated mapping from Buildkite (organization_id, pipeline_id) to
38-
# the GitHub-style "owner/repo" identity. Uses immutable IDs rather than
39-
# slugs to prevent identity hijacking via slug rename. Loaded from
40-
# ci_providers.yml so adding a new downstream repo only requires a config
41-
# change — no Lambda redeployment.
42-
BUILDKITE_REPO_MAP: Dict[Tuple[str, str], str] = {}
38+
# the GitHub-style "owner/repo" identity plus optional required claims.
39+
# Uses immutable IDs rather than slugs to prevent identity hijacking via slug
40+
# rename. Loaded from ci_providers.yml so adding a new downstream repo only
41+
# requires a config change — no Lambda redeployment.
42+
#
43+
# Values are dicts: {"repo": "owner/repo", "required_claims": {...}}
44+
# required_claims maps claim_name -> list of allowed values (any match passes).
45+
BUILDKITE_REPO_MAP: Dict[Tuple[str, str], dict] = {}
4346

4447

4548
def load_ci_provider_mappings(raw: dict) -> None:
4649
"""Populate provider repo maps from parsed ci_providers.yml content.
4750
48-
Currently supports ``buildkite``. Adding a new provider means adding
49-
a new section reader here and a corresponding ``_extract_repo_*``
50-
function below.
51+
Buildkite entries support two formats:
52+
org_id/pipeline_id: owner/repo # simple
53+
org_id/pipeline_id: # with constraints
54+
repo: owner/repo
55+
required_claims:
56+
build_branch: [main, nightly]
5157
"""
5258
BUILDKITE_REPO_MAP.clear()
5359
bk_section = raw.get("buildkite")
5460
if bk_section and isinstance(bk_section, dict):
55-
for bk_key, repo in bk_section.items():
61+
for bk_key, value in bk_section.items():
5662
bk_key_str = str(bk_key).strip()
57-
repo_str = str(repo).strip()
58-
if "/" not in bk_key_str or "/" not in repo_str:
59-
logger.warning(
60-
"Skipping invalid buildkite entry: %s -> %s", bk_key, repo
61-
)
63+
if "/" not in bk_key_str:
64+
logger.warning("Skipping buildkite entry without /: %s", bk_key)
6265
continue
6366
org, pipeline = bk_key_str.split("/", 1)
64-
BUILDKITE_REPO_MAP[(org, pipeline)] = repo_str
67+
68+
if isinstance(value, str):
69+
repo_str = value.strip()
70+
if "/" not in repo_str:
71+
logger.warning(
72+
"Skipping invalid buildkite entry: %s -> %s", bk_key, value
73+
)
74+
continue
75+
BUILDKITE_REPO_MAP[(org, pipeline)] = {
76+
"repo": repo_str,
77+
"required_claims": {},
78+
}
79+
elif isinstance(value, dict):
80+
repo_str = str(value.get("repo", "")).strip()
81+
if "/" not in repo_str:
82+
logger.warning(
83+
"Skipping buildkite entry with invalid repo: %s", bk_key
84+
)
85+
continue
86+
required = value.get("required_claims", {})
87+
if not isinstance(required, dict):
88+
required = {}
89+
normalized = {}
90+
for k, v in required.items():
91+
if isinstance(v, list):
92+
normalized[k] = [str(x) for x in v]
93+
else:
94+
normalized[k] = [str(v)]
95+
BUILDKITE_REPO_MAP[(org, pipeline)] = {
96+
"repo": repo_str,
97+
"required_claims": normalized,
98+
}
6599
if BUILDKITE_REPO_MAP:
66100
logger.info(
67101
"Loaded %d Buildkite repo mapping(s) from ci_providers",
@@ -137,13 +171,23 @@ def _extract_repo_buildkite(claims: dict) -> str:
137171
401,
138172
"Buildkite OIDC token missing 'organization_id' or 'pipeline_id'",
139173
)
140-
repo = BUILDKITE_REPO_MAP.get((org_id, pipeline_id))
141-
if not repo:
174+
entry = BUILDKITE_REPO_MAP.get((org_id, pipeline_id))
175+
if not entry:
142176
raise HTTPException(
143177
403,
144178
f"Buildkite pipeline {org_id}/{pipeline_id} is not registered with CRCR",
145179
)
146-
return repo
180+
181+
for claim_name, allowed_values in entry["required_claims"].items():
182+
actual = str(claims.get(claim_name, ""))
183+
if actual not in allowed_values:
184+
raise HTTPException(
185+
403,
186+
f"Buildkite claim '{claim_name}' value '{actual}' "
187+
f"not in allowed set for this pipeline",
188+
)
189+
190+
return entry["repo"]
147191

148192

149193
_REPO_EXTRACTORS = {

0 commit comments

Comments
 (0)