From 44fa31dfc9d3a398caec4e4194625cffd4f91e81 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Fri, 3 Apr 2026 14:44:35 -0600 Subject: [PATCH 1/5] Stop Playwright tracing during E2E login to prevent credential exposure Playwright traces capture page.fill() values verbatim, and traces are uploaded as publicly downloadable CI artifacts. This stops tracing before entering credentials and restarts it after login completes, navigating to about:blank before restart so the first snapshot doesn't capture residual form state. Also removes page HTML dump from login error messages since filled input values could appear in the DOM. Ref: shopify/bugbounty#3638393 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/e2e/helpers/browser-login.ts | 31 +++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/e2e/helpers/browser-login.ts b/packages/e2e/helpers/browser-login.ts index ba637b6d382..8ad7a8defd4 100644 --- a/packages/e2e/helpers/browser-login.ts +++ b/packages/e2e/helpers/browser-login.ts @@ -1,12 +1,27 @@ +/* eslint-disable no-console */ import type {Page} from '@playwright/test' /** * Completes the Shopify OAuth login flow on a Playwright page. */ export async function completeLogin(page: Page, loginUrl: string, email: string, password: string): Promise { - await page.goto(loginUrl) + const tracing = page.context().tracing + // Traces are uploaded as public CI artifacts, so we must stop + // tracing before entering credentials and restart it after login completes. + let tracingWasActive = false try { + await tracing.stop() + tracingWasActive = true + console.log('[e2e] Tracing paused for credential entry') + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // tracing.stop() throws if tracing was never started — that's fine + } + + try { + await page.goto(loginUrl) + // Fill in email await page.waitForSelector('input[name="account[email]"], input[type="email"]', {timeout: 60_000}) await page.locator('input[name="account[email]"], input[type="email"]').first().fill(email) @@ -27,12 +42,20 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, // No confirmation page — expected } } catch (error) { - const pageContent = await page.content().catch(() => '(failed to get content)') + // Intentionally omit page HTML from the error — it may contain filled + // credential values in input elements, which would leak into test reports. const pageUrl = page.url() throw new Error( `Login failed at ${pageUrl}\n` + - `Original error: ${error}\n` + - `Page HTML (first 2000 chars): ${pageContent.slice(0, 2000)}`, + `Original error: ${error}`, ) + } finally { + if (tracingWasActive) { + // Navigate to blank page before restarting tracing so the first snapshot + // does not capture any residual credentials on the login form. + await page.goto('about:blank').catch(() => {}) + await tracing.start({screenshots: true, snapshots: true}) + console.log('[e2e] Tracing resumed after credential entry') + } } } From c5abe00b5591e38469792e55f6e12b2269b46c2c Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Fri, 3 Apr 2026 14:53:47 -0600 Subject: [PATCH 2/5] Replace fill() with evaluate() to prevent credential capture in traces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous approach (context.tracing.stop()) did not work — Playwright's test runner instruments API calls at a level above context.tracing, so fill() values are logged in traces regardless. Using evaluate() to set input values via the DOM bypasses the Playwright action log entirely, so credentials never appear in trace files or HTML reports. Ref: shopify/bugbounty#3638393 Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/e2e/helpers/browser-login.ts | 49 +++++++++++++-------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/packages/e2e/helpers/browser-login.ts b/packages/e2e/helpers/browser-login.ts index 8ad7a8defd4..913322bf002 100644 --- a/packages/e2e/helpers/browser-login.ts +++ b/packages/e2e/helpers/browser-login.ts @@ -1,35 +1,40 @@ -/* eslint-disable no-console */ import type {Page} from '@playwright/test' +/** + * Sets an input field's value via the DOM, bypassing Playwright's fill() API. + * + * Security (shopify/bugbounty#3638393): Playwright's test runner logs every + * fill() call — including the literal value — into trace files, which are + * uploaded as publicly downloadable CI artifacts. Using evaluate() to set + * the value directly avoids the Playwright action log entirely. The runner's + * tracing instruments at a level above context.tracing, so context.tracing.stop() + * does NOT prevent the leak. + */ +async function fillSensitive(page: Page, selector: string, value: string): Promise { + const locator = page.locator(selector).first() + await locator.evaluate((el, val) => { + const input = el as HTMLInputElement + input.value = val + input.dispatchEvent(new Event('input', {bubbles: true})) + input.dispatchEvent(new Event('change', {bubbles: true})) + }, value) +} + /** * Completes the Shopify OAuth login flow on a Playwright page. */ export async function completeLogin(page: Page, loginUrl: string, email: string, password: string): Promise { - const tracing = page.context().tracing + await page.goto(loginUrl) - // Traces are uploaded as public CI artifacts, so we must stop - // tracing before entering credentials and restart it after login completes. - let tracingWasActive = false try { - await tracing.stop() - tracingWasActive = true - console.log('[e2e] Tracing paused for credential entry') - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // tracing.stop() throws if tracing was never started — that's fine - } - - try { - await page.goto(loginUrl) - // Fill in email await page.waitForSelector('input[name="account[email]"], input[type="email"]', {timeout: 60_000}) - await page.locator('input[name="account[email]"], input[type="email"]').first().fill(email) + await fillSensitive(page, 'input[name="account[email]"], input[type="email"]', email) await page.locator('button[type="submit"]').first().click() // Fill in password await page.waitForSelector('input[name="account[password]"], input[type="password"]', {timeout: 60_000}) - await page.locator('input[name="account[password]"], input[type="password"]').first().fill(password) + await fillSensitive(page, 'input[name="account[password]"], input[type="password"]', password) await page.locator('button[type="submit"]').first().click() // Handle any confirmation/approval page @@ -49,13 +54,5 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, `Login failed at ${pageUrl}\n` + `Original error: ${error}`, ) - } finally { - if (tracingWasActive) { - // Navigate to blank page before restarting tracing so the first snapshot - // does not capture any residual credentials on the login form. - await page.goto('about:blank').catch(() => {}) - await tracing.start({screenshots: true, snapshots: true}) - console.log('[e2e] Tracing resumed after credential entry') - } } } From 68ee26fb10747b010a6d9744b99efd66ca585bef Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Fri, 3 Apr 2026 15:04:30 -0600 Subject: [PATCH 3/5] Fix lint and type errors in browser-login - Fix prettier formatting for error string concatenation - Avoid HTMLInputElement (not in tsconfig types: ["node"]) by casting through unknown Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/e2e/helpers/browser-login.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/e2e/helpers/browser-login.ts b/packages/e2e/helpers/browser-login.ts index 913322bf002..76e9835fb92 100644 --- a/packages/e2e/helpers/browser-login.ts +++ b/packages/e2e/helpers/browser-login.ts @@ -12,11 +12,10 @@ import type {Page} from '@playwright/test' */ async function fillSensitive(page: Page, selector: string, value: string): Promise { const locator = page.locator(selector).first() - await locator.evaluate((el, val) => { - const input = el as HTMLInputElement - input.value = val - input.dispatchEvent(new Event('input', {bubbles: true})) - input.dispatchEvent(new Event('change', {bubbles: true})) + await locator.evaluate((el: Element, val: string) => { + ;(el as unknown as {value: string}).value = val + el.dispatchEvent(new Event('input', {bubbles: true})) + el.dispatchEvent(new Event('change', {bubbles: true})) }, value) } @@ -50,9 +49,6 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, // Intentionally omit page HTML from the error — it may contain filled // credential values in input elements, which would leak into test reports. const pageUrl = page.url() - throw new Error( - `Login failed at ${pageUrl}\n` + - `Original error: ${error}`, - ) + throw new Error(`Login failed at ${pageUrl}\n` + `Original error: ${error}`) } } From 657b467fa8f7e523552ab979ab4a11a6eadfece8 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Fri, 3 Apr 2026 15:08:15 -0600 Subject: [PATCH 4/5] Fix remaining type and lint errors - Drop explicit Element type annotation (not in node-only tsconfig) - Inline string concatenation to fix no-useless-concat Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/e2e/helpers/browser-login.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/e2e/helpers/browser-login.ts b/packages/e2e/helpers/browser-login.ts index 76e9835fb92..8d34417c8b4 100644 --- a/packages/e2e/helpers/browser-login.ts +++ b/packages/e2e/helpers/browser-login.ts @@ -12,7 +12,7 @@ import type {Page} from '@playwright/test' */ async function fillSensitive(page: Page, selector: string, value: string): Promise { const locator = page.locator(selector).first() - await locator.evaluate((el: Element, val: string) => { + await locator.evaluate((el, val) => { ;(el as unknown as {value: string}).value = val el.dispatchEvent(new Event('input', {bubbles: true})) el.dispatchEvent(new Event('change', {bubbles: true})) @@ -49,6 +49,6 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, // Intentionally omit page HTML from the error — it may contain filled // credential values in input elements, which would leak into test reports. const pageUrl = page.url() - throw new Error(`Login failed at ${pageUrl}\n` + `Original error: ${error}`) + throw new Error(`Login failed at ${pageUrl}\nOriginal error: ${error}`) } } From cf2cc00ef4de36b9d445484f4c3a8a87f559adeb Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Fri, 3 Apr 2026 15:38:23 -0600 Subject: [PATCH 5/5] Clear login page on auth failure to prevent credential capture Navigate to about:blank in the catch block before rethrowing so that failure artifacts (screenshots, trace snapshots) do not capture the login form with credentials still populated. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/e2e/helpers/browser-login.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/e2e/helpers/browser-login.ts b/packages/e2e/helpers/browser-login.ts index 8d34417c8b4..da5a4766c50 100644 --- a/packages/e2e/helpers/browser-login.ts +++ b/packages/e2e/helpers/browser-login.ts @@ -46,9 +46,10 @@ export async function completeLogin(page: Page, loginUrl: string, email: string, // No confirmation page — expected } } catch (error) { - // Intentionally omit page HTML from the error — it may contain filled - // credential values in input elements, which would leak into test reports. const pageUrl = page.url() + // Clear the page so failure artifacts (screenshots, trace snapshots) do + // not capture the login form with credentials still populated. + await page.goto('about:blank').catch(() => {}) throw new Error(`Login failed at ${pageUrl}\nOriginal error: ${error}`) } }