From 980722017b6ff114c5f5979437f2aa8dcc1634df Mon Sep 17 00:00:00 2001 From: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:01:23 -0700 Subject: [PATCH 1/3] fix(e2e): retry transient permission reads Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> --- .github/workflows/e2e.yaml | 186 +++++++++++-- test/e2e/RETRY_INVENTORY.md | 1 + .../e2e-collaborator-permission-retry.test.ts | 251 ++++++++++++++++++ .../e2e-operations-workflow-boundary.test.ts | 9 +- test/e2e/support/e2e-workflow.test.ts | 12 +- 5 files changed, 440 insertions(+), 19 deletions(-) create mode 100644 test/e2e/support/e2e-collaborator-permission-retry.test.ts diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index ce0ed3dd5dd..77673fc927a 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -288,6 +288,62 @@ jobs: run: | set -euo pipefail + read_collaborator_permission() { + local maintainer="$1" + local attempt curl_exit failure http_status permission_file + permission_file="$(mktemp "${RUNNER_TEMP:-/tmp}/nemoclaw-collaborator-permission.XXXXXX")" + + for attempt in 1 2 3; do + : >"$permission_file" + if http_status="$(curl --silent --proto '=https' --connect-timeout 10 --max-time 30 \ + --output "$permission_file" --write-out "%{http_code}" \ + --header "Authorization: Bearer ${GITHUB_TOKEN}" \ + --header "Accept: application/vnd.github+json" \ + --header "X-GitHub-Api-Version: 2022-11-28" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${maintainer}/permission" \ + 2>/dev/null)"; then + if [[ "$http_status" =~ ^2[0-9]{2}$ ]]; then + if jq -e 'type == "object" and (.user.login | type == "string") and (.role_name | type == "string")' "$permission_file" >/dev/null 2>&1; then + if (( attempt > 1 )); then + echo "::notice::Collaborator permission read passed after retry on attempt ${attempt}/3" >&2 + fi + cat "$permission_file" + rm -f "$permission_file" + return 0 + fi + echo "::error::Collaborator permission read attempt ${attempt}/3 failed: malformed response" >&2 + rm -f "$permission_file" + return 1 + fi + if [[ "$http_status" =~ ^[0-9]{3}$ ]]; then + failure="HTTP ${http_status}" + case "$http_status" in + 408 | 429 | 5??) ;; + *) echo "::error::Collaborator permission read attempt ${attempt}/3 failed: ${failure}" >&2; rm -f "$permission_file"; return 1 ;; + esac + else + echo "::error::Collaborator permission read attempt ${attempt}/3 failed: invalid HTTP status" >&2 + rm -f "$permission_file" + return 1 + fi + else + curl_exit=$? + case "$curl_exit" in + 5 | 6 | 7 | 16 | 18 | 28 | 35 | 52 | 55 | 56 | 92 | 95 | 96) failure="transport" ;; + *) echo "::error::Collaborator permission read attempt ${attempt}/3 failed: curl exit ${curl_exit}" >&2; rm -f "$permission_file"; return 1 ;; + esac + fi + + if (( attempt == 3 )); then + echo "::error::Collaborator permission read exhausted after attempt ${attempt}/3: ${failure}" >&2 + rm -f "$permission_file" + return 1 + fi + echo "::warning::Collaborator permission read attempt ${attempt}/3 failed: ${failure}; retrying" >&2 + sleep "$attempt" + done + } + require_maintainer() { local maintainer="$1" [[ "$maintainer" =~ ^[A-Za-z0-9-]{1,39}$ && "$maintainer" != -* && "$maintainer" != *- ]] || { @@ -295,11 +351,7 @@ jobs: exit 1 } local permission_json - permission_json="$(curl --fail --silent --show-error --proto '=https' \ - --header "Authorization: Bearer ${GITHUB_TOKEN}" \ - --header "Accept: application/vnd.github+json" \ - --header "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${maintainer}/permission")" + permission_json="$(read_collaborator_permission "$maintainer")" case "$(jq -r '.role_name // ""' <<< "$permission_json")" in maintain | admin) ;; *) echo "::error::Manual PR E2E requires a repository maintainer or administrator" >&2; exit 1 ;; @@ -361,6 +413,62 @@ jobs: run: | set -euo pipefail + read_collaborator_permission() { + local administrator="$1" + local attempt curl_exit failure http_status permission_file + permission_file="$(mktemp "${RUNNER_TEMP:-/tmp}/nemoclaw-collaborator-permission.XXXXXX")" + + for attempt in 1 2 3; do + : >"$permission_file" + if http_status="$(curl --silent --proto '=https' --connect-timeout 10 --max-time 30 \ + --output "$permission_file" --write-out "%{http_code}" \ + --header "Authorization: Bearer ${GITHUB_TOKEN}" \ + --header "Accept: application/vnd.github+json" \ + --header "X-GitHub-Api-Version: 2022-11-28" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${administrator}/permission" \ + 2>/dev/null)"; then + if [[ "$http_status" =~ ^2[0-9]{2}$ ]]; then + if jq -e 'type == "object" and (.user.login | type == "string") and (.role_name | type == "string")' "$permission_file" >/dev/null 2>&1; then + if (( attempt > 1 )); then + echo "::notice::Collaborator permission read passed after retry on attempt ${attempt}/3" >&2 + fi + cat "$permission_file" + rm -f "$permission_file" + return 0 + fi + echo "::error::Collaborator permission read attempt ${attempt}/3 failed: malformed response" >&2 + rm -f "$permission_file" + return 1 + fi + if [[ "$http_status" =~ ^[0-9]{3}$ ]]; then + failure="HTTP ${http_status}" + case "$http_status" in + 408 | 429 | 5??) ;; + *) echo "::error::Collaborator permission read attempt ${attempt}/3 failed: ${failure}" >&2; rm -f "$permission_file"; return 1 ;; + esac + else + echo "::error::Collaborator permission read attempt ${attempt}/3 failed: invalid HTTP status" >&2 + rm -f "$permission_file" + return 1 + fi + else + curl_exit=$? + case "$curl_exit" in + 5 | 6 | 7 | 16 | 18 | 28 | 35 | 52 | 55 | 56 | 92 | 95 | 96) failure="transport" ;; + *) echo "::error::Collaborator permission read attempt ${attempt}/3 failed: curl exit ${curl_exit}" >&2; rm -f "$permission_file"; return 1 ;; + esac + fi + + if (( attempt == 3 )); then + echo "::error::Collaborator permission read exhausted after attempt ${attempt}/3: ${failure}" >&2 + rm -f "$permission_file" + return 1 + fi + echo "::warning::Collaborator permission read attempt ${attempt}/3 failed: ${failure}; retrying" >&2 + sleep "$attempt" + done + } + require_admin() { local administrator="$1" if [[ ! "$administrator" =~ ^[A-Za-z0-9-]{1,39}$ || "$administrator" == -* || "$administrator" == *- ]]; then @@ -369,11 +477,7 @@ jobs: fi local permission_json - permission_json="$(curl --fail --silent --show-error --proto '=https' \ - --header "Authorization: Bearer ${GITHUB_TOKEN}" \ - --header "Accept: application/vnd.github+json" \ - --header "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${administrator}/permission")" + permission_json="$(read_collaborator_permission "$administrator")" if [[ "$(jq -r '.user.login // ""' <<< "$permission_json" | tr '[:upper:]' '[:lower:]')" != "$(tr '[:upper:]' '[:lower:]' <<< "$administrator")" ]]; then echo "::error::Release qualification waiver permission response did not match the actor" >&2 exit 1 @@ -500,6 +604,62 @@ jobs: run: | set -euo pipefail + read_collaborator_permission() { + local maintainer="$1" + local attempt curl_exit failure http_status permission_file + permission_file="$(mktemp "${RUNNER_TEMP:-/tmp}/nemoclaw-collaborator-permission.XXXXXX")" + + for attempt in 1 2 3; do + : >"$permission_file" + if http_status="$(curl --silent --proto '=https' --connect-timeout 10 --max-time 30 \ + --output "$permission_file" --write-out "%{http_code}" \ + --header "Authorization: Bearer ${GITHUB_TOKEN}" \ + --header "Accept: application/vnd.github+json" \ + --header "X-GitHub-Api-Version: 2022-11-28" \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${maintainer}/permission" \ + 2>/dev/null)"; then + if [[ "$http_status" =~ ^2[0-9]{2}$ ]]; then + if jq -e 'type == "object" and (.user.login | type == "string") and (.role_name | type == "string")' "$permission_file" >/dev/null 2>&1; then + if (( attempt > 1 )); then + echo "::notice::Collaborator permission read passed after retry on attempt ${attempt}/3" >&2 + fi + cat "$permission_file" + rm -f "$permission_file" + return 0 + fi + echo "::error::Collaborator permission read attempt ${attempt}/3 failed: malformed response" >&2 + rm -f "$permission_file" + return 1 + fi + if [[ "$http_status" =~ ^[0-9]{3}$ ]]; then + failure="HTTP ${http_status}" + case "$http_status" in + 408 | 429 | 5??) ;; + *) echo "::error::Collaborator permission read attempt ${attempt}/3 failed: ${failure}" >&2; rm -f "$permission_file"; return 1 ;; + esac + else + echo "::error::Collaborator permission read attempt ${attempt}/3 failed: invalid HTTP status" >&2 + rm -f "$permission_file" + return 1 + fi + else + curl_exit=$? + case "$curl_exit" in + 5 | 6 | 7 | 16 | 18 | 28 | 35 | 52 | 55 | 56 | 92 | 95 | 96) failure="transport" ;; + *) echo "::error::Collaborator permission read attempt ${attempt}/3 failed: curl exit ${curl_exit}" >&2; rm -f "$permission_file"; return 1 ;; + esac + fi + + if (( attempt == 3 )); then + echo "::error::Collaborator permission read exhausted after attempt ${attempt}/3: ${failure}" >&2 + rm -f "$permission_file" + return 1 + fi + echo "::warning::Collaborator permission read attempt ${attempt}/3 failed: ${failure}; retrying" >&2 + sleep "$attempt" + done + } + require_maintainer() { local maintainer="$1" if [[ ! "$maintainer" =~ ^[A-Za-z0-9-]{1,39}$ || "$maintainer" == -* || "$maintainer" == *- ]]; then @@ -508,11 +668,7 @@ jobs: fi local permission_json - permission_json="$(curl --fail --silent --show-error --proto '=https' \ - --header "Authorization: Bearer ${GITHUB_TOKEN}" \ - --header "Accept: application/vnd.github+json" \ - --header "X-GitHub-Api-Version: 2022-11-28" \ - "https://api.github.com/repos/${GITHUB_REPOSITORY}/collaborators/${maintainer}/permission")" + permission_json="$(read_collaborator_permission "$maintainer")" if [[ "$(jq -r '.user.login // ""' <<< "$permission_json" | tr '[:upper:]' '[:lower:]')" != "$(tr '[:upper:]' '[:lower:]' <<< "$maintainer")" ]]; then echo "::error::Launchable image publication permission response did not match the actor" >&2 exit 1 diff --git a/test/e2e/RETRY_INVENTORY.md b/test/e2e/RETRY_INVENTORY.md index dc39f9277a5..64c05e7c1b8 100644 --- a/test/e2e/RETRY_INVENTORY.md +++ b/test/e2e/RETRY_INVENTORY.md @@ -17,6 +17,7 @@ Exhaustion remains failed. | `hosted-runner-recovery` | Confirmed GitHub-hosted runner loss; `tools/e2e/hosted-runner-recovery.mts`, `tools/e2e/hosted-runner-loss*.mts` | Authenticated runner-allocation or internal-runner evidence that remains identical across 2 consecutive reads | 2 immediate evidence reads and at most 1 recovery request; no delay | GitHub reruns a workflow attempt | GitHub Actions | Dedicated runner-loss classifications | Source and recovery run links plus authenticated job evidence | External owner; governed by #7146, not this policy | | `pr-rerun-reconciliation` | PR E2E dispatch reconciliation; `tools/e2e/pr-e2e-dispatch-reconciliation.mts`, `tools/e2e/pr-e2e-retry-receipt.mts` | Trusted dispatch receipt state | Contract-defined single reconciliation | Reconciles workflow and commit identity before action | GitHub Actions | Receipt-specific terminal states | Signed workflow identity and receipt | External scope; governed by #7206 | | `github-publication-read` | GitHub API reads; `tools/e2e/base-image-publication.mts` | Fetch error, 408, rate limit, or 5xx | 3 attempts; Retry-After/rate-limit reset or linear delay capped at 10s | Read-only | GitHub API | Returned parsed selection on success; thrown terminal HTTP/fetch error on failure or exhaustion | Caller artifact records the returned publication selection; terminal errors identify exhausted fetch or HTTP status without response content | Eligible bounded read; existing implementation retained | +| `trusted-controller-collaborator-permission-read` | Collaborator-permission reads for manual PR dispatch, release waiver, and Launchable publication; `.github/workflows/e2e.yaml` | Curl exit 5, 6, 7, 16, 18, 28, 35, 52, 55, 56, 92, 95, or 96; HTTP 408, 429, or 5xx | 3 attempts; linear 1s then 2s | Read-only GitHub API request | GitHub API | Transient API read versus terminal authentication, authorization, actor, or response failure | Operation name, attempt number, and sanitized failure class or HTTP status; no response body, header, or token | Eligible bounded read; HTTP 401, 403, 404, and 422, malformed responses, actor failures, and insufficient roles remain terminal; no cached permission or workflow rerun | | `inference-switch-ts` | Verified inference route update; `test/e2e/fixtures/inference-switch-retry.ts` | Timeout, reset, DNS/connectivity/connect error, request transport error, or exact 502/503/504 status; authentication, authorization, policy, malformed-input, and invalid-request signals take precedence | 1-10 attempts; linear 5s | Setting the same desired provider/model is idempotent | Inference provider | Shared `RetryEvidence` classifications | Every attempt classification and aggregate outcome; command artifacts remain separate and redacted | Uses `runBoundedRetry`; deterministic verification mismatches stop; no `--no-verify` exhaustion bypass | | `inference-switch-shell` | Verified shell inference route update; `test/e2e/lib/inference-switch-retry.sh` | Same bounded transient and terminal-precedence signatures as the TypeScript helper | 1-10 attempts; linear 5s | Setting the same desired provider/model is idempotent | Inference provider | Exit status remains failed on exhaustion | Existing command output and retry progress | Bounded compatibility helper; no `--no-verify` exhaustion bypass | | `provider-install-standard` | Provider validation during Brave, cron, device-auth, Hermes-switch, network-policy, and restricted onboarding | `isTransientProviderValidationFailure` allowlist only | 1 local or 3 CI attempts; linear 10s backoff | Repeats the same desired onboarding state; restricted paths destroy the prior sandbox before retry | Inference provider | Transient allowlist versus terminal install failure | Per-attempt command artifacts; restricted paths add a terminal skip artifact | Existing bounded paths; no deterministic install retry | diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts new file mode 100644 index 00000000000..3730e09b003 --- /dev/null +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -0,0 +1,251 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { readWorkflow } from "../../helpers/e2e-workflow-contract"; + +type AuthorizationStep = { + deniedMessage: string; + name: string; +}; + +type PermissionScenario = + | "denied" + | "malformed-success" + | "terminal-http" + | "transient-then-success" + | "transport-exhaustion"; + +const AUTHORIZATION_STEPS: AuthorizationStep[] = [ + { + deniedMessage: "Manual PR E2E requires a repository maintainer or administrator", + name: "Authenticate manual PR dispatch", + }, + { + deniedMessage: "Release qualification waiver requires a repository administrator", + name: "Authorize release qualification waiver", + }, + { + deniedMessage: "Launchable image publication requires a repository maintainer or administrator", + name: "Authorize Launchable image publication", + }, +]; + +function authorizationScript(stepName: string): string { + const workflow = readWorkflow() as { + jobs: Record }>; + }; + const step = workflow.jobs["generate-matrix"]!.steps!.find( + (candidate) => candidate.name === stepName, + ); + expect(step?.run).toEqual(expect.any(String)); + return step!.run!; +} + +function lines(path: string): string[] { + if (!existsSync(path)) return []; + const value = readFileSync(path, "utf8").trim(); + return value === "" ? [] : value.split("\n"); +} + +function runAuthorization( + stepName: string, + scenario: PermissionScenario, + options: { actor?: string; status?: string } = {}, +) { + const fixture = mkdtempSync(join(tmpdir(), "nemoclaw-collaborator-permission-")); + const attemptFile = join(fixture, "attempts"); + const curlLog = join(fixture, "curl.log"); + const sleepLog = join(fixture, "sleep.log"); + const curlPath = join(fixture, "curl"); + const sleepPath = join(fixture, "sleep"); + writeFileSync( + curlPath, + `#!/usr/bin/env bash +set -euo pipefail +output_file="" +write_out="" +url="\${!#}" +while (( $# > 0 )); do + case "$1" in + --output) output_file="$2"; shift 2 ;; + --write-out) write_out="$2"; shift 2 ;; + *) shift ;; + esac +done + +if [[ "$url" == *"/collaborators/"*"/permission" ]]; then + actor="\${url%/permission}" + actor="\${actor##*/}" + attempt=0 + if [[ -f "$PERMISSION_ATTEMPT_FILE" ]]; then read -r attempt <"$PERMISSION_ATTEMPT_FILE"; fi + attempt=$((attempt + 1)) + printf '%s\n' "$attempt" >"$PERMISSION_ATTEMPT_FILE" + printf '%s\n' "permission" >>"$CURL_LOG" + status=200 + curl_exit=0 + printf -v body '{"user":{"login":"%s"},"role_name":"admin"}' "$actor" + case "$PERMISSION_SCENARIO" in + transient-then-success) + if (( attempt == 1 )); then status="$PERMISSION_TEST_STATUS"; body="private-response-body"; fi + ;; + transport-exhaustion) status=000; curl_exit=7; body="" ;; + terminal-http) status="$PERMISSION_TEST_STATUS"; body="private-response-body" ;; + malformed-success) body="private-response-body" ;; + denied) printf -v body '{"user":{"login":"%s"},"role_name":"write"}' "$actor" ;; + esac + if [[ -n "$output_file" ]]; then printf '%s' "$body" >"$output_file"; else printf '%s' "$body"; fi + if [[ -n "$write_out" ]]; then printf '%s' "$status"; fi + exit "$curl_exit" +fi + +if [[ "$url" == *"/pulls/42" ]]; then + printf '%s\n' "pull" >>"$CURL_LOG" + printf '%s' '{"state":"open","head":{"repo":{"full_name":"contributor/NemoClaw"},"sha":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},"base":{"sha":"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"}}' + exit 0 +fi + +exit 2 +`, + ); + writeFileSync( + sleepPath, + `#!/usr/bin/env bash +set -euo pipefail +printf '%s\n' "$1" >>"$SLEEP_LOG" +`, + ); + chmodSync(curlPath, 0o755); + chmodSync(sleepPath, 0o755); + + const workflowSha = "c".repeat(40); + const result = spawnSync("bash", ["--noprofile", "--norc", "-c", authorizationScript(stepName)], { + encoding: "utf8", + env: { + ...process.env, + ACTOR: options.actor ?? "dispatch-admin", + ALLOW_DGX_SPARK_RUNNER_QUEUE: "false", + ALLOW_JETSON_DISPATCH: "false", + BASE_SHA: "b".repeat(40), + CHECKOUT_REPOSITORY: "contributor/NemoClaw", + CHECKOUT_SHA: stepName === "Authenticate manual PR dispatch" ? "a".repeat(40) : "", + CURL_LOG: curlLog, + EXPECTED_WORKFLOW_SHA: workflowSha, + GITHUB_REPOSITORY: "NVIDIA/NemoClaw", + GITHUB_TOKEN: "private-test-token", + INCLUDE_LAUNCHABLE: stepName === "Authenticate manual PR dispatch" ? "false" : "true", + JOBS: "", + PATH: `${fixture}:${process.env.PATH ?? ""}`, + PERMISSION_ATTEMPT_FILE: attemptFile, + PERMISSION_SCENARIO: scenario, + PERMISSION_TEST_STATUS: options.status ?? "503", + PR_NUMBER: "42", + REVIEW_REASON: "Reviewed latest PR commit", + RUN_ATTEMPT: "1", + RUNNER_TEMP: fixture, + SLEEP_LOG: sleepLog, + TARGETS: "", + TRIGGERING_ACTOR: "dispatch-admin", + WAIVED_JOBS: "staging-brev-launchable", + WAIVER_REASON: "Brev credential expired", + WORKFLOW_EVENT: "workflow_dispatch", + WORKFLOW_REF: "refs/heads/main", + WORKFLOW_SHA: workflowSha, + }, + }); + const permissionAttempts = existsSync(attemptFile) + ? Number.parseInt(readFileSync(attemptFile, "utf8"), 10) + : 0; + const curlOperations = lines(curlLog); + const sleeps = lines(sleepLog); + rmSync(fixture, { force: true, recursive: true }); + return { ...result, curlOperations, permissionAttempts, sleeps }; +} + +describe.each(AUTHORIZATION_STEPS)( + "$name collaborator permission read", + ({ deniedMessage, name }) => { + it.each(["408", "429", "503"])( + "retries HTTP %s once before authorization succeeds (#9337)", + (status) => { + const result = runAuthorization(name, "transient-then-success", { status }); + + expect(result.status, result.stderr).toBe(0); + expect(result.permissionAttempts).toBe(2); + expect(result.sleeps).toEqual(["1"]); + expect(result.stderr).toContain( + `Collaborator permission read attempt 1/3 failed: HTTP ${status}; retrying`, + ); + expect(result.stderr).toContain( + "Collaborator permission read passed after retry on attempt 2/3", + ); + expect(result.stderr).not.toContain("private-response-body"); + expect(result.stderr).not.toContain("private-test-token"); + }, + ); + + it("stops after three transient transport failures (#9337)", () => { + const result = runAuthorization(name, "transport-exhaustion"); + + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(3); + expect(result.sleeps).toEqual(["1", "2"]); + expect(result.curlOperations).toEqual(["permission", "permission", "permission"]); + expect(result.stderr).toContain( + "Collaborator permission read exhausted after attempt 3/3: transport", + ); + }); + + it.each(["401", "403", "404", "422"])("does not retry HTTP %s (#9337)", (status) => { + const result = runAuthorization(name, "terminal-http", { status }); + + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(1); + expect(result.sleeps).toEqual([]); + expect(result.curlOperations).toEqual(["permission"]); + expect(result.stderr).toContain( + `Collaborator permission read attempt 1/3 failed: HTTP ${status}`, + ); + expect(result.stderr).not.toContain("private-response-body"); + }); + + it("does not retry a malformed HTTP 200 response (#9337)", () => { + const result = runAuthorization(name, "malformed-success"); + + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(1); + expect(result.sleeps).toEqual([]); + expect(result.curlOperations).toEqual(["permission"]); + expect(result.stderr).toContain( + "Collaborator permission read attempt 1/3 failed: malformed response", + ); + expect(result.stderr).not.toContain("private-response-body"); + }); + + it("does not retry a valid response with an unauthorized role (#9337)", () => { + const result = runAuthorization(name, "denied"); + + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(1); + expect(result.sleeps).toEqual([]); + expect(result.curlOperations).toEqual(["permission"]); + expect(result.stderr).toContain(deniedMessage); + }); + + it("rejects an invalid actor before the permission read (#9337)", () => { + const result = runAuthorization(name, "transient-then-success", { actor: "invalid actor" }); + + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(0); + expect(result.curlOperations).toEqual([]); + expect(result.sleeps).toEqual([]); + expect(result.stderr).toContain("actor is invalid"); + }); + }, +); diff --git a/test/e2e/support/e2e-operations-workflow-boundary.test.ts b/test/e2e/support/e2e-operations-workflow-boundary.test.ts index 33f69ec1f18..78c65a2f3c5 100644 --- a/test/e2e/support/e2e-operations-workflow-boundary.test.ts +++ b/test/e2e/support/e2e-operations-workflow-boundary.test.ts @@ -451,8 +451,13 @@ const interpolatedNeeds = \${{ toJSON ( needs ) }}; const workflowSha = "c".repeat(40); const prefix = [ "curl() {", - ' case "${@: -1}" in', - ` *collaborators*) printf '%s' '{"role_name":"${role}"}' ;;`, + ' local url="${@: -1}" output_file="" previous="" argument body', + ' for argument in "$@"; do', + ' if [[ "$previous" == "--output" ]]; then output_file="$argument"; fi', + ' previous="$argument"', + " done", + ' case "$url" in', + ` *collaborators*) body='{"user":{"login":"maintainer"},"role_name":"${role}"}'; if [[ -n "$output_file" ]]; then printf '%s' "$body" >"$output_file"; printf '200'; else printf '%s' "$body"; fi ;;`, ` *pulls/42) printf '%s' '{"state":"open","head":{"repo":{"full_name":"contributor/NemoClaw"},"sha":"${headSha}"},"base":{"sha":"${baseSha}"}}' ;;`, " *) return 1 ;;", " esac", diff --git a/test/e2e/support/e2e-workflow.test.ts b/test/e2e/support/e2e-workflow.test.ts index 2add2e7ca82..dea9b3443aa 100644 --- a/test/e2e/support/e2e-workflow.test.ts +++ b/test/e2e/support/e2e-workflow.test.ts @@ -47,13 +47,21 @@ set -euo pipefail url="\${!#}" administrator="\${url%/permission}" administrator="\${administrator##*/}" +output_file="" +previous="" +for argument in "$@"; do + if [[ "$previous" == "--output" ]]; then output_file="$argument"; fi + previous="$argument" +done printf '%s\n' "$administrator" >>"$CURL_LOG" case "$administrator" in maintainer) role=maintain ;; - mismatch) printf '%s\n' '{"user":{"login":"different-user"},"role_name":"admin"}'; exit 0 ;; + mismatch) login=different-user; role=admin ;; *) role=admin ;; esac -printf '{"user":{"login":"%s"},"role_name":"%s"}\n' "$administrator" "$role" +login="\${login:-$administrator}" +printf -v body '{"user":{"login":"%s"},"role_name":"%s"}' "$login" "$role" +if [[ -n "$output_file" ]]; then printf '%s' "$body" >"$output_file"; printf '200'; else printf '%s' "$body"; fi `, ); fs.chmodSync(curlPath, 0o755); From 52c2a447aeed4a9b214832c60b1d3ba3e492edfa Mon Sep 17 00:00:00 2001 From: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:15:20 -0700 Subject: [PATCH 2/3] test(e2e): keep permission fixture linear Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> --- test/e2e/support/e2e-collaborator-permission-retry.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index 3730e09b003..bb4edaf6105 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -49,8 +49,7 @@ function authorizationScript(stepName: string): string { } function lines(path: string): string[] { - if (!existsSync(path)) return []; - const value = readFileSync(path, "utf8").trim(); + const value = existsSync(path) ? readFileSync(path, "utf8").trim() : ""; return value === "" ? [] : value.split("\n"); } From 44d54505d0ffc367bac5da2071dbcd34e51f247f Mon Sep 17 00:00:00 2001 From: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:19:30 -0700 Subject: [PATCH 3/3] fix(e2e): verify permission response actor Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> --- .github/workflows/e2e.yaml | 4 ++++ .../e2e-collaborator-permission-retry.test.ts | 21 ++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 77673fc927a..b90dced0680 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -352,6 +352,10 @@ jobs: } local permission_json permission_json="$(read_collaborator_permission "$maintainer")" + if [[ "$(jq -r '.user.login // ""' <<< "$permission_json" | tr '[:upper:]' '[:lower:]')" != "$(tr '[:upper:]' '[:lower:]' <<< "$maintainer")" ]]; then + echo "::error::Manual PR E2E permission response did not match the actor" >&2 + exit 1 + fi case "$(jq -r '.role_name // ""' <<< "$permission_json")" in maintain | admin) ;; *) echo "::error::Manual PR E2E requires a repository maintainer or administrator" >&2; exit 1 ;; diff --git a/test/e2e/support/e2e-collaborator-permission-retry.test.ts b/test/e2e/support/e2e-collaborator-permission-retry.test.ts index bb4edaf6105..635d785e2ae 100644 --- a/test/e2e/support/e2e-collaborator-permission-retry.test.ts +++ b/test/e2e/support/e2e-collaborator-permission-retry.test.ts @@ -12,12 +12,14 @@ import { readWorkflow } from "../../helpers/e2e-workflow-contract"; type AuthorizationStep = { deniedMessage: string; + mismatchMessage: string; name: string; }; type PermissionScenario = | "denied" | "malformed-success" + | "mismatched-actor" | "terminal-http" | "transient-then-success" | "transport-exhaustion"; @@ -25,14 +27,17 @@ type PermissionScenario = const AUTHORIZATION_STEPS: AuthorizationStep[] = [ { deniedMessage: "Manual PR E2E requires a repository maintainer or administrator", + mismatchMessage: "Manual PR E2E permission response did not match the actor", name: "Authenticate manual PR dispatch", }, { deniedMessage: "Release qualification waiver requires a repository administrator", + mismatchMessage: "Release qualification waiver permission response did not match the actor", name: "Authorize release qualification waiver", }, { deniedMessage: "Launchable image publication requires a repository maintainer or administrator", + mismatchMessage: "Launchable image publication permission response did not match the actor", name: "Authorize Launchable image publication", }, ]; @@ -82,8 +87,7 @@ done if [[ "$url" == *"/collaborators/"*"/permission" ]]; then actor="\${url%/permission}" actor="\${actor##*/}" - attempt=0 - if [[ -f "$PERMISSION_ATTEMPT_FILE" ]]; then read -r attempt <"$PERMISSION_ATTEMPT_FILE"; fi + attempt="$(cat "$PERMISSION_ATTEMPT_FILE" 2>/dev/null || printf '0')" attempt=$((attempt + 1)) printf '%s\n' "$attempt" >"$PERMISSION_ATTEMPT_FILE" printf '%s\n' "permission" >>"$CURL_LOG" @@ -97,6 +101,7 @@ if [[ "$url" == *"/collaborators/"*"/permission" ]]; then transport-exhaustion) status=000; curl_exit=7; body="" ;; terminal-http) status="$PERMISSION_TEST_STATUS"; body="private-response-body" ;; malformed-success) body="private-response-body" ;; + mismatched-actor) body='{"user":{"login":"different-user"},"role_name":"admin"}' ;; denied) printf -v body '{"user":{"login":"%s"},"role_name":"write"}' "$actor" ;; esac if [[ -n "$output_file" ]]; then printf '%s' "$body" >"$output_file"; else printf '%s' "$body"; fi @@ -169,7 +174,7 @@ printf '%s\n' "$1" >>"$SLEEP_LOG" describe.each(AUTHORIZATION_STEPS)( "$name collaborator permission read", - ({ deniedMessage, name }) => { + ({ deniedMessage, mismatchMessage, name }) => { it.each(["408", "429", "503"])( "retries HTTP %s once before authorization succeeds (#9337)", (status) => { @@ -237,6 +242,16 @@ describe.each(AUTHORIZATION_STEPS)( expect(result.stderr).toContain(deniedMessage); }); + it("does not retry a permission response for a different actor (#9337)", () => { + const result = runAuthorization(name, "mismatched-actor"); + + expect(result.status).not.toBe(0); + expect(result.permissionAttempts).toBe(1); + expect(result.sleeps).toEqual([]); + expect(result.curlOperations).toEqual(["permission"]); + expect(result.stderr).toContain(mismatchMessage); + }); + it("rejects an invalid actor before the permission read (#9337)", () => { const result = runAuthorization(name, "transient-then-success", { actor: "invalid actor" });