diff --git a/.github/workflows/cloudflare-main.yml b/.github/workflows/cloudflare-main.yml index 44f00dd..613f198 100644 --- a/.github/workflows/cloudflare-main.yml +++ b/.github/workflows/cloudflare-main.yml @@ -76,7 +76,7 @@ jobs: permissions: contents: read deployments: write - uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@3332fc8e0049b837e851778cb1fb2be1f24acb1a # v1.7.0 + uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@bdb6a6c76b0fc229fa2a54ed91c3ddf78c2ea4ec # v1.7.1 with: # Under single topology the candidate and the release are the same # Worker, so deploying here would serve the merge immediately and there @@ -98,7 +98,7 @@ jobs: permissions: contents: read deployments: write - uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@3332fc8e0049b837e851778cb1fb2be1f24acb1a # v1.7.0 + uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@bdb6a6c76b0fc229fa2a54ed91c3ddf78c2ea4ec # v1.7.1 with: operation: preview target: release @@ -184,7 +184,7 @@ jobs: permissions: contents: read deployments: write - uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@3332fc8e0049b837e851778cb1fb2be1f24acb1a # v1.7.0 + uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@bdb6a6c76b0fc229fa2a54ed91c3ddf78c2ea4ec # v1.7.1 with: operation: deploy target: release diff --git a/.github/workflows/cloudflare-pr.yml b/.github/workflows/cloudflare-pr.yml index 1b1a2f8..21c5dc6 100644 --- a/.github/workflows/cloudflare-pr.yml +++ b/.github/workflows/cloudflare-pr.yml @@ -62,7 +62,7 @@ jobs: permissions: contents: read deployments: write - uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@3332fc8e0049b837e851778cb1fb2be1f24acb1a # v1.7.0 + uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@bdb6a6c76b0fc229fa2a54ed91c3ddf78c2ea4ec # v1.7.1 with: operation: preview target: candidate diff --git a/.github/workflows/cloudflare-release.yml b/.github/workflows/cloudflare-release.yml index 9d1b63c..63f944e 100644 --- a/.github/workflows/cloudflare-release.yml +++ b/.github/workflows/cloudflare-release.yml @@ -89,7 +89,7 @@ jobs: permissions: contents: read deployments: write - uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@3332fc8e0049b837e851778cb1fb2be1f24acb1a # v1.7.0 + uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@bdb6a6c76b0fc229fa2a54ed91c3ddf78c2ea4ec # v1.7.1 with: operation: preview target: release @@ -108,7 +108,7 @@ jobs: permissions: contents: read deployments: write - uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@3332fc8e0049b837e851778cb1fb2be1f24acb1a # v1.7.0 + uses: burnt-labs/github-workflows/.github/workflows/cloudflare-version.yml@bdb6a6c76b0fc229fa2a54ed91c3ddf78c2ea4ec # v1.7.1 with: operation: deploy target: release diff --git a/.github/workflows/cloudflare-version.yml b/.github/workflows/cloudflare-version.yml index b528fe7..e1c10b1 100644 --- a/.github/workflows/cloudflare-version.yml +++ b/.github/workflows/cloudflare-version.yml @@ -161,15 +161,19 @@ jobs: # available to them and GitHub is the only place they can hold a secret. # This carries the declared ones across. # - # Deploy only, never preview. `wrangler secret bulk` creates a Worker - # version and deploys it immediately, so running it on a preview would - # serve an intermediate version — on a chain repository that means mainnet - # starts serving off a job whose entire purpose is to *not* serve. A - # preview therefore runs against whatever secrets are already on the - # Worker, which is the same thing it does for bindings. + # Deploy only, never preview. A preview must not change what a target's + # secrets are, and on a chain repository the release target's secrets are + # mainnet's. A preview therefore runs against whatever secrets are already + # on the Worker, which is the same thing it does for bindings. # - # Two steps, deliberately. The first has the secrets but no Cloudflare - # credential and runs only jq from the runner image. The second has the + # This step writes the file; the deploy step below uploads it with the new + # version (`deploy --secrets-file`). A separate `wrangler secret` edit is + # deliberately not used: it edits the *latest* version and fails with + # Cloudflare error 10215 when that version is an undeployed upload, which + # is the normal state after any preview or single-topology candidate. + # + # Two steps, deliberately. This one has the secrets but no Cloudflare + # credential and runs only jq from the runner image. The deploy has the # credential but is SHA-pinned code. Neither has both a consumer-resolved # binary and the token. - name: Collect Worker secrets @@ -193,26 +197,6 @@ jobs: jq -n --argjson all "$ALL_SECRETS" --argjson want "$WANTED" \ '$want | map({key: ., value: $all[.]}) | from_entries' > "$RUNNER_TEMP/worker-secrets.json" echo "Publishing $(jq -r 'keys | join(", ")' "$RUNNER_TEMP/worker-secrets.json")" - - name: Publish Worker secrets - if: steps.worker-secrets.outcome == 'success' - # The same pinned action that performs the deploy. Running wrangler out - # of the consumer's node_modules instead would put a caller-resolved - # binary next to the deployment credential, and would also fail outright - # for a consumer that lets this action supply wrangler rather than - # depending on it directly. - uses: cloudflare/wrangler-action@ebbaa1584979971c8614a24965b4405ff95890e0 # v4.0.0 - with: - apiToken: ${{ secrets.cloudflare-api-token || secrets.BURNT_CLOUDFLARE_API_TOKEN }} - accountId: ${{ secrets.cloudflare-account-id || secrets.BURNT_CLOUDFLARE_ACCOUNT_ID }} - # deployment-policy.workingDirectory, matching the deploy step below. - # wrangler resolves its configuration relative to the working - # directory, and a monorepo consumer whose quality and deployment - # directories differ would otherwise update a different Worker or none. - workingDirectory: ${{ fromJSON(inputs.deployment-policy).workingDirectory }} - packageManager: ${{ fromJSON(inputs.deployment-policy).packageManager }} - command: >- - secret bulk ${{ runner.temp }}/worker-secrets.json - ${{ fromJSON(inputs.deployment-policy).topology != 'single' && format('--env {0}', fromJSON(inputs.deployment-policy).targets[inputs.target].wranglerEnv) || '' }} - name: Upload or deploy Worker version id: wrangler uses: cloudflare/wrangler-action@ebbaa1584979971c8614a24965b4405ff95890e0 # v4.0.0 @@ -227,11 +211,19 @@ jobs: # second form falls through to the right-hand side every time and # would always pass --env. Under single topology there is no wrangler # environment to name and passing one fails the deploy. + # + # --secrets-file follows the same shape. The collect step succeeds only + # on a deploy with a non-empty workerSecrets, so a preview never gets + # the flag. It needs wrangler 4.74.0 or newer. command: >- ${{ inputs.operation == 'preview' && 'versions upload' || 'deploy' }} ${{ fromJSON(inputs.deployment-policy).topology != 'single' && format('--env {0}', fromJSON(inputs.deployment-policy).targets[inputs.target].wranglerEnv) || '' }} --message=${{ inputs.version-tag }} --tag "${{ inputs.version-tag }}" + ${{ steps.worker-secrets.outcome == 'success' && format('--secrets-file {0}/worker-secrets.json', runner.temp) || '' }} + - name: Remove Worker secrets file + if: always() && steps.worker-secrets.outcome == 'success' + run: rm -f "$RUNNER_TEMP/worker-secrets.json" - name: Extract Cloudflare metadata id: metadata env: diff --git a/AGENTS.md b/AGENTS.md index 69ee6cb..eb134d8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -470,8 +470,10 @@ deploy carries the declared ones across: ``` Each name must exist as a secret on the target GitHub Environment. Before -creating the version, `cloudflare-version.yml` collects them and runs -`wrangler secret bulk`, so the version picks them up. +creating the version, `cloudflare-version.yml` collects them and passes them to +the deploy as `--secrets-file`, so the new version is created with them. The +consumer's wrangler must be 4.74.0 or newer, the first release with +`--secrets-file`. **The allowlist is the entire safety property.** `toJSON(secrets)` in that step contains every secret the caller inherited, the Cloudflare API token included. @@ -483,13 +485,15 @@ not skip the secret and carry on — absent configuration that degrades quietly how a Worker ends up running without a credential it needs and reporting success. -**Secrets are published on deploy, never on preview.** `wrangler secret bulk` -creates a Worker version and deploys it immediately, so running it on a preview -would serve an intermediate version — on a `chain` repository that means mainnet -starts serving from a job whose entire purpose is not to serve. A preview -therefore runs against whatever secrets are already on the Worker, the same way -it inherits its bindings. A brand-new secret is live from the first deploy that -publishes it, not from the preview before it. +**Secrets are published on deploy, never on preview.** A preview must not +change what a target's secrets are, and on `chain` the release target's secrets +are mainnet's. A preview therefore runs against whatever secrets are already on +the Worker, the same way it inherits its bindings. A brand-new secret is live +from the first deploy that publishes it, not from the preview before it. + +`wrangler secret bulk` is deliberately not used: it edits the _latest_ version +and fails with Cloudflare error 10215 when that version is an undeployed upload, +which is the normal state after any preview or `single`-topology candidate. **Removing a name from `workerSecrets` does not revoke it.** The list is an upsert, not a reconciliation: the deploy sets what it names and leaves diff --git a/tests/workflows.test.mjs b/tests/workflows.test.mjs index 9b890c7..f1af347 100644 --- a/tests/workflows.test.mjs +++ b/tests/workflows.test.mjs @@ -524,9 +524,8 @@ test("Worker secrets are allowlisted, never forwarded wholesale", () => { }); test("Worker secrets are published on deploy but never on preview", () => { - // `wrangler secret bulk` creates a version and deploys it immediately, so - // doing this on a preview would serve an intermediate version — on a chain - // repository, straight to mainnet from a job whose purpose is not to serve. + // A preview must not change what a target's secrets are, and on a chain + // repository the release target's secrets are mainnet's. const workflow = parse( fs.readFileSync(`${directory}/cloudflare-version.yml`, "utf8"), ); @@ -536,21 +535,32 @@ test("Worker secrets are published on deploy but never on preview", () => { assert.match(collect.if, /inputs\.operation == 'deploy'/); }); -test("Worker secrets are published by the pinned action, not consumer wrangler", () => { - // Resolving wrangler from the consumer's node_modules would put a - // caller-controlled binary in the same step as the deployment credential. +test("Worker secrets ride on the pinned deploy, not a separate secret edit", () => { + // `wrangler secret bulk` edits the latest version and fails with 10215 + // when that version is an undeployed 0% upload, which every single-topology + // flow leaves behind. `deploy --secrets-file` creates the version with the + // secrets instead. const workflow = parse( fs.readFileSync(`${directory}/cloudflare-version.yml`, "utf8"), ); - const publish = workflow.jobs.version.steps.find( - (step) => step.name === "Publish Worker secrets", + const steps = workflow.jobs.version.steps; + assert.equal( + steps.find((step) => step.name === "Publish Worker secrets"), + undefined, ); - assert.match(publish.uses, /^cloudflare\/wrangler-action@[0-9a-f]{40}$/); - assert.match(publish.command ?? publish.with.command, /secret bulk/); + const deploy = steps.find((step) => step.id === "wrangler"); + assert.match(deploy.uses, /^cloudflare\/wrangler-action@[0-9a-f]{40}$/); assert.match( - publish.with.workingDirectory, + deploy.with.workingDirectory, /deployment-policy\)\.workingDirectory/, ); + assert.match( + deploy.with.command, + /steps\.worker-secrets\.outcome == 'success' && format\('--secrets-file /, + ); + assert.doesNotMatch(deploy.with.command, /&& ''/); + const source = fs.readFileSync(`${directory}/cloudflare-version.yml`, "utf8"); + assert.doesNotMatch(source, /secret bulk/); }); test("a missing declared Worker secret fails the deploy", () => {