Skip to content

Commit e1b93dc

Browse files
emily-shenemily-shen
authored
fix: ask for confirmation before creating a new worker when uploading secrets (#7037)
* tidy up secret.test.ts * add confirmation before creating draft worker * changeset * explicitly set interactivity * messaging updates
1 parent 6131ef5 commit e1b93dc

File tree

3 files changed

+207
-97
lines changed

3 files changed

+207
-97
lines changed

‎.changeset/hungry-news-own.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: ask for confirmation before creating a new Worker when uploading secrets
6+
7+
Previously, `wrangler secret put KEY --name non-existent-worker` would automatically create a new Worker with the name `non-existent-worker`. This fix asks for confirmation before doing so (if running in an interactive context). Behaviour in non-interactive/CI contexts should be unchanged.

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

Lines changed: 181 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@ import { runInTempDir } from "./helpers/run-in-tmp";
1616
import { runWrangler } from "./helpers/run-wrangler";
1717
import type { Interface } from "node:readline";
1818

19-
function createFetchResult(result: unknown, success = true) {
19+
function createFetchResult(
20+
result: unknown,
21+
success = true,
22+
errors: { code: number; message: string }[] = []
23+
) {
2024
return {
2125
success,
22-
errors: [],
26+
errors,
2327
messages: [],
2428
result,
2529
};
@@ -39,6 +43,44 @@ export function mockGetMemberships(
3943
);
4044
}
4145

46+
function mockNoWorkerFound(isBulk = false) {
47+
if (isBulk) {
48+
msw.use(
49+
http.get(
50+
"*/accounts/:accountId/workers/scripts/:scriptName/settings",
51+
async () => {
52+
return HttpResponse.json(
53+
createFetchResult(null, false, [
54+
{
55+
code: 10007,
56+
message: "This Worker does not exist on your account.",
57+
},
58+
])
59+
);
60+
},
61+
{ once: true }
62+
)
63+
);
64+
} else {
65+
msw.use(
66+
http.put(
67+
"*/accounts/:accountId/workers/scripts/:scriptName/secrets",
68+
async () => {
69+
return HttpResponse.json(
70+
createFetchResult(null, false, [
71+
{
72+
code: 10007,
73+
message: "This Worker does not exist on your account.",
74+
},
75+
])
76+
);
77+
},
78+
{ once: true }
79+
)
80+
);
81+
}
82+
}
83+
4284
describe("wrangler secret", () => {
4385
const std = mockConsoleMethods();
4486
const { setIsTTY } = useMockIsTTY();
@@ -53,7 +95,8 @@ describe("wrangler secret", () => {
5395
function mockPutRequest(
5496
input: { name: string; text: string },
5597
env?: string,
56-
legacyEnv = false
98+
legacyEnv = false,
99+
expectedScriptName = "script-name"
57100
) {
58101
const servicesOrScripts = env && !legacyEnv ? "services" : "scripts";
59102
const environment = env && !legacyEnv ? "/environments/:envName" : "";
@@ -63,7 +106,9 @@ describe("wrangler secret", () => {
63106
async ({ request, params }) => {
64107
expect(params.accountId).toEqual("some-account-id");
65108
expect(params.scriptName).toEqual(
66-
legacyEnv && env ? `script-name-${env}` : "script-name"
109+
legacyEnv && env
110+
? `${expectedScriptName}-${env}`
111+
: expectedScriptName
67112
);
68113
if (!legacyEnv) {
69114
expect(params.envName).toEqual(env);
@@ -203,6 +248,26 @@ describe("wrangler secret", () => {
203248
`[Error: Required Worker name missing. Please specify the Worker name in wrangler.toml, or pass it as an argument with \`--name <worker-name>\`]`
204249
);
205250
});
251+
252+
it("should ask to create a new Worker if no Worker is found under the provided name and abort if declined", async () => {
253+
mockPrompt({
254+
text: "Enter a secret value:",
255+
options: { isSecret: true },
256+
result: `hunter2`,
257+
});
258+
mockNoWorkerFound();
259+
mockConfirm({
260+
text: `There doesn't seem to be a Worker called "non-existent-worker". Do you want to create a new Worker with that name and add secrets to it?`,
261+
result: false,
262+
});
263+
expect(
264+
await runWrangler("secret put the-key --name non-existent-worker")
265+
);
266+
expect(std.out).toMatchInlineSnapshot(`
267+
"🌀 Creating the secret for the Worker \\"non-existent-worker\\"
268+
Aborting. No secrets added."
269+
`);
270+
});
206271
});
207272

208273
describe("non-interactive", () => {
@@ -260,6 +325,35 @@ describe("wrangler secret", () => {
260325
expect(std.warn).toMatchInlineSnapshot(`""`);
261326
});
262327

328+
it("should create a new worker if no worker is found under the provided name", async () => {
329+
mockStdIn.send("hunter2");
330+
mockNoWorkerFound();
331+
msw.use(
332+
http.put(
333+
"*/accounts/:accountId/workers/scripts/:name",
334+
async ({ params }) => {
335+
expect(params.name).toEqual("non-existent-worker");
336+
return HttpResponse.json(
337+
createFetchResult({ name: params.name })
338+
);
339+
}
340+
)
341+
);
342+
mockPutRequest(
343+
{ name: "the-key", text: "hunter2" },
344+
undefined,
345+
undefined,
346+
"non-existent-worker"
347+
);
348+
expect(
349+
await runWrangler("secret put the-key --name non-existent-worker")
350+
);
351+
expect(std.out).toMatchInlineSnapshot(`
352+
"🌀 Creating the secret for the Worker \\"non-existent-worker\\"
353+
✨ Success! Uploaded secret the-key"
354+
`);
355+
});
356+
263357
describe("with accountId", () => {
264358
mockAccountId({ accountId: null });
265359

@@ -342,17 +436,14 @@ describe("wrangler secret", () => {
342436
expect(params.scriptName).toEqual(scriptName);
343437

344438
// Return our error
345-
return HttpResponse.json({
346-
success: false,
347-
errors: [
439+
return HttpResponse.json(
440+
createFetchResult(null, false, [
348441
{
349442
code: VERSION_NOT_DEPLOYED_ERR_CODE,
350443
message: "latest is not deployed",
351444
},
352-
],
353-
messages: [],
354-
result: null,
355-
});
445+
])
446+
);
356447
},
357448
{ once: true }
358449
)
@@ -622,6 +713,27 @@ describe("wrangler secret", () => {
622713
});
623714

624715
describe("bulk", () => {
716+
const mockBulkRequest = (returnNetworkError = false) => {
717+
msw.use(
718+
http.get(
719+
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
720+
({ params }) => {
721+
expect(params.accountId).toEqual("some-account-id");
722+
723+
return HttpResponse.json(createFetchResult({ bindings: [] }));
724+
}
725+
),
726+
http.patch(
727+
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
728+
({ params }) => {
729+
expect(params.accountId).toEqual("some-account-id");
730+
return HttpResponse.json(
731+
returnNetworkError ? null : createFetchResult(null)
732+
);
733+
}
734+
)
735+
);
736+
};
625737
it("should error helpfully if pages_build_output_dir is set", async () => {
626738
fs.writeFileSync(
627739
"wrangler.toml",
@@ -666,27 +778,7 @@ describe("wrangler secret", () => {
666778
password: "hunter2",
667779
}) as unknown as Interface
668780
);
669-
670-
msw.use(
671-
http.get(
672-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
673-
({ params }) => {
674-
expect(params.accountId).toEqual("some-account-id");
675-
676-
return HttpResponse.json(createFetchResult({ bindings: [] }));
677-
}
678-
)
679-
);
680-
msw.use(
681-
http.patch(
682-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
683-
({ params }) => {
684-
expect(params.accountId).toEqual("some-account-id");
685-
686-
return HttpResponse.json(createFetchResult(null));
687-
}
688-
)
689-
);
781+
mockBulkRequest();
690782

691783
await runWrangler(`secret bulk --name script-name`);
692784
expect(std.out).toMatchInlineSnapshot(`
@@ -710,26 +802,7 @@ describe("wrangler secret", () => {
710802
})
711803
);
712804

713-
msw.use(
714-
http.get(
715-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
716-
({ params }) => {
717-
expect(params.accountId).toEqual("some-account-id");
718-
719-
return HttpResponse.json(createFetchResult({ bindings: [] }));
720-
}
721-
)
722-
);
723-
msw.use(
724-
http.patch(
725-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
726-
({ params }) => {
727-
expect(params.accountId).toEqual("some-account-id");
728-
729-
return HttpResponse.json(createFetchResult(null));
730-
}
731-
)
732-
);
805+
mockBulkRequest();
733806

734807
await runWrangler("secret bulk ./secret.json --name script-name");
735808

@@ -783,27 +856,7 @@ describe("wrangler secret", () => {
783856
"secret-name-7": "secret_text",
784857
})
785858
);
786-
787-
msw.use(
788-
http.get(
789-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
790-
({ params }) => {
791-
expect(params.accountId).toEqual("some-account-id");
792-
793-
return HttpResponse.json(createFetchResult({ bindings: [] }));
794-
}
795-
)
796-
);
797-
msw.use(
798-
http.patch(
799-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
800-
({ params }) => {
801-
expect(params.accountId).toEqual("some-account-id");
802-
803-
return HttpResponse.json(null);
804-
}
805-
)
806-
);
859+
mockBulkRequest(true);
807860

808861
await expect(async () => {
809862
await runWrangler("secret bulk ./secret.json --name script-name");
@@ -835,27 +888,7 @@ describe("wrangler secret", () => {
835888
"secret-name-2": "secret_text",
836889
})
837890
);
838-
839-
msw.use(
840-
http.get(
841-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
842-
({ params }) => {
843-
expect(params.accountId).toEqual("some-account-id");
844-
845-
return HttpResponse.json(createFetchResult({ bindings: [] }));
846-
}
847-
)
848-
);
849-
msw.use(
850-
http.patch(
851-
`*/accounts/:accountId/workers/scripts/:scriptName/settings`,
852-
({ params }) => {
853-
expect(params.accountId).toEqual("some-account-id");
854-
855-
return HttpResponse.json(null);
856-
}
857-
)
858-
);
891+
mockBulkRequest(true);
859892

860893
await expect(async () => {
861894
await runWrangler("secret bulk ./secret.json --name script-name");
@@ -967,6 +1000,60 @@ describe("wrangler secret", () => {
9671000
expect(std.err).toMatchInlineSnapshot(`""`);
9681001
expect(std.warn).toMatchInlineSnapshot(`""`);
9691002
});
1003+
1004+
it("should, in interactive mode, ask to create a new Worker if no Worker is found under the provided name", async () => {
1005+
setIsTTY(true);
1006+
writeFileSync(
1007+
"secret.json",
1008+
JSON.stringify({
1009+
"secret-name-1": "secret_text",
1010+
"secret-name-2": "secret_text",
1011+
})
1012+
);
1013+
mockNoWorkerFound(true);
1014+
mockConfirm({
1015+
text: `There doesn't seem to be a Worker called "non-existent-worker". Do you want to create a new Worker with that name and add secrets to it?`,
1016+
result: false,
1017+
});
1018+
1019+
await runWrangler("secret bulk ./secret.json --name non-existent-worker");
1020+
expect(std.out).toMatchInlineSnapshot(`
1021+
"🌀 Creating the secrets for the Worker \\"non-existent-worker\\"
1022+
Aborting. No secrets added."
1023+
`);
1024+
});
1025+
1026+
it("should, in non-interactive mode, create a new worker if no worker is found under the provided name", async () => {
1027+
setIsTTY(false);
1028+
writeFileSync(
1029+
"secret.json",
1030+
JSON.stringify({
1031+
"secret-name-1": "secret_text",
1032+
"secret-name-2": "secret_text",
1033+
})
1034+
);
1035+
mockNoWorkerFound();
1036+
msw.use(
1037+
http.put(
1038+
"*/accounts/:accountId/workers/scripts/:name",
1039+
async ({ params }) => {
1040+
expect(params.name).toEqual("non-existent-worker");
1041+
return HttpResponse.json(createFetchResult({ name: params.name }));
1042+
}
1043+
)
1044+
);
1045+
mockBulkRequest();
1046+
1047+
await runWrangler("secret bulk ./secret.json --name script-name");
1048+
expect(std.out).toMatchInlineSnapshot(`
1049+
"🌀 Creating the secrets for the Worker \\"script-name\\"
1050+
✨ Successfully created secret for key: secret-name-1
1051+
✨ Successfully created secret for key: secret-name-2
1052+
1053+
Finished processing secrets JSON file:
1054+
✨ 2 secrets successfully uploaded"
1055+
`);
1056+
});
9701057
});
9711058

9721059
describe("secret:bulk [DEPRECATED]", () => {

0 commit comments

Comments
 (0)