diff --git a/docs/browser-scenario-contract.md b/docs/browser-scenario-contract.md index c2409cf307..29850acbc5 100644 --- a/docs/browser-scenario-contract.md +++ b/docs/browser-scenario-contract.md @@ -516,6 +516,9 @@ id the application assigns: the same name replaces the value. An integer past 2^53, which JSON parsing would round to another number, and a number that is not finite, such as `1e400`, fail the step that saves it. +- A scenario keeps at most 4,000,000 characters of saved values at once, + counting the redaction spellings of values saved as secret, and a save past + that fails its step before the value is kept. - 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/specs/agent-environments/scenario-saved-values/spec.md b/docs/specs/agent-environments/scenario-saved-values/spec.md index 458981fdc8..80efebde26 100644 --- a/docs/specs/agent-environments/scenario-saved-values/spec.md +++ b/docs/specs/agent-environments/scenario-saved-values/spec.md @@ -41,6 +41,9 @@ values. local storage entry. The encoded value and parts read from the page are redacted as page values are. A username with a colon fails the step. - Validation refuses a saved value no earlier step saves. +- A scenario keeps at most 4,000,000 characters of saved values at once, counting + the redaction spellings of values saved as secret; a save past that fails its + step before the value is kept. - A value saved with `secret: true` is redacted from records made after it is saved. The environment build warns about saved values named like credentials that are not marked secret (`credential-named-saved-values`). diff --git a/src/browser/BrowserScenarioSecrets.ts b/src/browser/BrowserScenarioSecrets.ts index 3ee11cd699..4e41f2df12 100644 --- a/src/browser/BrowserScenarioSecrets.ts +++ b/src/browser/BrowserScenarioSecrets.ts @@ -19,6 +19,9 @@ const REDACTION_PREFIX = "[REDACTED:"; /** Page values shorter than this are not redacted: redacting them would rewrite ids, ports and hosts throughout the capture. */ export const MIN_REDACTED_PAGE_VALUE_LENGTH = 8; +/** The most saved-value text a scenario keeps at once, with the redaction spellings of values saved as secret: as much as one saved response can carry. */ +export const MAX_RETAINED_SAVED_CHARACTERS = 4_000_000; + const encodedSecretValues = (secret: string): ReadonlySet => new Set([ encodeURIComponent(secret), @@ -75,6 +78,17 @@ const secretPatterns = ( }); }; +/** The code units a secret's redaction spellings keep, each distinct spelling once. */ +const spellingUnits = (secret: string): number => + [ + ...new Set( + [ + ...secretPatterns(secret, "text"), + ...secretPatterns(secret, "url"), + ].flatMap(({ text, searchText }) => [text, searchText]), + ), + ].reduce((units, spelling) => units + spelling.length, 0); + const declaredPatterns = ( values: ReadonlyMap, spellings: SecretSpellings, @@ -331,15 +345,35 @@ export class BrowserScenarioSecrets { /** Values steps saved from JSON responses, by name, as strings or numbers. */ private readonly saved = new Map(); - /** Keep a value a step saved from a JSON response; a secret one is redacted from everything recorded after it. */ + /** Code units the current saved values keep, and those the redaction spellings of secret saves keep for the rest of the scenario. */ + private savedUnits = 0; + private secretSpellingUnits = 0; + + /** Keep a value a step saved from a JSON response; a secret one is redacted from everything recorded after it. Refused before it is kept when the scenario would hold more than {@link MAX_RETAINED_SAVED_CHARACTERS}. */ save( label: string, name: string, value: string | number, secret: boolean, ): void { + const text = String(value); + const replaced = this.saved.get(name); + const savedUnits = + this.savedUnits - + (replaced === undefined ? 0 : String(replaced).length) + + text.length; + let kept = savedUnits + this.secretSpellingUnits; + const spellings = + secret && kept <= MAX_RETAINED_SAVED_CHARACTERS ? spellingUnits(text) : 0; + kept += spellings; + if (kept > MAX_RETAINED_SAVED_CHARACTERS) + throw new Error( + `${name} cannot be saved: the scenario would keep more than ${String(MAX_RETAINED_SAVED_CHARACTERS)} characters of saved values.`, + ); this.saved.set(name, value); - if (secret) this.admitCredential(label, String(value)); + this.savedUnits = savedUnits; + this.secretSpellingUnits += spellings; + if (secret) this.admitCredential(label, text); } /** A saved value with the JSON type it was saved with. */ diff --git a/src/browser/PlaywrightScenarioRequest.test.ts b/src/browser/PlaywrightScenarioRequest.test.ts index 07714950ae..07ff953673 100644 --- a/src/browser/PlaywrightScenarioRequest.test.ts +++ b/src/browser/PlaywrightScenarioRequest.test.ts @@ -1093,6 +1093,47 @@ describe("values a URL path or Basic header cannot carry", () => { }); }); +describe("saved value retention", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("refuses a new saved name that would keep more than the limit, before keeping it", () => { + const secrets = resolved(scenario([requestStep({})])); + secrets.save("a.first", "first", "x".repeat(3_000_000), false); + expect(() => + secrets.save("b.second", "second", "y".repeat(1_500_000), false), + ).toThrow("more than 4000000 characters of saved values"); + expect(() => secrets.savedValue("second")).toThrow("No earlier step saved"); + // A later save of the same name replaces its value rather than adding to it. + secrets.save("c.first", "first", "z".repeat(3_500_000), false); + expect(String(secrets.savedValue("first"))).toHaveLength(3_500_000); + }); + + it("counts the redaction spellings a secret save keeps", () => { + const plain = resolved(scenario([requestStep({})])); + plain.save("a.token", "token", "t".repeat(2_500_000), false); + const secret = resolved(scenario([requestStep({})])); + expect(() => + secret.save("a.token", "token", "t".repeat(2_500_000), true), + ).toThrow("more than 4000000 characters of saved values"); + }); + + it("fails the step that would save past the limit", async () => { + stubFetch(201, [`{"id":"${"y".repeat(200_000)}"}`]); + const parsed = scenario([ + requestStep({ save: [{ name: "item", json_pointer: "/id" }] }), + ]); + const secrets = resolved(parsed); + secrets.save("seed.big", "big", "x".repeat(3_900_000), false); + await expect( + perform(fakePage({}), parsed.actions[0]!, secrets), + ).rejects.toThrow( + "Step create: item cannot be saved: the scenario would keep more than", + ); + }); +}); + describe("goto placeholders", () => { it("fills a goto destination's placeholders, percent-encoded", async () => { const parsed = scenario([ diff --git a/src/browser/PlaywrightScenarioRequest.ts b/src/browser/PlaywrightScenarioRequest.ts index 8ee1691d88..20d61f2bb8 100644 --- a/src/browser/PlaywrightScenarioRequest.ts +++ b/src/browser/PlaywrightScenarioRequest.ts @@ -650,6 +650,12 @@ const saveValues = ( 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); + try { + secrets.save(`${stepId}.${name}`, name, value, secret); + } catch (cause) { + throw new ScenarioRequestError( + `Step ${stepId}: ${cause instanceof Error ? cause.message : String(cause)}`, + ); + } } };