diff --git a/docs/specs/agent-environments/environment-build/spec.md b/docs/specs/agent-environments/environment-build/spec.md index dd793d0a0b..497c42cfd5 100644 --- a/docs/specs/agent-environments/environment-build/spec.md +++ b/docs/specs/agent-environments/environment-build/spec.md @@ -47,7 +47,9 @@ before building. pointers, never values, and never refuse a build; the scan cannot overflow the stack on deeply nested bodies. A browser scenario's retained header values and the query parameters of its recorded URLs are scanned as a HAR's headers and - query strings are. + query strings are, and so are its request bodies recorded without a media + type. Each occurrence of a repeated form field counts, as each repeated + header or query parameter does. - Nothing is written before the bundle stage; `--dry-run` writes nothing, reports the files a build would write and refuses the package tarballs a build refuses, admitting and releasing the tarball as a build does: a close it diff --git a/src/application/environment/environmentBuildReport.test.ts b/src/application/environment/environmentBuildReport.test.ts index 10ceb1d03a..2e8f5ece41 100644 --- a/src/application/environment/environmentBuildReport.test.ts +++ b/src/application/environment/environmentBuildReport.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vitest"; +import { scenarioCaptureFixture } from "../../../tests/fixtures/browserScenarioCapture.js"; +import { scenarioCaptureRecords } from "../../domain/analysisView/scenarioCaptureRecords.js"; import type { WebNetworkCaptureRecord } from "../../domain/webNetworkCapture.js"; import { anonymousOrigin, @@ -130,6 +132,47 @@ describe("scanCredentialNames", () => { }); }); + it("counts every occurrence of a repeated form field that holds a value", () => { + const form = (text: string): WebNetworkCaptureRecord => { + const scanned = record(3, undefined, undefined); + ( + scanned.reported as { request: { postData?: unknown } } + ).request.postData = { + mimeType: "application/x-www-form-urlencoded", + text, + }; + return scanned; + }; + // A later empty occurrence does not hide the earlier value the Evidence ships. + expect(scanCredentialNames([form("api_key=secret&api_key=")])).toEqual({ + count: 1, + pointers: ["record 3 request /api_key"], + }); + expect(scanCredentialNames([form("token=a&page=1&token=b")])).toEqual({ + count: 2, + pointers: ["record 3 request /token", "record 3 request /token"], + }); + }); + + it("scans a scenario request body the capture recorded without a media type", () => { + const capture = scenarioCaptureFixture( + [ + { + method: "POST", + url: "/api/session", + status: 200, + mediaType: "application/json", + body: "{}", + requestBody: '{"password":"x"}', + }, + ], + { request_body: true, response_body: true, header_values: true }, + ); + expect( + scanCredentialNames(scenarioCaptureRecords(capture, "review").records), + ).toEqual({ count: 1, pointers: ["record 0 request /password"] }); + }); + it("walks bodies nested deeper than the call stack allows", () => { const depth = 20_000; const deep = record(1, undefined, undefined); diff --git a/src/application/environment/environmentBuildReport.ts b/src/application/environment/environmentBuildReport.ts index 98610df6e7..35bdd2d024 100644 --- a/src/application/environment/environmentBuildReport.ts +++ b/src/application/environment/environmentBuildReport.ts @@ -82,21 +82,26 @@ const member = (value: unknown, name: string): unknown => ? Reflect.get(value, name) : undefined; -/** A recorded body as JSON or form members, or undefined when it is neither. */ -const parsedBody = (text: unknown): unknown => { +/** A recorded body: a JSON value to walk, or form fields in order with every occurrence kept, as headers and query strings are. */ +type ScannedBody = + | { readonly kind: "json"; readonly value: unknown } + | { readonly kind: "form"; readonly fields: readonly [string, string][] }; + +/** A recorded body as JSON or form fields, or undefined when it is neither. */ +const parsedBody = (text: unknown): ScannedBody | undefined => { if (typeof text !== "string" || text.length > SCAN_BODY_LIMIT_BYTES) return undefined; const start = text.trimStart()[0]; if (start === "{" || start === "[") { try { - return JSON.parse(text); + return { kind: "json", value: JSON.parse(text) }; } catch { return undefined; } } if (!/^[^=&\s]+=[^&]*(?:&[^=&\s]+=[^&]*)*$/u.test(text)) return undefined; try { - return Object.fromEntries(new URLSearchParams(text)); + return { kind: "form", fields: [...new URLSearchParams(text)] }; } catch { return undefined; } @@ -147,6 +152,13 @@ export const scanCredentialNames = ( } } }; + const scanBody = (body: ScannedBody | undefined, label: string): void => { + if (body?.kind === "json") walk(body.value, label); + else if (body?.kind === "form") + for (const [name, value] of body.fields) + if (credentialNamed(name) && holdsValue(value)) + found(`${label} /${pointerToken(name)}`); + }; for (const record of records) { const ordinal = String(record.ordinal); const request = member(record.reported, "request"); @@ -161,11 +173,11 @@ export const scanCredentialNames = ( for (const [name, value] of namedValues(member(request, "queryString"))) if (credentialNamed(name) && holdsValue(value)) found(`record ${ordinal} request query ${name}`); - walk( + scanBody( parsedBody(member(member(request, "postData"), "text")), `record ${ordinal} request`, ); - walk( + scanBody( parsedBody( readRecordedResponseBody(record, SCAN_BODY_LIMIT_BYTES).bodyText, ), diff --git a/src/domain/analysisView/scenarioCaptureRecords.test.ts b/src/domain/analysisView/scenarioCaptureRecords.test.ts index b89a135b17..2e8276b299 100644 --- a/src/domain/analysisView/scenarioCaptureRecords.test.ts +++ b/src/domain/analysisView/scenarioCaptureRecords.test.ts @@ -293,3 +293,29 @@ it("keeps retained header values and URL query pairs only for review", () => { expect(projected?.reported).not.toHaveProperty("response.headers"); } }); + +it("keeps a request body without a media type for review only", () => { + const capture = scenarioCaptureFixture( + [ + { + method: "POST", + url: "/api/session", + status: 200, + mediaType: "application/json", + body: "{}", + requestBody: '{"password":"x"}', + }, + ], + { request_body: true, response_body: true, header_values: true }, + ); + // A body the capture could not classify is still shipped Evidence, so review sees its text. + expect( + scenarioCaptureRecords(capture, "review").records[0]?.reported, + ).toMatchObject({ + request: { postData: { mimeType: "", text: '{"password":"x"}' } }, + }); + // The manifest still reads only bodies whose media type it can classify. + expect( + scenarioCaptureRecords(capture, "manifest").records[0]?.reported, + ).not.toHaveProperty("request.postData"); +}); diff --git a/src/domain/analysisView/scenarioCaptureRecords.ts b/src/domain/analysisView/scenarioCaptureRecords.ts index 27f1434469..b4bd57c623 100644 --- a/src/domain/analysisView/scenarioCaptureRecords.ts +++ b/src/domain/analysisView/scenarioCaptureRecords.ts @@ -125,9 +125,10 @@ const reportedRequest = ( queryString: queryPairs(request.url.url), } : {}), - ...(text === undefined || mimeType === undefined + // Review scans every retained body, so one the capture could not classify keeps its text with an empty media type. + ...(text === undefined || (mimeType === undefined && purpose !== "review") ? {} - : { postData: { mimeType, text } }), + : { postData: { mimeType: mimeType ?? "", text } }), }; };