Shopify · jek · Oct 1, 2026 · Oct 1, 2026 · Oct 1, 2026 · Oct 1, 2026
@@ -13,7 +13,8 @@ import {randomBytes} from 'node:crypto'
 import {lstat, mkdir, realpath, rename, unlink, writeFile} from 'node:fs/promises'
 import type {Stats} from 'node:fs'
 
-const MAX_ARTIFACT_FILE_SIZE_BYTES = 5_000_000
+/** Readers reject an artifact larger than this, so writers check `encodedArtifactSize` against it first. */
+export const MAX_ARTIFACT_FILE_SIZE_BYTES = 5_000_000
 
 export interface AppSecurityArtifactPaths {
   artifactDirectory: string
@@ -140,6 +141,11 @@ async function readJsonArtifact(path: string): Promise<ReadArtifactResult<unknow
   }
 }
 
+/** The size in bytes of `value` once it's written as an artifact. */
+export function encodedArtifactSize(value: unknown): number {
+  return Buffer.byteLength(encodeArtifact(value), 'utf8')
+}
+
 function encodeArtifact(value: unknown): string {
   return `${JSON.stringify(value, null, 2)}\n`
 }

@@ -5,9 +5,10 @@ import {
   quoteShellArgument,
   resolveAppSecurityCommands,
   shellForPlatform,
+  type AppSecurityCommand,
   type AppSecurityShell,
 } from './app-security-commands.js'
-import {inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs'
+import {inTemporaryDirectory, readFile, writeFile} from '@shopify/cli-kit/node/fs'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {describe, expect, test} from 'vitest'
 import {spawnSync} from 'node:child_process'
@@ -204,7 +205,7 @@ describe('resolveAppSecurityCommands', () => {
       'shopify app security record --path "/tmp/app" < <findings.json>',
     )
     expect(formatAppSecurityCommand(commands.record, 'powershell')).toBe(
-      "Get-Content -Raw <findings.json> | shopify app security record --path '/tmp/app'",
+      "Get-Content -Raw -Encoding UTF8 <findings.json> | shopify app security record --path '/tmp/app'",
     )
     expect(formatAppSecurityCommand(commands.review, 'posix')).toBe("shopify app security review --path '/tmp/app'")
     expect(formatAppSecurityCommand(commands.clean, 'posix')).toBe("shopify app security clean --path '/tmp/app'")
@@ -321,7 +322,7 @@ describe('formatAppSecurityCommand', () => {
       const recordArguments = ['shopify', 'app', 'security', 'record', '--path', WINDOWS_APP_ROOT]
       expect(splitQuotedCommand(formatAppSecurityCommand(commands.record, shell), shell)).toEqual(
         shell === 'powershell'
-          ? ['Get-Content', '-Raw', '<findings.json>', '|', ...recordArguments]
+          ? ['Get-Content', '-Raw', '-Encoding', 'UTF8', '<findings.json>', '|', ...recordArguments]
           : [...recordArguments, '<', '<findings.json>'],
       )
       expect(splitQuotedCommand(formatAppSecurityCommand(commands.clean, shell), shell)).toEqual([
@@ -388,6 +389,40 @@ describe('formatAppSecurityCommand', () => {
       expect(result.stdout).not.toContain('EXPANDED')
     })
   })
+
+  test.skipIf(process.platform !== 'win32')(
+    'the PowerShell record command reads a findings file without a byte order mark as UTF-8 in Windows PowerShell 5.1',
+    async () => {
+      await inTemporaryDirectory(async (directory) => {
+        const findingsPath = joinPath(directory, 'findings.json')
+        const receivedPath = joinPath(directory, 'received.json')
+        const saveStdin = joinPath(directory, 'save-stdin.js')
+        // writeFile writes UTF-8 without a byte order mark.
+        await writeFile(findingsPath, '{"file": "app/café.ts"}')
+        // Saves stdin's bytes to a file, so PowerShell's console encoding can't change what the test reads back.
+        await writeFile(
+          saveStdin,
+          "const chunks = []\nprocess.stdin.on('data', (chunk) => chunks.push(chunk))\nprocess.stdin.on('end', () => require('fs').writeFileSync(process.argv[2], Buffer.concat(chunks)))\n",
+        )
+        // The record command's stdin form, with node in place of shopify.
+        const record: AppSecurityCommand = {
+          command: `& ${quoteShellArgument(process.execPath, 'powershell')}`,
+          args: [quoteShellArgument(saveStdin, 'powershell'), quoteShellArgument(receivedPath, 'powershell')],
+          stdinPlaceholder: quoteShellArgument(findingsPath, 'powershell'),
+        }
+        // As the instructions say: without this, Windows PowerShell 5.1 pipes text to node as ASCII.
+        const script = `$OutputEncoding = [System.Text.UTF8Encoding]::new(); ${formatAppSecurityCommand(record, 'powershell')}`
+        const result = spawnSync(
+          'powershell.exe',
+          ['-NoProfile', '-NonInteractive', '-EncodedCommand', Buffer.from(script, 'utf16le').toString('base64')],
+          {encoding: 'utf8', windowsHide: true},
+        )
+
+        expect(result.status, result.stderr).toBe(0)
+        expect(JSON.parse(await readFile(receivedPath))).toStrictEqual({file: 'app/café.ts'})
+      })
+    },
+  )
 })
 
 describe('formatAppSecurityInlineStdinCommand', () => {

@@ -97,15 +97,16 @@ function quoteCmdSegment(part: string): string {
 /**
  * Renders a command for the given shell. A command that reads stdin is shown reading its placeholder file:
  * redirected with `<` in POSIX shells and cmd.exe, and piped from `Get-Content -Raw` in PowerShell,
- * which has no `<` redirection.
+ * which has no `<` redirection. `-Encoding UTF8` because Windows PowerShell 5.1 otherwise reads a file
+ * without a byte order mark in the ANSI code page.
  */
 export function formatAppSecurityCommand(
   action: AppSecurityCommand,
   shell: AppSecurityShell = shellForPlatform(),
 ): string {
   const commandLine = formatCommandLine(action, shell)
   if (!action.stdinPlaceholder) return commandLine
-  if (shell === 'powershell') return `Get-Content -Raw ${action.stdinPlaceholder} | ${commandLine}`
+  if (shell === 'powershell') return `Get-Content -Raw -Encoding UTF8 ${action.stdinPlaceholder} | ${commandLine}`
   return `${commandLine} < ${action.stdinPlaceholder}`
 }
 

@@ -507,7 +507,9 @@ export function recordAgentFindings(document: unknown, options: RecordAgentFindi
 
   const grouped = groupFindingsByCheck(executed.reports, findings)
   errors.push(...grouped.errors)
-  if (errors.length > 0) return {ok: false, errors}
+  // Errors quote agent input (check IDs, paths, line values), which can hold secrets. They're shown in the
+  // terminal and in --json output, so they're redacted like everything that's stored.
+  if (errors.length > 0) return {ok: false, errors: errors.map(redactText)}
 
   const storedChecks = grouped.groups
     .map(

@@ -156,7 +156,11 @@ describe('appSecurityInstructions', () => {
       expect(instructions).toContain(
         codeBlock('powershell', "@'", '<the findings document from step 4>', `'@ | ${record}`),
       )
-      expect(instructions).toContain(codeBlock('powershell', `Get-Content -Raw <findings.json> | ${record}`))
+      // The encoding note follows both forms, since both pipe text to record.
+      expect(instructions).toContain(
+        `${codeBlock('powershell', `Get-Content -Raw -Encoding UTF8 <findings.json> | ${record}`)}\n\nWindows PowerShell 5.1 pipes text to \`record\` as ASCII by default.`,
+      )
+      expect(instructions).toContain('`$OutputEncoding = [System.Text.UTF8Encoding]::new()` before either command.')
       expect(instructions).toContain("The closing `'@` must start its line.")
       expect(instructions).not.toContain("<<'EOF'")
       expect(instructions).not.toContain(`${record} <`)

@@ -69,16 +69,21 @@ ${fileCommand}`
 
   const shellNotes =
     shell === 'powershell'
-      ? `The closing \`'@\` must start its line. The single-quoted here-string keeps PowerShell from expanding \`$\` in the document. Windows PowerShell 5.1 pipes text as ASCII by default, so if the document contains non-ASCII characters, run \`$OutputEncoding = [System.Text.UTF8Encoding]::new()\` first.`
+      ? `The closing \`'@\` must start its line. The single-quoted here-string keeps PowerShell from expanding \`$\` in the document.`
       : `The quoted \`'EOF'\` keeps the shell from expanding \`$\` and backticks in the document.`
+  // Both PowerShell forms pipe text to a native command, which Windows PowerShell 5.1 encodes as ASCII.
+  const encodingNote =
+    shell === 'powershell'
+      ? `\n\nWindows PowerShell 5.1 pipes text to \`record\` as ASCII by default. If the document contains non-ASCII characters, run \`$OutputEncoding = [System.Text.UTF8Encoding]::new()\` before either command.`
+      : ''
 
   return `${codeBlock(inlineCommand, shell)}
 
 Replace \`${RECORD_DOCUMENT_PLACEHOLDER}\` with the document itself; you don't need to write a file. ${shellNotes}
 
 If you'd rather write the document to a file, pipe the file instead, ${replaceFilePlaceholder}:
 
-${fileCommand}`
+${fileCommand}${encodingNote}`
 }
 
 function instructionPaths(

@@ -1,9 +1,9 @@
 import securityRecord, {renderSecurityRecordResult} from './security-record.js'
 import {securityRecordJsonOutputSchema} from './security-record-json.js'
 import {resolveAppSecurityRoot} from './app-security-api.js'
-import {appSecurityArtifactPaths, writeAgentFindings} from './app-security-artifacts.js'
+import {appSecurityArtifactPaths, readFindingsDocument, writeAgentFindings} from './app-security-artifacts.js'
 import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js'
-import {fileExists, inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs'
+import {fileExists, fileSize, inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs'
 import {AbortError, handler} from '@shopify/cli-kit/node/error'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
@@ -274,6 +274,64 @@ describe('securityRecord', () => {
     })
   })
 
+  describe('redacts secrets quoted in validation errors', () => {
+    const documentWithSecrets = JSON.stringify({
+      schema_version: 1,
+      checks_executed: [{check_id: FAKE_SHOPIFY_TOKEN, check_version: 1}],
+      findings: [
+        finding({check_id: FAKE_SHOPIFY_TOKEN}),
+        finding({file: `../${FAKE_SHOPIFY_TOKEN}`}),
+        finding({line: FAKE_SHOPIFY_TOKEN}),
+      ],
+    })
+    const redactedErrors = [
+      expect.stringMatching(/^checks_executed\[0\] \(.*REDACTED.*\): unknown check_id$/),
+      expect.stringMatching(/^findings\[0\] \(.*REDACTED.*\): unknown check_id$/),
+      expect.stringMatching(/^findings\[1\] \(MISSING_TENANT_ISOLATION\): unsafe file path .*REDACTED/),
+      expect.stringMatching(/^findings\[2\] \(MISSING_TENANT_ISOLATION\): invalid line number: .*REDACTED/),
+    ]
+
+    test('in the terminal', async () => {
+      await inTemporaryDirectory(async (directory) => {
+        const appRoot = await createApp(directory)
+        const error = await recordError(appRoot, testDependencies(documentWithSecrets))
+        expect(error.details).toStrictEqual({errors: redactedErrors})
+
+        const output = mockAndCaptureOutput()
+        output.clear()
+        try {
+          await handler(error)
+
+          expect(output.error()).toContain('unknown check_id')
+          expect(output.error()).toContain('REDACTED')
+          expect(output.error()).not.toContain(FAKE_SHOPIFY_TOKEN)
+        } finally {
+          output.clear()
+        }
+      })
+    })
+
+    test('in JSON mode', async () => {
+      await inTemporaryDirectory(async (directory) => {
+        const appRoot = await createApp(directory)
+        const error = await recordError(appRoot, testDependencies(documentWithSecrets))
+
+        const output = mockAndCaptureOutput()
+        output.clear()
+        vi.stubEnv('SHOPIFY_FLAG_JSON', '1')
+        try {
+          await handler(error)
+
+          expect(JSON.parse(output.info()).error.details).toStrictEqual({errors: redactedErrors})
+          expect(output.info()).not.toContain(FAKE_SHOPIFY_TOKEN)
+        } finally {
+          vi.unstubAllEnvs()
+          output.clear()
+        }
+      })
+    })
+  })
+
   test('fails when nothing is piped on stdin', async () => {
     await inTemporaryDirectory(async (directory) => {
       const appRoot = await createApp(directory)
@@ -315,6 +373,40 @@ describe('securityRecord', () => {
     await expectRejected(document, [expect.stringContaining('the limit is 5 MB')])
   })
 
+  describe('near the 5 MB limit', () => {
+    // 1,000 findings of about 5 KB each. The stored document adds about 180 KB of snapshots and indentation,
+    // so the reasoning length decides which side of the limit the stored file lands on.
+    function nearLimitDocument(reasoningLength: number): string {
+      return JSON.stringify({
+        schema_version: 1,
+        findings: Array.from({length: 1_000}, (_, index) =>
+          finding({line: index + 1, message: 'm'.repeat(4_000), reasoning: 'r'.repeat(reasoningLength)}),
+        ),
+      })
+    }
+
+    test('rejects a document under the input limit whose stored form review could not read', async () => {
+      const document = nearLimitDocument(700)
+      expect(Buffer.byteLength(document)).toBeLessThan(5_000_000)
+
+      await expectRejected(document, [
+        expect.stringMatching(/^The recorded findings would be stored as \d+ bytes; the limit is 5 MB\./),
+      ])
+    })
+
+    test('records a document that review can read', async () => {
+      await inTemporaryDirectory(async (directory) => {
+        const appRoot = await createApp(directory)
+        const {agentFindingsPath} = appSecurityArtifactPaths(appRoot)
+
+        await securityRecord({appRoot}, testDependencies(nearLimitDocument(500)))
+
+        await expect(fileSize(agentFindingsPath)).resolves.toBeGreaterThan(4_900_000)
+        await expect(readFindingsDocument(agentFindingsPath, 'agent')).resolves.toMatchObject({status: 'ok'})
+      })
+    })
+  })
+
   test('rejects invalid JSON', async () => {
     await expectRejected('{"schema_version": 1,', [expect.stringContaining('The findings document is not valid JSON')])
   })

@@ -1,4 +1,4 @@
-import {writeAgentFindings} from './app-security-artifacts.js'
+import {encodedArtifactSize, MAX_ARTIFACT_FILE_SIZE_BYTES, writeAgentFindings} from './app-security-artifacts.js'
 import {formatAppSecurityCommand, resolveAppSecurityCommands} from './app-security-commands.js'
 import {countLabel} from './app-security-format.js'
 import {
@@ -110,6 +110,16 @@ export default async function securityRecord(
   })
   if (!recorded.ok) throw rejectedDocumentError(recorded.errors, commands)
 
+  // The stored document adds check snapshots and indentation, so an input under the limit can still be stored
+  // over it. `review` can't read a file that large, so refuse to replace the existing one with it.
+  const storedSize = encodedArtifactSize(recorded.document)
+  if (storedSize > MAX_ARTIFACT_FILE_SIZE_BYTES) {
+    throw rejectedDocumentError(
+      [`The recorded findings would be stored as ${storedSize} bytes; the limit is 5 MB. Shorten or remove findings.`],
+      commands,
+    )
+  }
+
   const path = await dependencies.writeAgentFindings(options.appRoot, recorded.document)
   return {
     path,

@@ -623,6 +623,60 @@ describe('buildSecurityReviewAlerts', () => {
     ])
   })
 
+  test('lists the suppressed and superseded findings of unresolved checks in full with --verbose', () => {
+    // The agent's prefer-agent result supersedes both deterministic findings, and its own finding is suppressed,
+    // so the unresolved check has no active findings.
+    const agent: AgentFindingsDocument = {
+      ...agentFindingsDocument,
+      checks: agentFindingsDocument.checks.map((check) =>
+        check.id === 'CREDENTIAL_LOG_LEAKAGE'
+          ? {
+              ...check,
+              status: 'unresolved',
+              reason: {code: 'dynamic_logger', message: 'The logger is configured at runtime.'},
+              findings: check.findings.map((finding) => ({
+                ...finding,
+                suppression: {justification: 'The token is redacted by the logger.'},
+              })),
+            }
+          : check,
+      ),
+    }
+    const sources: Sources = {deterministic: deterministicFindingsDocument, agent}
+
+    const [concise] = buildSecurityReviewAlerts(presenterInput(sources, {checkIds: ['CREDENTIAL_LOG_LEAKAGE']}))
+    const [verbose] = buildSecurityReviewAlerts(
+      presenterInput(sources, {checkIds: ['CREDENTIAL_LOG_LEAKAGE'], verbose: true}),
+    )
+
+    expect(concise!.options.headline).toBe('1 check unresolved.')
+    expect(concise!.options.customSections).toHaveLength(1)
+    expect(verbose!.options.headline).toBe('1 check unresolved.')
+    const sections = verbose!.options.customSections!
+    expect(sections[0]).toEqual(concise!.options.customSections![0])
+    expect(sections.slice(1).map((section) => section.title)).toEqual([
+      'CREDENTIAL_LOG_LEAKAGE \u00b7 deterministic \u00b7 app/routes/orders.tsx:12 (superseded)',
+      'CREDENTIAL_LOG_LEAKAGE \u00b7 agent \u00b7 app/routes/orders.tsx:12 (suppressed)',
+      'CREDENTIAL_LOG_LEAKAGE \u00b7 deterministic \u00b7 app/shopify.server.ts:40 (superseded)',
+    ])
+    expect(sections[1]!.body).toContainEqual({
+      subdued: '\nEvidence: app/routes/orders.tsx:12 \u2014 console.log(session.accessToken)',
+    })
+    expect(sections[2]).toEqual({
+      title: 'CREDENTIAL_LOG_LEAKAGE \u00b7 agent \u00b7 app/routes/orders.tsx:12 (suppressed)',
+      body: [
+        'The session access token is written to the server log.',
+        {subdued: '\nConfidence: high'},
+        {
+          subdued:
+            '\nReasoning: The logged object is the authenticated session, whose accessToken is a live credential.',
+        },
+        {subdued: '\nEvidence: app/routes/orders.tsx:12 \u2014 console.log(session.accessToken)'},
+        {subdued: '\nSuppressed: The token is redacted by the logger.'},
+      ],
+    })
+  })
+
   test('lists passed and not applicable checks with their disposition counts', () => {
     // With no active findings, MISSING_TENANT_ISOLATION passes; its one finding is suppressed.
     const agent: AgentFindingsDocument = {

@@ -119,7 +119,7 @@ export function buildSecurityReviewAlerts(input: SecurityReviewPresenterInput):
   const withFindings = checks.filter((check) => activeFindings(check).length > 0)
 
   return [
-    ...(unresolved.length > 0 ? [unresolvedChecksAlert(unresolved)] : []),
+    ...(unresolved.length > 0 ? [unresolvedChecksAlert(unresolved, input.verbose)] : []),
     ...(passedOrNotApplicable.length > 0 ? [passedChecksAlert(passedOrNotApplicable, input.verbose)] : []),
     ...withFindings.map((check) => checkWithFindingsAlert(check, input.verbose, input.now)),
     summaryAlert(buildSecurityReviewSummary(input)),
@@ -331,15 +331,22 @@ function resultsFileCells(row: ResultsFileRow): InlineToken[] {
   return [row.name, row.updated, row.commit, row.engine]
 }
 
-function unresolvedChecksAlert(checks: CombinedCheck[]): SecurityReviewAlert {
+/**
+ * Unresolved checks with no active findings. With `--verbose`, each check's suppressed and superseded findings
+ * follow it in full, prefixed by the check ID, as in the passed checks box.
+ */
+function unresolvedChecksAlert(checks: CombinedCheck[], verbose: boolean): SecurityReviewAlert {
   return {
     type: 'warning',
     options: {
       headline: `${countLabel(checks.length, 'check')} unresolved.`,
-      customSections: checks.map((check) => ({
-        title: `${check.id} · ${check.title}`,
-        body: {tabularData: sourceStatusRows(check, {withDispositions: true}), firstColumnSubdued: true},
-      })),
+      customSections: checks.flatMap((check) => [
+        {
+          title: `${check.id} · ${check.title}`,
+          body: {tabularData: sourceStatusRows(check, {withDispositions: true}), firstColumnSubdued: true},
+        },
+        ...(verbose ? check.findings.map((finding) => findingSection(finding, verbose, `${check.id} · `)) : []),
+      ]),
     },
   }
 }