refactor: retire the unlogged HTTP retrieval path in the target observer

CHANGELOG.md

@@ -10,6 +10,8 @@ This project uses a Keep a Changelog-style structure. Until a tagged release is
 
 - Node.js 22 or later is now required (package `engines`, `doctor`, the Web capability contract and the documentation). Node.js 20 reached end of life on 2026-04-30 and is no longer supported or tested. CI runs Node.js 22 and 24 on Ubuntu, Windows and macOS; workflow actions move to versions that run on Node.js 24 and are pinned by commit, and the browser E2E workflows pin Ubuntu 24.04. Contributors install test dependencies with `npm install` through a root `package.json`, which also stops an unrelated parent-directory `package.json` from capturing the install.
 
+- HTTP target observation always runs through the logged, size-bounded network session under the saved run policy. The second retrieval path, reachable only by direct library callers, did not reject content encodings, cap total bytes or keep a request log; it was removed. No CLI command used it.
+
 - Removed internal plans, implementation reviews, status notes, and research source copies from the public source tree; retained official source links and attribution.
 
 - Reviewed the Digital Agency accessibility policy against the monitored source on 2026-09-25. The 18 additional criterion mappings and routing are unchanged; the catalog source hash, review date and attribution binding now match the reviewed page.

shared/skill/scripts/lib/target-observer.mjs

@@ -1,8 +1,6 @@
 import crypto from "node:crypto";
 import { createNetworkSession } from "./network-transport.mjs";
 import fs from "node:fs";
-import http from "node:http";
-import https from "node:https";
 import path from "node:path";
 import { spawnSync } from "node:child_process";
 import { fileURLToPath } from "node:url";
@@ -11,7 +9,7 @@ import { canonicalJson } from "./canonical-json.mjs";
 import { isRfc3339DateTime } from "./date-time.mjs";
 import { isSafeRelativePath } from "./evidence-identity-validation.mjs";
 import { createTargetIdentity, targetDigest, targetIdentityErrors, compareTargetIdentities } from "./target-identity.mjs";
-import { parseTargetUrl, resolveInspectionEndpoint } from "./inspection-endpoint.mjs";
+import { parseTargetUrl } from "./inspection-endpoint.mjs";
 
 const MAX_BYTES = 50 * 1024 * 1024;
 const DEFAULT_BYTES = 10 * 1024 * 1024;
@@ -55,108 +53,24 @@ function fileObservation(spec, options) {
   return { snapshot, files: [file], bytes: file.bytes };
 }
 
-function permittedOrigins(policy) {
-  if (policy?.network !== "allowlisted" || !Array.isArray(policy.allowedOrigins) || !policy.allowedOrigins.length) throw new Error("HTTP target observation requires an explicit network allowlist.");
-  return new Set(policy.allowedOrigins.map((value) => {
-    const url = parseTargetUrl(value, { allowLocalhost: policy.allowLocalhost === true });
-    if (url.pathname !== "/" || url.search || url.hash) throw new Error("Allowed origins must not contain a path, query or fragment.");
-    return url.origin;
-  }));
-}
-
-function deadline(promise, signal) {
-  if (signal.aborted) return Promise.reject(signal.reason);
-  return new Promise((resolve, reject) => {
-    const abort = () => reject(signal.reason);
-    signal.addEventListener("abort", abort, { once: true });
-    promise.then(resolve, reject).finally(() => signal.removeEventListener("abort", abort));
-  });
-}
-
-async function retrieve(url, endpoint, options, signal) {
-  return await new Promise((resolve, reject) => {
-    const transport = url.protocol === "https:" ? https : http;
-    const request = transport.request(url, {
-      method: "GET", agent: false, autoSelectFamily: false, signal,
-      lookup: (_hostname, lookupOptions, callback) => lookupOptions?.all
-        ? callback(null, [{ address: endpoint.address, family: endpoint.family }])
-        : callback(null, endpoint.address, endpoint.family),
-      headers: { "accept-encoding": "identity", "user-agent": "information-accessibility-target-observer/1", "connection": "close" },
-      maxHeaderSize: 16384
-    }, (response) => {
-      const chunks = [];
-      let size = 0;
-      response.on("data", (chunk) => {
-        size += chunk.length;
-        if (size > options.maxBytes) {
-          response.destroy(new Error(`HTTP response exceeds the ${options.maxBytes}-byte limit.`));
-          return;
-        }
-        chunks.push(chunk);
-      });
-      response.on("error", reject);
-      response.on("end", () => resolve({ status: response.statusCode, headers: response.headers, bytes: Buffer.concat(chunks, size) }));
-    });
-    request.on("error", reject);
-    request.end();
-  });
-}
-
 async function httpObservation(spec, options) {
   exactSpec(spec, ["kind", "target_ref"]);
-  if (options.runNetworkPolicy || options.runId) {
-    const session = createNetworkSession({ runId: options.runId, policy: options.runNetworkPolicy, caller: options.networkPolicy });
-    const response = await session.fetch({ url: spec.target_ref, method: "GET", resource_type: "main_document" });
-    const header = (name) => typeof response.headers[name] === "string" ? response.headers[name] : null;
-    const sha256 = targetDigest(response.bytes);
-    const snapshot = createTargetIdentity({ kind: "http", targetRef: spec.target_ref,
-      identity: { requested_url: spec.target_ref, final_url: response.final_url, status: response.status,
-        etag: header("etag"), last_modified: header("last-modified"), content_type: header("content-type"), content_encoding: header("content-encoding"),
-        response_sha256: sha256, size: response.bytes.length, redirects: response.redirects, request_profile: "credential_free_get_identity_encoding_v1" },
-      evidenceBindings: [{ evidence_type: "other", sha256 }],
-      limitations: ["Captures one credential-free HTTP response under the saved run policy and explicit caller authorization.",
-        "Dynamic responses can drift between requests; rendered or authenticated states require an explicitly captured web state."] });
-    return { snapshot, files: [], bytes: response.bytes, network: session.log() };
+  // Retrieval always goes through the logged, size-bounded network session under the saved run policy.
+  if (!options.runNetworkPolicy || !options.runId) {
+    throw new Error("HTTP target observation requires the saved run network policy and run ID; use the run target commands.");
   }
-  const policy = options.networkPolicy;
-  const origins = permittedOrigins(policy);
-  const requested = parseTargetUrl(spec.target_ref, { allowLocalhost: policy.allowLocalhost === true });
-  requested.hash = "";
-  let current = requested;
-  const controller = new AbortController();
-  const timer = setTimeout(() => controller.abort(new Error("HTTP target observation timed out.")), options.timeoutMs);
-  timer.unref();
-  const redirects = [];
-  const endpoints = [];
-  const visited = new Set();
-  try {
-    while (true) {
-      if (!origins.has(current.origin)) throw new Error("Target or redirect origin is outside the explicit network allowlist.");
-      if (visited.has(current.href)) throw new Error("HTTP target redirect loop detected.");
-      visited.add(current.href);
-      const endpoint = await deadline(resolveInspectionEndpoint(current, { allowLocalhost: policy.allowLocalhost === true }), controller.signal);
-      endpoints.push(endpoint);
-      const response = await retrieve(current, endpoint, options, controller.signal);
-      if ([301, 302, 303, 307, 308].includes(response.status)) {
-        if (redirects.length >= 5) throw new Error("HTTP target exceeded the redirect limit.");
-        if (typeof response.headers.location !== "string") throw new Error("HTTP target redirect has no Location.");
-        const next = parseTargetUrl(new URL(response.headers.location, current).href, { allowLocalhost: policy.allowLocalhost === true });
-        next.hash = "";
-        redirects.push({ from: current.href, to: next.href, status: response.status });
-        current = next;
-        continue;
-      }
-      const header = (name) => typeof response.headers[name] === "string" ? response.headers[name] : null;
-      const sha256 = targetDigest(response.bytes);
-      const snapshot = createTargetIdentity({ kind: "http", targetRef: spec.target_ref,
-        identity: { requested_url: requested.href, final_url: current.href, status: response.status,
-          etag: header("etag"), last_modified: header("last-modified"), content_type: header("content-type"), content_encoding: header("content-encoding"),
-          response_sha256: sha256, size: response.bytes.length, redirects, request_profile: "credential_free_get_identity_encoding_v1" },
-        evidenceBindings: [{ evidence_type: "other", sha256 }],
-        limitations: ["Captures one credential-free HTTP response, without executing JavaScript or authenticating.", "Dynamic responses can drift between requests; rendered or authenticated states require an explicitly captured web state."] });
-      return { snapshot, files: [], bytes: response.bytes, network: { dns_binding: "pinned_per_request", endpoints } };
-    }
-  } finally { clearTimeout(timer); }
+  const session = createNetworkSession({ runId: options.runId, policy: options.runNetworkPolicy, caller: options.networkPolicy });
+  const response = await session.fetch({ url: spec.target_ref, method: "GET", resource_type: "main_document" });
+  const header = (name) => typeof response.headers[name] === "string" ? response.headers[name] : null;
+  const sha256 = targetDigest(response.bytes);
+  const snapshot = createTargetIdentity({ kind: "http", targetRef: spec.target_ref,
+    identity: { requested_url: spec.target_ref, final_url: response.final_url, status: response.status,
+      etag: header("etag"), last_modified: header("last-modified"), content_type: header("content-type"), content_encoding: header("content-encoding"),
+      response_sha256: sha256, size: response.bytes.length, redirects: response.redirects, request_profile: "credential_free_get_identity_encoding_v1" },
+    evidenceBindings: [{ evidence_type: "other", sha256 }],
+    limitations: ["Captures one credential-free HTTP response under the saved run policy and explicit caller authorization.",
+      "Dynamic responses can drift between requests; rendered or authenticated states require an explicitly captured web state."] });
+  return { snapshot, files: [], bytes: response.bytes, network: session.log() };
 }
 
 function gitExecutable(root) {

tests/target-observer.test.mjs

@@ -9,6 +9,7 @@ import test from "node:test";
 import { observeTarget, assertTargetUnchanged } from "../codex/skills/information-accessibility-practice/scripts/lib/target-observer.mjs";
 import { targetDigest, targetIdentityErrors, compareTargetIdentities } from "../codex/skills/information-accessibility-practice/scripts/lib/target-identity.mjs";
 import { assertStableFile, readStableFile } from "../codex/skills/information-accessibility-practice/scripts/lib/audit-run.mjs";
+import { createNetworkPolicy } from "../codex/skills/information-accessibility-practice/scripts/lib/network-policy.mjs";
 
 function temporary(t) {
   const root = fs.mkdtempSync(path.join(os.tmpdir(), "a11y-target-identity-"));
@@ -126,6 +127,8 @@ test("Git records untracked and deleted files, and refuses executable filters an
   assert.equal(fs.existsSync(marker), false);
 });
 
+const observeRunId = "RUN-20260918T000000Z-OBSERVE1";
+
 async function serverFixture(t) {
   let body = "<main>Static response</main>";
   let etag = '"version-1"';
@@ -135,56 +138,67 @@ async function serverFixture(t) {
     if (request.url === "/redirect") { response.writeHead(302, { location: "/page" }); response.end(); return; }
     if (request.url === "/outside") { response.writeHead(302, { location: "http://169.254.169.254/metadata" }); response.end(); return; }
     if (request.url === "/loop") { response.writeHead(302, { location: "/loop" }); response.end(); return; }
-    if (request.url === "/slow") return;
+    if (request.url === "/encoded") { response.writeHead(200, { "content-encoding": "gzip" }); response.end(body); return; }
     response.writeHead(200, { "content-type": "text/html", etag, "last-modified": "Fri, 18 Sep 2026 00:00:00 GMT" });
     response.end(request.url === "/dynamic" ? `${body} ${count}` : body);
   });
   await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
   t.after(() => new Promise((resolve) => { server.closeAllConnections(); server.close(resolve); }));
   const origin = `http://127.0.0.1:${server.address().port}`;
-  return { origin, policy: { network: "allowlisted", allowedOrigins: [origin], allowLocalhost: true },
+  const caller = { network: "allowlisted", allowedOrigins: [origin], allowLocalhost: true };
+  const runPolicy = createNetworkPolicy({ targetOrigins: [origin], allowLocalhost: true });
+  return { origin, caller, runPolicy, options: { runId: observeRunId, runNetworkPolicy: runPolicy, networkPolicy: caller },
     setBody(value) { body = value; }, setEtag(value) { etag = value; }, requestCount() { return count; } };
 }
 
 test("HTTP identities pin endpoints and preserve final URL, response hash, headers and retrieval time", async (t) => {
   const server = await serverFixture(t);
-  const spec = { kind: "http", target_ref: `${server.origin}/redirect#content` };
-  const options = { networkPolicy: server.policy };
-  const before = await observeTarget(spec, options);
+  const spec = { kind: "http", target_ref: `${server.origin}/redirect` };
+  const before = await observeTarget(spec, server.options);
   assert.equal(before.snapshot.identity.final_url, `${server.origin}/page`);
   assert.equal(before.snapshot.identity.redirects.length, 1);
   assert.equal(before.snapshot.identity.etag, '"version-1"');
   assert.equal(before.snapshot.identity.response_sha256, targetDigest(before.bytes));
-  assert.equal(before.network.dns_binding, "pinned_per_request");
+  assert.equal(before.network.kind, "network-request-log");
+  assert.equal(before.network.entries.length, 2);
+  assert.ok(before.network.entries.every((entry) => entry.pinned_address === "127.0.0.1" && entry.request_sent && entry.outcome === "succeeded"));
   assert.ok(before.snapshot.captured_at);
-  await assertTargetUnchanged(before.snapshot, spec, options);
+  await assertTargetUnchanged(before.snapshot, spec, server.options);
   server.setEtag('"version-2"');
-  await assert.rejects(assertTargetUnchanged(before.snapshot, spec, options), (error) => error.comparison.changed_identity_fields.includes("etag"));
+  await assert.rejects(assertTargetUnchanged(before.snapshot, spec, server.options), (error) => error.comparison.changed_identity_fields.includes("etag"));
   server.setBody("<main>New response</main>");
-  const after = await observeTarget(spec, options);
+  const after = await observeTarget(spec, server.options);
   assert.ok(compareTargetIdentities(before.snapshot, after.snapshot).changed_identity_fields.includes("response_sha256"));
 });
 
 test("dynamic HTTP responses cannot be silently reused as the same snapshot", async (t) => {
   const server = await serverFixture(t);
   const spec = { kind: "http", target_ref: `${server.origin}/dynamic` };
-  const options = { networkPolicy: server.policy };
-  const before = await observeTarget(spec, options);
+  const before = await observeTarget(spec, server.options);
   assert.ok(before.snapshot.limitations.some((item) => item.includes("Dynamic")));
-  await assert.rejects(assertTargetUnchanged(before.snapshot, spec, options), /Target drift/);
+  await assert.rejects(assertTargetUnchanged(before.snapshot, spec, server.options), /Target drift/);
 });
 
-test("HTTP observation fails closed for network denial, credentials, redirects, size and elapsed time", async (t) => {
+test("HTTP observation needs the saved run policy and fails closed before or during transmission", async (t) => {
   const server = await serverFixture(t);
   const spec = { kind: "http", target_ref: `${server.origin}/page` };
-  await assert.rejects(observeTarget(spec), /explicit network allowlist/);
-  await assert.rejects(observeTarget(spec, { networkPolicy: { ...server.policy, network: "denied" } }), /allowlist/);
+  const retired = /saved run network policy and run ID/;
+  await assert.rejects(observeTarget(spec), retired);
+  await assert.rejects(observeTarget(spec, { networkPolicy: server.caller }), retired);
+  await assert.rejects(observeTarget(spec, { runId: observeRunId, networkPolicy: server.caller }), retired);
+  await assert.rejects(observeTarget(spec, { runNetworkPolicy: server.runPolicy, networkPolicy: server.caller }), retired);
+  await assert.rejects(observeTarget(spec, { ...server.options, networkPolicy: { ...server.caller, network: "denied" } }), /allowlist|caller/i);
+  await assert.rejects(observeTarget({ ...spec, target_ref: spec.target_ref.replace("http://", "http://user:secret@") }, server.options), /credentials|Network operation stopped/);
+  await assert.rejects(observeTarget({ ...spec, target_ref: `${spec.target_ref}#fragment` }, server.options), /fragments|Network operation stopped/);
   assert.equal(server.requestCount(), 0);
-  await assert.rejects(observeTarget({ ...spec, target_ref: spec.target_ref.replace("http://", "http://user:secret@") }, { networkPolicy: server.policy }), /credentials/);
-  await assert.rejects(observeTarget({ ...spec, target_ref: `${server.origin}/outside` }, { networkPolicy: server.policy }), /outside.*allowlist/);
-  await assert.rejects(observeTarget({ ...spec, target_ref: `${server.origin}/loop` }, { networkPolicy: server.policy }), /loop/);
-  await assert.rejects(observeTarget(spec, { networkPolicy: server.policy, maxBytes: 3 }), /byte limit/);
-  await assert.rejects(observeTarget({ ...spec, target_ref: `${server.origin}/slow` }, { networkPolicy: server.policy, timeoutMs: 40 }), /timed out|aborted/i);
+  await assert.rejects(observeTarget({ ...spec, target_ref: `${server.origin}/outside` }, server.options), (error) => /Network operation stopped/.test(error.message) && error.networkLog?.entries.some((entry) => entry.decision === "denied"));
+  await assert.rejects(observeTarget({ ...spec, target_ref: `${server.origin}/loop` }, server.options), /redirect_loop/);
+  await assert.rejects(observeTarget({ ...spec, target_ref: `${server.origin}/encoded` }, server.options), /unexpected_content_encoding/);
+  // The abort and size-limit codes race, so the log is the stable evidence of a transmitted-then-failed request.
+  await assert.rejects(observeTarget(spec, { ...server.options, runNetworkPolicy: { ...server.runPolicy, max_response_bytes: 3 } }), (error) => {
+    const entry = error.networkLog.entries[0];
+    return entry.decision === "allowed" && entry.outcome === "failed" && entry.request_sent === true && entry.response_bytes > 3;
+  });
 });
 
 function webState(t) {