ci(preview): fix Cloudflare registry image deployment (#2124)

* ci(preview): fix Cloudflare registry image deployment Keep the preview workflow's secret-bearing deploy path on trusted tooling while rewriting Wrangler config through registry-shaped image refs for push and deploy. Centralize preview log redaction and extend resolver/security guard coverage for artifact identity conflicts. * ci(preview): keep failure diagnostics resilient * ci(preview): redact private key diagnostics
2026-06-05 18:59:04 +00:00 · 2026-06-02 15:11:36 -07:00
parent f7b66d40ff
commit 92fa73ef00
5 changed files with 347 additions and 51 deletions
--- 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",
+  "<redacted-token>",
+  "<redacted-account>"
+].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")