From 212063ff6c2aee2548aeabc111179e1c56daa4c0 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Sun, 25 Oct 2026 19:49:33 -0700 Subject: [PATCH] fix(browser): address review of saved values and slug spellings - Basic credentials and values saved with `secret: true` are redacted however short, as declared secrets are; page values keep the 8-character floor - a literal Basic password other than empty is refused, since Evidence records literals as written - `save_response` takes only a request the step starts, so an earlier autosave cannot answer it, matches its path as the browser spells it, and stops watching requests when the step ends - saving reads are bounded: a request step streams its answer up to 4,000,000 characters; a UI step refuses a larger received size unread - goto placeholders fill the destination URL before structured query values join it - reading a composed string stops at the reading cap before comparing or allocating more - a slug's `&` spelling follows every create the plan accepted and every held entity; direct slug rules and composed strings take it from held entities when the create slugs none, and a slug rule they contradict is not learned Co-Authored-By: Claude Opus 5.5 --- .../references/environments.md | 3 +- docs/browser-scenario-contract.md | 30 +- docs/web-network-captures.md | 6 +- src/browser/BrowserScenarioSecrets.ts | 15 +- src/browser/PlaywrightScenarioActions.ts | 37 ++- src/browser/PlaywrightScenarioRequest.test.ts | 296 +++++++++++++++--- src/browser/PlaywrightScenarioRequest.ts | 90 ++++-- .../EnvironmentStateWriteSimulation.ts | 1 + .../environmentStateDerivedWrites.test.ts | 84 ++++- .../environmentStateWriteComposed.test.ts | 16 + .../environmentStateWriteComposed.ts | 40 ++- .../environmentStateWriteCreate.ts | 20 +- .../environmentStateWriteCreateCalls.ts | 87 ++++- .../environmentStateWriteEffects.ts | 3 + .../environmentStateWriteExplain.ts | 36 ++- src/domain/browserScenario.ts | 11 + 16 files changed, 655 insertions(+), 120 deletions(-) diff --git a/.agents/skills/reverse-engineer-anything/references/environments.md b/.agents/skills/reverse-engineer-anything/references/environments.md index de4f69d7c7..0e61e7113e 100644 --- a/.agents/skills/reverse-engineer-anything/references/environments.md +++ b/.agents/skills/reverse-engineer-anything/references/environments.md @@ -159,7 +159,8 @@ About `request` steps: base64 of `username:password` and redacted), with an optional `prefix` such as `"Bearer "` or `"Basic "`. Each credential part is a literal, secret, saved value, cookie or local storage entry, so a header the frontend builds from a - login token (the username and the token) can be copied too. Copy the + login token (the username and the token) can be copied too; the password is + a secret or saved value, never a literal. Copy the headers the application's own requests send (a CSRF header such as `X-CSRFToken` from the `csrftoken` cookie, a bearer token). Values read from the page are redacted from records made after they are read, not from the login response that returned them, and values diff --git a/docs/browser-scenario-contract.md b/docs/browser-scenario-contract.md index de4a9801c0..f6a1f51d27 100644 --- a/docs/browser-scenario-contract.md +++ b/docs/browser-scenario-contract.md @@ -406,9 +406,10 @@ data you may change. This scenario creates one item: `prefix` such as `"Basic "` or with none, as some APIs take encoded credentials in their own header. Each part is a literal, a declared secret, a saved value, a cookie or a local storage entry, so a header a frontend builds - from a login token can be sent too. The encoded value is redacted like a page value, as - `[REDACTED:page:.
]`, and parts read from the page are - redacted as page values are. + from a login token can be sent too; the password is never a literal other + than empty, since Evidence records literals as written. The encoded value is + redacted however short, as `[REDACTED:page:.
]`, and parts + read from the page are redacted as page values are. - Validation refuses a header name given twice in a step, header names a page's `fetch` cannot set, such as `Cookie`, `Origin`, `Host` or `Sec-*`, and `User-Agent`, which Chrome drops; it refuses line breaks and other ASCII @@ -478,13 +479,19 @@ id the application assigns: - A `request` step saves with `save: [{name, json_pointer, secret}]`, from its own response. A `click` or `press` step saves with `save_response: {method, path, save}`, from the response to the request it triggers, such as the - create a form's Save button sends: the recorder starts waiting before the - step acts, matches the method and a path on the start origin (`{*}` matches - one segment, and the query is ignored), and fails the step unless a 2xx JSON - response arrives before its `timeout_ms`. A pointer that names no string or - number fails the step, so later steps never send a missing value. + create a form's Save button sends: the recorder starts watching before the + step acts and takes only a request the step starts, so one already running, + such as an earlier autosave, cannot answer it. It matches the method and a + path on the start origin as the browser spells it, so a path may name + `My List` or `My%20List` (`{*}` matches one segment, and the query is + ignored), and fails the step unless a 2xx JSON response arrives before its + `timeout_ms`. A pointer that names no string or number fails the step, so + later steps never send a missing value, as does a response over 4,000,000 + characters: a request step stops reading it there, and a UI step does not + read one whose received size is larger. - `{saved:name}` sends a saved value in a request step's `path`, including its - query, and in a `goto` destination's URL, percent-encoded. In a request + query, and in a `goto` destination's `url`, percent-encoded; a destination's + structured `query` values are sent as declared. In a request step's `json` body, a string that is only `{saved:name}` becomes the saved value with the JSON type it was saved with, so a saved number id is sent as a number, and `{saved:name}` inside a longer string is filled as text. A header @@ -492,8 +499,9 @@ id the application assigns: Other braces, such as `{id}`, are sent as written. - Validation refuses a saved value that no earlier step saves. A later save of the same name replaces the value. -- A value saved with `secret: true` is redacted like a page value, as - `[REDACTED:page:.]`, from records made after it is saved. The +- A value saved with `secret: true` is redacted however short, as a declared + secret is, as `[REDACTED:page:.]`, from records made after it + is saved. The recorder reads response bodies on their own schedule, so the response it was saved from may keep it, as a login response may keep a token a later step reads from the page. Other saved values, such as ids, stay visible, since diff --git a/docs/web-network-captures.md b/docs/web-network-captures.md index 3d0bf1bb44..3f9ec75307 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -770,7 +770,8 @@ and a field neither rules holds one value. Members either call sent are accepted required, and an echoed member a call left out takes the value that call stored (`body.defaults`). A clock rule widens to `trimmed` only when no held entity or earlier call showed that clock at a fixed width, and a slug to spelling `&` as -the word (`ampersand: "and"`) only when the other call slugged no `&`. A later +the word (`ampersand: "and"`) only when no call the plan accepted nor entity the +type held dropped one. A later create that disagrees leaves the plan as it was. A field the first create left unexplained, such as an address whose two parts were the same text, takes the `composed` rule a later create learns for it when that rule spells the first @@ -785,7 +786,8 @@ write: a field's value, its slug, or a clock's calendar digits (`YYYYMMDDhhmmss`, `YYYYMMDD`, `YYYY-MM-DD` or `YYYY/MM/DD`, in the clock's zone). A slug drops `&` unless the application spells it as the word, as Go's gosimple/slug and JavaScript's slugify do (`ampersand: "and"`, on a `slug` rule -or part). A create composes a string only when at least one other entity of the +or part). When a create slugs no `&`, the entities the type holds show which, +and a create whose slug spells `&` otherwise than they do has no `slug` rule. A create composes a string only when at least one other entity of the type spells it with the same parts and none spells it otherwise. Where one text could come from two sources serving could tell apart, such as an id that equals the title's slug, the entities the type holds must leave exactly one reading. A diff --git a/src/browser/BrowserScenarioSecrets.ts b/src/browser/BrowserScenarioSecrets.ts index 1700f481f4..3ee11cd699 100644 --- a/src/browser/BrowserScenarioSecrets.ts +++ b/src/browser/BrowserScenarioSecrets.ts @@ -272,10 +272,19 @@ export class BrowserScenarioSecrets { * {@link MIN_REDACTED_PAGE_VALUE_LENGTH} is not redacted. */ admitPageValue(label: string, value: string): void { + this.admit(label, value, MIN_REDACTED_PAGE_VALUE_LENGTH); + } + + /** Redact a credential a step built or a value it saved as secret from everything recorded after it, whatever its length, as a declared secret is. */ + admitCredential(label: string, value: string): void { + this.admit(label, value, 1); + } + + private admit(label: string, value: string, shortest: number): void { // fetch sends a header value without surrounding spaces and tabs, so that is the spelling records hold. const sent = value.replace(/^[\t ]+|[\t ]+$/gu, ""); - if (sent !== value) this.admitPageValue(label, sent); - if (value.length < MIN_REDACTED_PAGE_VALUE_LENGTH) return; + if (sent !== value) this.admit(label, sent, shortest); + if (value.length < shortest) return; for (const held of this.values.values()) if (held === value) return; let id = `page:${label}`; for (let suffix = 2; this.values.has(id); suffix += 1) @@ -330,7 +339,7 @@ export class BrowserScenarioSecrets { secret: boolean, ): void { this.saved.set(name, value); - if (secret) this.admitPageValue(label, String(value)); + if (secret) this.admitCredential(label, String(value)); } /** A saved value with the JSON type it was saved with. */ diff --git a/src/browser/PlaywrightScenarioActions.ts b/src/browser/PlaywrightScenarioActions.ts index 58d6e84c3e..72cf469c6a 100644 --- a/src/browser/PlaywrightScenarioActions.ts +++ b/src/browser/PlaywrightScenarioActions.ts @@ -9,6 +9,7 @@ import { awaitSavedResponse, performScenarioRequest, withSavedPlaceholders, + type SavedResponseWait, } from "./PlaywrightScenarioRequest.js"; type ScenarioLocator = Extract< @@ -50,7 +51,7 @@ const savingResponse = ( input: PerformActionInput, action: Extract, timeout: number | undefined, -): (() => Promise) | undefined => +): SavedResponseWait | undefined => action.save_response === undefined ? undefined : awaitSavedResponse(input.page, { @@ -70,19 +71,27 @@ export const performPlaywrightScenarioAction = async ( const options = timeout === undefined ? {} : { timeout }; switch (action.action) { case "goto": + // Placeholders fill the URL as written, before structured query values join it. await page.goto( - withSavedPlaceholders(secrets.url(action.destination), secrets), + secrets.url({ + ...action.destination, + url: withSavedPlaceholders(action.destination.url, secrets), + }), { ...options, waitUntil: action.wait_until }, ); return; case "click": { - const saved = savingResponse(input, action, timeout); - await resolveLocator(page, action.locator).click({ - ...options, - button: action.button, - clickCount: action.click_count, - }); - await saved?.(); + const saving = savingResponse(input, action, timeout); + try { + await resolveLocator(page, action.locator).click({ + ...options, + button: action.button, + clickCount: action.click_count, + }); + await saving?.saved(); + } finally { + saving?.stop(); + } return; } case "fill": @@ -92,9 +101,13 @@ export const performPlaywrightScenarioAction = async ( ); return; case "press": { - const saved = savingResponse(input, action, timeout); - await resolveLocator(page, action.locator).press(action.key, options); - await saved?.(); + const saving = savingResponse(input, action, timeout); + try { + await resolveLocator(page, action.locator).press(action.key, options); + await saving?.saved(); + } finally { + saving?.stop(); + } return; } case "select_option": diff --git a/src/browser/PlaywrightScenarioRequest.test.ts b/src/browser/PlaywrightScenarioRequest.test.ts index 31c3ba753c..8d95b35283 100644 --- a/src/browser/PlaywrightScenarioRequest.test.ts +++ b/src/browser/PlaywrightScenarioRequest.test.ts @@ -661,6 +661,25 @@ describe("saved value schema", () => { ).toEqual([]); }); + it("refuses a literal Basic password, which Evidence would record as written", () => { + const basic = (password: unknown) => + requestStep({ + headers: [ + { + name: "Authorization", + value: { + source: "basic_credentials", + parts: [{ source: "literal", value: "admin" }, password], + }, + }, + ], + }); + expect( + issues([basic({ source: "literal", value: "hunter2" })]), + ).toContainEqual(expect.stringContaining("not a literal")); + expect(issues([basic({ source: "literal", value: "" })])).toEqual([]); + }); + it("builds Basic credentials from page values and refuses undeclared secret parts", () => { const basic = (username: unknown, password: unknown) => requestStep({ @@ -778,6 +797,69 @@ describe("saved value execution", () => { "folder 42 with [REDACTED:page:create.token]", ); }); +}); + +describe("saved value credentials and limits", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("redacts Basic credentials and secret saves however short", async () => { + stubFetch(200, ['{"pin":"x7"}']); + const parsed = scenario( + [ + requestStep({ + headers: [ + { + name: "X-Auth", + value: { + source: "basic_credentials", + parts: [ + { source: "secret", secret_id: "user" }, + { source: "secret", secret_id: "password" }, + ], + }, + }, + ], + save: [{ name: "pin", json_pointer: "/pin", secret: true }], + }), + ], + [ + { secret_id: "user", environment_variable: "APP_USER" }, + { secret_id: "password", environment_variable: "APP_PASSWORD" }, + ], + ); + const secrets = resolved(parsed, { APP_USER: "a", APP_PASSWORD: "b" }); + await perform(fakePage({}), parsed.actions[0]!, secrets); + const encoded = Buffer.from("a:b").toString("base64"); + const redacted = secrets.redact(`X-Auth: ${encoded} pin x7`); + expect(redacted).not.toContain(encoded); + expect(redacted).not.toContain("x7"); + }); + + it("stops reading a response to save from once it passes the limit", async () => { + const chunk = new Uint8Array(3_000_000).fill(97); + const read = vi + .fn() + .mockResolvedValueOnce({ done: false, value: chunk }) + .mockResolvedValueOnce({ done: false, value: chunk }) + .mockResolvedValueOnce({ done: false, value: chunk }) + .mockResolvedValueOnce({ done: true }); + const cancel = vi.fn(async () => undefined); + vi.stubGlobal("fetch", async () => ({ + status: 200, + type: "basic", + body: { getReader: () => ({ read, cancel, releaseLock: vi.fn() }) }, + })); + const parsed = scenario([ + requestStep({ save: [{ name: "folder", json_pointer: "/id" }] }), + ]); + await expect( + perform(fakePage({}), parsed.actions[0]!, resolved(parsed)), + ).rejects.toThrow("too large to save values from"); + expect(read).toHaveBeenCalledTimes(2); + expect(cancel).toHaveBeenCalledOnce(); + }); it("fails a step whose response holds no value to save", async () => { stubFetch(200, ['{"items":[]}', "not json"]); @@ -836,7 +918,9 @@ describe("saved value execution", () => { for (const value of [encoded, "operator-7", "cookie-key-5d1c"]) expect(redacted).not.toContain(value); }); +}); +describe("goto placeholders", () => { it("fills a goto destination's placeholders, percent-encoded", async () => { const parsed = scenario([ requestStep({ save: [{ name: "item", json_pointer: "/id" }] }), @@ -856,61 +940,126 @@ describe("saved value execution", () => { expect.anything(), ); }); -}); -describe("saving from a UI step's response", () => { - interface FakeResponse { - readonly method: string; - readonly url: string; - readonly status: number; - readonly body: string; - } - - /** A page whose click triggers the given responses; waitForResponse resolves with the first one it names. */ - const uiPage = (responses: readonly FakeResponse[]) => { - const order: string[] = []; - const page = { - getByRole: () => ({ - click: async () => { - order.push("click"); - }, - }), - waitForResponse: async ( - names: (response: unknown) => boolean, - options: { timeout?: number }, - ) => { - order.push(`wait ${String(options.timeout)}`); - await Promise.resolve(); - const found = responses - .map((response) => ({ - request: () => ({ method: () => response.method }), - url: () => response.url, - status: () => response.status, - text: async () => response.body, - })) - .find(names); - if (found === undefined) throw new Error("Timeout exceeded"); - return found; - }, - } as unknown as Page; - return { page, order }; - }; - - const clickSaving = (path: string) => - scenario([ + it("keeps a structured query value that spells a placeholder as written", async () => { + const parsed = scenario([ + requestStep({ save: [{ name: "item", json_pointer: "/id" }] }), { - step_id: "save-item", - action: "click", - timeout_ms: 5000, - locator: { kind: "role", role: "button", name: "Save" }, - save_response: { - method: "POST", - path, - save: [{ name: "item", json_pointer: "/id" }], + step_id: "open", + action: "goto", + destination: { + url: `${ORIGIN}/items/{saved:item}`, + query: [ + { + name: "note", + value: { source: "literal", value: "{saved:item}" }, + }, + ], }, + wait_until: "load", }, ]); + const secrets = resolved(parsed); + secrets.save("create.item", "item", "7", false); + const goto = vi.fn(async () => null); + await perform({ goto } as unknown as Page, parsed.actions[1]!, secrets); + const [address] = goto.mock.calls[0] as unknown as [string]; + const url = new URL(address); + expect(url.pathname).toBe("/items/7"); + expect(url.searchParams.get("note")).toBe("{saved:item}"); + }); +}); +interface FakeResponse { + readonly method: string; + readonly url: string; + readonly status: number; + readonly body: string; + /** A request already running when the step began, such as an earlier autosave. */ + readonly earlier?: boolean; + /** The body size the browser received, when it differs from the body's. */ + readonly size?: number; +} + +/** + * A page whose click starts the given requests, emitting each that did not start earlier to request listeners, and + * answers them in order; waitForResponse resolves with the first answer it names once the click acted. + */ +const uiPage = (responses: readonly FakeResponse[], clickFails = false) => { + const order: string[] = []; + const reads = { count: 0 }; + const listeners = new Set<(request: unknown) => void>(); + let acted: () => void = () => undefined; + const clicked = new Promise((resolve) => { + acted = resolve; + }); + const answers = responses.map((response) => { + const request = { + method: () => response.method, + url: () => response.url, + sizes: async () => ({ + responseBodySize: response.size ?? response.body.length, + }), + }; + return { + response, + request, + answer: { + request: () => request, + url: () => response.url, + status: () => response.status, + text: async () => { + reads.count += 1; + return response.body; + }, + }, + }; + }); + const page = { + on: (_event: string, listener: (request: unknown) => void) => + listeners.add(listener), + off: (_event: string, listener: (request: unknown) => void) => + listeners.delete(listener), + getByRole: () => ({ + click: async () => { + order.push("click"); + if (clickFails) throw new Error("Element is not attached"); + for (const { response, request } of answers) + if (response.earlier !== true) + for (const listener of listeners) listener(request); + acted(); + }, + }), + waitForResponse: async ( + names: (response: unknown) => boolean, + options: { timeout?: number }, + ) => { + order.push(`wait ${String(options.timeout)}`); + await clicked; + const found = answers.map(({ answer }) => answer).find(names); + if (found === undefined) throw new Error("Timeout exceeded"); + return found; + }, + } as unknown as Page; + return { page, order, listeners, reads }; +}; + +const clickSaving = (path: string) => + scenario([ + { + step_id: "save-item", + action: "click", + timeout_ms: 5000, + locator: { kind: "role", role: "button", name: "Save" }, + save_response: { + method: "POST", + path, + save: [{ name: "item", json_pointer: "/id" }], + }, + }, + ]); + +describe("saving from a UI step's response", () => { it("waits before clicking, and saves from the response the step names", async () => { const parsed = clickSaving("/api/lists/{*}/items"); const secrets = resolved(parsed); @@ -945,6 +1094,55 @@ describe("saving from a UI step's response", () => { expect(secrets.savedValue("item")).toBe("item-7"); }); + it("saves from a request the step started, not one already running, and matches an encoded path", async () => { + const parsed = clickSaving("/api/lists/My List/items"); + const secrets = resolved(parsed); + const { page, listeners } = uiPage([ + { + method: "POST", + url: `${ORIGIN}/api/lists/My%20List/items`, + status: 201, + body: '{"id":"autosaved"}', + earlier: true, + }, + { + method: "POST", + url: `${ORIGIN}/api/lists/My%20List/items`, + status: 201, + body: '{"id":"item-8"}', + }, + ]); + await perform(page, parsed.actions[0]!, secrets); + expect(secrets.savedValue("item")).toBe("item-8"); + expect(listeners.size).toBe(0); + }); + + it("refuses a response too large to save from without reading it", async () => { + const parsed = clickSaving("/api/items"); + const { page, reads } = uiPage([ + { + method: "POST", + url: `${ORIGIN}/api/items`, + status: 201, + body: '{"id":"x"}', + size: 5_000_000, + }, + ]); + await expect( + perform(page, parsed.actions[0]!, resolved(parsed)), + ).rejects.toThrow("too large to save values from"); + expect(reads.count).toBe(0); + }); + + it("stops watching requests when the step fails before its response", async () => { + const parsed = clickSaving("/api/items"); + const { page, listeners } = uiPage([], true); + await expect( + perform(page, parsed.actions[0]!, resolved(parsed)), + ).rejects.toThrow("Element is not attached"); + expect(listeners.size).toBe(0); + }); + it("fails the step when no named response arrives or it is not 2xx", async () => { const parsed = clickSaving("/api/items"); await expect( diff --git a/src/browser/PlaywrightScenarioRequest.ts b/src/browser/PlaywrightScenarioRequest.ts index fd1a08e7f3..65ead8a089 100644 --- a/src/browser/PlaywrightScenarioRequest.ts +++ b/src/browser/PlaywrightScenarioRequest.ts @@ -1,4 +1,4 @@ -import type { Page, Route } from "playwright-core"; +import type { Page, Request, Route } from "playwright-core"; import { HEADER_FIELD_VALUE, @@ -162,7 +162,7 @@ const headerValue = async ( const user = await credentialPart(step, username!, `${label}.username`); const pass = await credentialPart(step, password!, `${label}.password`); const encoded = Buffer.from(`${user}:${pass}`).toString("base64"); - secrets.admitPageValue(label, encoded); + secrets.admitCredential(label, encoded); return encoded; } return readPageValue(step, value, label); @@ -249,10 +249,25 @@ const sendFromPage = ( }); if (answer.type === "opaqueredirect") return { redirected: true }; if (request.readLimit !== undefined) { - const text = await answer.text(); - return text.length > request.readLimit - ? { status: answer.status } - : { status: answer.status, body: text }; + // Keep the text only up to the limit, so a large answer is never held whole. + const reader = answer.body?.getReader(); + if (reader === undefined) return { status: answer.status, body: "" }; + const decoder = new TextDecoder(); + let text = ""; + try { + for (;;) { + const chunk = await reader.read(); + if (chunk.done) break; + text += decoder.decode(chunk.value, { stream: true }); + if (text.length > request.readLimit) { + await reader.cancel(); + return { status: answer.status }; + } + } + } finally { + reader.releaseLock(); + } + return { status: answer.status, body: text + decoder.decode() }; } const reader = answer.body?.getReader(); if (reader !== undefined) @@ -399,16 +414,26 @@ export const withSavedPlaceholders = ( ); /** Whether a request a UI step triggered is the one its `save_response` names: method, start origin, and path with `{*}` matching one segment. */ +/** A `save_response` path's segments as a browser spells a URL path, percent-encoded as its URL parser would, `{*}` kept. */ +const pathPattern = (path: string): readonly string[] => + path + .split("?")[0]! + .split("/") + .map((segment) => + segment === "{*}" + ? segment + : new URL(`http://path.invalid/${segment}`).pathname.slice(1), + ); + const namesResponse = ( spec: BrowserScenarioSaveResponse, + pattern: readonly string[], startOrigin: string, - method: string, - address: string, + request: { readonly method: string; readonly url: string }, ): boolean => { - if (method !== spec.method) return false; - const url = URL.parse(address); + if (request.method !== spec.method) return false; + const url = URL.parse(request.url); if (url === null || url.origin !== startOrigin) return false; - const pattern = spec.path.split("?")[0]!.split("/"); const segments = url.pathname.split("/"); return ( segments.length === pattern.length && @@ -416,10 +441,16 @@ const namesResponse = ( ); }; +/** A UI step's wait for the response its `save_response` names: `saved` waits and saves, `stop` stops watching requests. */ +export interface SavedResponseWait { + readonly saved: () => Promise; + readonly stop: () => void; +} + /** - * Start waiting, before a UI step acts, for the response its `save_response` names; the returned call waits for that - * response and saves its values. The step fails unless a 2xx JSON response arrives before its timeout, or the page's - * default timeout when it sets none. + * Start waiting, before a UI step acts, for the response its `save_response` names to a request the step starts; one + * already running when the step began, such as an earlier autosave, cannot answer it. The step fails unless a 2xx JSON + * response arrives before its timeout, or the page's default timeout when it sets none. */ export const awaitSavedResponse = ( page: Page, @@ -430,21 +461,27 @@ export const awaitSavedResponse = ( readonly timeout: number | undefined; readonly secrets: BrowserScenarioSecrets; }, -): (() => Promise) => { +): SavedResponseWait => { const { stepId, spec, startOrigin, timeout, secrets } = step; + const pattern = pathPattern(spec.path); + const started = new Set(); + const watch = (request: Request): void => { + if ( + namesResponse(spec, pattern, startOrigin, { + method: request.method(), + url: request.url(), + }) + ) + started.add(request); + }; + page.on("request", watch); const waiting = page.waitForResponse( - (response) => - namesResponse( - spec, - startOrigin, - response.request().method(), - response.url(), - ), + (response) => started.has(response.request()), timeout === undefined ? {} : { timeout }, ); // The step may fail before it waits; the rejection is reported by the wait, or not at all. waiting.catch(() => undefined); - return async () => { + const saved = async (): Promise => { let response: Awaited; try { response = await waiting; @@ -457,8 +494,15 @@ export const awaitSavedResponse = ( throw new ScenarioRequestError( `Step ${stepId}: the ${spec.method} ${spec.path} response answered ${String(response.status())}; values are saved only from a 2xx response.`, ); + // Playwright reads a body whole, so the size the browser received decides before it is read. + const { responseBodySize } = await response.request().sizes(); + if (responseBodySize > MAX_SAVED_RESPONSE_CHARACTERS) + throw new ScenarioRequestError( + `Step ${stepId}: the response is too large to save values from.`, + ); saveValues(stepId, await response.text(), spec.save, secrets); }; + return { saved, stop: () => page.off("request", watch) }; }; /** diff --git a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts index 2b524a14fe..e8043622b1 100644 --- a/src/domain/analysisView/EnvironmentStateWriteSimulation.ts +++ b/src/domain/analysisView/EnvironmentStateWriteSimulation.ts @@ -221,6 +221,7 @@ export class EnvironmentStateWriteSimulation { unstamped: new Set(), kept: new Set(), fixedWidth: new Map(), + ampersands: new Map(), linked: link.collection, }); } diff --git a/src/domain/analysisView/environmentStateDerivedWrites.test.ts b/src/domain/analysisView/environmentStateDerivedWrites.test.ts index ece947e36b..d8c9a7a2b6 100644 --- a/src/domain/analysisView/environmentStateDerivedWrites.test.ts +++ b/src/domain/analysisView/environmentStateDerivedWrites.test.ts @@ -559,7 +559,7 @@ const addressParts = (ampersand?: "and") => [ }, ]; -describe("slugs, ambiguous addresses and templates across creates", () => { +describe("slug spellings across creates", () => { it("learns that a slug spells & as the word from the first create that sends one", () => { const rows = folderCreates( [ @@ -605,6 +605,88 @@ describe("slugs, ambiguous addresses and templates across creates", () => { expect(planOf(mixed, "post_folders")).toMatchObject({ served: false }); }); + it("keeps a dropped & that an accepted create showed when a later one spells it as the word", () => { + const tag = (id: number, name: string, slug: string) => ({ + id, + name, + slug, + }); + const seed = tag(1, "Lamp Oil", "lamp-oil"); + const created = [ + tag(2, "Alpha Notes", "alpha-notes"), + tag(3, "Bravo & Tide", "bravo-tide"), + tag(4, "Charlie & Wind", "charlie-and-wind"), + ]; + const rows: TimedCaptureRow[] = [ + ["GET", "/api/tags", [seed]], + ["GET", "/api/tags/1", seed], + ]; + created.forEach((entity, index) => + rows.push( + ["POST", "/api/tags", entity, 201, { name: entity.name }], + ["GET", `/api/tags/${String(entity.id)}`, entity], + ["GET", "/api/tags", [seed, ...created.slice(0, index + 1)]], + ), + ); + const plan = planOf(rows, "post_tags"); + expect(plan.rules).toContainEqual({ + field: "slug", + rule: "slug", + from: "name", + }); + expect(plan.served).toBe(false); + }); + + it("spells & as the held entities do when the creates slug none", () => { + const rows = folderCreates( + [ + { uid: numberedUuid(102), title: "Alpha Harbour", sent: false }, + { uid: numberedUuid(103), title: "Bravo Tide", sent: false }, + ], + wordSlug, + { uid: numberedUuid(101), title: "Lamp & Oil" }, + ); + expect(planOf(rows, "post_folders")).toMatchObject({ + served: true, + rules: expect.arrayContaining([ + { field: "url", rule: "composed", parts: addressParts("and") }, + ]), + }); + }); + + it("learns a slug field's & spelling from held tags, and no slug rule they contradict", () => { + const tagRows = (held: string, created: string, slug: string) => { + const seed = { id: 1, name: held, slug: wordSlug(held) }; + const tag = { id: 2, name: created, slug }; + return [ + ["GET", "/api/tags", [seed]], + ["GET", "/api/tags/1", seed], + ["POST", "/api/tags", tag, 201, { name: created }], + ["GET", "/api/tags/2", tag], + ["GET", "/api/tags", [seed, tag]], + ] as TimedCaptureRow[]; + }; + expect( + planOf( + tagRows("Salt & Pepper", "Harbour Notes", "harbour-notes"), + "post_tags", + ).rules, + ).toContainEqual({ + field: "slug", + rule: "slug", + from: "name", + ampersand: "and", + }); + expect( + planOf(tagRows("Salt & Pepper", "Tide & Wind", "tide-wind"), "post_tags") + .rules, + ).not.toContainEqual( + expect.objectContaining({ field: "slug", rule: "slug" }), + ); + }); +}); + +describe("ambiguous addresses and templates across creates", () => { it("learns an address the first create cannot tell apart, its uid being its title's slug, from the second", () => { const rows = folderCreates( [ diff --git a/src/domain/analysisView/environmentStateWriteComposed.test.ts b/src/domain/analysisView/environmentStateWriteComposed.test.ts index 258e3d0b58..c34caebb1a 100644 --- a/src/domain/analysisView/environmentStateWriteComposed.test.ts +++ b/src/domain/analysisView/environmentStateWriteComposed.test.ts @@ -357,4 +357,20 @@ describe("slug spellings", () => { ])("url"), ).toBeUndefined(); }); + + it("reads no address that more sources spell than it can tell apart", () => { + const fields = Object.fromEntries( + Array.from({ length: 40 }, (_, index) => [ + `alias${String(index)}`, + "harbour-notes", + ]), + ); + const rules: FieldRule[] = Object.keys(fields).map((field) => ({ + field, + rule: "echo", + })); + const created = { ...fields, url: "/notes/harbour-notes/harbour-notes" }; + const held = [{ ...fields, url: "/notes/harbour-notes/harbour-notes" }]; + expect(createdComposer(created, rules, held)("url")).toBeUndefined(); + }); }); diff --git a/src/domain/analysisView/environmentStateWriteComposed.ts b/src/domain/analysisView/environmentStateWriteComposed.ts index 1c3c319ad1..4678af1b9b 100644 --- a/src/domain/analysisView/environmentStateWriteComposed.ts +++ b/src/domain/analysisView/environmentStateWriteComposed.ts @@ -125,6 +125,23 @@ const matchesAt = ( return []; }; +/** Parts with each slug of a source holding no `&` spelling it as the word, or undefined when no such slug part exists. */ +const ampersandWord = ( + parts: readonly ComposedPart[], + entity: JsonObject, +): ComposedPart[] | undefined => { + let changed = false; + const spelled = parts.map((part): ComposedPart => { + if (!("as" in part) || part.as !== "slug" || part.ampersand !== undefined) + return part; + const source = entity[part.from]; + if (typeof source === "string" && source.includes("&")) return part; + changed = true; + return { ...part, ampersand: "and" }; + }); + return changed ? spelled : undefined; +}; + /** More ways to read one string than this leave it unexplained. */ const MAX_SEGMENTATIONS = 8; @@ -148,14 +165,17 @@ const segmentations = ( at += 1; continue; } + // Stop at the cap before comparing or allocating more, however many sources spell the text. const choices: SourcePart[] = []; for (const { part } of matches) - if (!choices.some((choice) => alike(choice, part))) choices.push(part); + if (!choices.some((choice) => alike(choice, part))) { + choices.push(part); + if (readings.length * choices.length > MAX_SEGMENTATIONS) return []; + } const lead: ComposedPart[] = literal === "" ? [] : [{ text: literal }]; readings = readings.flatMap((parts) => choices.map((choice) => [...parts, ...lead, choice]), ); - if (readings.length > MAX_SEGMENTATIONS) return []; literal = ""; shown = true; at += matches[0]!.token.length; @@ -241,11 +261,17 @@ export const createdComposer = ( : [], ), ); - const fits = segmentations(text, index, alike).filter( - (parts) => - !parts.some((part) => "from" in part && part.from === field) && - (heldSpellings(parts, field, held) ?? 0) > 0, - ); + const fits = segmentations(text, index, alike).flatMap((parts) => { + if (parts.some((part) => "from" in part && part.from === field)) + return []; + // A slug whose source here holds no `&` spells it as the held entities do. + const spelled = [parts, ampersandWord(parts, entity)].find( + (candidate) => + candidate !== undefined && + (heldSpellings(candidate, field, held) ?? 0) > 0, + ); + return spelled === undefined ? [] : [spelled]; + }); return fits.length === 1 ? { field, rule: "composed", parts: fits[0]! } : undefined; diff --git a/src/domain/analysisView/environmentStateWriteCreate.ts b/src/domain/analysisView/environmentStateWriteCreate.ts index e3c1e7e9ec..66e0dc51d3 100644 --- a/src/domain/analysisView/environmentStateWriteCreate.ts +++ b/src/domain/analysisView/environmentStateWriteCreate.ts @@ -77,7 +77,11 @@ import { fittedClockFormat, noteFixedWidths, } from "./environmentStateWriteClockFormats.js"; -import { mergedCreatePlan } from "./environmentStateWriteCreateCalls.js"; +import { + mergedCreatePlan, + noteAmpersands, + noteHeldAmpersands, +} from "./environmentStateWriteCreateCalls.js"; import { containsDelimited, generatedIntegerDigits, @@ -276,16 +280,24 @@ const refineCreate = ( { rules: draft.plan.rules, body: draft.plan.body! }, draft.observed!, later, - { kept: draft.kept, clockOf: (field) => clockEvidence(draft, field) }, + earlierOf(draft), ) ?? reexplained(draft, explained.rules, later); if (widened === undefined) return false; retainFixed(context, widened.body.fixed.slice(draft.plan.body!.fixed.length)); draft.plan.rules = widened.rules; draft.plan.body = widened.body; draft.exemplarInput = undefined; + noteAmpersands(draft.ampersands, explained.rules, evidence.observed.object); return true; }; +/** What the plan's earlier calls showed, for merging a later one. */ +const earlierOf = (draft: WriteDraft) => ({ + kept: draft.kept, + clockOf: (field: string) => clockEvidence(draft, field), + ampersands: draft.ampersands, +}); + /** A later create as the plan merge reads it. */ const laterOf = ( evidence: CreateCallEvidence, @@ -331,7 +343,7 @@ const reexplained = ( }, draft.observed!, later, - { kept: draft.kept, clockOf: (field) => clockEvidence(draft, field) }, + earlierOf(draft), ); }; @@ -441,6 +453,8 @@ const buildCreate = ( }); draft.observed = evidence.observed.object; draft.exemplarInput = input; + noteHeldAmpersands(draft.ampersands, explained.rules, held); + noteAmpersands(draft.ampersands, explained.rules, evidence.observed.object); for (const entity of held) noteFixedWidths(draft.fixedWidth, entity); return true; }; diff --git a/src/domain/analysisView/environmentStateWriteCreateCalls.ts b/src/domain/analysisView/environmentStateWriteCreateCalls.ts index 30c561e021..e7f63facf1 100644 --- a/src/domain/analysisView/environmentStateWriteCreateCalls.ts +++ b/src/domain/analysisView/environmentStateWriteCreateCalls.ts @@ -2,11 +2,13 @@ import type { JsonValue } from "../jsonValue.js"; import { same, type JsonObject } from "./environmentStateRules.js"; import { storedKinds } from "./environmentStateWriteApply.js"; import { fittedClockFormat } from "./environmentStateWriteClockFormats.js"; +import { composedValue } from "./environmentStateWriteComposed.js"; import { alikeRules, type FieldRule, type WriteBody, } from "./environmentStateWriteRules.js"; +import { slugOf, type Ampersand } from "./environmentStateWriteValues.js"; type Default = { readonly member: string; readonly value: JsonValue }; @@ -14,8 +16,13 @@ type Default = { readonly member: string; readonly value: JsonValue }; export interface EarlierEvidence { readonly kept: ReadonlySet; readonly clockOf: (field: string) => JsonValue | undefined; + /** How the plan's accepted calls and the type's held entities spelled `&` in each field's slug; absent where none showed one. */ + readonly ampersands?: ReadonlyMap; } +/** How a field's slug showed `&`: dropped, or as the word. */ +export type AmpersandSpelling = "dropped" | Ampersand; + /** The plan's rule for a field and a later call's agree: equal, or clocks one spelling covers given what earlier calls showed. */ const agreeing = ( held: FieldRule, @@ -31,8 +38,9 @@ const agreeing = ( const alike = alikeRules(held, later, values.earlier.kept); if (alike !== undefined) return alike; const spelled = ampersandSpelled( - { rule: held, entity: values.exemplar }, + held, { rule: later, entity: values.laterEntity }, + values.earlier.ampersands?.get(values.field), ); if (spelled !== undefined) return spelled; if (held.rule !== "clock" || later.rule !== "clock") return undefined; @@ -75,17 +83,82 @@ const showsAmpersand = (rule: FieldRule, entity: JsonObject): boolean => return typeof value === "string" && value.includes("&"); }); -/** Two calls' rules that differ only in how a slug spells `&`: the one that spells it as the word, when the other call slugged no `&`. */ +/** + * Two calls' rules that differ only in how a slug spells `&`: the one that spells it as the word, when the call + * spelling it otherwise slugged no `&`, and no call the plan accepted nor entity the type held dropped one (`shown`). + */ const ampersandSpelled = ( - held: { readonly rule: FieldRule; readonly entity: JsonObject }, + held: FieldRule, later: { readonly rule: FieldRule; readonly entity: JsonObject }, + shown: AmpersandSpelling | undefined, ): FieldRule | undefined => { - const plainHeld = unspelled(held.rule); + const plainHeld = unspelled(held); if (!same(plainHeld, unspelled(later.rule))) return undefined; - const heldSpells = !same(held.rule, plainHeld); + const heldSpells = !same(held, plainHeld); if (heldSpells === !same(later.rule, unspelled(later.rule))) return undefined; - const [spelling, plain] = heldSpells ? [held, later] : [later, held]; - return showsAmpersand(plain.rule, plain.entity) ? undefined : spelling.rule; + if (heldSpells) + return showsAmpersand(later.rule, later.entity) ? undefined : held; + return shown === "dropped" ? undefined : later.rule; +}; + +/** How a rule's slug spells `&`, for a rule that slugs a field. */ +const slugSpelling = (rule: FieldRule): AmpersandSpelling | undefined => { + if (rule.rule === "slug") return rule.ampersand ?? "dropped"; + if (rule.rule !== "composed") return undefined; + for (const part of rule.parts) + if ("as" in part && part.as === "slug") return part.ampersand ?? "dropped"; + return undefined; +}; + +/** Note how a call that slugged `&` spelled it in each field its own rules slug. */ +export const noteAmpersands = ( + ampersands: Map, + rules: readonly FieldRule[], + entity: JsonObject, +): void => { + for (const rule of rules) { + const spelling = slugSpelling(rule); + if (spelling !== undefined && showsAmpersand(rule, entity)) + ampersands.set(rule.field, spelling); + } +}; + +/** Note how held entities whose slugged source holds `&` spelled each field the plan's rules slug. */ +export const noteHeldAmpersands = ( + ampersands: Map, + rules: readonly FieldRule[], + held: readonly JsonObject[], +): void => { + for (const rule of rules) { + if (slugSpelling(rule) === undefined) continue; + const plain = unspelled(rule); + for (const entity of held) { + if (!showsAmpersand(rule, entity)) continue; + const spelled = (spelling: Ampersand | undefined): string | undefined => + rule.rule === "slug" + ? typeof entity[rule.from] === "string" + ? slugOf(entity[rule.from] as string, spelling) + : undefined + : plain.rule === "composed" + ? composedValue( + plain.parts.map((part) => + "as" in part && part.as === "slug" && spelling !== undefined + ? { ...part, ampersand: spelling } + : part, + ), + entity, + ) + : undefined; + // A field that spells neither way shows nothing of how its slug spells `&`; dropping it wins over the word. + if (spelled(undefined) === entity[rule.field]) + ampersands.set(rule.field, "dropped"); + else if ( + spelled("and") === entity[rule.field] && + ampersands.get(rule.field) !== "dropped" + ) + ampersands.set(rule.field, "and"); + } + } }; /** Defaults joined, undefined when two calls showed one member left out at different values. */ diff --git a/src/domain/analysisView/environmentStateWriteEffects.ts b/src/domain/analysisView/environmentStateWriteEffects.ts index a469204952..8de2d19eb1 100644 --- a/src/domain/analysisView/environmentStateWriteEffects.ts +++ b/src/domain/analysisView/environmentStateWriteEffects.ts @@ -50,6 +50,7 @@ import { import { plannedBody, restampedFields } from "./environmentStateWriteStamps.js"; import { readClock } from "./environmentStateWriteValues.js"; import type { CreateEvidence } from "./environmentStateWriteExplain.js"; +import type { AmpersandSpelling } from "./environmentStateWriteCreateCalls.js"; import { fittedClockFormat, matchesClockFormat, @@ -99,6 +100,8 @@ export interface WriteDraft { readonly fixedWidth: Map; /** What explained a create's exemplar, kept until a later call widens the plan, which it may then explain again. */ exemplarInput?: CreateEvidence | undefined; + /** How the calls a create plan accepted and the entities its type held spelled `&` in each slugged field. */ + readonly ampersands: Map; } /** The value a draft fits a clock field against: one showing it at a fixed width, else the exemplar's. */ diff --git a/src/domain/analysisView/environmentStateWriteExplain.ts b/src/domain/analysisView/environmentStateWriteExplain.ts index ca10740b7d..aea344373e 100644 --- a/src/domain/analysisView/environmentStateWriteExplain.ts +++ b/src/domain/analysisView/environmentStateWriteExplain.ts @@ -25,6 +25,7 @@ import { isUuid, readClock, sequenceNumber, + slugOf, slugSpellings, sortedStrings, type ClockFormat, @@ -304,6 +305,35 @@ const bodyOf = ( }); /** Explain every field of a created entity: echoed, copied, path, slug, generated, template or constant (unlisted). */ +/** + * A slug rule as the held entities spell `&`: as the word when the call slugged none and they spell it so, and no rule + * when they spell it otherwise than the call did. Other rules pass unchanged. + */ +const heldSlugRule = ( + rule: FieldRule | undefined, + sent: JsonObject, + held: readonly JsonObject[], +): FieldRule | undefined => { + if (rule?.rule !== "slug") return rule; + const shown = new Set( + held.flatMap((entity) => { + const from = entity[rule.from]; + const value = entity[rule.field]; + if (typeof from !== "string" || !from.includes("&")) return []; + if (value === slugOf(from)) return ["dropped"]; + return value === slugOf(from, "and") ? ["and"] : []; + }), + ); + const source = sent[rule.from]; + if (typeof source === "string" && source.includes("&")) + return shown.has(rule.ampersand === "and" ? "dropped" : "and") + ? undefined + : rule; + return shown.has("and") && !shown.has("dropped") + ? { ...rule, ampersand: "and" } + : rule; +}; + /** Whether a composed string spells a field besides the one a template rebuilds it around, such as a title's slug beside a generated id. */ const spellsMore = (composed: FieldRule, template: FieldRule): boolean => composed.rule === "composed" && @@ -322,7 +352,11 @@ export const explainCreate = ( const index = indexBody(evidence.call.body); for (const [field, value] of Object.entries(evidence.entity)) { const rule = - requestRule(evidence, index, field, value) ?? + heldSlugRule( + requestRule(evidence, index, field, value), + evidence.call.body, + evidence.type, + ) ?? generatedRule(evidence, field, value) ?? (evidence.named === undefined ? undefined diff --git a/src/domain/browserScenario.ts b/src/domain/browserScenario.ts index f4fd1867ef..b5ec7a4825 100644 --- a/src/domain/browserScenario.ts +++ b/src/domain/browserScenario.ts @@ -193,6 +193,17 @@ const validateRequestSteps = ( ["actions", index, "headers", position, "value", "value"], "A header value cannot carry line breaks or control characters", ); + // Evidence records literals as written, so a password must come from a reference. + const password = + header.value.source === "basic_credentials" + ? header.value.parts[1] + : undefined; + if (password?.source === "literal" && password.value !== "") + addIssue( + context, + ["actions", index, "headers", position, "value", "parts", 1], + "Basic credentials take their password from a declared secret, a saved value, a cookie or local storage, not a literal", + ); }); }); };