Send OAuth token request params in POST body instead of URL

The cli-kit `tokenRequest()` helper was placing all OAuth parameters
(`subject_token`, `refresh_token`, `device_code`, `client_id`, etc.)
into the URL query string of POST requests to `/oauth/token`.

RFC 6749 specifies that these parameters are sent in the request body as
`application/x-www-form-urlencoded` data, and putting credentials in
URLs causes them to leak into verbose CLI debug output, HTTP access logs,
proxies, and anywhere else URLs are recorded.

This commit:

- Moves the OAuth parameters from the URL query string into the POST
  request body (form-encoded).
- Expands `sanitizeURL` to mask all sensitive OAuth parameter names
  (`access_token`, `refresh_token`, `id_token`, `subject_token`,
  `actor_token`, `device_code`, `client_secret`, `code`) so that any
  URL that ever carries one of these values in the future is still
  redacted as defence in depth.
- Updates tests to assert the new behaviour and adds coverage for every
  sanitized parameter.

Reported through Shopify's bug bounty programme (report #3686978).

Requested by Josh Bregar <josh.bregar@shopify.com>

Co-authored-by: Josh Bregar <josh.bregar@shopify.com>

Author: shopify-river

.changeset/oauth-credentials-to-body.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli-kit': patch
+---
+
+Send OAuth `/oauth/token` parameters in the POST request body (per RFC 6749) instead of the URL query string, so credentials such as `subject_token`, `refresh_token`, and `device_code` are not written to verbose CLI debug output. Also extends `sanitizeURL` to mask `access_token`, `refresh_token`, `id_token`, `subject_token`, `actor_token`, `device_code`, `client_secret`, and `code` for defence in depth.

packages/cli-kit/src/private/node/api/urls.test.ts

@@ -45,4 +45,38 @@ describe('sanitizeURL', () => {
     // Then
     expect(sanitizedUrl).toBe('https://example.com/?subject_token=****&other_param=def456')
   })
+
+  test.each([
+    'access_token',
+    'refresh_token',
+    'id_token',
+    'subject_token',
+    'actor_token',
+    'device_code',
+    'client_secret',
+    'code',
+    'token',
+  ])('sanitizes %s query parameter', (param) => {
+    // Given
+    const url = `https://example.com?${param}=secret-value`
+
+    // When
+    const sanitizedUrl = sanitizeURL(url)
+
+    // Then
+    expect(sanitizedUrl).toBe(`https://example.com/?${param}=****`)
+  })
+
+  test('sanitizes all sensitive query parameters together', () => {
+    // Given
+    const url = 'https://example.com?access_token=a&refresh_token=b&device_code=c&subject_token=d&other=keep'
+
+    // When
+    const sanitizedUrl = sanitizeURL(url)
+
+    // Then
+    expect(sanitizedUrl).toBe(
+      'https://example.com/?access_token=****&refresh_token=****&device_code=****&subject_token=****&other=keep',
+    )
+  })
 })

packages/cli-kit/src/private/node/api/urls.ts

@@ -1,15 +1,30 @@
+// Query-string parameter names that may carry sensitive credentials and must
+// not be written to logs, verbose debug output, or any other user-visible
+// destination. Covers OAuth 2.0 / device-authorization / token-exchange
+// parameters.
+const SENSITIVE_QUERY_PARAMS = [
+  'access_token',
+  'refresh_token',
+  'id_token',
+  'subject_token',
+  'actor_token',
+  'device_code',
+  'client_secret',
+  'code',
+  'token',
+]
+
 /**
  * Removes the sensitive data from the url and outputs them as a string.
  * @param url - HTTP headers.
  * @returns A sanitized version of the url as a string.
  */
 export function sanitizeURL(url: string): string {
   const parsedUrl = new URL(url)
-  if (parsedUrl.searchParams.has('subject_token')) {
-    parsedUrl.searchParams.set('subject_token', '****')
-  }
-  if (parsedUrl.searchParams.has('token')) {
-    parsedUrl.searchParams.set('token', '****')
+  for (const param of SENSITIVE_QUERY_PARAMS) {
+    if (parsedUrl.searchParams.has(param)) {
+      parsedUrl.searchParams.set(param, '****')
+    }
   }
   return parsedUrl.toString()
 }

packages/cli-kit/src/private/node/session/exchange.test.ts

@@ -271,8 +271,10 @@ describe.each(tokenExchangeMethods)(
     test(`Executing ${tokenExchangeMethod.name} returns access token and user ID for a valid CLI token`, async () => {
       // Given
       let capturedUrl = ''
+      let capturedInit: {method?: string; headers?: Record<string, string>; body?: string} = {}
       vi.mocked(shopifyFetch).mockImplementation(async (url, options) => {
         capturedUrl = url.toString()
+        capturedInit = (options ?? {}) as typeof capturedInit
         return Promise.resolve(
           new Response(
             JSON.stringify({
@@ -292,12 +294,21 @@ describe.each(tokenExchangeMethods)(
       await expect(getLastSeenUserIdAfterAuth()).resolves.toBe(userId)
       await expect(getLastSeenAuthMethod()).resolves.toBe('partners_token')
 
-      // Assert token exchange parameters are correct
+      // Request is sent as POST form-encoded body (not query string), so the
+      // URL must not contain any OAuth parameters.
       const actualUrl = new URL(capturedUrl)
       expect(actualUrl).toBeDefined()
-      expect(actualUrl.href).toContain('https://fqdn.com/oauth/token')
+      expect(actualUrl.href).toBe('https://fqdn.com/oauth/token')
+      expect(actualUrl.search).toBe('')
 
-      const params = actualUrl.searchParams
+      expect(capturedInit.method).toBe('POST')
+      expect(capturedInit.headers).toMatchObject({
+        'Content-Type': 'application/x-www-form-urlencoded',
+      })
+      expect(typeof capturedInit.body).toBe('string')
+
+      // Assert token exchange parameters are correct and sent in the body.
+      const params = new URLSearchParams(capturedInit.body)
       expect(params.get('grant_type')).toBe(grantType)
       expect(params.get('requested_token_type')).toBe(accessTokenType)
       expect(params.get('subject_token_type')).toBe(accessTokenType)

packages/cli-kit/src/private/node/session/exchange.ts

@@ -226,10 +226,19 @@ async function tokenRequest(
   params: Record<string, string>,
 ): Promise<Result<TokenRequestResult, {error: string; store?: string}>> {
   const fqdn = await identityFqdn()
-  const url = new URL(`https://${fqdn}/oauth/token`)
-  url.search = new URLSearchParams(Object.entries(params)).toString()
+  const url = `https://${fqdn}/oauth/token`
 
-  const res = await shopifyFetch(url.href, {method: 'POST'})
+  // Send OAuth parameters in the request body (per RFC 6749) rather than the
+  // URL query string. This prevents sensitive credentials (subject_token,
+  // refresh_token, device_code, client_id, etc.) from being written to
+  // verbose CLI debug output or otherwise leaking through URLs.
+  const body = new URLSearchParams(Object.entries(params)).toString()
+
+  const res = await shopifyFetch(url, {
+    method: 'POST',
+    headers: {'Content-Type': 'application/x-www-form-urlencoded'},
+    body,
+  })
   try {
     const responseText = await res.text()