add warning about env not specified to potentially risky wrangler commands (#9344)

* add warning about env not specified to potentially risky wrangler commands

* Apply suggestions from code review

Co-authored-by: Pete Bacon Darwin <[email protected]>

* remove skip committed by mistake

* remove extra debugging logs

* use configPath instead

* remove unused import

* remove unnecessary displayName meta property

* [wrangler] allow passing an empty string to the `-e|--env` flag to target the top-level environment

* Update packages/wrangler/src/__tests__/deploy.test.ts

Co-authored-by: Pete Bacon Darwin <[email protected]>

* revert silly change

* fix incorrect code change

---------

Co-authored-by: Pete Bacon Darwin <[email protected]>

Author: dario-piotrowicz

.changeset/some-cloths-move.md

@@ -0,0 +1,23 @@
+---
+"wrangler": patch
+---
+
+add warning about env not specified to potentially risky wrangler commands
+
+add a warning suggesting users to specify their target environment (via `-e` or `--env`)
+when their wrangler config file contains some environments and they are calling one
+of the following commands:
+
+- wrangler deploy
+- wrangler versions upload
+- wrangler versions deploy
+- wrangler versions secret bulk
+- wrangler versions secret put
+- wrangler versions secret delete
+- wrangler secret bulk
+- wrangler secret put
+- wrangler secret delete
+- wrangler triggers deploy (TODO: manually test)
+
+this is a measure we're putting in place to try to prevent developers from accidentally applying
+changes to an incorrect (potentially even production) environment

.changeset/spotty-worlds-rest.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+allow passing an empty string to the `-e|--env` flag to target the top-level environment

packages/wrangler/src/__tests__/deploy.test.ts

@@ -13006,6 +13006,65 @@ export default{
 			await runWrangler("deploy ./index.js");
 		});
 	});
+
+	describe("multi-env warning", () => {
+		it("should warn if the wrangler config contains environments but none was specified in the command", async () => {
+			writeWorkerSource();
+			writeWranglerConfig({
+				main: "./index.js",
+				env: {
+					test: {},
+				},
+			});
+			mockSubDomainRequest();
+			mockUploadWorkerRequest();
+
+			await runWrangler("deploy");
+
+			expect(std.warn).toMatchInlineSnapshot(`
+				"▲ [WARNING] Multiple environments are defined in the Wrangler configuration file, but no target environment was specified for the deploy command.
+
+				  To avoid unintentional changes to the wrong environment, it is recommended to explicitly specify
+				  the target environment using the \`-e|--env\` flag.
+				  If your intention is to use the top-level environment of your configuration simply pass an empty
+				  string to the flag to target such environment. For example \`--env=\\"\\"\`.
+
+				"
+			`);
+		});
+
+		it("should not warn if the wrangler config contains environments and one was specified in the command", async () => {
+			writeWorkerSource();
+			writeWranglerConfig({
+				main: "./index.js",
+				env: {
+					test: {},
+				},
+			});
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({
+				env: "test",
+				legacyEnv: true,
+			});
+
+			await runWrangler("deploy -e test");
+
+			expect(std.warn).toMatchInlineSnapshot(`""`);
+		});
+
+		it("should not warn if the wrangler config doesn't contain environments and none was specified in the command", async () => {
+			writeWorkerSource();
+			writeWranglerConfig({
+				main: "./index.js",
+			});
+			mockSubDomainRequest();
+			mockUploadWorkerRequest();
+
+			await runWrangler("deploy");
+
+			expect(std.warn).toMatchInlineSnapshot(`""`);
+		});
+	});
 });
 
 /** Write mock assets to the file system so they can be uploaded. */

packages/wrangler/src/__tests__/secret.test.ts

@@ -14,6 +14,7 @@ import { useMockStdin } from "./helpers/mock-stdin";
 import { msw } from "./helpers/msw";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
+import { writeWranglerConfig } from "./helpers/write-wrangler-config";
 import type { Interface } from "node:readline";
 
 function createFetchResult(
@@ -458,6 +459,49 @@ describe("wrangler secret", () => {
 					`);
 				});
 			});
+
+			describe("multi-env warning", () => {
+				it("should warn if the wrangler config contains environments but none was specified in the command", async () => {
+					writeWranglerConfig({
+						env: {
+							test: {},
+						},
+					});
+					mockStdIn.send("the-secret");
+					mockPutRequest({ name: "the-key", text: "the-secret" });
+					await runWrangler("secret put the-key --name script-name");
+					expect(std.warn).toMatchInlineSnapshot(`
+						"▲ [WARNING] Multiple environments are defined in the Wrangler configuration file, but no target environment was specified for the secret put command.
+
+						  To avoid unintentional changes to the wrong environment, it is recommended to explicitly specify
+						  the target environment using the \`-e|--env\` flag.
+						  If your intention is to use the top-level environment of your configuration simply pass an empty
+						  string to the flag to target such environment. For example \`--env=\\"\\"\`.
+
+						"
+					`);
+				});
+
+				it("should not warn if the wrangler config contains environments and one was specified in the command", async () => {
+					writeWranglerConfig({
+						env: {
+							test: {},
+						},
+					});
+					mockStdIn.send("the-secret");
+					mockPutRequest({ name: "the-key", text: "the-secret" }, "test", true);
+					await runWrangler("secret put the-key --name script-name -e test");
+					expect(std.warn).toMatchInlineSnapshot(`""`);
+				});
+
+				it("should not warn if the wrangler config doesn't contain environments and none was specified in the command", async () => {
+					writeWranglerConfig();
+					mockStdIn.send("the-secret");
+					mockPutRequest({ name: "the-key", text: "the-secret" });
+					await runWrangler("secret put the-key --name script-name");
+					expect(std.warn).toMatchInlineSnapshot(`""`);
+				});
+			});
 		});
 
 		it("should error if the latest version is not deployed", async () => {
@@ -640,6 +684,62 @@ describe("wrangler secret", () => {
 				`[Error: Required Worker name missing. Please specify the Worker name in your Wrangler configuration file, or pass it as an argument with \`--name <worker-name>\`]`
 			);
 		});
+
+		describe("multi-env warning", () => {
+			it("should warn if the wrangler config contains environments but none was specified in the command", async () => {
+				writeWranglerConfig({
+					env: {
+						test: {},
+					},
+				});
+				mockDeleteRequest({ scriptName: "script-name", secretName: "the-key" });
+				mockConfirm({
+					text: "Are you sure you want to permanently delete the secret the-key on the Worker script-name?",
+					result: true,
+				});
+				await runWrangler("secret delete the-key --name script-name");
+				expect(std.warn).toMatchInlineSnapshot(`
+					"▲ [WARNING] Multiple environments are defined in the Wrangler configuration file, but no target environment was specified for the secret delete command.
+
+					  To avoid unintentional changes to the wrong environment, it is recommended to explicitly specify
+					  the target environment using the \`-e|--env\` flag.
+					  If your intention is to use the top-level environment of your configuration simply pass an empty
+					  string to the flag to target such environment. For example \`--env=\\"\\"\`.
+
+					"
+				`);
+			});
+
+			it("should not warn if the wrangler config contains environments and one was specified in the command", async () => {
+				writeWranglerConfig({
+					env: {
+						test: {},
+					},
+				});
+				mockDeleteRequest(
+					{ scriptName: "script-name", secretName: "the-key" },
+					"test",
+					true
+				);
+				mockConfirm({
+					text: "Are you sure you want to permanently delete the secret the-key on the Worker script-name-test?",
+					result: true,
+				});
+				await runWrangler("secret delete the-key --name script-name -e test");
+				expect(std.warn).toMatchInlineSnapshot(`""`);
+			});
+
+			it("should not warn if the wrangler config doesn't contain environments and none was specified in the command", async () => {
+				writeWranglerConfig();
+				mockDeleteRequest({ scriptName: "script-name", secretName: "the-key" });
+				mockConfirm({
+					text: "Are you sure you want to permanently delete the secret the-key on the Worker script-name?",
+					result: true,
+				});
+				await runWrangler("secret delete the-key --name script-name");
+				expect(std.warn).toMatchInlineSnapshot(`""`);
+			});
+		});
 	});
 
 	describe("list", () => {
@@ -1240,5 +1340,63 @@ describe("wrangler secret", () => {
 				✨ 2 secrets successfully uploaded"
 			`);
 		});
+
+		describe("multi-env warning", () => {
+			it("should warn if the wrangler config contains environments but none was specified in the command", async () => {
+				writeWranglerConfig({
+					name: "test-name",
+					main: "./index.js",
+					env: {
+						test: {},
+					},
+				});
+				writeFileSync("secret.json", JSON.stringify({}));
+
+				mockBulkRequest();
+
+				await runWrangler("secret bulk ./secret.json --name script-name");
+				expect(std.warn).toMatchInlineSnapshot(`
+					"▲ [WARNING] Multiple environments are defined in the Wrangler configuration file, but no target environment was specified for the secret bulk command.
+
+					  To avoid unintentional changes to the wrong environment, it is recommended to explicitly specify
+					  the target environment using the \`-e|--env\` flag.
+					  If your intention is to use the top-level environment of your configuration simply pass an empty
+					  string to the flag to target such environment. For example \`--env=\\"\\"\`.
+
+					"
+				`);
+			});
+
+			it("should not warn if the wrangler config contains environments and one was specified in the command", async () => {
+				writeWranglerConfig({
+					name: "test-name",
+					main: "./index.js",
+					env: {
+						test: {},
+					},
+				});
+				writeFileSync("secret.json", JSON.stringify({}));
+
+				mockBulkRequest();
+
+				await runWrangler(
+					"secret bulk ./secret.json --name script-name -e test"
+				);
+				expect(std.warn).toMatchInlineSnapshot(`""`);
+			});
+
+			it("should not warn if the wrangler config doesn't contain environments and none was specified in the command", async () => {
+				writeWranglerConfig({
+					name: "test-name",
+					main: "./index.js",
+				});
+				writeFileSync("secret.json", JSON.stringify({}));
+
+				mockBulkRequest();
+
+				await runWrangler("secret bulk ./secret.json --name script-name");
+				expect(std.warn).toMatchInlineSnapshot(`""`);
+			});
+		});
 	});
 });

packages/wrangler/src/__tests__/versions/secrets/bulk.test.ts

@@ -279,4 +279,66 @@ describe("versions secret bulk", () => {
 		);
 		expect(std.err).toMatchInlineSnapshot(`""`);
 	});
+
+	describe("multi-env warning", () => {
+		it("should warn if the wrangler config contains environments but none was specified in the command", async () => {
+			vi.spyOn(readline, "createInterface").mockImplementation(
+				() =>
+					// `readline.Interface` is an async iterator: `[Symbol.asyncIterator](): AsyncIterableIterator<string>`
+					JSON.stringify({
+						SECRET_1: "secret-1",
+					}) as unknown as Interface
+			);
+
+			writeWranglerConfig({ env: { test: {} } });
+			mockSetupApiCalls();
+			mockPostVersion();
+
+			await runWrangler(`versions secret bulk --name script-name`);
+			expect(std.warn).toMatchInlineSnapshot(`
+				"▲ [WARNING] Multiple environments are defined in the Wrangler configuration file, but no target environment was specified for the versions secret bulk command.
+
+				  To avoid unintentional changes to the wrong environment, it is recommended to explicitly specify
+				  the target environment using the \`-e|--env\` flag.
+				  If your intention is to use the top-level environment of your configuration simply pass an empty
+				  string to the flag to target such environment. For example \`--env=\\"\\"\`.
+
+				"
+			`);
+		});
+
+		it("should not warn if the wrangler config contains environments and one was specified in the command", async () => {
+			vi.spyOn(readline, "createInterface").mockImplementation(
+				() =>
+					// `readline.Interface` is an async iterator: `[Symbol.asyncIterator](): AsyncIterableIterator<string>`
+					JSON.stringify({
+						SECRET_1: "secret-1",
+					}) as unknown as Interface
+			);
+
+			writeWranglerConfig({ env: { test: {} } });
+			mockSetupApiCalls();
+			mockPostVersion();
+
+			await runWrangler(`versions secret bulk --name script-name --env test`);
+			expect(std.warn).toMatchInlineSnapshot(`""`);
+		});
+
+		it("should not warn if the wrangler config doesn't contain environments and none was specified in the command", async () => {
+			vi.spyOn(readline, "createInterface").mockImplementation(
+				() =>
+					// `readline.Interface` is an async iterator: `[Symbol.asyncIterator](): AsyncIterableIterator<string>`
+					JSON.stringify({
+						SECRET_1: "secret-1",
+					}) as unknown as Interface
+			);
+
+			writeWranglerConfig();
+			mockSetupApiCalls();
+			mockPostVersion();
+
+			await runWrangler(`versions secret bulk --name script-name`);
+			expect(std.warn).toMatchInlineSnapshot(`""`);
+		});
+	});
 });