diff --git a/packages/cli-kit/src/public/node/analytics.test.ts b/packages/cli-kit/src/public/node/analytics.test.ts index 15f64abd05b..ea764c23fcc 100644 --- a/packages/cli-kit/src/public/node/analytics.test.ts +++ b/packages/cli-kit/src/public/node/analytics.test.ts @@ -429,6 +429,153 @@ describe('event tracking', () => { }) }) + test('does not send signup JWTs passed as command arguments to Monorail', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const signupJwt = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJzdG9yZSJ9.s1gn4tur3' + const commandContent = {command: 'stripe-auth', topic: 'store'} + const argsWithSignup = args.concat(['--signup', signupJwt, '--scopes', `--signup=${signupJwt}`]) + await startAnalytics({commandContent, args: argsWithSignup, currentTime: currentDate.getTime() - 100}) + + // When + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() + + // Then + expect(publishEventMock).toHaveBeenCalledOnce() + const sensitivePayload = publishEventMock.mock.calls[0]![2] + expect(sensitivePayload.args).toContain('--signup *****') + expect(sensitivePayload.args).toContain('--signup=*****') + expect(JSON.stringify(sensitivePayload)).not.toContain('s1gn4tur3') + }) + }) + + test('does not send signup JWTs that were quoted on the command line to Monorail', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const signupJwt = 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJzdG9yZSJ9.s1gn4tur3' + const commandContent = {command: 'stripe-auth', topic: 'store'} + const argsWithSignup = args.concat(['--signup', `"${signupJwt}"`]) + await startAnalytics({commandContent, args: argsWithSignup, currentTime: currentDate.getTime() - 100}) + + // When + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() + + // Then + expect(publishEventMock).toHaveBeenCalledOnce() + const sensitivePayload = publishEventMock.mock.calls[0]![2] + expect(sensitivePayload.args).toContain('--signup *****') + expect(JSON.stringify(sensitivePayload)).not.toContain('s1gn4tur3') + }) + }) + + test('does not send signup environment flags to Monorail', async () => { + await inProjectWithFile('package.json', async (args) => { + const commandContent = {command: 'stripe-auth', topic: 'store'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + await addSensitiveMetadata(() => ({ + environmentFlags: JSON.stringify({signup: 'eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJzdG9yZSJ9.s1gn4tur3'}), + })) + + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() + + expect(publishEventMock).toHaveBeenCalledOnce() + expect(publishEventMock.mock.calls[0]![2]).toMatchObject({ + cmd_all_environment_flags: JSON.stringify({signup: '*****'}), + }) + }) + }) + + test('does not send signup credentials carried in an authorization URL to Monorail', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'stripe-auth', topic: 'store'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + + // When + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + await reportAnalyticsEvent({ + config, + errorMessage: + 'Could not open https://shop.myshopify.com/admin/oauth/authorize?client_id=abc&signup=eyJhbGciOiJIUzI1NiJ9.s1gn4tur3&state=xyz', + exitMode: 'unexpected_error', + }) + await sendReportedAnalyticsPayload() + + // Then + expect(publishEventMock).toHaveBeenCalledOnce() + const sensitivePayload = publishEventMock.mock.calls[0]![2] + expect(sensitivePayload.error_message).toContain('signup=*****&state=xyz') + expect(JSON.stringify(sensitivePayload)).not.toContain('s1gn4tur3') + }) + }) + + test('sends URLs whose path merely mentions signup without redacting them', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'dev', topic: 'app'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + + // When + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + await reportAnalyticsEvent({ + config, + errorMessage: 'Create an account at https://partners.shopify.com/signup and retry with from_signup=true', + exitMode: 'unexpected_error', + }) + await sendReportedAnalyticsPayload() + + // Then + expect(publishEventMock).toHaveBeenCalledOnce() + expect(publishEventMock.mock.calls[0]![2].error_message).toBe( + 'Create an account at https://partners.shopify.com/signup and retry with from_signup=true', + ) + }) + }) + + test('sends analytics when a redacted flag value was quoted on the command line', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'dev', topic: 'app'} + const argsWithPassword = args.concat(['--store-password', '"store secret"']) + await startAnalytics({commandContent, args: argsWithPassword, currentTime: currentDate.getTime() - 100}) + + // When + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() + + // Then + expect(publishEventMock).toHaveBeenCalledOnce() + const sensitivePayload = publishEventMock.mock.calls[0]![2] + expect(sensitivePayload.args).toContain('--store-password *****') + expect(JSON.stringify(sensitivePayload)).not.toContain('store secret') + }) + }) + test('sends only allowlisted Shopify environment variables in sensitive payload', async () => { const originalShopifyInvokedBy = process.env.SHOPIFY_INVOKED_BY const originalShopifyCliAgent = process.env.SHOPIFY_CLI_AGENT diff --git a/packages/cli-kit/src/public/node/analytics.ts b/packages/cli-kit/src/public/node/analytics.ts index d20f7c90a8f..574783f4af5 100644 --- a/packages/cli-kit/src/public/node/analytics.ts +++ b/packages/cli-kit/src/public/node/analytics.ts @@ -267,11 +267,17 @@ async function buildPayload({ function sanitizePayload(payload: T): T { const payloadString = JSON.stringify(payload) - // Remove Theme Access passwords from the payload + // Redaction runs on the serialized payload, so a flag value that was quoted arrives with its quotes + // escaped. Every flag rule below matches that escaped form first, and its bare-value alternative + // treats a JSON escape as part of the value, so a value can never be redacted only halfway or leave + // behind a string that no longer parses. const sanitizedPayloadString = payloadString .replace(/shptka_\w*/g, '*****') - .replace(/(--store-password(?:=|\s+))(?:"[^"]*"|'[^']*'|[^\s"]+)/g, '$1*****') + .replace(/(--store-password(?:=|\s+))(?:\\"[^"\\]*\\"|'[^']*'|(?:\\.|[^\s"\\])+)/g, '$1*****') .replace(/((?:store-password|SHOPIFY_FLAG_STORE_PASSWORD)\\?":\\?")[^"\\]*/g, '$1*****') + .replace(/(--signup(?:=|\s+))(?:\\"[^"\\]*\\"|'[^']*'|(?:\\.|[^\s"\\])+)/g, '$1*****') + .replace(/((?:signup|SHOPIFY_FLAG_SIGNUP)\\?":\\?")[^"\\]*/g, '$1*****') + .replace(/(?