From 92c7be5a2e2d59b2c986231e59532a9e631b44c9 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sat, 8 Aug 2026 10:26:28 -0400 Subject: [PATCH 1/3] Encode device authorization request parameters Assisted-By: devx/bb1c6644-dd32-425e-b0c8-33f7efa1bb93 --- .../encode-device-authorization-request.md | 5 +++ .../node/session/device-authorization.test.ts | 39 ++++++++++++++++++- .../node/session/device-authorization.ts | 5 +-- 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 .changeset/encode-device-authorization-request.md diff --git a/.changeset/encode-device-authorization-request.md b/.changeset/encode-device-authorization-request.md new file mode 100644 index 00000000000..1e7cbae8923 --- /dev/null +++ b/.changeset/encode-device-authorization-request.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Encode device authorization request parameters safely. diff --git a/packages/cli-kit/src/private/node/session/device-authorization.test.ts b/packages/cli-kit/src/private/node/session/device-authorization.test.ts index e87ab4fc703..cc08383dcf5 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.test.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.test.ts @@ -63,11 +63,48 @@ describe('requestDeviceAuthorization', () => { expect(shopifyFetch).toBeCalledWith('https://fqdn.com/oauth/device_authorization', { method: 'POST', headers: {'Content-type': 'application/x-www-form-urlencoded'}, - body: 'client_id=clientId&scope=scope1 scope2', + body: new URLSearchParams({client_id: 'clientId', scope: 'scope1 scope2'}).toString(), }) expect(got).toEqual(dataExpected) }) + test('encodes special characters in authorization scopes', async () => { + // Given + const response = new Response(JSON.stringify(data)) + vi.mocked(shopifyFetch).mockResolvedValue(response) + vi.mocked(identityFqdn).mockResolvedValue('fqdn.com') + vi.mocked(clientId).mockReturnValue('clientId') + const scopes = ['scope&name', 'scope=value', 'scope%value'] + + // When + await requestDeviceAuthorization(scopes) + + // Then + expect(shopifyFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: new URLSearchParams({client_id: 'clientId', scope: scopes.join(' ')}).toString(), + }), + ) + }) + + test('omits empty authorization scope values', async () => { + // Given + const response = new Response(JSON.stringify(data)) + vi.mocked(shopifyFetch).mockResolvedValue(response) + vi.mocked(identityFqdn).mockResolvedValue('fqdn.com') + vi.mocked(clientId).mockReturnValue('clientId') + + // When + await requestDeviceAuthorization([]) + + // Then + expect(shopifyFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({body: new URLSearchParams({client_id: 'clientId'}).toString()}), + ) + }) + test('opens the browser directly in an interactive terminal', async () => { // Given const outputInfo = vi.spyOn(output, 'outputInfo') diff --git a/packages/cli-kit/src/private/node/session/device-authorization.ts b/packages/cli-kit/src/private/node/session/device-authorization.ts index 8ac629e7cc0..769a3a8bdc0 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.ts @@ -152,10 +152,7 @@ export async function pollForDeviceAuthorization(code: string, interval = 5): Pr } function convertRequestToParams(queryParams: {client_id: string; scope: string}): string { - return Object.entries(queryParams) - .map(([key, value]) => value && `${key}=${value}`) - .filter((hasValue) => Boolean(hasValue)) - .join('&') + return new URLSearchParams(Object.entries(queryParams).filter(([, value]) => Boolean(value))).toString() } /** From 0733652932e12b9c0b2648764ae219599774c186 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sat, 8 Aug 2026 10:47:17 -0400 Subject: [PATCH 2/3] Strengthen device authorization encoding tests Assisted-By: devx/05a127b1-5522-4d26-afd2-fdde93868c5e --- .../node/session/device-authorization.test.ts | 20 +++++++++---------- .../node/session/device-authorization.ts | 1 + 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/cli-kit/src/private/node/session/device-authorization.test.ts b/packages/cli-kit/src/private/node/session/device-authorization.test.ts index cc08383dcf5..305ea26cca1 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.test.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.test.ts @@ -80,12 +80,11 @@ describe('requestDeviceAuthorization', () => { await requestDeviceAuthorization(scopes) // Then - expect(shopifyFetch).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ - body: new URLSearchParams({client_id: 'clientId', scope: scopes.join(' ')}).toString(), - }), - ) + expect(shopifyFetch).toHaveBeenCalledWith('https://fqdn.com/oauth/device_authorization', { + method: 'POST', + headers: {'Content-type': 'application/x-www-form-urlencoded'}, + body: 'client_id=clientId&scope=scope%26name+scope%3Dvalue+scope%25value', + }) }) test('omits empty authorization scope values', async () => { @@ -99,10 +98,11 @@ describe('requestDeviceAuthorization', () => { await requestDeviceAuthorization([]) // Then - expect(shopifyFetch).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({body: new URLSearchParams({client_id: 'clientId'}).toString()}), - ) + expect(shopifyFetch).toHaveBeenCalledWith('https://fqdn.com/oauth/device_authorization', { + method: 'POST', + headers: {'Content-type': 'application/x-www-form-urlencoded'}, + body: 'client_id=clientId', + }) }) test('opens the browser directly in an interactive terminal', async () => { diff --git a/packages/cli-kit/src/private/node/session/device-authorization.ts b/packages/cli-kit/src/private/node/session/device-authorization.ts index 769a3a8bdc0..cb7b5c0c34e 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.ts @@ -152,6 +152,7 @@ export async function pollForDeviceAuthorization(code: string, interval = 5): Pr } function convertRequestToParams(queryParams: {client_id: string; scope: string}): string { + // URLSearchParams applies form encoding so scope values cannot change the request structure. return new URLSearchParams(Object.entries(queryParams).filter(([, value]) => Boolean(value))).toString() } From 250f635e74c75b86067248abc93ad7249b6f5179 Mon Sep 17 00:00:00 2001 From: Donald Merand Date: Sat, 8 Aug 2026 11:10:34 -0400 Subject: [PATCH 3/3] Assert literal encoded body in basic scope test Assisted-By: devx/36a3e4d9-c136-46e2-a221-9934ba078d6f --- .../src/private/node/session/device-authorization.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/private/node/session/device-authorization.test.ts b/packages/cli-kit/src/private/node/session/device-authorization.test.ts index 305ea26cca1..7f66b5a4ceb 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.test.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.test.ts @@ -63,7 +63,7 @@ describe('requestDeviceAuthorization', () => { expect(shopifyFetch).toBeCalledWith('https://fqdn.com/oauth/device_authorization', { method: 'POST', headers: {'Content-type': 'application/x-www-form-urlencoded'}, - body: new URLSearchParams({client_id: 'clientId', scope: 'scope1 scope2'}).toString(), + body: 'client_id=clientId&scope=scope1+scope2', }) expect(got).toEqual(dataExpected) })