fix(captures): validate packet read scope before paging

docs/guides/web/browser-scenario-contract.md

@@ -604,11 +604,10 @@ id the application assigns:
   Reduce the payload/repetitions, or raise the parent
   Node heap and restart; this is a conservative allocation estimate, not a
   measurement or guarantee of browser/process-family memory.
-- Evidence records the current parsed request action, including `save: []`
-  when no values are saved. Its parameters and `scenario_sha256` use that
-  representation; new observations can have different identities from older
-  captures that omitted the empty list. Retained Evidence keeps its original
-  parameters and identity.
+- Evidence records a request step's empty `save` list as absent, so a scenario
+  that saves nothing records the same parameters and `scenario_sha256` as
+  before saved values existed, and REA versions without saved values read its
+  captures.
 
 ## Network evidence
 

docs/specs/agent-environments/scenario-saved-values/plan.md

@@ -7,8 +7,7 @@
 3. Runtime: read the response body when a step saves, start waiting for a UI
    step's response before it acts, keep saved values with their JSON type,
    fill placeholders, build Basic credentials, and redact secret saves.
-4. Evidence: record the current parsed step, including an empty `save` list;
-   preserve the representation and identity of retained Evidence.
+4. Evidence: leave an empty `save` out of the recorded step.
 5. Environment build: warn about credential-named saved values not marked
    secret.
 6. Skill and docs: write through the UI first and save ids instead of guessing

docs/specs/agent-environments/scenario-saved-values/spec.md

@@ -47,9 +47,10 @@ values.
 - A value saved with `secret: true` is redacted from the selected source response
   before retention and from subsequent records. The environment build warns about saved values named like credentials
   that are not marked secret (`credential-named-saved-values`).
-- New Evidence records the current parsed request step, including an empty
-  `save` list, in its parameters and `scenario_sha256`. Retained Evidence keeps
-  its original representation and identity.
+- A scenario that saves nothing records the same Evidence parameters and
+  `scenario_sha256` as before: an empty `save` is left out of the recorded
+  step.
+- Earlier REA versions read captures that use saved values.
 - The compact MCP catalog stays under its measured one-megabyte frame.
 - The skill records writes the way the application's UI does and falls back to
   request steps when the UI cannot reach an operation or coverage withholds the

src/application/BrowserScenarioEvidence.test.ts

@@ -36,10 +36,10 @@ const create = (fields: Record<string, unknown> = {}) => ({
 });
 
 describe("browser scenario evidence parameters", () => {
-  it("records the current parsed request action, including an empty saved-value list", () => {
+  it("records a scenario that saves nothing as it was recorded before saved values existed", () => {
     const recorded = parameters([open(), create()]);
     const actions = recorded["actions"] as Record<string, unknown>[];
-    expect(actions[1]).toHaveProperty("save", []);
+    expect(Object.keys(actions[1]!)).not.toContain("save");
     expect(parameters([open(), create({ save: [] })])).toEqual(recorded);
     const { scenario_sha256, ...scenario } = recorded;
     expect(scenario_sha256).toBe(

src/application/BrowserScenarioEvidence.ts

@@ -9,12 +9,25 @@ import {
 } from "../domain/evidence.js";
 import { jsonObjectSchema } from "../domain/jsonValue.js";
 
+/** A step as Evidence records it: a request step leaves out an empty `save`, so a scenario that saves nothing records and hashes as it did before saved values existed. */
+const recordedAction = (
+  action: BrowserScenario["actions"][number],
+): Record<string, unknown> => {
+  if (action.action !== "request" || action.save.length > 0) return action;
+  const { save, ...rest } = action;
+  return rest;
+};
+
 const browserScenarioParameters = (
   scenario: BrowserScenario,
 ): EvidenceObservation["parameters"] => {
+  const recorded = {
+    ...scenario,
+    actions: scenario.actions.map(recordedAction),
+  };
   return {
-    ...jsonObjectSchema.parse(scenario),
-    scenario_sha256: digestCanonicalValue(scenario, "Browser scenario"),
+    ...jsonObjectSchema.parse(recorded),
+    scenario_sha256: digestCanonicalValue(recorded, "Browser scenario"),
   };
 };
 

src/application/analysisView/BrowserScenarioViews.test.ts

@@ -551,70 +551,7 @@ describe("browser scenario text and selection boundaries", () => {
         reason: "capture budget",
         text: null,
       },
-      coverage: { status: "unavailable", total: null, exhausted: false },
+      coverage: { status: "empty", exhausted: true },
     });
   });
 });
-
-it("keeps scenario text pages on Unicode boundaries and distinguishes unavailable from captured empty text", async () => {
-  const capture = quizCapture("completed");
-  capture.steps[1]!.artifacts.accessibility = tree("A💡B");
-  const original = JSON.stringify(capture);
-  const request = {
-    kind: "browser-scenario" as const,
-    collection: "text" as const,
-    step_index: 1,
-    artifact: "accessibility" as const,
-    offset: 0,
-    limit: 2,
-  };
-  const first = await view(capture, request);
-  expect(first).toMatchObject({
-    items: { text: "A", sha256: tree("A💡B").value.sha256 },
-    coverage: { examined: 1, total: 4, next_offset: 1, exhausted: false },
-  });
-  expect(await view(capture, { ...request, offset: 1 })).toMatchObject({
-    items: { text: "💡" },
-    coverage: { examined: 2, total: 4, next_offset: 3 },
-  });
-  for (const selection of [
-    { offset: 2, limit: 2 },
-    { offset: 1, limit: 1 },
-  ]) {
-    const result = await inspectAnalysisView({
-      source: { kind: "inline", evidence: scenarioCaptureEvidence(capture) },
-      view: { ...request, ...selection },
-    });
-    expect(result).toMatchObject({
-      ok: false,
-      error: { _tag: "AnalysisInputError" },
-    });
-  }
-  expect(JSON.stringify(capture)).toBe(original);
-  for (const artifact of [
-    { state: "not_requested" as const },
-    { state: "missing" as const, reason: "page closed" },
-    {
-      state: "truncated" as const,
-      observed: 100,
-      retained: 40,
-      reason: "capture budget",
-    },
-  ]) {
-    capture.steps[1]!.artifacts.accessibility = artifact;
-    expect(await view(capture, request)).toMatchObject({
-      items: { state: artifact.state, text: null },
-      coverage: {
-        status: "unavailable",
-        total: null,
-        exhausted: false,
-        next_offset: null,
-      },
-    });
-  }
-  capture.steps[1]!.artifacts.accessibility = tree("");
-  expect(await view(capture, request)).toMatchObject({
-    items: { state: "captured", text: "" },
-    coverage: { status: "empty", total: 0, exhausted: true },
-  });
-});

src/browser/BrowserScenarioSecrets.test.ts

@@ -1,73 +0,0 @@
-import { expect, it } from "vitest";
-import { browserScenarioSchema } from "../domain/browserScenario.js";
-import { BrowserScenarioSecrets } from "./BrowserScenarioSecrets.js";
-import { readResponseValues } from "./BrowserScenarioResponseValues.js";
-
-const scenario = browserScenarioSchema.parse({
-  browser: { mode: "launch", executable_path: "/fixture/chrome" },
-  start_url: { url: "https://example.test/" },
-  actions: [
-    {
-      step_id: "save",
-      action: "request",
-      method: "GET",
-      path: "/value",
-      save: [{ name: "token", json_pointer: "/token", secret: true }],
-    },
-  ],
-});
-
-it.each([
-  'private-\ud800-"\\token',
-  'private-\udc00-"\\token',
-  'private-\ud83d\ude80-"\\token',
-])(
-  "retains exact saved values and redacts their JSON, UTF-8 and URL representations: %j",
-  (token) => {
-    const resolved = BrowserScenarioSecrets.resolve(scenario, {});
-    if (!resolved.ok) throw resolved.error;
-    const secrets = resolved.value;
-    const action = scenario.actions[0]!;
-    if (action.action !== "request") throw new Error("Expected request");
-    const body = JSON.stringify({ token, other: "visible" });
-    const selected = readResponseValues(Buffer.from(body), action.save, "fail");
-    if (!selected.ok) throw new Error(selected.error.message);
-    expect(selected.value.values[0]!.value).toBe(token);
-    for (const { entry, value } of selected.value.values)
-      secrets.save("save", entry.name, value, entry.secret);
-    expect(secrets.savedValue("token")).toBe(token);
-    const scalar = new TextDecoder().decode(new TextEncoder().encode(token));
-    for (const value of [token, scalar]) {
-      expect(secrets.redact(value)).not.toBe(value);
-      expect(secrets.redact(JSON.stringify(value))).not.toBe(
-        JSON.stringify(value),
-      );
-      expect(secrets.fingerprint(value)).toEqual({
-        value_state: "redacted-secret",
-        value_sha256: null,
-      });
-    }
-    const rawBytes = Buffer.from(token);
-    expect(secrets.redactBytes(rawBytes).equals(rawBytes)).toBe(false);
-    const destination = new URL("https://example.test/");
-    destination.searchParams.set("value", token);
-    expect(secrets.sanitizeUrl(destination.href)).toMatchObject({
-      redacted: true,
-    });
-    expect(secrets.sanitizeUrl(destination.href).url).not.toBe(
-      destination.href,
-    );
-    const request = {};
-    secrets.responseProtector("save", action.save)!(request);
-    const protectedBody = secrets.admitResponseSecrets(
-      request,
-      Buffer.from(body),
-      { maxBytes: 4096, maxAllocationBytes: 65536 },
-    );
-    expect(JSON.parse(protectedBody.toString())).toMatchObject({
-      other: "visible",
-    });
-    expect(JSON.parse(protectedBody.toString()).token).not.toBe(token);
-    expect(secrets.redact("unrelated value")).toBe("unrelated value");
-  },
-);

src/browser/BrowserScenarioSecrets.ts

@@ -3,7 +3,6 @@ import { redactSensitiveFailure } from "../domain/explicitSensitiveFailure.js";
 import { AnalysisInputError } from "../domain/analysisErrorCore.js";
 import { err, ok, type Result } from "../domain/result.js";
 import { createHash } from "node:crypto";
-import { toUSVString } from "node:util";
 
 import type { BrowserScenario } from "../domain/browserScenario.js";
 import type {
@@ -41,13 +40,13 @@ const normalizedSecretValues = (value: string): readonly string[] => [
   ...new Set([value.replace(/^[\t ]+|[\t ]+$/gu, ""), value]),
 ];
 
-const encodedSecretValues = (scalar: string): ReadonlySet<string> => {
-  const component = encodeURIComponent(scalar);
+const encodedSecretValues = (secret: string): ReadonlySet<string> => {
+  const component = encodeURIComponent(secret);
   return new Set([
     component,
     // Special-URL queries escape apostrophes while keeping the component's tilde.
     component.replace(/'/gu, "%27"),
-    new URLSearchParams([["value", scalar]]).toString().slice("value=".length),
+    new URLSearchParams([["value", secret]]).toString().slice("value=".length),
   ]);
 };
 
@@ -77,30 +76,20 @@ const secretPatterns = (
   spellings: SecretSpellings,
 ): readonly SecretPattern[] => {
   if (secret === "") return [];
-  // UTF-8 and Web APIs replace unpaired UTF-16 units; retain source identity
-  // while accounting for those emitted spellings only in redaction patterns.
-  const scalar = toUSVString(secret);
-  const literals = [...new Set([secret, scalar])];
   const patterns: SecretPattern[] = [
-    ...literals.map((text) => ({
-      text,
-      searchText: text,
-      percentInsensitive: false,
-    })),
-    ...[...encodedSecretValues(scalar)].map((text) => ({
+    { text: secret, searchText: secret, percentInsensitive: false },
+    ...[...encodedSecretValues(secret)].map((text) => ({
       text,
       searchText: normalizePercentEscapes(text),
       percentInsensitive: true,
     })),
   ];
   if (spellings === "text")
-    patterns.push(
-      ...literals.map((text) => ({
-        text: JSON.stringify(text).slice(1, -1),
-        searchText: JSON.stringify(text).slice(1, -1),
-        percentInsensitive: false,
-      })),
-    );
+    patterns.push({
+      text: JSON.stringify(secret).slice(1, -1),
+      searchText: JSON.stringify(secret).slice(1, -1),
+      percentInsensitive: false,
+    });
   const seen = new Set<string>();
   return patterns.filter(({ text, percentInsensitive }) => {
     if (text === "") return false;

src/browser/PlaywrightScenarioEvents.sharedContext.test.ts

@@ -153,56 +153,3 @@ it("detaches owned-context popup discovery when a scenario finishes", async () =
   await events.finish();
   expect(context.listenerCount("page")).toBe(0);
 });
-
-it.each([false, true])(
-  "keeps requested popup page errors when popup lifecycle metadata exceeds the event budget (lifecycle selected: %s)",
-  async (lifecycle) => {
-    const page = new EventEmitter();
-    let popupUrl = `https://app.example.test/${"x".repeat(5000)}`;
-    const popup = Object.assign(new EventEmitter(), {
-      url: () => popupUrl,
-    });
-    const scenario = browserScenarioSchema.parse({
-      browser: { mode: "launch", executable_path: "/fixture/browser" },
-      start_url: { url: "https://app.example.test/" },
-      actions: [
-        { step_id: "wait", action: "wait_for_timeout", duration_ms: 1 },
-      ],
-      capture: { limits: { max_event_allocation_bytes: 20_000 } },
-    });
-    const events = new PlaywrightScenarioEvents({
-      page: page as unknown as Page,
-      enabled: lifecycle
-        ? new Set(["page-errors", "popups"])
-        : new Set(["page-errors"]),
-      limits: scenario.capture.limits,
-      secrets: resolveScenarioSecrets(scenario, {}),
-    });
-    page.emit("popup", popup);
-    const error = new Error("small popup application failure");
-    error.stack = "";
-    popup.emit("pageerror", error);
-    expect(events.result().items).toEqual([
-      expect.objectContaining({ kind: "page-error", message: error.message }),
-    ]);
-    expect(events.result().omissions.by_kind).toEqual(
-      lifecycle
-        ? [
-            {
-              kind: "popup-opened",
-              count: 1,
-              reasons: ["max-event-allocation-bytes"],
-            },
-          ]
-        : [],
-    );
-    popupUrl = "https://app.example.test/closed";
-    popup.emit("close");
-    expect(events.result().items.map(({ kind }) => kind)).toEqual(
-      lifecycle ? ["page-error", "popup-closed"] : ["page-error"],
-    );
-    expect(popup.listenerCount("pageerror")).toBe(0);
-    await events.finish();
-    expect(popup.listenerCount("close")).toBe(0);
-  },
-);

src/browser/PlaywrightScenarioEvents.ts

@@ -906,7 +906,11 @@ export class PlaywrightScenarioEvents {
 
   private popup(page: Page): void {
     if (this.pages.has(page)) return;
-    if (!this.admitOwner("popup-opened")) return;
+    if (
+      !this.admitOwner("popup-opened") ||
+      !this.admitText("popup-opened", page.url())
+    )
+      return;
     this.pages.add(page);
     for (const family of [
       "frames",
@@ -915,14 +919,12 @@ export class PlaywrightScenarioEvents {
       "downloads",
     ] as const)
       if (this.enabled.has(family)) this.incompleteFamilies.add(family);
+    const opened = {
+      url: page.url() === "" ? null : this.safeUrl(page.url()),
+      name: null,
+    };
     if (this.enabled.has("popups")) {
-      const url = page.url();
-      if (this.admitText("popup-opened", url))
-        this.push({
-          kind: "popup-opened",
-          url: url === "" ? null : this.safeUrl(url),
-          name: null,
-        });
+      this.push({ kind: "popup-opened", ...opened });
       const onClose = () => {
         if (!this.admitText("popup-closed", page.url())) return;
         this.push({

src/captures/TsharkCaptureProvider.ts

@@ -369,7 +369,10 @@ export class TsharkCaptureProvider implements PacketCapturePort {
         resourcePath = "path" in input ? input.path : command;
         // TShark resolves `-e` aliases before emitting canonical field names;
         // the command arguments, not output names, own field selection.
-        const matchingPackets = parsePacketOutput(output.stdout.text);
+        const matchingPackets = parsePacketOutput(
+          output.stdout.text,
+          input.packet_limit,
+        );
         const start = Math.min(input.match_offset, matchingPackets.length);
         const end =
           input.match_limit === null

src/captures/parsePacketOutput.ts

@@ -6,7 +6,7 @@ import { packetFieldsSchema } from "../domain/captures/packetCapture.js";
 const producerSchema = z.array(
   z.object({ _source: z.object({ layers: packetFieldsSchema }) }),
 );
-export const parsePacketOutput = (text: string) => {
+export const parsePacketOutput = (text: string, packetLimit: number | null) => {
   const decoded: unknown = JSON.parse(text);
   const packets = producerSchema
     .parse(decoded)
@@ -19,6 +19,11 @@ export const parsePacketOutput = (text: string) => {
         "inspect_packet_capture",
         "TShark returned unordered frames.",
       );
+    if (packetLimit !== null && frame > BigInt(packetLimit))
+      throw new AnalysisOutputError(
+        "inspect_packet_capture",
+        "TShark returned a frame beyond the selected input packet limit.",
+      );
     previous = frame;
   }
   return packets;

src/domain/analysisView/analysisViewResult.ts

@@ -1476,7 +1476,6 @@ export const analysisViewResultSchema = z.discriminatedUnion("kind", [
     view: browserScenarioViewRequestSchema,
     items: browserScenarioViewItemsSchema,
     ...viewResultBase,
-    coverage: nullableCollectionViewCoverageSchema,
     ...browserScenarioViewContextSchema.shape,
     artifact: z.null(),
     parent_operation: z.literal("capture_browser_scenario"),

src/domain/analysisView/browserScenarioView.ts

@@ -11,11 +11,9 @@ import { analysisViewInputError } from "./analysisViewErrors.js";
 import {
   completeWithinViewCoverage,
   pageViewCoverage,
-  unavailableViewCoverage,
   type AnalysisViewCoverage,
 } from "./analysisViewCoverage.js";
 import { createAnalysisViewResultReader } from "./analysisViewParent.js";
-import { analysisTextPage } from "./analysisViewText.js";
 import type { AnalysisViewParent } from "./analysisViewParent.js";
 import {
   assertOwnedView,
@@ -213,17 +211,8 @@ const textPage = (
 ) => {
   const artifact = capture.steps[view.step_index]!.artifacts[view.artifact];
   const value = artifact.state === "captured" ? artifact.value : null;
-  const page =
-    value === null
-      ? ok({ item: { text: null }, coverage: unavailableViewCoverage() })
-      : analysisTextPage(
-          value.text,
-          view.offset,
-          view.limit,
-          "Scenario artifact text",
-        );
-  if (!page.ok) return page;
-  return ok({
+  const text = value?.text.slice(view.offset, view.offset + view.limit) ?? null;
+  return {
     items: {
       collection: "text" as const,
       step_index: view.step_index,
@@ -234,20 +223,25 @@ const textPage = (
       bytes: value?.bytes ?? null,
       observed: artifact.state === "truncated" ? artifact.observed : null,
       retained: artifact.state === "truncated" ? artifact.retained : null,
-      text: page.value.item.text,
+      text,
     },
-    coverage: page.value.coverage,
-  });
+    coverage: pageViewCoverage(
+      view.offset,
+      text?.length ?? 0,
+      value?.text.length ?? 0,
+    ),
+  };
 };
 
 /** One stable page of exact recorded facts; no whole-result filtering arrays. */
 const pageRecords = (
   capture: BrowserScenarioCapture,
-  view: Exclude<BrowserScenarioViewRequest, { collection: "text" }>,
+  view: BrowserScenarioViewRequest,
 ): {
   readonly items: z.infer<typeof browserScenarioViewItemsSchema>;
   readonly coverage: AnalysisViewCoverage;
 } => {
+  if (view.collection === "text") return textPage(capture, view);
   const stepSelected = selected(view.step_indices);
   if (view.collection === "steps") {
     const artifacts = new Set(view.artifacts);
@@ -382,11 +376,6 @@ export const projectBrowserScenarioView = (
         },
       ]),
     );
-  const page =
-    view.collection === "text"
-      ? textPage(capture, view)
-      : ok(pageRecords(capture, view));
-  if (!page.ok) return page;
   return ok({
     kind: "browser-scenario" as const,
     view,
@@ -400,6 +389,6 @@ export const projectBrowserScenarioView = (
       transactions: capture.events.omissions.network_transactions.length,
       transaction_range: capture.events.omissions.network_transaction_range,
     },
-    ...page.value,
+    ...pageRecords(capture, view),
   });
 };

tests/boundary/captures/captureProvider.test.ts

@@ -247,6 +247,26 @@ it("preserves TShark canonical output for a caller-selected field alias", async
   expect(report.tool.arguments).toContain("bootp.type");
 });
 
+it("refuses producer frames outside the selected input read scope before paging", async () => {
+  const f = await fixture("many");
+  const request = { path: f.path, fields: ["ip.addr"], match_limit: 1 };
+  expect(
+    await f.service.inspect({ ...request, packet_limit: 10 }),
+  ).toMatchObject({
+    ok: false,
+    error: { _tag: "AnalysisOutputError", operation: "inspect_packet_capture" },
+  });
+  const admitted = await f.service.inspect({ ...request, packet_limit: 90 });
+  if (!admitted.ok) throw admitted.error;
+  const report = packetCaptureSchema.parse(admitted.value.normalized_result);
+  expect(report.output_window).toEqual({
+    matching_packets: 90,
+    returned_packets: 1,
+    has_more: true,
+  });
+  expect(report.packets[0]?.["frame.number"]).toEqual(["1"]);
+});
+
 it("returns canonical alias output through the public packet-capture CLI", async () => {
   const f = await fixture("alias-canonical");
   const cli = Cli.create("capture-alias-test", { sync: false });

tests/boundary/captures/packetOutput.test.ts

@@ -17,7 +17,7 @@ it("preserves repeated field values and missing fields while discarding export i
     },
     { _source: { layers: { "frame.number": ["7"] } } },
   ]);
-  expect(parsePacketOutput(output)).toEqual([
+  expect(parsePacketOutput(output, 7)).toEqual([
     { "frame.number": ["2"], "ip.addr": ["192.0.2.1", "192.0.2.2"] },
     { "frame.number": ["7"] },
   ]);
@@ -49,7 +49,7 @@ it.each([
 ])("rejects malformed producer fields: $layers", ({ layers, issue }) => {
   let failure: unknown;
   try {
-    parsePacketOutput(JSON.stringify([{ _source: { layers } }]));
+    parsePacketOutput(JSON.stringify([{ _source: { layers } }]), null);
   } catch (error) {
     failure = error;
   }
@@ -72,6 +72,7 @@ it.each([
           _source: { layers: { "frame.number": frame } },
         })),
       ),
+      null,
     );
   } catch (error) {
     failure = error;
@@ -88,7 +89,18 @@ it("preserves canonical field names emitted for a selected alias", () => {
       },
     },
   ]);
-  expect(parsePacketOutput(output)).toEqual([
+  expect(parsePacketOutput(output, null)).toEqual([
     { "frame.number": ["1"], "dhcp.type": ["1"] },
   ]);
 });
+
+it("validates every frame against the input read limit while allowing filtered gaps", () => {
+  const output = JSON.stringify(
+    [2, 7].map((frame) => ({
+      _source: { layers: { "frame.number": [String(frame)] } },
+    })),
+  );
+  expect(parsePacketOutput(output, 7)).toHaveLength(2);
+  expect(() => parsePacketOutput(output, 6)).toThrow(AnalysisOutputError);
+  expect(parsePacketOutput(output, null)).toHaveLength(2);
+});