fix: ensure that version secrets commands do not write wrangler confi…

…g warnings (#7450)

.changeset/yellow-cooks-battle.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: ensure that version secrets commands do not write wrangler config warnings

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

@@ -9,7 +9,7 @@ import { runWrangler } from "../../helpers/run-wrangler";
 import { mockPostVersion, mockSetupApiCalls } from "./utils";
 import type { Interface } from "node:readline";
 
-describe("versions secret put", () => {
+describe("versions secret bulk", () => {
 	const std = mockConsoleMethods();
 	runInTempDir();
 	mockAccountId();
@@ -75,6 +75,18 @@ describe("versions secret put", () => {
 		expect(std.err).toMatchInlineSnapshot(`""`);
 	});
 
+	test("no wrangler configuration warnings shown", async () => {
+		await writeFile("secrets.json", JSON.stringify({ SECRET_1: "secret-1" }));
+		await writeFile("wrangler.json", JSON.stringify({ invalid_field: true }));
+		mockSetupApiCalls();
+		mockPostVersion();
+		await runWrangler(
+			`versions secret bulk secrets.json --name script-name --x-versions`
+		);
+		expect(std.warn).toMatchInlineSnapshot(`""`);
+		expect(std.err).toMatchInlineSnapshot(`""`);
+	});
+
 	test("uploading secrets from stdin", async () => {
 		vi.spyOn(readline, "createInterface").mockImplementation(
 			() =>

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

@@ -1,3 +1,4 @@
+import { writeFile } from "node:fs/promises";
 import { describe, expect, test } from "vitest";
 import { mockAccountId, mockApiToken } from "../../helpers/mock-account-id";
 import { mockConsoleMethods } from "../../helpers/mock-console";
@@ -8,7 +9,7 @@ import { runWrangler } from "../../helpers/run-wrangler";
 import { writeWranglerConfig } from "../../helpers/write-wrangler-config";
 import { mockGetVersion, mockPostVersion, mockSetupApiCalls } from "./utils";
 
-describe("versions secret put", () => {
+describe("versions secret delete", () => {
 	const std = mockConsoleMethods();
 	const { setIsTTY } = useMockIsTTY();
 	runInTempDir();
@@ -103,4 +104,20 @@ describe("versions secret put", () => {
 		`);
 		expect(std.err).toMatchInlineSnapshot(`""`);
 	});
+
+	test("no wrangler configuration warnings shown", async () => {
+		await writeFile("wrangler.json", JSON.stringify({ invalid_field: true }));
+		setIsTTY(false);
+
+		mockSetupApiCalls();
+		mockGetVersion();
+		mockPostVersion();
+
+		await runWrangler(
+			"versions secret delete SECRET --name script-name --x-versions"
+		);
+
+		expect(std.warn).toMatchInlineSnapshot(`""`);
+		expect(std.err).toMatchInlineSnapshot(`""`);
+	});
 });

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

@@ -1,13 +1,16 @@
+import { writeFile } from "node:fs/promises";
 import { http, HttpResponse } from "msw";
 import { describe, expect, test } from "vitest";
 import { mockAccountId, mockApiToken } from "../../helpers/mock-account-id";
 import { mockConsoleMethods } from "../../helpers/mock-console";
 import { createFetchResult, 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 { ApiDeployment, ApiVersion } from "../../../versions/types";
 
 describe("versions secret list", () => {
+	runInTempDir();
 	const std = mockConsoleMethods();
 	mockAccountId();
 	mockApiToken();
@@ -253,4 +256,15 @@ describe("versions secret list", () => {
 		`);
 		expect(std.err).toMatchInlineSnapshot(`""`);
 	});
+
+	test("no wrangler configuration warnings shown", async () => {
+		await writeFile("wrangler.json", JSON.stringify({ invalid_field: true }));
+		mockGetDeployments();
+		mockGetVersion("version-id-1");
+
+		await runWrangler("versions secret list --name script-name --x-versions");
+
+		expect(std.warn).toMatchInlineSnapshot(`""`);
+		expect(std.err).toMatchInlineSnapshot(`""`);
+	});
 });

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

@@ -1,3 +1,4 @@
+import { writeFile } from "node:fs/promises";
 import { http, HttpResponse } from "msw";
 import { File, FormData } from "undici";
 import { describe, expect, test } from "vitest";
@@ -54,41 +55,60 @@ describe("versions secret put", () => {
 		expect(std.err).toMatchInlineSnapshot(`""`);
 	});
 
-	// For some reason, this always hangs. Not sure why
-	test.skip("can add a new secret (non-interactive)", async () => {
-		setIsTTY(false);
-		const mockStdIn = useMockStdin({ isTTY: false });
+	test("no wrangler configuration warnings shown", async () => {
+		await writeFile("wrangler.json", JSON.stringify({ invalid_field: true }));
+		setIsTTY(true);
 
-		mockSetupApiCalls();
-		mockPostVersion((metadata) => {
-			expect(metadata.bindings).toStrictEqual([
-				{ type: "secret_text", name: "NEW_SECRET", text: "the-secret" },
-			]);
-			expect(metadata.keep_bindings).toStrictEqual([
-				"secret_key",
-				"secret_text",
-			]);
-			expect(metadata.keep_assets).toBeTruthy();
+		mockPrompt({
+			text: "Enter a secret value:",
+			options: { isSecret: true },
+			result: "the-secret",
 		});
 
-		mockStdIn.send(
-			`the`,
-			`-`,
-			`secret
-			` // whitespace & newline being removed
-		);
+		mockSetupApiCalls();
+		mockPostVersion();
 		await runWrangler(
 			"versions secret put NEW_SECRET --name script-name --x-versions"
 		);
 
-		expect(std.out).toMatchInlineSnapshot(`
-			"🌀 Creating the secret for the Worker \\"script-name\\"
-			✨ Success! Created version id with secret NEW_SECRET.
-			➡️  To deploy this version with secret NEW_SECRET to production traffic use the command wrangler versions deploy."
-		`);
+		expect(std.warn).toMatchInlineSnapshot(`""`);
 		expect(std.err).toMatchInlineSnapshot(`""`);
 	});
 
+	describe("(non-interactive)", () => {
+		const mockStdIn = useMockStdin({ isTTY: false });
+		test("can add a new secret (non-interactive)", async () => {
+			mockSetupApiCalls();
+			mockPostVersion((metadata) => {
+				expect(metadata.bindings).toStrictEqual([
+					{ type: "secret_text", name: "NEW_SECRET", text: "the-secret" },
+				]);
+				expect(metadata.keep_bindings).toStrictEqual([
+					"secret_key",
+					"secret_text",
+				]);
+				expect(metadata.keep_assets).toBeTruthy();
+			});
+
+			mockStdIn.send(
+				`the`,
+				`-`,
+				`secret
+			` // whitespace & newline being removed
+			);
+			await runWrangler(
+				"versions secret put NEW_SECRET --name script-name --x-versions"
+			);
+
+			expect(std.out).toMatchInlineSnapshot(`
+				"🌀 Creating the secret for the Worker \\"script-name\\"
+				✨ Success! Created version id with secret NEW_SECRET.
+				➡️  To deploy this version with secret NEW_SECRET to production traffic use the command \\"wrangler versions deploy\\"."
+			`);
+			expect(std.err).toMatchInlineSnapshot(`""`);
+		});
+	});
+
 	test("can add a new secret, read Worker name from wrangler.toml", async () => {
 		writeWranglerConfig({ name: "script-name" });
 

packages/wrangler/src/__tests__/versions/secrets/utils.ts

@@ -82,8 +82,12 @@ export function mockGetVersion(versionInfo?: VersionDetails) {
 function mockGetVersionContent() {
 	msw.use(
 		http.get(
-			`*/accounts/:accountId/workers/scripts/:scriptName/content/v2?version=ce15c78b-cc43-4f60-b5a9-15ce4f298c2a`,
-			async ({ params }) => {
+			`*/accounts/:accountId/workers/scripts/:scriptName/content/v2`,
+			async ({ params, request }) => {
+				const url = new URL(request.url);
+				expect(url.searchParams.get("version")).toEqual(
+					"ce15c78b-cc43-4f60-b5a9-15ce4f298c2a"
+				);
 				expect(params.accountId).toEqual("some-account-id");
 				expect(params.scriptName).toEqual("script-name");
 

packages/wrangler/src/api/pages/deploy.ts

@@ -160,7 +160,11 @@ export async function deploy({
 	let config: Config | undefined;
 
 	try {
-		config = readConfig(undefined, { ...args, env }, true);
+		config = readConfig(
+			undefined,
+			{ ...args, env },
+			{ requirePagesConfig: true }
+		);
 	} catch (err) {
 		if (
 			!(

packages/wrangler/src/cloudchamber/common.ts

@@ -105,10 +105,7 @@ export function handleFailure<
 ) => Promise<void> {
 	return async (t) => {
 		try {
-			const config = readConfig(
-				t.config,
-				t as unknown as Parameters<typeof readConfig>[1]
-			);
+			const config = readConfig(t.config, t);
 			await fillOpenAPIConfiguration(config, t.json);
 			await cb(t, config);
 		} catch (err) {

packages/wrangler/src/config/index.ts

@@ -75,21 +75,22 @@ export function readConfig(
 	configPath: string | undefined,
 	// Include command specific args as well as the wrangler global flags
 	args: ReadConfigCommandArgs,
-	requirePagesConfig: true
+	options: { requirePagesConfig: true }
 ): Omit<Config, "pages_build_output_dir"> & { pages_build_output_dir: string };
 export function readConfig(
 	configPath: string | undefined,
 	// Include command specific args as well as the wrangler global flags
 	args: ReadConfigCommandArgs,
-	requirePagesConfig?: boolean,
-	hideWarnings?: boolean
+	options?: { requirePagesConfig?: boolean; hideWarnings?: boolean }
 ): Config;
 export function readConfig(
 	configPath: string | undefined,
 	// Include command specific args as well as the wrangler global flags
 	args: ReadConfigCommandArgs,
-	requirePagesConfig?: boolean,
-	hideWarnings: boolean = false
+	{
+		requirePagesConfig,
+		hideWarnings = false,
+	}: { requirePagesConfig?: boolean; hideWarnings?: boolean } = {}
 ): Config {
 	let rawConfig: RawConfig = {};
 
@@ -686,10 +687,11 @@ export function printBindings(
 export function withConfig<T>(
 	handler: (
 		t: OnlyCamelCase<T & CommonYargsOptions> & { config: Config }
-	) => Promise<void>
+	) => Promise<void>,
+	options?: Parameters<typeof readConfig>[2]
 ) {
 	return (t: OnlyCamelCase<T & CommonYargsOptions>) => {
-		return handler({ ...t, config: readConfig(t.config, t) });
+		return handler({ ...t, config: readConfig(t.config, t, options) });
 	};
 }
 

packages/wrangler/src/core/register-yargs-command.ts

@@ -82,12 +82,9 @@ function createHandler(def: CommandDefinition) {
 			await def.handler(args, {
 				config:
 					def.behaviour?.provideConfig ?? true
-						? readConfig(
-								args.config,
-								args,
-								undefined,
-								!(def.behaviour?.printConfigWarnings ?? true)
-							)
+						? readConfig(args.config, args, {
+								hideWarnings: !(def.behaviour?.printConfigWarnings ?? true),
+							})
 						: defaultWranglerConfig,
 				errors: { UserError, FatalError },
 				logger,

packages/wrangler/src/pages/build-env.ts

@@ -63,10 +63,10 @@ export const Handler = async (args: PagesBuildEnvArgs) => {
 				// eslint-disable-next-line turbo/no-undeclared-env-vars
 				env: process.env.PAGES_ENVIRONMENT,
 			},
-			true
+			{ requirePagesConfig: true }
 		);
 	} catch (err) {
-		// found `wrangler.toml` but `pages_build_output_dir` is not specififed
+		// found `wrangler.toml` but `pages_build_output_dir` is not specified
 		if (
 			err instanceof FatalError &&
 			err.code === EXIT_CODE_INVALID_PAGES_CONFIG

packages/wrangler/src/pages/build.ts

@@ -366,7 +366,7 @@ async function maybeReadPagesConfig(
 				// eslint-disable-next-line turbo/no-undeclared-env-vars
 				env: process.env.PAGES_ENVIRONMENT,
 			},
-			true
+			{ requirePagesConfig: true }
 		);
 
 		return {

packages/wrangler/src/pages/deploy.ts

@@ -123,7 +123,11 @@ export const Handler = async (args: PagesDeployArgs) => {
 		 * need for now. We will perform a second config file read later
 		 * in `/api/pages/deploy`, that will get the environment specific config
 		 */
-		config = readConfig(configPath, { ...args, env: undefined }, true);
+		config = readConfig(
+			configPath,
+			{ ...args, env: undefined },
+			{ requirePagesConfig: true }
+		);
 	} catch (err) {
 		if (
 			!(

packages/wrangler/src/pages/secret/index.ts

@@ -49,7 +49,11 @@ async function pagesProject(
 		 * return the top-level config. This contains all the information we
 		 * need.
 		 */
-		config = readConfig(configPath, { env: undefined }, true);
+		config = readConfig(
+			configPath,
+			{ env: undefined },
+			{ requirePagesConfig: true }
+		);
 	} catch (err) {
 		if (
 			!(

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

@@ -9,6 +9,7 @@ import { logger } from "../../logger";
 import { parseJSON, readFileSync } from "../../parse";
 import { validateJSONFileSecrets } from "../../secret";
 import { requireAuth } from "../../user";
+import { getConfig } from "../utils/config";
 import { copyWorkerVersionWithNewSecrets } from "./index";
 import type { WorkerVersion } from "./index";
 
@@ -18,6 +19,9 @@ export const versionsSecretBulkCommand = createCommand({
 		owner: "Workers: Authoring and Testing",
 		status: "stable",
 	},
+	behaviour: {
+		provideConfig: false,
+	},
 	args: {
 		json: {
 			describe: `The JSON file of key-value pairs to upload, in form {"key": value, ...}`,
@@ -40,7 +44,8 @@ export const versionsSecretBulkCommand = createCommand({
 		},
 	},
 	positionalArgs: ["json"],
-	handler: async function versionsSecretPutBulkHandler(args, { config }) {
+	handler: async function versionsSecretPutBulkHandler(args) {
+		const config = getConfig(args, { hideWarnings: true });
 		const scriptName = getLegacyScriptName(args, config);
 		if (!scriptName) {
 			throw new UserError(

packages/wrangler/src/versions/secrets/delete.ts

@@ -6,6 +6,7 @@ import { UserError } from "../../errors";
 import { getLegacyScriptName, isLegacyEnv } from "../../index";
 import { logger } from "../../logger";
 import { requireAuth } from "../../user";
+import { getConfig } from "../utils/config";
 import { copyWorkerVersionWithNewSecrets } from "./index";
 import type { VersionDetails, WorkerVersion } from "./index";
 
@@ -15,6 +16,9 @@ export const versionsSecretDeleteCommand = createCommand({
 		owner: "Workers: Authoring and Testing",
 		status: "stable",
 	},
+	behaviour: {
+		provideConfig: false,
+	},
 	args: {
 		key: {
 			describe: "The variable name to be accessible in the Worker",
@@ -38,7 +42,8 @@ export const versionsSecretDeleteCommand = createCommand({
 		},
 	},
 	positionalArgs: ["key"],
-	handler: async function versionsSecretDeleteHandler(args, { config }) {
+	handler: async function versionsSecretDeleteHandler(args) {
+		const config = getConfig(args, { hideWarnings: true });
 		const scriptName = getLegacyScriptName(args, config);
 		if (!scriptName) {
 			throw new UserError(