Skip to content

fix(ci): bind canonical E2E check authorization #7529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
apurvvkumaria wants to merge 2 commits into from
Closed

fix(ci): bind canonical E2E check authorization #7529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7133453
fix(ci): bind canonical E2E check authorization
apurvvkumaria Jul 25, 2026
5c0ea66
merge: sync current main into E2E authorization
apurvvkumaria Jul 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions .github/workflows/e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,10 @@ jobs:
[[ "$CONTROLLER_CHECK_ID" =~ ^[1-9][0-9]*$ ]] || { echo "::error::controller_check_id must be a positive integer"; exit 1; }
expected_external_id="nemoclaw-pr-e2e:v2:${PR_NUMBER}:${CHECKOUT_SHA}:${BASE_SHA}"
expected_run_url="https://github.com/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}"
expected_check_url="https://github.com/${GITHUB_REPOSITORY}/runs/${CONTROLLER_CHECK_ID}"
jobs_summary="${JOBS//,/, }"
targets_summary="${TARGETS//,/, }"
expected_summary="Risk plan ${PLAN_HASH} selected jobs: ${jobs_summary:-none}; targets: ${targets_summary:-none}. Child run: ${expected_run_url}."
expected_summary="[Selected E2E run ${RUN_ID}](${expected_run_url})"$'\n\n'"Risk plan ${PLAN_HASH} selected jobs: ${jobs_summary:-none}; targets: ${targets_summary:-none}."
controller_auth_max_attempts=45
controller_auth_poll_seconds=2
check_json=""
Expand All @@ -236,9 +237,14 @@ jobs:
--header "Cache-Control: no-cache" \
--header "Pragma: no-cache" \
--header "X-GitHub-Api-Version: 2022-11-28" \
"https://api.github.com/repos/${GITHUB_REPOSITORY}/check-runs/${CONTROLLER_CHECK_ID}")" &&
[[ "$(jq -r '.output.summary // ""' <<< "$check_json")" == "$expected_summary" ]]; then
break
"https://api.github.com/repos/${GITHUB_REPOSITORY}/check-runs/${CONTROLLER_CHECK_ID}")"; then
details_url="$(jq -r '.details_url // ""' <<< "$check_json")"
summary="$(jq -r '.output.summary // ""' <<< "$check_json")"
if [[ "$summary" == "$expected_summary" ]] &&
{ [[ "$details_url" == "$expected_run_url" ]] ||
[[ "$details_url" == "$expected_check_url" ]]; }; then
break
fi
fi
check_json=""
if (( _attempt < controller_auth_max_attempts )); then
Expand All @@ -250,6 +256,8 @@ jobs:
--argjson check_id "$CONTROLLER_CHECK_ID" \
--arg external_id "$expected_external_id" \
--arg head_sha "$CHECKOUT_SHA" \
--arg check_url "$expected_check_url" \
--arg run_url "$expected_run_url" \
--arg summary "$expected_summary" \
'
.id == $check_id and
Expand All @@ -260,6 +268,7 @@ jobs:
.external_id == $external_id and
.status == "in_progress" and
.conclusion == null and
(.details_url == $run_url or .details_url == $check_url) and
.output.summary == $summary
' <<< "$check_json" >/dev/null ||
{ echo "::error::controller check does not authorize this exact E2E run"; exit 1; }
Expand Down
128 changes: 72 additions & 56 deletions test/e2e/support/e2e-operations-workflow-boundary.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,61 @@ function workflowScript(jobName: string, stepName: string): string {
return step?.with?.script as string;
}

function runControllerAuthentication(detailsUrl: string, summaryRunId = 23) {
const workflow = readE2eOperationsWorkflow();
const authentication = workflow.jobs["generate-matrix"].steps!.find(
(step) => step.name === "Authenticate controller dispatch",
)!;
const headSha = "a".repeat(40);
const baseSha = "b".repeat(40);
const planHash = "c".repeat(64);
const check = JSON.stringify({
id: 17,
name: "E2E / PR Gate Coordination",
app: { id: 15368, slug: "github-actions" },
head_sha: headSha,
external_id: `nemoclaw-pr-e2e:v2:42:${headSha}:${baseSha}`,
status: "in_progress",
conclusion: null,
details_url: detailsUrl,
output: {
summary: `[Selected E2E run ${summaryRunId}](https://github.com/NVIDIA/NemoClaw/actions/runs/${summaryRunId})\n\nRisk plan ${planHash} selected jobs: credential-sanitization; targets: none.`,
},
});
return spawnSync(
"bash",
[
"--noprofile",
"--norc",
"-e",
"-o",
"pipefail",
"-c",
`curl() { printf '%s' "$FAKE_CHECK"; }\nsleep() { :; }\n${authentication.run!}`,
],
{
encoding: "utf8",
env: {
...process.env,
ACTOR: "github-actions[bot]",
BASE_SHA: baseSha,
CHECKOUT_SHA: headSha,
CONTROLLER_CHECK_ID: "17",
CORRELATION_ID: "123e4567-e89b-42d3-a456-426614174000",
FAKE_CHECK: check,
GITHUB_REPOSITORY: "NVIDIA/NemoClaw",
GITHUB_TOKEN: "unused",
JOBS: "credential-sanitization",
PLAN_HASH: planHash,
PR_NUMBER: "42",
RUN_ATTEMPT: "1",
RUN_ID: "23",
TARGETS: "",
},
},
);
}

describe("E2E operations workflow boundary", () => {
it("accepts the checked-in workflow and rejects aggregation, permission, and secret-scope drift", () => {
expect(validateE2eOperationsWorkflowBoundary()).toEqual([]);
Expand Down Expand Up @@ -146,6 +201,7 @@ describe("E2E operations workflow boundary", () => {
"Controller authentication must bind CONTROLLER_CHECK_ID",
'Controller authentication must retain "$ACTOR" == "github-actions[bot]"',
"Controller authentication must retain .app.id == 15368",
"Controller authentication must retain (.details_url == $run_url or .details_url == $check_url)",
"Controller validation must be activated only by checkout_sha",
"Controller validation must bind BASE_SHA",
"Controller validation must bind CHECKOUT_REPOSITORY",
Expand Down Expand Up @@ -185,13 +241,13 @@ describe("E2E operations workflow boundary", () => {
)!;
authentication.run = String(authentication.run)
.replace('--header "Cache-Control: no-cache"', '--header "Cache-Control: max-age=60"')
.replace(" Child run: ${expected_run_url}.", "")
.replace("[Selected E2E run ${RUN_ID}](${expected_run_url})", "")
.replace("controller_auth_max_attempts=45", "controller_auth_max_attempts=15");

expect(validateE2eOperationsWorkflow(workflow)).toEqual(
expect.arrayContaining([
'Controller authentication must retain --header "Cache-Control: no-cache"',
"Controller authentication must retain Child run: ${expected_run_url}.",
"Controller authentication must retain [Selected E2E run ${RUN_ID}](${expected_run_url})",
"Controller authentication polling window must exceed GitHub's 60-second check cache",
]),
);
Expand Down Expand Up @@ -255,63 +311,23 @@ describe("E2E operations workflow boundary", () => {
expect(result.stderr).not.toContain("curl:");
});

it("accepts a summary-bound child when GitHub canonicalizes the details URL (#7140)", () => {
const workflow = readE2eOperationsWorkflow();
const authentication = workflow.jobs["generate-matrix"].steps!.find(
(step) => step.name === "Authenticate controller dispatch",
)!;
const headSha = "a".repeat(40);
const baseSha = "b".repeat(40);
const planHash = "c".repeat(64);
const check = JSON.stringify({
id: 17,
name: "E2E / PR Gate Coordination",
app: { id: 15368, slug: "github-actions" },
head_sha: headSha,
external_id: `nemoclaw-pr-e2e:v2:42:${headSha}:${baseSha}`,
status: "in_progress",
conclusion: null,
details_url: "https://github.com/NVIDIA/NemoClaw/runs/17",
output: {
summary: `Risk plan ${planHash} selected jobs: credential-sanitization; targets: none. Child run: https://github.com/NVIDIA/NemoClaw/actions/runs/23.`,
},
});
const result = spawnSync(
"bash",
[
"--noprofile",
"--norc",
"-e",
"-o",
"pipefail",
"-c",
`curl() { printf '%s' "$FAKE_CHECK"; }\n${authentication.run!}`,
],
{
encoding: "utf8",
env: {
...process.env,
ACTOR: "github-actions[bot]",
BASE_SHA: baseSha,
CHECKOUT_SHA: headSha,
CONTROLLER_CHECK_ID: "17",
CORRELATION_ID: "123e4567-e89b-42d3-a456-426614174000",
FAKE_CHECK: check,
GITHUB_REPOSITORY: "NVIDIA/NemoClaw",
GITHUB_TOKEN: "unused",
JOBS: "credential-sanitization",
PLAN_HASH: planHash,
PR_NUMBER: "42",
RUN_ATTEMPT: "1",
RUN_ID: "23",
TARGETS: "",
},
},
);

it.each([
["child-run URL", "https://github.com/NVIDIA/NemoClaw/actions/runs/23"],
["GitHub canonical check URL", "https://github.com/NVIDIA/NemoClaw/runs/17"],
])("accepts a controller check with the %s and exact child-run summary", (_label, url) => {
const result = runControllerAuthentication(url);
expect(result.status, result.stderr || result.stdout).toBe(0);
});

it("rejects a canonical check URL whose summary names a different child run", () => {
const result = runControllerAuthentication("https://github.com/NVIDIA/NemoClaw/runs/17", 24);

expect(result.status).toBe(1);
expect(result.stdout).toContain(
"trusted controller authorization was not published for this run",
);
});

it("binds controller dispatch to the exact checkout, plan, and correlation identity (#6955)", () => {
const workflow = readE2eOperationsWorkflow();
const validation = workflow.jobs["generate-matrix"].steps!.find(
Expand Down
9 changes: 5 additions & 4 deletions test/pr-e2e-gate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1260,9 +1260,7 @@ describe("PR E2E controller", () => {
expect(runningUpdate?.body).toMatchObject({
details_url: "https://github.com/NVIDIA/NemoClaw/actions/runs/23",
output: {
summary: expect.stringContaining(
"Child run: https://github.com/NVIDIA/NemoClaw/actions/runs/23.",
),
summary: expect.stringContaining("[Selected E2E run 23]"),
},
Comment on lines 1260 to 1264

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the complete selected-run hyperlink.

The test only checks [Selected E2E run 23], so it would pass if the summary linked to the wrong child run. Assert the full [Selected E2E run 23](https://github.com/NVIDIA/NemoClaw/actions/runs/23) value, or the exact summary contract.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-e2e-gate.test.ts` around lines 1260 - 1264, Update the summary
assertion in the running update expectation to verify the complete selected-run
Markdown hyperlink, including the exact URL for run 23, rather than only
matching its label. Keep the existing details_url assertion and ensure the
assertion rejects summaries linking to a different child run.

Source: Path instructions

});
} finally {
Expand Down Expand Up @@ -1418,9 +1416,12 @@ describe("PR E2E controller", () => {
status: "in_progress",
output: {
title: "Running 13 E2E checks",
summary: expect.stringContaining("upgrade-stale-sandbox"),
summary: expect.stringContaining(
"[Selected E2E run 23](https://github.com/NVIDIA/NemoClaw/actions/runs/23)",
),
},
});
expect(JSON.stringify(checkUpdates[1]?.body)).toContain("upgrade-stale-sandbox");

@coderabbitai coderabbitai Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the assertion scoped to output.summary.

Matching JSON.stringify(checkUpdates[1]?.body) allows upgrade-stale-sandbox to appear in any body field and does not prove that the summary reports the selected job. Assert directly on checkUpdates[1]?.body.output?.summary.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-e2e-gate.test.ts` at line 1424, Update the assertion for
checkUpdates[1] to inspect only body.output?.summary, rather than stringifying
the entire body. Preserve the existing expectation that the summary contains
"upgrade-stale-sandbox".

Source: Path instructions

expect(checkUpdates[2]?.body).toMatchObject({
status: "completed",
conclusion: "success",
Expand Down
7 changes: 5 additions & 2 deletions tools/e2e/operations-workflow-boundary.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,16 +187,19 @@ function validateControllerAuthorization(
'"$CONTROLLER_CHECK_ID" =~ ^[1-9][0-9]*$',
"nemoclaw-pr-e2e:v2:${PR_NUMBER}:${CHECKOUT_SHA}:${BASE_SHA}",
"https://github.com/${GITHUB_REPOSITORY}/actions/runs/${RUN_ID}",
"https://github.com/${GITHUB_REPOSITORY}/runs/${CONTROLLER_CHECK_ID}",
"https://api.github.com/repos/${GITHUB_REPOSITORY}/check-runs/${CONTROLLER_CHECK_ID}",
"[Selected E2E run ${RUN_ID}](${expected_run_url})",
'--header "Cache-Control: no-cache"',
"Child run: ${expected_run_url}.",
`[[ "$(jq -r '.output.summary // ""' <<< "$check_json")" == "$expected_summary" ]]`,
'--header "Pragma: no-cache"',
'[[ "$summary" == "$expected_summary" ]]',
'.name == "E2E / PR Gate Coordination"',
".app.id == 15368",
'.app.slug == "github-actions"',
".external_id == $external_id",
'.status == "in_progress"',
".conclusion == null",
"(.details_url == $run_url or .details_url == $check_url)",
".output.summary == $summary",
]) {
if (!source.includes(fragment)) {
Expand Down
6 changes: 5 additions & 1 deletion tools/e2e/pr-e2e-gate.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1467,7 +1467,11 @@ async function updateRunningCheck(
const childRunUrl = `https://github.com/${context.repository}/actions/runs/${options.childRunId}`;
const selectionCount = options.jobs.length + options.targets.length;
const title = `Running ${selectionCount} E2E ${selectionCount === 1 ? "check" : "checks"}`;
const summary = `Risk plan ${options.planHash} selected jobs: ${options.jobs.join(", ") || "none"}; targets: ${options.targets.join(", ") || "none"}. Child run: ${childRunUrl}.`;
const summary = [
`[Selected E2E run ${options.childRunId}](${childRunUrl})`,
"",
`Risk plan ${options.planHash} selected jobs: ${options.jobs.join(", ") || "none"}; targets: ${options.targets.join(", ") || "none"}.`,
].join("\n");
const check = await githubApi<unknown>(
`repos/${context.repository}/check-runs/${context.checkRunId}`,
token,
Expand Down
Loading