diff --git a/.github/workflows/preview-deploy.yml b/.github/workflows/preview-deploy.yml index 63ea9590d..62d2266af 100644 --- a/.github/workflows/preview-deploy.yml +++ b/.github/workflows/preview-deploy.yml @@ -25,6 +25,7 @@ jobs: head_sha: ${{ steps.preview.outputs.head_sha }} is_fork: ${{ steps.preview.outputs.is_fork }} pr_number: ${{ steps.preview.outputs.pr_number }} + resolution_source: ${{ steps.preview.outputs.resolution_source }} should_deploy: ${{ steps.preview.outputs.should_deploy }} steps: @@ -105,6 +106,7 @@ jobs: HEAD_SHA: ${{ needs.preview-gate.outputs.head_sha }} IS_FORK: ${{ needs.preview-gate.outputs.is_fork }} PR_NUMBER: ${{ needs.preview-gate.outputs.pr_number }} + RESOLUTION_SOURCE: ${{ needs.preview-gate.outputs.resolution_source }} steps: - name: Checkout trusted preview tooling @@ -202,6 +204,9 @@ jobs: cp trusted/workers/preview/tsconfig.json "$preview_dir/tsconfig.json" cp trusted/workers/preview/wrangler.toml "$preview_dir/wrangler.toml" cp -R trusted/workers/preview/src "$preview_dir/src" + mkdir -p "$preview_dir/deploy" + cp trusted/workers/preview/deploy/redact_preview_log.sh "$preview_dir/deploy/redact_preview_log.sh" + chmod 0755 "$preview_dir/deploy/redact_preview_log.sh" diagnostics_nonce="$(openssl rand -hex 32)" sed -i "s/\${PR_NUMBER}/${PR_NUMBER}/g" "$preview_dir/wrangler.toml" @@ -245,32 +250,54 @@ jobs: cd "$RUNNER_TEMP/sure-preview-worker" config_path="$RUNNER_TEMP/sure-preview-worker/wrangler.toml" image_tag="sure-preview-pr-${PR_NUMBER}:${HEAD_SHA}" + temporary_image_ref="registry.cloudflare.com/${CLOUDFLARE_ACCOUNT_ID}/${image_tag}" push_log="$RUNNER_TEMP/wrangler-containers-push.log" clean_log="$RUNNER_TEMP/wrangler-containers-push.clean.log" + push_status=0 # wrangler containers push validates wrangler.toml, so point the trusted - # config at the loaded CI image before replacing it with the registry ref. - LOCAL_IMAGE_TAG="$image_tag" node - "$config_path" <<'NODE' + # config at a registry-shaped ref while it pushes the verified local image. + TEMPORARY_IMAGE_REF="$temporary_image_ref" node - "$config_path" <<'NODE' const fs = require('node:fs'); const configPath = process.argv[2]; - const imageTag = process.env.LOCAL_IMAGE_TAG; + const imageRef = process.env.TEMPORARY_IMAGE_REF; - if (!/^sure-preview-pr-[1-9][0-9]*:[a-f0-9]{40}$/.test(imageTag || '')) { - throw new Error('Expected local preview image tag for wrangler containers push'); + if (!/^registry\.cloudflare\.com\/[A-Za-z0-9_-]+\/sure-preview-pr-[1-9][0-9]*:[a-f0-9]{40}$/.test(imageRef || '')) { + throw new Error('Expected registry-shaped preview image ref before wrangler containers push'); } const original = fs.readFileSync(configPath, 'utf8'); - const updated = original.replace(/image = "[^"]+"/, `image = ${JSON.stringify(imageTag)}`); + const updated = original.replace(/image = "[^"]+"/, `image = ${JSON.stringify(imageRef)}`); if (updated === original) { throw new Error('Expected wrangler.toml to contain an image entry to rewrite before push'); } fs.writeFileSync(configPath, updated); NODE - ./node_modules/.bin/wrangler containers push "$image_tag" 2>&1 | tee "$push_log" + set +e + ./node_modules/.bin/wrangler containers push "$image_tag" 2>&1 | tee "$push_log" | ./deploy/redact_preview_log.sh + push_status=${PIPESTATUS[0]} + set -e perl -pe 's/\e\[[0-9;]*[A-Za-z]//g' "$push_log" > "$clean_log" - image_ref=$(grep -Eo 'registry\.cloudflare\.com/[^[:space:]]+' "$clean_log" | tail -n 1 | tr -d '\r') + + if [ "$push_status" -ne 0 ]; then + exit "$push_status" + fi + + image_ref="$(node - "$clean_log" <<'NODE' + const fs = require('node:fs'); + + const logPath = process.argv[2]; + const log = fs.readFileSync(logPath, 'utf8'); + const expectedSuffix = `sure-preview-pr-${process.env.PR_NUMBER}:${process.env.HEAD_SHA}`; + const pattern = /registry\.cloudflare\.com\/[A-Za-z0-9_-]+\/sure-preview-pr-[1-9][0-9]*:[a-f0-9]{40}/g; + const matches = [...log.matchAll(pattern)].map((match) => match[0]); + const imageRef = matches.findLast((candidate) => candidate.endsWith(`/${expectedSuffix}`)); + + if (imageRef) process.stdout.write(imageRef); + NODE + )" if [ -z "$image_ref" ]; then echo "Could not find Cloudflare registry image reference in wrangler output" >&2 @@ -292,11 +319,16 @@ jobs: const configPath = process.argv[2]; const imageRef = process.env.IMAGE_REF; + const expectedSuffix = `sure-preview-pr-${process.env.PR_NUMBER}:${process.env.HEAD_SHA}`; - if (!imageRef || !imageRef.startsWith('registry.cloudflare.com/')) { + if (!/^registry\.cloudflare\.com\/[A-Za-z0-9_-]+\/sure-preview-pr-[1-9][0-9]*:[a-f0-9]{40}$/.test(imageRef || '')) { throw new Error('Expected a Cloudflare registry image reference'); } + if (!imageRef.endsWith(`/${expectedSuffix}`)) { + throw new Error('Cloudflare registry image reference does not match this preview artifact'); + } + const original = fs.readFileSync(configPath, 'utf8'); const updated = original.replace(/image = "[^"]+"/, `image = ${JSON.stringify(imageRef)}`); if (updated === original) { @@ -305,7 +337,10 @@ jobs: fs.writeFileSync(configPath, updated); NODE - cat "$config_path" + # Print a redacted copy for logs without mutating the config used by deploy. + redacted_config="$RUNNER_TEMP/wrangler-redacted.toml" + "$RUNNER_TEMP/sure-preview-worker/deploy/redact_preview_log.sh" < "$config_path" > "$redacted_config" + cat "$redacted_config" - name: Deploy to Cloudflare Containers id: deploy @@ -321,7 +356,11 @@ jobs: clean_deploy_log="$RUNNER_TEMP/wrangler-deploy.clean.log" deploy_once() { - ./node_modules/.bin/wrangler deploy --config wrangler.toml --var "PR_NUMBER:${PR_NUMBER}" 2>&1 | tee "$deploy_log" + set +e + ./node_modules/.bin/wrangler deploy --config wrangler.toml --var "PR_NUMBER:${PR_NUMBER}" 2>&1 | tee "$deploy_log" | ./deploy/redact_preview_log.sh + local deploy_status=${PIPESTATUS[0]} + set -e + return "$deploy_status" } if ! deploy_once; then @@ -406,12 +445,90 @@ jobs: if-no-files-found: error retention-days: 3 + - name: Collect preview failure diagnostics + if: failure() + run: | + set -euo pipefail + + diagnostics_dir="$RUNNER_TEMP/preview-failure-diagnostics" + manifest_file="$RUNNER_TEMP/preview-image/sure-preview-image.manifest.json" + redaction_helper="$RUNNER_TEMP/sure-preview-worker/deploy/redact_preview_log.sh" + mkdir -p "$diagnostics_dir" + + jq -n \ + --arg artifactName "$ARTIFACT_NAME" \ + --arg headSha "$HEAD_SHA" \ + --arg isFork "$IS_FORK" \ + --arg prNumber "$PR_NUMBER" \ + --arg resolutionSource "$RESOLUTION_SOURCE" \ + '{ + artifactName: $artifactName, + headSha: $headSha, + isFork: $isFork, + prNumber: $prNumber, + resolutionSource: $resolutionSource + }' > "$diagnostics_dir/preview-request.json" + + sanitize_copy() { + local source="$1" + local destination="$2" + if [ -f "$source" ]; then + if [ -x "$redaction_helper" ]; then + "$redaction_helper" < "$source" > "$destination" + else + cp "$source" "$destination" + fi + fi + } + + if [ -f "$manifest_file" ]; then + jq '{ + artifactVersion, + archivePath, + archiveSha256, + headSha, + imageId, + imageTag, + prNumber + }' "$manifest_file" > "$diagnostics_dir/preview-image-manifest.json" + fi + + sanitize_copy "$RUNNER_TEMP/sure-preview-worker/wrangler.toml" "$diagnostics_dir/wrangler.toml" + sanitize_copy "$RUNNER_TEMP/wrangler-containers-push.clean.log" "$diagnostics_dir/wrangler-containers-push.log" + if [ -f "$RUNNER_TEMP/wrangler-deploy.clean.log" ]; then + sanitize_copy "$RUNNER_TEMP/wrangler-deploy.clean.log" "$diagnostics_dir/wrangler-deploy.log" + else + sanitize_copy "$RUNNER_TEMP/wrangler-deploy.log" "$diagnostics_dir/wrangler-deploy.log" + fi + + find "$diagnostics_dir" -maxdepth 1 -type f -print + + - name: Upload preview failure diagnostics + if: failure() + uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6 + with: + name: preview-failure-diagnostics-pr-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }} + path: ${{ runner.temp }}/preview-failure-diagnostics + if-no-files-found: error + retention-days: 3 + + - name: Prepare cleanup metadata + if: success() + run: | + set -euo pipefail + + metadata_dir="$RUNNER_TEMP/preview-cleanup-metadata" + mkdir -p "$metadata_dir" + "$RUNNER_TEMP/sure-preview-worker/deploy/redact_preview_log.sh" \ + < "$RUNNER_TEMP/sure-preview-worker/wrangler.toml" \ + > "$metadata_dir/wrangler.toml" + - name: Store cleanup metadata if: success() uses: actions/upload-artifact@b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6 with: name: preview-cleanup-pr-${{ env.PR_NUMBER }} - path: ${{ runner.temp }}/sure-preview-worker/wrangler.toml + path: ${{ runner.temp }}/preview-cleanup-metadata/wrangler.toml retention-days: 2 deployment_status: diff --git a/bin/preview_deploy_security_check.rb b/bin/preview_deploy_security_check.rb index 5b4b5b3b1..72339cd6e 100644 --- a/bin/preview_deploy_security_check.rb +++ b/bin/preview_deploy_security_check.rb @@ -8,6 +8,7 @@ PREVIEW_WORKFLOW_PATH = File.join(ROOT, ".github/workflows/preview-deploy.yml") PR_WORKFLOW_PATH = File.join(ROOT, ".github/workflows/pr.yml") LOCKFILE_PATH = File.join(ROOT, "workers/preview/package-lock.json") RESOLVER_PATH = File.join(ROOT, "workers/preview/deploy/resolve_preview_request.cjs") +REDACTION_HELPER_PATH = File.join(ROOT, "workers/preview/deploy/redact_preview_log.sh") PREVIEW_WORKER_PATH = File.join(ROOT, "workers/preview/src/index.ts") PREVIEW_DOCKERFILE_PATH = File.join(ROOT, "Dockerfile.preview") PINNED_ACTION = /\A[^@\s]+@[a-f0-9]{40}\z/ @@ -51,12 +52,17 @@ EXPECTED_COMMENT_PERMISSIONS = { }.freeze EXPECTED_DEPLOY_SECRET_ENV = %w[CLOUDFLARE_ACCOUNT_ID CLOUDFLARE_API_TOKEN CLOUDFLARE_WORKERS_SUBDOMAIN].freeze EXPECTED_PUSH_SECRET_ENV = %w[CLOUDFLARE_ACCOUNT_ID CLOUDFLARE_API_TOKEN].freeze +EXPECTED_FAILURE_DIAGNOSTICS_PATH = "${{ runner.temp }}/preview-failure-diagnostics" +EXPECTED_CLEANUP_METADATA_PATH = "${{ runner.temp }}/preview-cleanup-metadata/wrangler.toml" REQUIRED_PREPARE_LINES = [ 'cp trusted/workers/preview/package.json "$preview_dir/package.json"', 'cp trusted/workers/preview/package-lock.json "$preview_dir/package-lock.json"', 'cp trusted/workers/preview/tsconfig.json "$preview_dir/tsconfig.json"', 'cp trusted/workers/preview/wrangler.toml "$preview_dir/wrangler.toml"', 'cp -R trusted/workers/preview/src "$preview_dir/src"', + 'mkdir -p "$preview_dir/deploy"', + 'cp trusted/workers/preview/deploy/redact_preview_log.sh "$preview_dir/deploy/redact_preview_log.sh"', + 'chmod 0755 "$preview_dir/deploy/redact_preview_log.sh"', 'diagnostics_nonce="$(openssl rand -hex 32)"', 'sed -i "s/\${PREVIEW_DIAGNOSTICS_NONCE}/${diagnostics_nonce}/g" "$preview_dir/src/index.ts"', "Preview diagnostics nonce placeholder was not replaced", @@ -154,6 +160,7 @@ preview_workflow = YAML.safe_load_file(PREVIEW_WORKFLOW_PATH, aliases: true) pr_workflow = YAML.safe_load_file(PR_WORKFLOW_PATH, aliases: true) lockfile = JSON.parse(File.read(LOCKFILE_PATH)) resolver_script = File.read(RESOLVER_PATH) +redaction_helper_script = File.read(REDACTION_HELPER_PATH) preview_worker_script = File.read(PREVIEW_WORKER_PATH) preview_dockerfile = File.read(PREVIEW_DOCKERFILE_PATH) @@ -195,6 +202,10 @@ deploy = step!(deploy_steps, "Deploy to Cloudflare Containers") warm_preview = step!(deploy_steps, "Warm preview container") collect_diagnostics = step!(deploy_steps, "Collect preview diagnostics") upload_diagnostics = step!(deploy_steps, "Upload preview diagnostics") +collect_failure_diagnostics = step!(deploy_steps, "Collect preview failure diagnostics") +upload_failure_diagnostics = step!(deploy_steps, "Upload preview failure diagnostics") +prepare_cleanup_metadata = step!(deploy_steps, "Prepare cleanup metadata") +store_cleanup_metadata = step!(deploy_steps, "Store cleanup metadata") update_deployment_status = step!(deployment_status_steps, "Update Deployment Status") comment_on_pr = step!(preview_comment_steps, "Comment on PR") @@ -215,6 +226,7 @@ comment_on_pr = step!(preview_comment_steps, "Comment on PR") [ "preview gate head output", gate_job.dig("outputs", "head_sha"), "${{ steps.preview.outputs.head_sha }}" ], [ "preview gate fork output", gate_job.dig("outputs", "is_fork"), "${{ steps.preview.outputs.is_fork }}" ], [ "preview gate PR output", gate_job.dig("outputs", "pr_number"), "${{ steps.preview.outputs.pr_number }}" ], + [ "preview gate resolution source output", gate_job.dig("outputs", "resolution_source"), "${{ steps.preview.outputs.resolution_source }}" ], [ "preview image needs", image_job.fetch("needs"), "ci" ], [ "preview image permissions", image_job.fetch("permissions"), EXPECTED_IMAGE_PERMISSIONS ], [ "preview image timeout", image_job.fetch("timeout-minutes"), 30 ], @@ -240,6 +252,7 @@ comment_on_pr = step!(preview_comment_steps, "Comment on PR") [ "deploy HEAD_SHA env", deploy_job.dig("env", "HEAD_SHA"), "${{ needs.preview-gate.outputs.head_sha }}" ], [ "deploy IS_FORK env", deploy_job.dig("env", "IS_FORK"), "${{ needs.preview-gate.outputs.is_fork }}" ], [ "deploy PR_NUMBER env", deploy_job.dig("env", "PR_NUMBER"), "${{ needs.preview-gate.outputs.pr_number }}" ], + [ "deploy RESOLUTION_SOURCE env", deploy_job.dig("env", "RESOLUTION_SOURCE"), "${{ needs.preview-gate.outputs.resolution_source }}" ], [ "deployment status needs", deployment_status_job.fetch("needs"), [ "preview-gate", "deployment_record", "deploy-preview" ] ], [ "deployment status permissions", deployment_status_job.fetch("permissions"), EXPECTED_DEPLOYMENT_PERMISSIONS ], [ "deployment status timeout", deployment_status_job.fetch("timeout-minutes"), 5 ], @@ -271,6 +284,16 @@ comment_on_pr = step!(preview_comment_steps, "Comment on PR") [ "diagnostics upload name", upload_diagnostics.dig("with", "name"), "preview-diagnostics-pr-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }}" ], [ "diagnostics upload path", upload_diagnostics.dig("with", "path"), "${{ runner.temp }}/preview-diagnostics.json" ], [ "diagnostics upload retention", upload_diagnostics.dig("with", "retention-days"), 3 ], + [ "failure diagnostics collect if", collect_failure_diagnostics.fetch("if"), "failure()" ], + [ "failure diagnostics upload if", upload_failure_diagnostics.fetch("if"), "failure()" ], + [ "failure diagnostics upload name", upload_failure_diagnostics.dig("with", "name"), "preview-failure-diagnostics-pr-${{ env.PR_NUMBER }}-${{ env.HEAD_SHA }}" ], + [ "failure diagnostics upload path", upload_failure_diagnostics.dig("with", "path"), EXPECTED_FAILURE_DIAGNOSTICS_PATH ], + [ "failure diagnostics upload retention", upload_failure_diagnostics.dig("with", "retention-days"), 3 ], + [ "cleanup metadata prepare if", prepare_cleanup_metadata.fetch("if"), "success()" ], + [ "cleanup metadata upload if", store_cleanup_metadata.fetch("if"), "success()" ], + [ "cleanup metadata upload name", store_cleanup_metadata.dig("with", "name"), "preview-cleanup-pr-${{ env.PR_NUMBER }}" ], + [ "cleanup metadata upload path", store_cleanup_metadata.dig("with", "path"), EXPECTED_CLEANUP_METADATA_PATH ], + [ "cleanup metadata upload retention", store_cleanup_metadata.dig("with", "retention-days"), 2 ], [ "Wrangler binary", wrangler.dig("bin", "wrangler"), "bin/wrangler.js" ] ].each { |label, actual, expected| assert(actual == expected, "#{label}: expected #{actual.inspect} to equal #{expected.inspect}") } @@ -298,7 +321,28 @@ assert(gate_trusted_checkout.dig("with", "sparse-checkout").to_s.include?("worke assert(trusted_checkout.dig("with", "sparse-checkout").to_s.include?("workers/preview"), "trusted checkout must include preview tooling") assert(deploy_step_names.compact.uniq == deploy_step_names.compact, "workflow step names must stay unique for security checks") assert([ gate_trusted_checkout, resolve_preview ].map { |step| gate_steps.index(step) }.each_cons(2).all? { |left, right| left < right }, "gate workflow steps must checkout trusted resolver before use") -assert([ trusted_checkout, download_artifact, verify_checksum, prepare, load_image, push_image, configure_image, deploy, warm_preview, collect_diagnostics, upload_diagnostics ].map { |step| deploy_steps.index(step) }.each_cons(2).all? { |left, right| left < right }, "deploy workflow steps must preserve safe cross-run artifact deploy order") +required_deploy_order = [ + trusted_checkout, + download_artifact, + verify_checksum, + prepare, + load_image, + push_image, + configure_image, + deploy, + warm_preview, + collect_diagnostics, + upload_diagnostics, + collect_failure_diagnostics, + upload_failure_diagnostics, + prepare_cleanup_metadata, + store_cleanup_metadata +] +assert( + required_deploy_order.map { |step| deploy_steps.index(step) } + .each_cons(2).all? { |left, right| left < right }, + "deploy workflow steps must preserve safe cross-run artifact deploy order" +) assert(deploy_steps.none? { |step| step["name"] == "Checkout PR code" }, "privileged deploy job must not checkout PR code") assert(env_hash(deploy_job).keys.none? { |name| name.start_with?("CLOUDFLARE_") }, "Cloudflare secrets must not be job-wide") assert(env_hash(gate_job).keys.none? { |name| name.start_with?("CLOUDFLARE_") }, "preview gate must not receive Cloudflare secrets") @@ -344,6 +388,9 @@ assert_run_includes( "workflowRun.pull_requests?.[0]", "github.rest.repos.listPullRequestsAssociatedWithCommit", "parsePreviewArtifactName", + "artifactPullRequestNumbers.length === 1", + "conflicts with workflow_run PR", + "conflicts with commit-associated PRs", "pullRequest.head.sha !== headSha", "is stale for PR", "preview-cf", @@ -352,9 +399,30 @@ assert_run_includes( "!item.expired", "core.setOutput(\"artifact_name\", artifactName)", "core.setOutput(\"is_fork\", String(isFork))", + "core.setOutput(\"resolution_source\", selected.source)", "core.setOutput(\"should_deploy\", \"true\")" ].each { |needle| assert(resolver_script.include?(needle), "preview resolver must include #{needle.inspect}") } +artifact_resolution_index = resolver_script.index("artifactPullRequestNumbers.length === 1") +workflow_run_fallback_index = resolver_script.index("if (workflowRunPullRequestNumber)") +assert(artifact_resolution_index, "preview resolver must inspect exact preview artifacts") +assert(workflow_run_fallback_index, "preview resolver must retain workflow_run PR fallback") +assert( + artifact_resolution_index < workflow_run_fallback_index, + "preview resolver must prefer exact preview artifacts before workflow_run PR metadata" +) +assert(File.executable?(REDACTION_HELPER_PATH), "preview log redaction helper must be executable") +[ + "registry\\.cloudflare\\.com/[^/]+/", + "Authorization|Proxy-Authorization", + "X-Auth-Key|X-Auth-Email|X-Api-Key|Api-Key", + "api_key|access_token|refresh_token|auth_token|key|private_key", + "CLOUDFLARE_ACCOUNT_ID=", + "CLOUDFLARE_API_TOKEN|API_KEY|ACCESS_TOKEN|REFRESH_TOKEN|AUTH_TOKEN|PRIVATE_KEY", + "", + "" +].each { |needle| assert(redaction_helper_script.include?(needle), "preview log redaction helper must include #{needle.inspect}") } + prepare_run = assert_run_includes(prepare, *REQUIRED_PREPARE_LINES) assert(!prepare_run.include?("npm install"), "prepare step must not use npm install") assert(!prepare_run.include?("CLOUDFLARE_API_TOKEN"), "prepare step must not receive Cloudflare secrets") @@ -371,24 +439,31 @@ assert_run_includes(load_image, 'gzip -dc "$image_archive" | docker load', 'dock push_image_run = assert_run_includes( push_image, "./node_modules/.bin/wrangler containers push", - "registry\\.cloudflare\\.com/", + "registry.cloudflare.com/${CLOUDFLARE_ACCOUNT_ID}/${image_tag}", + "registry\\.cloudflare\\.com\\/", "image_ref=", 'config_path="$RUNNER_TEMP/sure-preview-worker/wrangler.toml"', - 'LOCAL_IMAGE_TAG="$image_tag" node - "$config_path"', - "Expected local preview image tag for wrangler containers push", + 'temporary_image_ref="registry.cloudflare.com/${CLOUDFLARE_ACCOUNT_ID}/${image_tag}"', + 'TEMPORARY_IMAGE_REF="$temporary_image_ref" node - "$config_path"', + "Expected registry-shaped preview image ref before wrangler containers push", "Expected wrangler.toml to contain an image entry to rewrite before push" ) -push_rewrite_index = push_image_run.index('LOCAL_IMAGE_TAG="$image_tag" node - "$config_path"') +push_rewrite_index = push_image_run.index('TEMPORARY_IMAGE_REF="$temporary_image_ref" node - "$config_path"') push_command_index = push_image_run.index("./node_modules/.bin/wrangler containers push") assert( push_rewrite_index < push_command_index, - "push step must rewrite wrangler.toml to the loaded local image tag before wrangler validates it" + "push step must rewrite wrangler.toml to a registry-shaped image ref before wrangler validates it" ) -assert_run_includes(configure_image, "imageRef.startsWith('registry.cloudflare.com/')", 'const original = fs.readFileSync', 'const updated = original.replace(/image = "[^"]+"/', "updated === original", "Expected wrangler.toml to contain an image entry to rewrite", "JSON.stringify(imageRef)") +assert(!push_image_run.include?("LOCAL_IMAGE_TAG"), "push step must not write a local Docker tag into wrangler.toml") +assert(!push_image_run.include?("Expected local preview image tag"), "push step must not accept local Docker tags as wrangler config image refs") +assert_run_includes(push_image, 'tee "$push_log" | ./deploy/redact_preview_log.sh', "push_status=${PIPESTATUS[0]}") +assert_run_includes(configure_image, "registry\\.cloudflare\\.com", "expectedSuffix", "imageRef.endsWith", "Cloudflare registry image reference does not match this preview artifact", 'const original = fs.readFileSync', 'const updated = original.replace(/image = "[^"]+"/', "updated === original", "Expected wrangler.toml to contain an image entry to rewrite", "JSON.stringify(imageRef)", 'redact_preview_log.sh" < "$config_path"') assert_run_includes(create_deployment, "github.rest.repos.createDeployment", "ref: headSha", "preview-pr-${prNumber}") -assert_run_includes(deploy, 'cd "$RUNNER_TEMP/sure-preview-worker"', "deploy_once()", "./node_modules/.bin/wrangler deploy --config wrangler.toml", '--var "PR_NUMBER:${PR_NUMBER}"', "associated with a different durable object namespace", 'if ! ./node_modules/.bin/wrangler delete --name "sure-preview-${PR_NUMBER}" --force', "Preview Worker delete failed", "retrying once") +assert_run_includes(deploy, 'cd "$RUNNER_TEMP/sure-preview-worker"', "deploy_once()", "./node_modules/.bin/wrangler deploy --config wrangler.toml", '--var "PR_NUMBER:${PR_NUMBER}"', 'tee "$deploy_log" | ./deploy/redact_preview_log.sh', "deploy_status=${PIPESTATUS[0]}", "associated with a different durable object namespace", 'if ! ./node_modules/.bin/wrangler delete --name "sure-preview-${PR_NUMBER}" --force', "Preview Worker delete failed", "retrying once") assert_run_includes(warm_preview, "$PREVIEW_URL/_container_status", "--connect-timeout 5", "--max-time 15") assert_run_includes(collect_diagnostics, "$PREVIEW_URL/_container_status", "--connect-timeout 5", "--max-time 15", "seq 1 40", "preview-diagnostics.json", "jq -e '.previewReady == true or .previewFailed == true'", "jq -e '.previewFailed == true'", "Preview diagnostics from _container_status reported previewFailed=true", "jq -e '.previewReady == true'", "Preview diagnostics from _container_status did not reach previewReady=true", ".timings.previewReadyAt != null and .timings.secondsToPreviewReady != null", "Preview diagnostics are missing readiness timing fields", "exit 1") +assert_run_includes(collect_failure_diagnostics, "preview-failure-diagnostics", "preview-request.json", "preview-image-manifest.json", "wrangler.toml", "wrangler-containers-push.log", "wrangler-deploy.log", "redaction_helper=", 'sanitize_copy "$RUNNER_TEMP/sure-preview-worker/wrangler.toml"', "wrangler-deploy.clean.log", "resolutionSource") +assert_run_includes(prepare_cleanup_metadata, "preview-cleanup-metadata", "redact_preview_log.sh", "$RUNNER_TEMP/sure-preview-worker/wrangler.toml", "$metadata_dir/wrangler.toml") assert_run_includes(update_deployment_status, "github.rest.repos.createDeploymentStatus", "process.env.DEPLOY_RESULT === 'success'", "deployment_id: Number(process.env.DEPLOYMENT_ID)") assert_run_includes(comment_on_pr, "github.rest.issues.listComments", "github.rest.issues.updateComment", "github.rest.issues.createComment", "Preview Deployment Ready") diff --git a/test/javascript/preview_deploy/resolve_preview_request_test.cjs b/test/javascript/preview_deploy/resolve_preview_request_test.cjs index 7dc799c7a..a3fb32a1f 100644 --- a/test/javascript/preview_deploy/resolve_preview_request_test.cjs +++ b/test/javascript/preview_deploy/resolve_preview_request_test.cjs @@ -123,7 +123,7 @@ describe("selectPullRequestNumber", () => { const headSha = "4f1159e99c7785bc370f53510284c251fabdb75b"; const context = contextFor({ id: 123, head_sha: headSha }); - it("falls back to commit association when workflow_run has no PR payload", () => { + it("prefers the preview artifact when commit association matches", () => { const selected = selectPullRequestNumber({ runPullRequest: undefined, artifacts: [previewArtifact(2017, headSha)], @@ -134,7 +134,37 @@ describe("selectPullRequestNumber", () => { assert.deepEqual(selected, { prNumber: 2017, - source: "commit_association", + source: "artifact_name+commit_association", + }); + }); + + it("records workflow_run as a corroborating source when it matches the preview artifact", () => { + const selected = selectPullRequestNumber({ + runPullRequest: { number: 2017 }, + artifacts: [previewArtifact(2017, headSha)], + associatedPullRequests: [], + context, + headSha, + }); + + assert.deepEqual(selected, { + prNumber: 2017, + source: "artifact_name+workflow_run", + }); + }); + + it("records workflow_run and commit association when both match the preview artifact", () => { + const selected = selectPullRequestNumber({ + runPullRequest: { number: 2017 }, + artifacts: [previewArtifact(2017, headSha)], + associatedPullRequests: [openPullRequest(2017, headSha, "Rene0422/sure")], + context, + headSha, + }); + + assert.deepEqual(selected, { + prNumber: 2017, + source: "artifact_name+workflow_run+commit_association", }); }); @@ -152,10 +182,38 @@ describe("selectPullRequestNumber", () => { assert.deepEqual(selected, { prNumber: 2060, - source: "artifact_and_commit_association", + source: "artifact_name+commit_association", }); }); + it("fails closed when workflow metadata disagrees with the preview artifact", () => { + const selected = selectPullRequestNumber({ + runPullRequest: { number: 1985 }, + artifacts: [previewArtifact(1798, headSha)], + associatedPullRequests: [openPullRequest(1798, headSha)], + context, + headSha, + }); + + assert.equal(selected.prNumber, undefined); + assert.equal(typeof selected.error, "string"); + assert.match(selected.error, /conflicts with workflow_run PR 1985/); + }); + + it("fails closed when commit association disagrees with the preview artifact", () => { + const selected = selectPullRequestNumber({ + runPullRequest: undefined, + artifacts: [previewArtifact(1798, headSha)], + associatedPullRequests: [openPullRequest(1985, headSha)], + context, + headSha, + }); + + assert.equal(selected.prNumber, undefined); + assert.equal(typeof selected.error, "string"); + assert.match(selected.error, /conflicts with commit-associated PRs 1985/); + }); + it("refuses ambiguous associated PRs without a single matching artifact", () => { const selected = selectPullRequestNumber({ runPullRequest: undefined, @@ -197,6 +255,7 @@ describe("resolvePreviewRequest", () => { assert.equal(state.outputs.head_sha, headSha); assert.equal(state.outputs.artifact_name, `preview-image-pr-2017-${headSha}`); assert.equal(state.outputs.is_fork, "true"); + assert.equal(state.outputs.resolution_source, "artifact_name+commit_association"); }); it("resolves PRs from artifact names when workflow and commit association metadata are unavailable", async () => { @@ -216,6 +275,7 @@ describe("resolvePreviewRequest", () => { assert.equal(state.outputs.head_sha, headSha); assert.equal(state.outputs.artifact_name, `preview-image-pr-2017-${headSha}`); assert.equal(state.outputs.is_fork, "true"); + assert.equal(state.outputs.resolution_source, "artifact_name"); assert.match(state.messages.join("\n"), /Resolved PR 2017 from artifact_name; fork=true/); }); diff --git a/workers/preview/deploy/redact_preview_log.sh b/workers/preview/deploy/redact_preview_log.sh new file mode 100755 index 000000000..40633ff4e --- /dev/null +++ b/workers/preview/deploy/redact_preview_log.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash +set -euo pipefail + +perl -pe ' + s#registry\.cloudflare\.com/[^/]+/#registry.cloudflare.com//#g; + s#((?:Authorization|Proxy-Authorization):\s*Bearer\s+)[^[:space:]]+#$1#gi; + s#((?:X-Auth-Key|X-Auth-Email|X-Api-Key|Api-Key):\s*)[^[:space:]]+#$1#gi; + s#([?&](?:token|api_key|access_token|refresh_token|auth_token|key|private_key)=)[^&[:space:]]+#$1#gi; + s#("(?:token|api_key|access_token|refresh_token|auth_token|secret|client_secret|private_key)"\s*:\s*")[^"]*#$1#gi; + s#(CLOUDFLARE_ACCOUNT_ID=)[^[:space:]]+#$1#g; + s#((?:CLOUDFLARE_API_TOKEN|API_KEY|ACCESS_TOKEN|REFRESH_TOKEN|AUTH_TOKEN|PRIVATE_KEY)=)[^[:space:]]+#$1#g; +' diff --git a/workers/preview/deploy/resolve_preview_request.cjs b/workers/preview/deploy/resolve_preview_request.cjs index 7aec276c1..596a1e3c0 100644 --- a/workers/preview/deploy/resolve_preview_request.cjs +++ b/workers/preview/deploy/resolve_preview_request.cjs @@ -32,6 +32,10 @@ function uniqueNumbers(candidates) { return [...new Set(candidates.map((candidate) => candidate.parsed.prNumber))]; } +function uniquePullRequestNumbers(pullRequests) { + return [...new Set(pullRequests.map((pullRequest) => pullRequest.number))]; +} + function associatedPullRequestsForHead(associatedPullRequests, context, headSha) { const baseRepo = repoFullName(context); @@ -43,15 +47,65 @@ function associatedPullRequestsForHead(associatedPullRequests, context, headSha) } function selectPullRequestNumber({ runPullRequest, artifacts, associatedPullRequests, context, headSha }) { - if (runPullRequest?.number) { + const associatedHeadPullRequests = associatedPullRequestsForHead(associatedPullRequests, context, headSha); + const associatedPullRequestNumbers = uniquePullRequestNumbers(associatedHeadPullRequests); + const artifactPullRequestNumbers = uniqueNumbers(artifactCandidates(artifacts, headSha)); + const workflowRunPullRequestNumber = runPullRequest?.number ?? null; + + if (artifactPullRequestNumbers.length > 1) { return { - prNumber: runPullRequest.number, - source: "workflow_run", + error: `Workflow run ${headSha} published preview artifacts for multiple pull requests`, }; } - const associatedHeadPullRequests = associatedPullRequestsForHead(associatedPullRequests, context, headSha); - const artifactPullRequestNumbers = uniqueNumbers(artifactCandidates(artifacts, headSha)); + if (artifactPullRequestNumbers.length === 1) { + const artifactPullRequestNumber = artifactPullRequestNumbers[0]; + + if (workflowRunPullRequestNumber && workflowRunPullRequestNumber !== artifactPullRequestNumber) { + return { + error: `Preview artifact PR ${artifactPullRequestNumber} conflicts with workflow_run PR ${workflowRunPullRequestNumber}`, + }; + } + + if ( + associatedPullRequestNumbers.length > 0 && + !associatedPullRequestNumbers.includes(artifactPullRequestNumber) + ) { + return { + error: `Preview artifact PR ${artifactPullRequestNumber} conflicts with commit-associated PRs ${associatedPullRequestNumbers.join(", ")}`, + }; + } + + const corroboratingSources = []; + if (workflowRunPullRequestNumber === artifactPullRequestNumber) corroboratingSources.push("workflow_run"); + if (associatedPullRequestNumbers.includes(artifactPullRequestNumber)) { + corroboratingSources.push("commit_association"); + } + + return { + prNumber: artifactPullRequestNumber, + source: + corroboratingSources.length > 0 + ? `artifact_name+${corroboratingSources.join("+")}` + : "artifact_name", + }; + } + + if (workflowRunPullRequestNumber) { + if ( + associatedPullRequestNumbers.length > 0 && + !associatedPullRequestNumbers.includes(workflowRunPullRequestNumber) + ) { + return { + error: `workflow_run PR ${workflowRunPullRequestNumber} conflicts with commit-associated PRs ${associatedPullRequestNumbers.join(", ")}`, + }; + } + + return { + prNumber: workflowRunPullRequestNumber, + source: "workflow_run", + }; + } if (associatedHeadPullRequests.length === 1) { return { @@ -61,34 +115,11 @@ function selectPullRequestNumber({ runPullRequest, artifacts, associatedPullRequ } if (associatedHeadPullRequests.length > 1) { - const associatedNumbers = new Set(associatedHeadPullRequests.map((pullRequest) => pullRequest.number)); - const artifactMatches = artifactPullRequestNumbers.filter((number) => associatedNumbers.has(number)); - - if (artifactMatches.length === 1) { - return { - prNumber: artifactMatches[0], - source: "artifact_and_commit_association", - }; - } - return { error: `Workflow run head SHA ${headSha} is associated with multiple open pull requests and no single preview artifact matched`, }; } - if (artifactPullRequestNumbers.length === 1) { - return { - prNumber: artifactPullRequestNumbers[0], - source: "artifact_name", - }; - } - - if (artifactPullRequestNumbers.length > 1) { - return { - error: `Workflow run ${headSha} published preview artifacts for multiple pull requests`, - }; - } - return { prNumber: null, source: "none", @@ -186,6 +217,7 @@ async function resolvePreviewRequest({ github, context, core }) { core.setOutput("head_sha", headSha); core.setOutput("is_fork", String(isFork)); core.setOutput("pr_number", String(prNumber)); + core.setOutput("resolution_source", selected.source); core.setOutput("should_deploy", "true"); }