From 80ad27e3d84d278f92fd249a070664aa4c916005 Mon Sep 17 00:00:00 2001 From: Nagi-Inaba <221035391+Nagi-Inaba@users.noreply.github.com> Date: Fri, 9 Oct 2026 03:32:26 +0900 Subject: [PATCH] refactor: retire the unlogged HTTP retrieval path in the target observer target-observer.mjs carried two HTTP clients. The run-policy path goes through the network session: it rejects any content encoding, caps the bytes of the whole session, enforces the per-run request budget and keeps a request log. The older path, taken when no run policy was supplied, had none of those guards. It also returned a hash of whatever bytes arrived while its identity recorded request_profile credential_free_get_identity_encoding_v1. Every CLI route already passes the saved run policy and run ID (run-targets.mjs sets both before observing an HTTP target), so the older path was reachable only by direct library callers and the tests. It is removed; observeTarget now rejects an HTTP target that lacks the saved run network policy or run ID before any request is made. File, Git and saved web-state targets are unchanged. target-observer.mjs shrinks from 327 to 241 lines. The three HTTP tests of target-observer.test.mjs now run through the session path with a real run policy. They keep the identity, drift and fail-closed assertions and add the rejection of a gzip response and the request-log evidence, which the older path could not provide. The retired per-call maxBytes and timeoutMs options no longer apply to HTTP; the run policy and the session fix those limits. Verified: verify-all passes on Node.js 22.19.0 (Windows), 91 test files. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 2 + shared/skill/scripts/lib/target-observer.mjs | 118 +++---------------- tests/target-observer.test.mjs | 54 +++++---- 3 files changed, 52 insertions(+), 122 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4be5b9bb..a0ade17c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ This project uses a Keep a Changelog-style structure. Until a tagged release is ### Changed +- 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. diff --git a/shared/skill/scripts/lib/target-observer.mjs b/shared/skill/scripts/lib/target-observer.mjs index 8cc0089c..6d1a78f7 100644 --- a/shared/skill/scripts/lib/target-observer.mjs +++ b/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) { diff --git a/tests/target-observer.test.mjs b/tests/target-observer.test.mjs index 352a2607..cd495573 100644 --- a/tests/target-observer.test.mjs +++ b/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 = "
Static response
"; 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("
New response
"); - 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) {