From e42362c92255afe6df50efb192af2584be38e603 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Mon, 26 Oct 2026 07:46:12 -0700 Subject: [PATCH] fix(browser): keep saved ids exact, encoded text and unknown slugs as written - a saved integer past 2^53, which JSON parsing would round to another id, fails the step that saves it - request paths and JSON strings read only `{saved:name}` as written; the percent-encoded spelling is read only in a goto URL, whose parsed form spells braces that way - a held slug that neither `&` spelling gives counts as unknown: a create whose slug spells `&` learns no slug rule beside it, and no later create widens the spelling - `save_response.path` takes the request path's portable pattern, shared as START_ORIGIN_PATH, so JSON Schema clients compiling it with the `v` flag read it Co-Authored-By: Claude Opus 5.5 --- docs/browser-scenario-contract.md | 7 +- docs/web-network-captures.md | 4 +- src/browser/PlaywrightScenarioActions.ts | 13 ++- src/browser/PlaywrightScenarioRequest.test.ts | 97 +++++++++++++------ src/browser/PlaywrightScenarioRequest.ts | 10 +- .../environmentStateDerivedWrites.test.ts | 22 +++++ .../environmentStateWriteCreateCalls.ts | 16 +-- .../environmentStateWriteExplain.ts | 11 ++- src/domain/browserScenario.ts | 7 +- src/domain/browserScenarioValues.ts | 26 +++-- 10 files changed, 149 insertions(+), 64 deletions(-) diff --git a/docs/browser-scenario-contract.md b/docs/browser-scenario-contract.md index f6a1f51d27..a48e744ecb 100644 --- a/docs/browser-scenario-contract.md +++ b/docs/browser-scenario-contract.md @@ -496,9 +496,12 @@ id the application assigns: 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 value or Basic credentials part sends one as `{"source": "saved", "name"}`. - Other braces, such as `{id}`, are sent as written. + Other braces, such as `{id}`, are sent as written, and so is a percent-encoded + `%7Bsaved:name%7D` anywhere but a `goto` URL, whose parsed form spells the + braces that way. - Validation refuses a saved value that no earlier step saves. A later save of - the same name replaces the value. + the same name replaces the value. An integer past 2^53, which JSON parsing + would round to another number, fails the step that saves it. - 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 diff --git a/docs/web-network-captures.md b/docs/web-network-captures.md index 98b745e1dc..4ee640342a 100644 --- a/docs/web-network-captures.md +++ b/docs/web-network-captures.md @@ -788,7 +788,9 @@ write: a field's value, its slug, or a clock's calendar digits 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). 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 +and a create whose slug spells `&` otherwise than they do, or alongside one whose +slug neither spelling gives, has no `slug` rule; such a held slug also keeps a +later create from widening the spelling. 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/PlaywrightScenarioActions.ts b/src/browser/PlaywrightScenarioActions.ts index 72cf469c6a..a406726520 100644 --- a/src/browser/PlaywrightScenarioActions.ts +++ b/src/browser/PlaywrightScenarioActions.ts @@ -1,8 +1,9 @@ import type { Locator, Page } from "playwright-core"; -import type { - BrowserScenarioAction, - BrowserScenarioValue, +import { + SAVED_URL_PLACEHOLDER, + type BrowserScenarioAction, + type BrowserScenarioValue, } from "../domain/browserScenarioValues.js"; import type { BrowserScenarioSecrets } from "./BrowserScenarioSecrets.js"; import { @@ -75,7 +76,11 @@ export const performPlaywrightScenarioAction = async ( await page.goto( secrets.url({ ...action.destination, - url: withSavedPlaceholders(action.destination.url, secrets), + url: withSavedPlaceholders( + action.destination.url, + secrets, + SAVED_URL_PLACEHOLDER, + ), }), { ...options, waitUntil: action.wait_until }, ); diff --git a/src/browser/PlaywrightScenarioRequest.test.ts b/src/browser/PlaywrightScenarioRequest.test.ts index 8d95b35283..1afa726fc4 100644 --- a/src/browser/PlaywrightScenarioRequest.test.ts +++ b/src/browser/PlaywrightScenarioRequest.test.ts @@ -590,30 +590,30 @@ describe("page value redaction", () => { }); }); -describe("saved value schema", () => { - const saved = (name: string) => ({ source: "saved", name }); - const saveId = (stepId: string, name: string) => - requestStep({ - step_id: stepId, - save: [{ name, json_pointer: "/id" }], - }); - const clickSaving = (name: string) => ({ - step_id: "save-item", - action: "click", - locator: { kind: "role", role: "button", name: "Save" }, - save_response: { - method: "POST", - path: "/api/items", - save: [{ name, json_pointer: "/id" }], - }, - }); - const gotoItem = (url: string) => ({ - step_id: "open", - action: "goto", - destination: { url }, - wait_until: "load", +const savedValue = (name: string) => ({ source: "saved", name }); +const saveIdStep = (stepId: string, name: string) => + requestStep({ + step_id: stepId, + save: [{ name, json_pointer: "/id" }], }); +const clickSavingItem = (name: string) => ({ + step_id: "save-item", + action: "click", + locator: { kind: "role", role: "button", name: "Save" }, + save_response: { + method: "POST", + path: "/api/items", + save: [{ name, json_pointer: "/id" }], + }, +}); +const gotoStep = (url: string) => ({ + step_id: "open", + action: "goto", + destination: { url }, + wait_until: "load", +}); +describe("saved value schema", () => { it("refuses a saved value no earlier step saves, wherever a step sends it", () => { const sendsItem = requestStep({ step_id: "edit", @@ -623,25 +623,25 @@ describe("saved value schema", () => { expect(issues([sendsItem])).toEqual([ "Step edit sends saved value item, which no earlier step saves", ]); - expect(issues([sendsItem, saveId("create", "item")])).toContainEqual( + expect(issues([sendsItem, saveIdStep("create", "item")])).toContainEqual( expect.stringContaining("which no earlier step saves"), ); - expect(issues([saveId("create", "item"), sendsItem])).toEqual([]); - expect(issues([clickSaving("item"), sendsItem])).toEqual([]); + expect(issues([saveIdStep("create", "item"), sendsItem])).toEqual([]); + expect(issues([clickSavingItem("item"), sendsItem])).toEqual([]); for (const step of [ - gotoItem(`${ORIGIN}/items/{saved:item}`), - gotoItem(`${ORIGIN}/items/%7Bsaved%3Aitem%7D`), + gotoStep(`${ORIGIN}/items/{saved:item}`), + gotoStep(`${ORIGIN}/items/%7Bsaved%3Aitem%7D`), requestStep({ path: "/api/items?parent={saved:item}" }), requestStep({ json: { list: [{ parent: "{saved:item}" }] } }), requestStep({ json: { note: "Copied from {saved:item}" } }), - requestStep({ headers: [{ name: "X-Item", value: saved("item") }] }), + requestStep({ headers: [{ name: "X-Item", value: savedValue("item") }] }), requestStep({ headers: [ { name: "Authorization", value: { source: "basic_credentials", - parts: [saved("item"), { source: "literal", value: "x" }], + parts: [savedValue("item"), { source: "literal", value: "x" }], }, }, ], @@ -652,15 +652,23 @@ describe("saved value schema", () => { ); }); + it("reads only `{saved:name}` as written in a JSON string, not its encoded spelling", () => { + expect( + issues([requestStep({ json: { note: "%7Bsaved:item%7D" } })]), + ).toEqual([]); + }); + it("sends other braces as written", () => { expect( issues([ - gotoItem(`${ORIGIN}/search/{q}`), + gotoStep(`${ORIGIN}/search/{q}`), requestStep({ path: "/api/items/{id}", json: { name: "{draft}" } }), ]), ).toEqual([]); }); +}); +describe("saved value schema: credentials and response paths", () => { it("refuses a literal Basic password, which Evidence would record as written", () => { const basic = (password: unknown) => requestStep({ @@ -724,7 +732,7 @@ describe("saved value schema", () => { it("requires a save_response to name a same-origin path and something to save", () => { const click = (saveResponse: unknown) => ({ - ...clickSaving("item"), + ...clickSavingItem("item"), save_response: saveResponse, }); for (const path of ["//evil.test/api", "api/items", "https://evil.test/"]) @@ -837,6 +845,33 @@ describe("saved value credentials and limits", () => { expect(redacted).not.toContain("x7"); }); + it("saves no integer JSON parsing would round, so no later step names another object", async () => { + stubFetch(201, ['{"id":9007199254740993}']); + const parsed = scenario([ + requestStep({ save: [{ name: "folder", json_pointer: "/id" }] }), + ]); + const secrets = resolved(parsed); + await expect( + perform(fakePage({}), parsed.actions[0]!, secrets), + ).rejects.toThrow("too large to keep exactly"); + expect(() => secrets.savedValue("folder")).toThrow(); + }); + + it("sends a JSON string that spells a placeholder percent-encoded as written", async () => { + const calls = stubFetch(200, ['{"id":7}']); + const parsed = scenario([ + requestStep({ save: [{ name: "item", json_pointer: "/id" }] }), + requestStep({ + step_id: "note", + json: { note: "%7Bsaved:item%7D", id: "{saved:item}" }, + }), + ]); + const secrets = resolved(parsed); + for (const action of parsed.actions) + await perform(fakePage({}), action, secrets); + expect(calls[1]!.init.body).toBe('{"note":"%7Bsaved:item%7D","id":7}'); + }); + 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 diff --git a/src/browser/PlaywrightScenarioRequest.ts b/src/browser/PlaywrightScenarioRequest.ts index 65ead8a089..572fa110c4 100644 --- a/src/browser/PlaywrightScenarioRequest.ts +++ b/src/browser/PlaywrightScenarioRequest.ts @@ -404,12 +404,13 @@ export const performScenarioRequest = async (input: { saveValues(action.step_id, response.body, action.save, secrets); }; -/** A path or URL with its `{saved:name}` placeholders filled, each value percent-encoded. */ +/** A path or URL with its `placeholder`s filled, each value percent-encoded; a request path's are spelled as written. */ export const withSavedPlaceholders = ( text: string, secrets: BrowserScenarioSecrets, + placeholder: RegExp = SAVED_PLACEHOLDER, ): string => - text.replace(SAVED_PLACEHOLDER, (_placeholder, name: string) => + text.replace(placeholder, (_placeholder, name: string) => encodeURIComponent(String(secrets.savedValue(name))), ); @@ -558,6 +559,11 @@ const saveValues = ( throw new ScenarioRequestError( `Step ${stepId}: the response has no string or number at ${json_pointer} to save as ${name}.`, ); + // JSON parsing rounds an integer past 2^53, so a later step would name another object. + if (Number.isInteger(value) && !Number.isSafeInteger(value)) + throw new ScenarioRequestError( + `Step ${stepId}: the number at ${json_pointer} is too large to keep exactly, so ${name} cannot be saved from it.`, + ); secrets.save(`${stepId}.${name}`, name, value, secret); } }; diff --git a/src/domain/analysisView/environmentStateDerivedWrites.test.ts b/src/domain/analysisView/environmentStateDerivedWrites.test.ts index d8c9a7a2b6..7d9c95be80 100644 --- a/src/domain/analysisView/environmentStateDerivedWrites.test.ts +++ b/src/domain/analysisView/environmentStateDerivedWrites.test.ts @@ -684,6 +684,28 @@ describe("slug spellings across creates", () => { expect.objectContaining({ field: "slug", rule: "slug" }), ); }); + + it("learns no & spelling from a held slug neither spelling gives", () => { + const rows = (created: string, slug: string): TimedCaptureRow[] => { + const seed = { id: 1, name: "Salt & Pepper", slug: "condiments" }; + 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]], + ]; + }; + expect( + planOf(rows("Tide & Wind", "tide-and-wind"), "post_tags").rules, + ).not.toContainEqual( + expect.objectContaining({ field: "slug", rule: "slug" }), + ); + expect( + planOf(rows("Harbour Notes", "harbour-notes"), "post_tags").rules, + ).toContainEqual({ field: "slug", rule: "slug", from: "name" }); + }); }); describe("ambiguous addresses and templates across creates", () => { diff --git a/src/domain/analysisView/environmentStateWriteCreateCalls.ts b/src/domain/analysisView/environmentStateWriteCreateCalls.ts index cc009fecd5..36ca4d204f 100644 --- a/src/domain/analysisView/environmentStateWriteCreateCalls.ts +++ b/src/domain/analysisView/environmentStateWriteCreateCalls.ts @@ -20,8 +20,8 @@ export interface EarlierEvidence { readonly ampersands?: ReadonlyMap; } -/** How a field's slug showed `&`: dropped, or as the word. */ -export type AmpersandSpelling = "dropped" | Ampersand; +/** How a field's slug showed `&`: dropped, as the word, or in a way neither spelling gives. */ +export type AmpersandSpelling = "dropped" | Ampersand | "unknown"; /** 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 = ( @@ -98,7 +98,7 @@ const ampersandSpelled = ( if (heldSpells === !same(later.rule, unspelled(later.rule))) return undefined; if (heldSpells) return showsAmpersand(later.rule, later.entity) ? undefined : held; - return shown === "dropped" ? undefined : later.rule; + return shown === "dropped" || shown === "unknown" ? undefined : later.rule; }; /** How a rule's slug spells `&`, for a rule that slugs a field. */ @@ -152,12 +152,12 @@ export const noteHeldAmpersands = ( entity, ) : undefined; - // A field that spells neither way shows nothing of how its slug spells `&`; dropping it wins over the word. + // Dropping `&` outranks a spelling neither way gives, which outranks the word. + const known = ampersands.get(rule.field); if (spelled(undefined) === stored) ampersands.set(rule.field, "dropped"); - else if ( - spelled("and") === stored && - ampersands.get(rule.field) !== "dropped" - ) + else if (stored !== undefined && spelled("and") !== stored) { + if (known !== "dropped") ampersands.set(rule.field, "unknown"); + } else if (stored !== undefined && known === undefined) ampersands.set(rule.field, "and"); } } diff --git a/src/domain/analysisView/environmentStateWriteExplain.ts b/src/domain/analysisView/environmentStateWriteExplain.ts index 9a4a237ae2..c57a1dafbd 100644 --- a/src/domain/analysisView/environmentStateWriteExplain.ts +++ b/src/domain/analysisView/environmentStateWriteExplain.ts @@ -310,7 +310,7 @@ 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. + * when they spell it otherwise than the call did or in a way neither spelling gives. Other rules pass unchanged. */ const heldSlugRule = ( rule: FieldRule | undefined, @@ -324,15 +324,18 @@ const heldSlugRule = ( const value = ownMember(entity, rule.field); if (typeof from !== "string" || !from.includes("&")) return []; if (value === slugOf(from)) return ["dropped"]; - return value === slugOf(from, "and") ? ["and"] : []; + if (value === slugOf(from, "and")) return ["and"]; + // A slug neither spelling gives leaves how the application spells `&` unknown. + return value === undefined ? [] : ["unknown"]; }), ); const source = ownMember(sent, rule.from); if (typeof source === "string" && source.includes("&")) - return shown.has(rule.ampersand === "and" ? "dropped" : "and") + return shown.has("unknown") || + shown.has(rule.ampersand === "and" ? "dropped" : "and") ? undefined : rule; - return shown.has("and") && !shown.has("dropped") + return shown.has("and") && !shown.has("dropped") && !shown.has("unknown") ? { ...rule, ampersand: "and" } : rule; }; diff --git a/src/domain/browserScenario.ts b/src/domain/browserScenario.ts index b5ec7a4825..42fe661272 100644 --- a/src/domain/browserScenario.ts +++ b/src/domain/browserScenario.ts @@ -14,6 +14,7 @@ import { type BrowserScenarioAction, type BrowserScenarioUrl, type BrowserScenarioValue, + SAVED_URL_PLACEHOLDER, savedPlaceholderNames, } from "./browserScenarioValues.js"; import type { JsonValue } from "./jsonValue.js"; @@ -83,11 +84,11 @@ const jsonStrings = (value: JsonValue | undefined): readonly string[] => { /** The saved values a step sends: `{saved:name}` in a request path, JSON string or goto URL, and saved header values and credential parts. */ const sentSavedNames = (action: BrowserScenarioAction): readonly string[] => { if (action.action === "goto") - return savedPlaceholderNames(action.destination.url); + return savedPlaceholderNames(action.destination.url, SAVED_URL_PLACEHOLDER); if (action.action !== "request") return []; return [ - ...[action.path, ...jsonStrings(action.json)].flatMap( - savedPlaceholderNames, + ...[action.path, ...jsonStrings(action.json)].flatMap((text) => + savedPlaceholderNames(text), ), ...headerValues(action).flatMap((value) => value.source === "saved" ? [value.name] : [], diff --git a/src/domain/browserScenarioValues.ts b/src/domain/browserScenarioValues.ts index b8e6f69136..ed4a8ac117 100644 --- a/src/domain/browserScenarioValues.ts +++ b/src/domain/browserScenarioValues.ts @@ -138,16 +138,25 @@ const browserScenarioPageValueSchemas = [ }), ] as const; +/** A path on the start origin: one leading slash, never `//` or `/\`. Written without look-around or reserved class syntax, so every JSON Schema regex dialect reads it. */ +const START_ORIGIN_PATH = /^\x2F(?:[^\x2F\x5C]|$)/u; + +/** `{saved:name}` as written, which sends a saved value in a request path or a JSON body string. */ +export const SAVED_PLACEHOLDER = /\{saved:([A-Za-z][A-Za-z0-9._-]*)\}/gu; + /** - * `{saved:name}`, which sends a saved value in a request path, a goto URL or a JSON body string. A parsed URL spells the - * braces, and may spell the colon, percent-encoded. + * `{saved:name}` in a goto destination's URL, which the scenario schema parsed, so it may spell the braces and the colon + * percent-encoded. Other text keeps an encoded spelling as written. */ -export const SAVED_PLACEHOLDER = +export const SAVED_URL_PLACEHOLDER = /(?:\{|%7[Bb])saved(?::|%3[Aa])([A-Za-z][A-Za-z0-9._-]*)(?:\}|%7[Dd])/gu; -/** The saved value names a text's `{saved:name}` placeholders send. */ -export const savedPlaceholderNames = (text: string): readonly string[] => - [...text.matchAll(SAVED_PLACEHOLDER)].map(([, name]) => name!); +/** The saved value names a text's placeholders send, as `placeholder` spells them. */ +export const savedPlaceholderNames = ( + text: string, + placeholder: RegExp = SAVED_PLACEHOLDER, +): readonly string[] => + [...text.matchAll(placeholder)].map(([, name]) => name!); /** A value an earlier step saved from a JSON response. */ const savedValueSchema = z.strictObject({ @@ -207,7 +216,7 @@ const saveResponseSchema = z method: z.enum(BROWSER_SCENARIO_REQUEST_METHODS), path: z .string() - .regex(/^\/(?![/\\])/u) + .regex(START_ORIGIN_PATH) .describe("`{*}` matches one segment; the query is ignored."), save: z.array(saveEntrySchema).min(1), }) @@ -402,8 +411,7 @@ export const browserScenarioActionSchema = z.discriminatedUnion("action", [ method: z.enum(BROWSER_SCENARIO_REQUEST_METHODS), path: z .string() - // Preserve the prefix grammar without look-around or reserved class syntax. - .regex(/^\x2F(?:[^\x2F\x5C]|$)/u) + .regex(START_ORIGIN_PATH) .describe( "Path, and optional literal query, on the start URL's origin, such as /api/folders; it must resolve to that origin.", ),