Skip to content

Improve wrangler deploy flow to also check for potential (and likely unintentional) secrets overrides#11389

Merged
dario-piotrowicz merged 3 commits intomainfrom
dario/DEVX-2347/wrangler-deploy-from-dash-secrets
Nov 27, 2025
Merged

Improve wrangler deploy flow to also check for potential (and likely unintentional) secrets overrides#11389
dario-piotrowicz merged 3 commits intomainfrom
dario/DEVX-2347/wrangler-deploy-from-dash-secrets

Conversation

@dario-piotrowicz
Copy link
Member

@dario-piotrowicz dario-piotrowicz commented Nov 23, 2025

Fixes https://jira.cfdata.org/browse/DEVX-2347

Before in case of names overlaps wrangler deploy would just proceed with the deployment and override your remote secrets, now instead it will do something like this:
Screenshot 2025-11-23 at 18 05 06


@changeset-bot
Copy link

changeset-bot bot commented Nov 23, 2025

🦋 Changeset detected

Latest commit: de03c97

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Nov 23, 2025
@dario-piotrowicz dario-piotrowicz changed the title Improve wrangler deploy flow to also check for potential (and likely y unintentional) secrets overrides Improve wrangler deploy flow to also check for potential (and likely unintentional) secrets overrides Nov 23, 2025
@dario-piotrowicz dario-piotrowicz force-pushed the dario/DEVX-2347/wrangler-deploy-from-dash-secrets branch 5 times, most recently from 7328e0d to 9180084 Compare November 24, 2025 17:22
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 24, 2025

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@11389

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@11389

miniflare

npm i https://pkg.pr.new/miniflare@11389

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@11389

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@11389

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@11389

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@11389

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@11389

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@11389

wrangler

npm i https://pkg.pr.new/wrangler@11389

commit: de03c97

@dario-piotrowicz dario-piotrowicz force-pushed the dario/DEVX-2347/wrangler-deploy-from-dash-secrets branch from 9180084 to 6fe3f86 Compare November 24, 2025 17:44
@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review November 24, 2025 18:34
@dario-piotrowicz dario-piotrowicz requested a review from a team as a code owner November 24, 2025 18:34
@github-actions
Copy link
Contributor

Failed to automatically backport this PR's changes to Wrangler v3. Please manually create a PR targeting the v3-maintenance branch with your changes. Thank you for helping us keep Wrangler v3 supported!

Depending on your changes, running git rebase --onto v3-maintenance main dario/DEVX-2347/wrangler-deploy-from-dash-secrets might be a good starting point.

Notes:

  • your PR branch should be named v3-backport-11389
  • add the skip-v3-pr label to the current PR to stop this workflow from failing

@ascorbic
Copy link
Contributor

Does it actually delete it, or just override it?

@dario-piotrowicz
Copy link
Member Author

Does it actually delete it, or just override it?

Technically always delete it and then add the new value in.

When the one in question is an env variable it does look like an override from the user perspective since secrets and variables are shown in the same plain in the dashboard. But when it is a binding, it does look like a delete since bindings are presented in their own dashboard page and the secret in the other page just disappears.

@dario-piotrowicz dario-piotrowicz force-pushed the dario/DEVX-2347/wrangler-deploy-from-dash-secrets branch from 7e2ff4f to a1f8a59 Compare November 26, 2025 22:22
const untypedValue = entry[1];

switch (key) {
case "durable_objects": {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like it has potential to go stale as new binding types are added?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I had the same exact thought, but I couldn't think of a solution for this... do you have some ideas? 😓

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you include a type guard (assert never or something?) to make sure all bindings types are covered?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I don't think a never-assertion is possible here since we do handle all possible keys, if the key happens to be a binding then we return the corresponding array of names otherwise we return an empty array 😕

(it's not like we need to ensure that key is the key for a bindings, since any other key is also valid here)

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha... I don't want to overindex on this problem since it's probably fine

Copy link
Member Author

Choose a reason for hiding this comment

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

ok thanks 🙂 🙏

dario-piotrowicz and others added 2 commits November 27, 2025 13:12
Co-authored-by: Somhairle MacLeòid <smacleod@cloudflare.com>
penalosa
penalosa previously approved these changes Nov 27, 2025
@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Nov 27, 2025
@penalosa penalosa self-requested a review November 27, 2025 17:05
@dario-piotrowicz dario-piotrowicz merged commit 2342d2f into main Nov 27, 2025
54 of 55 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Nov 27, 2025
@dario-piotrowicz dario-piotrowicz deleted the dario/DEVX-2347/wrangler-deploy-from-dash-secrets branch November 27, 2025 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants