diff --git a/apps/loopover-ui/content/docs/privacy-security.mdx b/apps/loopover-ui/content/docs/privacy-security.mdx index 4332364d28..bc78fada6c 100644 --- a/apps/loopover-ui/content/docs/privacy-security.mdx +++ b/apps/loopover-ui/content/docs/privacy-security.mdx @@ -68,6 +68,7 @@ LOOPOVER_REVIEW_OPS="true" # read-only anomaly scan + outcome stats endpoint LOOPOVER_REVIEW_SELFTUNE="true" # self-tightening tuning loop, never loosens LOOPOVER_REVIEW_PARITY_AUDIT="true" # shadow-record gate-decision parity readiness LOOPOVER_REVIEW_CONTENT_LANE="true" # dedicated content/registry-repo review lane +LOOPOVER_REVIEW_SURFACE_VERIFICATION="true" # fetch + verify registry surface entries (needs the content lane too) LOOPOVER_REVIEW_DRAFT="true" # public draft-submission (contributor fork PR) flow`} /> diff --git a/apps/loopover-ui/content/docs/tuning.mdx b/apps/loopover-ui/content/docs/tuning.mdx index e1db382204..5b1f85b714 100644 --- a/apps/loopover-ui/content/docs/tuning.mdx +++ b/apps/loopover-ui/content/docs/tuning.mdx @@ -165,6 +165,15 @@ any per-PR feature to run on a given repo. So a per-PR feature activates only wh registries) through the dedicated content lane — duplicate detection, source-evidence reachability, security scanning, scope classification, registry grounding — instead of the code gate. Global. +- `LOOPOVER_REVIEW_SURFACE_VERIFICATION` — live verification of registry surface + entries, on top of the content lane above (both must be on). An entry that passes the + static shape/safety checks is additionally fetched: its declared source is checked for + evidence corroborating the claimed netuid, owner, or host, and an `openapi` / + `subnet-api` / `sse` entry is probed to confirm its URL really serves the interface its + `kind` declares. A confirmed not-served surface **closes**; unverified corroboration and + any **inconclusive** probe **hold for review** — an inconclusive check is never reported + as a pass. This is the only content-lane capability that makes outbound requests to + submitter-supplied URLs, so it is a separate flag you can roll back on its own. Global. - `LOOPOVER_REVIEW_DRAFT` — the public draft-submission flow (the `/v1/drafts` endpoints: contributor draft → GitHub OAuth → fork PR). With the flag off every draft endpoint 404s. Requires the diff --git a/packages/loopover-engine/src/review/content-lane/flag.ts b/packages/loopover-engine/src/review/content-lane/flag.ts index 9b80330cff..ccc2703e9f 100644 --- a/packages/loopover-engine/src/review/content-lane/flag.ts +++ b/packages/loopover-engine/src/review/content-lane/flag.ts @@ -15,6 +15,9 @@ export interface ContentLaneEnv { /** When truthy ("1"/"true"/"on"/"yes"), the content lane is enabled. Default OFF. */ LOOPOVER_REVIEW_CONTENT_LANE?: string; + /** When truthy, a surface entry that passes STATIC validation is ADDITIONALLY verified against live content + * (#8908 evidence grounding, #8909 functional-surface probing) before it can merge. Default OFF. */ + LOOPOVER_REVIEW_SURFACE_VERIFICATION?: string; } /** Is the content lane enabled? Default OFF — only a recognized truthy flag turns it on. */ @@ -22,3 +25,17 @@ export function isContentLaneEnabled(env: ContentLaneEnv | undefined | null): bo if (!env) return false; return /^(1|true|yes|on)$/i.test((env.LOOPOVER_REVIEW_CONTENT_LANE ?? "").trim()); } + +/** + * Is LIVE surface-entry verification enabled (#8908, #8909)? Default OFF, and deliberately its OWN flag rather + * than riding on LOOPOVER_REVIEW_CONTENT_LANE: it is the first thing in this lane that makes OUTBOUND requests + * to submitter-controlled URLs, and it can HOLD or CLOSE submissions that merge today. Both properties need to + * be rollable back on their own without taking the whole (already-cutover) content lane down with them. + * + * The caller ANDs this with the lane's existing activation, so verification runs only where a RegistryLaneSpec + * already resolved — i.e. it inherits the per-repo scoping for free and needs no config-surface of its own. + */ +export function isSurfaceVerificationEnabled(env: ContentLaneEnv | undefined | null): boolean { + if (!env) return false; + return /^(1|true|yes|on)$/i.test((env.LOOPOVER_REVIEW_SURFACE_VERIFICATION ?? "").trim()); +} diff --git a/packages/loopover-engine/test/content-lane-flag.test.ts b/packages/loopover-engine/test/content-lane-flag.test.ts new file mode 100644 index 0000000000..ffbc0bb8c2 --- /dev/null +++ b/packages/loopover-engine/test/content-lane-flag.test.ts @@ -0,0 +1,49 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { isContentLaneEnabled, isSurfaceVerificationEnabled } from "../dist/review/content-lane/flag.js"; + +test("isContentLaneEnabled is OFF by default (unset / empty / undefined env)", () => { + assert.equal(isContentLaneEnabled(undefined), false); + assert.equal(isContentLaneEnabled(null), false); + assert.equal(isContentLaneEnabled({}), false); + assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: "" }), false); +}); + +test("isContentLaneEnabled is ON for recognized truthy values (case/whitespace insensitive)", () => { + for (const v of ["1", "true", "on", "yes", "TRUE", " On ", "Yes"]) { + assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: v }), true); + } +}); + +test("isContentLaneEnabled is OFF for non-truthy strings", () => { + for (const v of ["0", "false", "off", "no", "enabled", "maybe"]) { + assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: v }), false); + } +}); + +// #8908/#8909: live surface verification is its OWN flag, independent of the lane's, so the first +// outbound-probe behavior in this lane can be rolled back without taking the whole lane down. +test("isSurfaceVerificationEnabled is OFF by default (unset / empty / undefined env)", () => { + assert.equal(isSurfaceVerificationEnabled(undefined), false); + assert.equal(isSurfaceVerificationEnabled(null), false); + assert.equal(isSurfaceVerificationEnabled({}), false); + assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "" }), false); +}); + +test("isSurfaceVerificationEnabled is ON for recognized truthy values (case/whitespace insensitive)", () => { + for (const v of ["1", "true", "on", "yes", "TRUE", " On ", "Yes"]) { + assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: v }), true); + } +}); + +test("isSurfaceVerificationEnabled is OFF for non-truthy strings", () => { + for (const v of ["0", "false", "off", "no", "enabled", "maybe"]) { + assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: v }), false); + } +}); + +test("isSurfaceVerificationEnabled is independent of the content-lane flag in BOTH directions", () => { + assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: "true" }), false); + assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "true" }), false); +}); diff --git a/src/env.d.ts b/src/env.d.ts index 7b88315aac..53c2bb03e2 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -516,6 +516,16 @@ declare global { * gate disposition byte-identical. AI-FREE (pure structured-data adjudication), so independent of the AI * reviewer; a generic hard blocker (e.g. a committed secret) is always preserved over a surface "merge". */ LOOPOVER_REVIEW_CONTENT_LANE?: string; + /** Convergence (surface LIVE VERIFICATION, #8908/#8909): when truthy *AND* the surface lane above is active + * for the repo, a surface entry that passes STATIC validation is additionally checked against what its URLs + * actually serve — `computeGrounding` over the fetched source/target evidence (is the declared netuid / + * owner / host corroborated at all?) and `probeFunctionalSurface` for the openapi/subnet-api/sse kinds (does + * the url serve the interface its `kind` claims?). A CONFIRMED not-served functional surface CLOSES; + * unconfirmed grounding and any INCONCLUSIVE probe HOLD for review — an inconclusive check is never reported + * as a pass. Default OFF: unset/false makes no outbound probe and leaves the surface verdict byte-identical. + * Its own flag, separate from the lane's, so the first outbound-fetch behavior in this lane can be rolled + * back on its own — see review/content-lane/surface-verification. */ + LOOPOVER_REVIEW_SURFACE_VERIFICATION?: string; /** Convergence (self-improve / auto-tune): when truthy, the ported self-improvement loop * (src/review/auto-tune.ts + auto-apply.ts) runs on the cron tick over loopover's OWN review-outcome * data — it computes tuning recommendations, SHADOW-SOAKS any STRICTLY-TIGHTENING recommendation in the diff --git a/src/review/content-lane-wire.ts b/src/review/content-lane-wire.ts index 6964d8cdd2..15c62fbe04 100644 --- a/src/review/content-lane-wire.ts +++ b/src/review/content-lane-wire.ts @@ -37,10 +37,11 @@ // genuinely critical finding, or one of these other configured gates, still wins outright). import { AI_JUDGMENT_BLOCKER_CODES, type GateCheckEvaluation, isAiJudgmentOnlyFailure, isDuplicateOnlyFailure } from "../rules/advisory"; import { LOOPOVER_GATE_CHECK_NAME } from "./check-names"; -import { isContentLaneEnabled } from "./content-lane/flag"; +import { isContentLaneEnabled, isSurfaceVerificationEnabled } from "./content-lane/flag"; import { runSurfaceReview, type SurfaceReviewInput, type SurfaceReviewResult } from "./content-lane/orchestrator"; import type { RegistryLaneSpec } from "./content-lane/registry-logic"; import { registeredValidatorIds, resolveRegistryLaneSpec, unregisteredValidatorId } from "./content-lane/spec-resolver"; +import { makeSurfaceEntryVerifier } from "./content-lane/surface-verification"; import { makeGithubFileFetcher } from "./grounding-wire"; import { MAX_FETCH_CHARS } from "./review-grounding"; import type { FocusManifest } from "../signals/focus-manifest"; @@ -185,6 +186,7 @@ export async function runRegistrySurfaceGate( files: { path: string; status?: string | null | undefined }[]; }, loadFileOverride?: SurfaceReviewInput["loadFile"], + verifyEntryOverride?: SurfaceReviewInput["verifyEntry"], ): Promise { let fetcherPromise: ReturnType | null = null; const githubLoad = async (path: string, ref: "head" | "base"): Promise => { @@ -219,10 +221,15 @@ export async function runRegistrySurfaceGate( if (ref === "base" && content === null && statusByPath.get(path) === "modified") deferUnreadable = true; return content; }; + // #8908/#8909: live verification is its OWN flag on top of the lane's activation, so it can be rolled back + // without taking the (already-cutover) surface lane down. Flag-OFF, `verifyEntry` is undefined, the + // orchestrator runs no verification and makes no outbound probe, and the verdict is byte-identical to today. + const verifyEntry = verifyEntryOverride ?? (isSurfaceVerificationEnabled(env) ? makeSurfaceEntryVerifier() : undefined); const result = await runSurfaceReview(spec, { changedFiles: args.files.map((file) => file.path), loadFile, opts: { secretsScan: true, sourceUrlValidation: true }, + ...(verifyEntry ? { verifyEntry } : {}), }); if (result === null) return null; // not a registry submission → the generic gate applies if (deferUnreadable) return null; // a fetch blip on a file that must be readable → defer, never auto-close diff --git a/src/review/content-lane/index.ts b/src/review/content-lane/index.ts index b888f8e568..a3b1f6e96f 100644 --- a/src/review/content-lane/index.ts +++ b/src/review/content-lane/index.ts @@ -116,4 +116,20 @@ export { type ProviderLike, type Verdict, } from "./registry-logic"; +export { + fetchSurfaceProbe, + makeSurfaceEntryVerifier, + probeToEvidence, + verifySurfaceEntry, + FUNCTIONAL_INCONCLUSIVE_REASON, + FUNCTIONAL_NOT_SERVED_REASON, + GROUNDING_INCONCLUSIVE_REASON, + GROUNDING_UNCONFIRMED_REASON, + MAX_PROBE_BODY_CHARS, + SURFACE_GROUNDING_MIN_STRONG, + type SurfaceCheckOutcome, + type SurfaceCheckResult, + type SurfaceEntryVerification, + type SurfaceProbe, +} from "./surface-verification"; export { runSurfaceReview, diffAppendedSurfaceEntries, type SurfaceReviewInput, type SurfaceReviewResult } from "./orchestrator"; diff --git a/src/review/content-lane/orchestrator.ts b/src/review/content-lane/orchestrator.ts index 110e1c3e8b..5dc22c35ca 100644 --- a/src/review/content-lane/orchestrator.ts +++ b/src/review/content-lane/orchestrator.ts @@ -30,6 +30,13 @@ export interface SurfaceReviewInput { /** Loads decoded file content at a ref; injected so unit tests need no network. Returns null when absent. */ loadFile: (path: string, ref: "head" | "base") => Promise; opts?: { secretsScan?: boolean; sourceUrlValidation?: boolean }; + /** Optional LIVE content verification for an entry that already passed static validation (#8908, #8909) — + * injected I/O, exactly like `loadFile`, so the orchestrator stays pure and domain-agnostic and any registry + * can supply its own verifier (metagraphed's is `makeSurfaceEntryVerifier` in ./surface-verification). + * Returns an OVERRIDING Assessment when verification downgrades the entry (hold or close), or `null` when it + * confirms it (the static assessment stands). Omitted ⇒ no verification runs and no fetch is made, so the + * verdict is byte-identical to the pre-#8908 lane. */ + verifyEntry?: (entry: unknown) => Promise; } export interface SurfaceReviewResult { @@ -192,6 +199,42 @@ function pickAggregateAssessment(assessments: Assessment[]): Assessment { return first as Assessment; } +/** + * Apply the injected live-content verifier (#8908, #8909) to every appended entry that passed STATIC validation, + * and return the per-entry assessments with any downgrade folded in. + * + * Only "merged" entries are verified, for two reasons: an entry already closing or held has its verdict decided + * (verification could only ever confirm it), and skipping them means an invalid submission — the common bad case + * — pays for no network I/O at all. The surviving entries are verified CONCURRENTLY (independent URLs, no data + * dependency), mirroring how this function's caller already parallelizes its GitHub reads. + * + * A verifier that THROWS must never take the review down or, worse, be swallowed into a pass: the entry is held + * for review instead. `makeSurfaceEntryVerifier` is itself written not to throw, so this is a belt-and-braces + * guard against a future/third-party verifier that is less careful. + */ +async function verifyMergedEntries( + staticAssessments: Assessment[], + appendedEntries: readonly unknown[], + verifyEntry: SurfaceReviewInput["verifyEntry"], +): Promise { + if (!verifyEntry) return staticAssessments; + return await Promise.all( + staticAssessments.map(async (assessment, idx) => { + if (assessment.verdict !== "merged") return assessment; + try { + return (await verifyEntry(appendedEntries[idx])) ?? assessment; + } catch { + return { + verdict: "manual-review" as const, + summary: "Live verification of this surface entry could not be completed — routing to review rather than accepting an unverified entry.", + candidate: assessment.candidate, + reason: "verification-error", + }; + } + }), + ); +} + /** * Adjudication policy (deterministic, DECISIVE): the overwhelming majority of outcomes are merge or close — * manual review is the rare exception. A clean valid submission MERGES; anything invalid or non-standard @@ -279,9 +322,8 @@ export async function runSurfaceReview(spec: RegistryLaneSpec, input: SurfaceRev return { verdict: "manual", summary: NO_VALIDATOR_ENTRY_SUMMARY }; } const headDoc = safeParseJson(headRaw); - const assessment = pickAggregateAssessment( - appendedEntries.map((appendedEntry) => assessEntry(headDoc, { ...input.opts, appendedEntry })), - ); + const staticAssessments = appendedEntries.map((appendedEntry) => assessEntry(headDoc, { ...input.opts, appendedEntry })); + const assessment = pickAggregateAssessment(await verifyMergedEntries(staticAssessments, appendedEntries, input.verifyEntry)); if (companionProviderFile !== null) { // Guaranteed non-null: the no-validator short-circuit above already returned when this spec lacks one. const assessProvider = spec.assessProviderEntry as NonNullable; diff --git a/src/review/content-lane/surface-verification.ts b/src/review/content-lane/surface-verification.ts new file mode 100644 index 0000000000..605d94a6e7 --- /dev/null +++ b/src/review/content-lane/surface-verification.ts @@ -0,0 +1,385 @@ +// Live surface-entry content verification (#8908, #8909) — the fetch plumbing that finally CONSULTS the two +// calibration primitives registry-logic.ts has always exported but nothing ever called: +// +// - computeGrounding (#8908): does the fetched evidence actually corroborate the declared netuid / +// owner / host, or is the submission an unbacked claim? +// - probeFunctionalSurface (#8909): for the FUNCTIONAL kinds (openapi / subnet-api / sse), does the declared +// url actually SERVE the surface its `kind` claims? +// +// Both were fully implemented + unit-tested but had zero callers outside their own test file, so the live +// surface-entry gate (assessSurfaceEntry → runSurfaceReview) merged a submission having never once looked at +// what its URLs serve. This module supplies the missing half: SSRF-guarded fetches, evidence extraction, and the +// gating decision — leaving both primitives untouched. +// +// SHAPE: all I/O is the injected `fetchImpl`, exactly like source-evidence.ts and netuid-verification.ts, so the +// whole module is unit-testable without network. The orchestrator consumes it through ONE injected +// `verifyEntry` hook (SurfaceReviewInput), so the orchestrator itself stays domain-agnostic and any other +// registry can supply its own verifier. +// +// THREE-STATE, NEVER FAIL-OPEN. Each check resolves to pass / fail / inconclusive and those are kept strictly +// distinct: an inconclusive result (a probe that never got a usable response, or a submission with no fetchable +// corroborating evidence) is NEVER reported as a pass. It holds the PR for a human instead. This is the whole +// point of the issues — a check that silently passes when it could not run is worse than no check, because it +// launders "we didn't look" into "we verified". +import { + type Assessment, + type CandidateLike, + type MetaVerdict, + computeGrounding, + functionalRequired, + probeFunctionalSurface, + registrableDomain, +} from "./registry-logic"; +import { isSafeHttpUrl } from "./safe-url"; + +/** + * Minimum `computeGrounding().strong` score an entry must reach for its claim to count as independently + * corroborated. ONE positive signal — the evidence names the declared netuid, OR names the claimed + * owner, OR the source independently backs the target host — net of computeGrounding's own cross-origin-redirect + * penalty. + * + * Deliberately 1, not 2: metagraphed is a PUBLIC registry whose own identity-token code comments state that + * corroboration here is "ACCURACY corroboration, NOT ownership gating". Requiring two independent signals would + * hold a large share of legitimate submissions (a real subnet's docs page routinely fails to literally print its + * netuid, and a docs host routinely shares no apex with its source repo), and a false HOLD on good work is the + * expensive mistake for a one-shot gate. One signal still forecloses the case these issues exist to catch: a + * surface whose fetched evidence corroborates NOTHING about the subnet it claims to belong to. + */ +export const SURFACE_GROUNDING_MIN_STRONG = 1; + +/** Max characters read from a probed body. probeFunctionalSurface is explicitly truncation-tolerant (it accepts + * an openapi version key with "paths beyond the window"), so a bounded read is sufficient and keeps a hostile + * or merely enormous response from being pulled into memory whole. */ +export const MAX_PROBE_BODY_CHARS = 64_000; +/** Max characters of extracted text handed to computeGrounding as an evidence snippet. */ +const MAX_EVIDENCE_SNIPPET_CHARS = 8_000; +const PROBE_TIMEOUT_MS = 10_000; +const MAX_PROBE_REDIRECTS = 4; + +// Same browser-like headers source-evidence.ts uses, and for the same reason: major doc hosts answer a missing +// or bot-looking User-Agent with a 403 even for fully public pages, which would false-fail valid surfaces. +const PROBE_FETCH_HEADERS: Readonly> = { + "user-agent": + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36", + accept: "application/json,application/yaml,text/event-stream;q=0.9,text/html;q=0.8,*/*;q=0.5", + "accept-language": "en-US,en;q=0.9", +}; + +/** A single check's three-state outcome. `inconclusive` is NOT a pass and NOT a fail — the check could not run. */ +export type SurfaceCheckOutcome = "pass" | "fail" | "inconclusive"; + +export interface SurfaceCheckResult { + outcome: SurfaceCheckOutcome; + /** Human-readable reason, rendered into the public PR comment when the check gates. */ + detail: string; +} + +/** One fetched URL's result. `ok` is true only for a real 2xx response — every other shape (non-2xx, redirect + * loop, SSRF-rejected host, thrown/timed-out fetch) is a NON-ok probe and therefore inconclusive evidence. */ +export interface SurfaceProbe { + ok: boolean; + httpStatus: number | null; + contentType: string | null; + body: string; + /** The fetch was redirected to a different registrable domain — computeGrounding's bait-and-switch penalty. */ + crossOriginRedirect: boolean; + /** Why the probe is not ok (an outcome token, never raw provider text). */ + error: string | null; +} + +const unreachableProbe = (error: string, httpStatus: number | null = null): SurfaceProbe => ({ + ok: false, + httpStatus, + contentType: null, + body: "", + crossOriginRedirect: false, + error, +}); + +function redirectLocation(response: Response, currentUrl: string): string { + const location = response.headers.get("location"); + if (!location) return ""; + try { + return new URL(location, currentUrl).toString(); + } catch { + return ""; + } +} + +/** + * Fetch one public URL and report what it actually served. SSRF-guarded via `isSafeHttpUrl` on EVERY hop (the + * redirect chain is followed manually for exactly this reason — an https origin that 302s to 127.0.0.1 must not + * be followed), bounded to MAX_PROBE_REDIRECTS hops and a read of MAX_PROBE_BODY_CHARS. + * + * NEVER THROWS: every failure mode collapses into an `ok:false` probe carrying an outcome token, because a + * thrown fetch here must degrade to "inconclusive" (a human looks) rather than escaping into the review + * pipeline or, worse, being caught somewhere upstream and read as a pass. + */ +export async function fetchSurfaceProbe(url: string, fetchImpl: typeof fetch = fetch): Promise { + const startDomain = registrableDomain(url); + let currentUrl = url; + for (let hop = 0; hop <= MAX_PROBE_REDIRECTS; hop += 1) { + if (!isSafeHttpUrl(currentUrl)) return unreachableProbe("probe_url_not_public"); + let response: Response; + try { + response = await fetchImpl(currentUrl, { + method: "GET", + redirect: "manual", + headers: { ...PROBE_FETCH_HEADERS }, + signal: AbortSignal.timeout(PROBE_TIMEOUT_MS), + }); + } catch { + return unreachableProbe("probe_fetch_failed"); + } + if (response.status >= 300 && response.status < 400) { + const nextUrl = redirectLocation(response, currentUrl); + if (!nextUrl) return unreachableProbe("probe_redirect_without_location", response.status); + if (hop === MAX_PROBE_REDIRECTS) return unreachableProbe("probe_too_many_redirects", response.status); + currentUrl = nextUrl; + continue; + } + if (!response.ok) return unreachableProbe("probe_http_error", response.status); + let body: string; + try { + body = (await response.text()).slice(0, MAX_PROBE_BODY_CHARS); + } catch { + // A 2xx whose body cannot be read (a broken/aborted stream) is NOT a served surface — the response line + // alone proves nothing about what the URL serves, so this stays inconclusive rather than an empty pass. + return unreachableProbe("probe_body_unreadable", response.status); + } + return { + ok: true, + httpStatus: response.status, + contentType: response.headers.get("content-type"), + body, + // A redirect that lands on a different registrable domain than the submission declared is the classic + // bait-and-switch; computeGrounding already knows how to penalize it, it just needs to be TOLD. With no + // redirect, currentUrl is still the original url, so this compares equal and reports false. + crossOriginRedirect: registrableDomain(currentUrl) !== startDomain, + error: null, + }; + } + /* v8 ignore next -- unreachable: the loop runs hops 0..MAX inclusive and every path returns (the hop===MAX + redirect returns too_many_redirects); this only satisfies the type checker. */ + return unreachableProbe("probe_too_many_redirects"); +} + +/** `` text of an HTML body, or null. */ +function extractTitle(body: string): string | null { + const match = /<title[^>]*>([\s\S]{0,300}?)<\/title>/i.exec(body); + const raw = match?.[1]; + return raw ? raw.replace(/\s+/g, " ").trim() || null : null; +} + +/** + * A probed body reduced to the plain text computeGrounding matches its netuid/owner/host signals against. + * Script and style blocks are dropped first (their contents are code, not page claims, and a bundled script + * mentioning an unrelated "subnet 14" would otherwise forge a grounding signal), then tags are stripped and + * whitespace collapsed. A non-HTML body (JSON/YAML/SSE) is used as-is — it is already text. + */ +export function probeToEvidence(probe: SurfaceProbe): { title: string; snippet: string; cross_origin_redirect: boolean } { + const isHtml = /html/i.test(probe.contentType ?? "") || /^\s*<(?:!doctype|html)\b/i.test(probe.body); + const text = isHtml + ? probe.body + .replace(/<(script|style)[^>]*>[\s\S]*?<\/\1>/gi, " ") + .replace(/<[^>]+>/g, " ") + .replace(/\s+/g, " ") + .trim() + : probe.body; + return { + title: (isHtml ? extractTitle(probe.body) : null) ?? "", + snippet: text.slice(0, MAX_EVIDENCE_SNIPPET_CHARS), + cross_origin_redirect: probe.crossOriginRedirect, + }; +} + +/** The entry's declared corroborating source URL (`source_url`, else the first `source_urls[]` entry). */ +function entrySourceUrl(entry: CandidateLike): string { + const single = entry.source_url; + if (typeof single === "string" && single.trim()) return single.trim(); + const list = entry.source_urls; + const first = Array.isArray(list) ? list[0] : null; + return typeof first === "string" ? first.trim() : ""; +} + +export interface SurfaceEntryVerification { + grounding: SurfaceCheckResult; + functional: SurfaceCheckResult; + /** What this verification forces onto an entry that already passed static validation. */ + disposition: MetaVerdict; + /** Public, actionable text for a non-merged disposition; null when the entry verified clean. */ + summary: string | null; + /** Machine reason code for a non-merged disposition; null when the entry verified clean. */ + reason: string | null; +} + +/** Reason codes this module can attach. Deliberately NOT added to registry-logic's REVIEWER_CLOSE_REASONS: that + * set exists solely so its private `fail()` helper can map a reason to a verdict, and these assessments are + * built here with an explicit verdict rather than routed through it. */ +export const FUNCTIONAL_NOT_SERVED_REASON = "functional-surface-not-served"; +export const FUNCTIONAL_INCONCLUSIVE_REASON = "functional-probe-inconclusive"; +export const GROUNDING_INCONCLUSIVE_REASON = "grounding-inconclusive"; +export const GROUNDING_UNCONFIRMED_REASON = "grounding-unconfirmed"; + +/** + * Run both live checks for ONE surface entry that has already passed static validation. + * + * Fetches at most two URLs, concurrently: + * - the declared `source_url` — the corroborating evidence #8908's grounding check needs. Its probe is what + * makes grounding conclusive at all: with no readable source there is nothing to corroborate AGAINST, which + * is inconclusive, never a pass. + * - the entry's own `url` — needed by #8909's functional probe, and additionally fed to grounding as target + * evidence. Skipped when it is not an https URL, which is the normal, expected shape for a base-layer + * `wss://` endpoint: grounding then rests on the source evidence alone (still conclusive — computeGrounding + * matches the netuid and the target HOST out of the source body), and no functional kind is a wss kind, so + * nothing silently degrades. + */ +export async function verifySurfaceEntry( + entry: CandidateLike, + opts: { fetchImpl?: typeof fetch } = {}, +): Promise<SurfaceEntryVerification> { + const fetchImpl = opts.fetchImpl ?? fetch; + const sourceUrl = entrySourceUrl(entry); + const targetUrl = typeof entry.url === "string" ? entry.url.trim() : ""; + const targetFetchable = isSafeHttpUrl(targetUrl); + const [sourceProbe, targetProbe] = await Promise.all([ + isSafeHttpUrl(sourceUrl) ? fetchSurfaceProbe(sourceUrl, fetchImpl) : Promise.resolve(unreachableProbe("probe_url_not_public")), + targetFetchable ? fetchSurfaceProbe(targetUrl, fetchImpl) : Promise.resolve(null), + ]); + + const functional = assessFunctional(entry, targetProbe); + const grounding = assessGrounding(entry, sourceProbe, targetProbe); + return decide(grounding, functional); +} + +/** #8909: does the declared url actually serve the surface its `kind` claims? A null `targetProbe` means the + * url was never https-fetchable, so no probe was attempted (see verifySurfaceEntry). */ +function assessFunctional(entry: CandidateLike, targetProbe: SurfaceProbe | null): SurfaceCheckResult { + if (!functionalRequired(entry.kind)) { + return { outcome: "pass", detail: "n/a — this kind declares no functional surface" }; + } + // A functional kind whose url isn't even https-fetchable can't be probed. assessSurfaceEntry's own url safety + // check normally rejects this first, but it is skippable via `sourceUrlValidation:false`, so guard it here too + // rather than letting an unprobed functional surface fall through as verified. + if (targetProbe === null) { + return { outcome: "inconclusive", detail: "the declared url is not a fetchable public HTTPS URL, so the surface could not be probed" }; + } + if (!targetProbe.ok) { + const status = targetProbe.httpStatus === null ? targetProbe.error : `HTTP ${targetProbe.httpStatus}`; + return { outcome: "inconclusive", detail: `the declared url could not be probed (${status})` }; + } + // A 2xx with a blank body proves nothing about what is served; probeFunctionalSurface would read a json + // content-type alone as a served API. Treat a degraded/empty response as inconclusive, never a pass. + if (targetProbe.body.trim() === "") { + return { outcome: "inconclusive", detail: "the declared url returned an empty body, so what it serves could not be confirmed" }; + } + const probe = probeFunctionalSurface(entry.kind, targetProbe.contentType, targetProbe.body); + return probe.served ? { outcome: "pass", detail: probe.detail } : { outcome: "fail", detail: probe.detail }; +} + +/** #8908: does the fetched evidence actually corroborate the declared netuid / owner / host? */ +function assessGrounding(entry: CandidateLike, sourceProbe: SurfaceProbe, targetProbe: SurfaceProbe | null): SurfaceCheckResult { + if (!sourceProbe.ok) { + const status = sourceProbe.httpStatus === null ? sourceProbe.error : `HTTP ${sourceProbe.httpStatus}`; + return { outcome: "inconclusive", detail: `the declared source URL could not be fetched (${status}), so nothing corroborates this entry` }; + } + const signals = computeGrounding( + entry, + targetProbe !== null && targetProbe.ok ? probeToEvidence(targetProbe) : null, + probeToEvidence(sourceProbe), + ); + if (signals.strong >= SURFACE_GROUNDING_MIN_STRONG) { + const named = [ + signals.netuidMentioned ? "netuid" : "", + signals.ownerMentioned ? "owner" : "", + signals.hostMatchesClaim ? "host" : "", + ].filter(Boolean); + return { outcome: "pass", detail: `corroborated by ${named.join(" + ")}` }; + } + const penalty = signals.crossOriginRedirect ? " (a cross-domain redirect was followed, which discounts the evidence)" : ""; + return { + outcome: "fail", + detail: `the fetched evidence corroborates none of the declared netuid, owner, or host${penalty}`, + }; +} + +/** + * The gating decision — the part these issues actually asked to be designed, not just wired. + * + * CLOSE only for a CONFIRMED functional failure (#8909's "holding/closing on served:false"): a 2xx response + * whose body demonstrably is not the declared surface — an `openapi` url serving an HTML marketing page, an + * `sse` url that is not an event stream. That is an objective, reproducible fact about the submission, in the + * same class as the shape/kind violations assessSurfaceEntry already closes on, and it is trivially fixable by + * resubmitting with the right `kind` or the right url. + * + * HOLD (manual review) for everything softer: any inconclusive probe, and a failed GROUNDING check. Grounding + * is a heuristic corroboration score over fetched page text, not a fact about the response — a legitimate + * surface can genuinely fail it (a docs page that never prints its netuid, hosted on a domain unrelated to its + * source repo). Closing on it would one-shot-close good contributions on a heuristic, so it holds for a human + * instead. Crucially it no longer MERGES: before this module, an entry corroborated by nothing at all sailed + * through, which is exactly the gap #8908 describes. + * + * Precedence is fail-closed-first: a confirmed functional failure outranks everything, then any inconclusive + * probe, then unconfirmed grounding. Inconclusive and failed grounding carry DISTINCT reason codes and distinct + * public text even though both hold, so "we could not check" is never rendered as "we checked and disliked it". + */ +function decide(grounding: SurfaceCheckResult, functional: SurfaceCheckResult): SurfaceEntryVerification { + const base = { grounding, functional }; + if (functional.outcome === "fail") { + return { + ...base, + disposition: "closed", + reason: FUNCTIONAL_NOT_SERVED_REASON, + summary: `Surface entry's url does not serve the interface its \`kind\` declares — ${functional.detail}. Resubmit with a url that serves the declared surface, or with the kind that matches what it actually serves.`, + }; + } + if (functional.outcome === "inconclusive") { + return { + ...base, + disposition: "manual-review", + reason: FUNCTIONAL_INCONCLUSIVE_REASON, + summary: `Could not confirm the declared surface is served — ${functional.detail}. Routing to review rather than accepting an unverified functional surface.`, + }; + } + if (grounding.outcome === "inconclusive") { + return { + ...base, + disposition: "manual-review", + reason: GROUNDING_INCONCLUSIVE_REASON, + summary: `Could not verify this entry against its declared source — ${grounding.detail}. Routing to review rather than accepting an unverified claim.`, + }; + } + if (grounding.outcome === "fail") { + return { + ...base, + disposition: "manual-review", + reason: GROUNDING_UNCONFIRMED_REASON, + summary: `The declared source does not corroborate this entry — ${grounding.detail}. Routing to review to confirm the surface really belongs to this subnet.`, + }; + } + return { ...base, disposition: "merged", reason: null, summary: null }; +} + +/** + * Adapt `verifySurfaceEntry` into the orchestrator's `verifyEntry` hook: return an OVERRIDING Assessment when + * live verification downgrades the entry (hold or close), or `null` when it confirms it — in which case the + * entry's existing static "merged" assessment stands unchanged. + */ +export function makeSurfaceEntryVerifier(opts: { fetchImpl?: typeof fetch } = {}): (entry: unknown) => Promise<Assessment | null> { + return async (entry: unknown): Promise<Assessment | null> => { + if (!entry || typeof entry !== "object") return null; + const candidate = entry as CandidateLike; + const verification = await verifySurfaceEntry(candidate, opts); + if (verification.disposition === "merged") return null; + return { + verdict: verification.disposition, + // Both are non-null for every non-merged disposition (see `decide`), so the fallback/spread arms below are + // unreachable; they only satisfy the Assessment field types under exactOptionalPropertyTypes. + /* v8 ignore next */ + summary: verification.summary ?? "Registry surface verification.", + candidate, + /* v8 ignore next */ + ...(verification.reason !== null ? { reason: verification.reason } : {}), + }; + }; +} diff --git a/test/unit/content-lane-flag.test.ts b/test/unit/content-lane-flag.test.ts index 4fc731467d..530b41e18e 100644 --- a/test/unit/content-lane-flag.test.ts +++ b/test/unit/content-lane-flag.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { isContentLaneEnabled } from "../../src/review/content-lane/flag"; +import { isContentLaneEnabled, isSurfaceVerificationEnabled } from "../../src/review/content-lane/flag"; describe("isContentLaneEnabled", () => { it("is OFF by default (unset / empty / undefined env)", () => { @@ -21,3 +21,31 @@ describe("isContentLaneEnabled", () => { } }); }); + +// #8908/#8909: live surface verification is its OWN flag, independent of the lane's, so the first +// outbound-probe behavior in this lane can be rolled back without taking the whole lane down. +describe("isSurfaceVerificationEnabled", () => { + it("is OFF by default (unset / empty / undefined env)", () => { + expect(isSurfaceVerificationEnabled(undefined)).toBe(false); + expect(isSurfaceVerificationEnabled(null)).toBe(false); + expect(isSurfaceVerificationEnabled({})).toBe(false); + expect(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "" })).toBe(false); + }); + + it("is ON for recognized truthy values (case/whitespace insensitive)", () => { + for (const v of ["1", "true", "on", "yes", "TRUE", " On ", "Yes"]) { + expect(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: v })).toBe(true); + } + }); + + it("is OFF for non-truthy strings", () => { + for (const v of ["0", "false", "off", "no", "enabled", "maybe"]) { + expect(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: v })).toBe(false); + } + }); + + it("is independent of the content-lane flag in BOTH directions", () => { + expect(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: "true" })).toBe(false); + expect(isContentLaneEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "true" })).toBe(false); + }); +}); diff --git a/test/unit/content-lane-orchestrator.test.ts b/test/unit/content-lane-orchestrator.test.ts index 04ec893ff9..9b1ac38fce 100644 --- a/test/unit/content-lane-orchestrator.test.ts +++ b/test/unit/content-lane-orchestrator.test.ts @@ -552,3 +552,70 @@ describe("runSurfaceReview (deterministic + decisive: merge/close, rarely manual }); }); }); + +// ── Live entry verification hook (#8908, #8909) ─────────────────────────────────────────────── +// The orchestrator itself stays domain-agnostic: it only knows "run the injected verifier on entries that +// passed static validation, and let it downgrade them". metagraphed's actual verifier (the fetches, the +// grounding/functional checks, the close-vs-hold policy) is tested in content-lane-surface-verification.test.ts. +describe("runSurfaceReview — verifyEntry hook", () => { + const doc = (surfaces: unknown[]) => JSON.stringify({ netuid: 14, surfaces }); + const files = (surfaces: unknown[], base: unknown[] = [existing]) => ({ [`head:${SUBNET}`]: doc(surfaces), [`base:${SUBNET}`]: doc(base) }); + const withVerifier = (surfaces: unknown[], verifyEntry: SurfaceReviewInput["verifyEntry"], base: unknown[] = [existing]) => + runSurfaceReview(METAGRAPHED_LANE_SPEC, { changedFiles: [SUBNET], loadFile: loader(files(surfaces, base)), ...(verifyEntry ? { verifyEntry } : {}) }); + + it("is entirely optional — omitted, the verdict is byte-identical to the pre-verification lane", async () => { + expect(await withVerifier([existing, newEntry], undefined)).toEqual({ verdict: "merge", summary: undefined, reason: undefined }); + }); + + it("keeps the static merged verdict when the verifier confirms the entry (null)", async () => { + const r = await withVerifier([existing, newEntry], () => Promise.resolve(null)); + expect(r?.verdict).toBe("merge"); + }); + + it("CLOSES the PR when the verifier closes an otherwise-valid entry (#8909 served:false)", async () => { + const r = await withVerifier([existing, newEntry], () => + Promise.resolve({ verdict: "closed" as const, summary: "url does not serve the declared surface", candidate: null, reason: "functional-surface-not-served" }), + ); + expect(r).toEqual({ verdict: "close", summary: "url does not serve the declared surface", reason: "functional-surface-not-served" }); + }); + + it("HOLDS the PR when the verifier downgrades an otherwise-valid entry (#8908 ungrounded)", async () => { + const r = await withVerifier([existing, newEntry], () => + Promise.resolve({ verdict: "manual-review" as const, summary: "source does not corroborate", candidate: null, reason: "grounding-unconfirmed" }), + ); + expect(r).toEqual({ verdict: "manual", summary: "source does not corroborate", reason: "grounding-unconfirmed" }); + }); + + it("never verifies an entry that already failed static validation (no wasted network I/O)", async () => { + const seen: unknown[] = []; + // public_safe:false closes statically, so the verifier must never be handed this entry. + const r = await withVerifier([existing, { ...newEntry, public_safe: false }], (entry) => { + seen.push(entry); + return Promise.resolve(null); + }); + expect(r?.verdict).toBe("close"); + expect(seen).toEqual([]); + }); + + it("verifies EACH appended entry independently, and one bad entry closes the whole PR", async () => { + const seen: unknown[] = []; + const r = await withVerifier([existing, newEntry, newEntry2], (entry) => { + seen.push(entry); + const bad = (entry as { kind?: string }).kind === "openapi"; + return Promise.resolve(bad ? { verdict: "closed" as const, summary: "not served", candidate: null, reason: "functional-surface-not-served" } : null); + }); + expect(seen).toEqual([newEntry, newEntry2]); + // pickAggregateAssessment prefixes a multi-entry reason with its position so the offender is identifiable. + expect(r?.verdict).toBe("close"); + expect(r?.summary).toBe("Surface entry 2 of 2: not served"); + }); + + it("HOLDS rather than merging when the verifier itself throws — a broken check never reads as a pass", async () => { + const r = await withVerifier([existing, newEntry], () => Promise.reject(new Error("probe exploded"))); + expect(r).toEqual({ + verdict: "manual", + summary: "Live verification of this surface entry could not be completed — routing to review rather than accepting an unverified entry.", + reason: "verification-error", + }); + }); +}); diff --git a/test/unit/content-lane-surface-verification.test.ts b/test/unit/content-lane-surface-verification.test.ts new file mode 100644 index 0000000000..8cc8be42f4 --- /dev/null +++ b/test/unit/content-lane-surface-verification.test.ts @@ -0,0 +1,438 @@ +import { describe, expect, it } from "vitest"; +import { + FUNCTIONAL_INCONCLUSIVE_REASON, + FUNCTIONAL_NOT_SERVED_REASON, + GROUNDING_INCONCLUSIVE_REASON, + GROUNDING_UNCONFIRMED_REASON, + MAX_PROBE_BODY_CHARS, + SURFACE_GROUNDING_MIN_STRONG, + fetchSurfaceProbe, + makeSurfaceEntryVerifier, + probeToEvidence, + verifySurfaceEntry, +} from "../../src/review/content-lane/surface-verification"; + +// ── fetch stubs ─────────────────────────────────────────────────────────────────────────────── +// A Response-shaped stub. Built by hand rather than with `new Response()` so a body-read FAILURE (a broken +// stream on an otherwise-2xx response) is expressible — that arm is a real degraded-fetch case the gate must +// treat as inconclusive, and a genuine Response can't be made to reject from text(). +const res = (init: { status?: number; headers?: Record<string, string>; body?: string; textThrows?: boolean }): Response => { + const status = init.status ?? 200; + return { + status, + ok: status >= 200 && status < 300, + headers: new Headers(init.headers ?? {}), + text: init.textThrows ? () => Promise.reject(new Error("stream broken")) : () => Promise.resolve(init.body ?? ""), + } as unknown as Response; +}; + +/** A fetch stub routing on the exact request URL; an unmapped URL fails the test loudly rather than silently. */ +const routes = (map: Record<string, Response | "throw">): typeof fetch => + ((url: string | URL): Promise<Response> => { + const hit = map[String(url)]; + if (hit === undefined) return Promise.reject(new Error(`unexpected fetch: ${String(url)}`)); + if (hit === "throw") return Promise.reject(new Error("network down")); + return Promise.resolve(hit); + }) as unknown as typeof fetch; + +const html = (body: string) => res({ headers: { "content-type": "text/html" }, body }); +const json = (body: string) => res({ headers: { "content-type": "application/json" }, body }); + +const TARGET = "https://sn14.example.ai/docs"; +const SOURCE = "https://github.com/acme/sn14"; +/** A website entry whose source page names the subnet → one grounding signal → verifies clean. */ +const groundedEntry = { netuid: 14, kind: "website", url: TARGET, source_url: SOURCE, public_safe: true }; + +describe("fetchSurfaceProbe", () => { + it("returns an ok probe carrying the served status, content-type and body", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: json('{"a":1}') })); + expect(probe.ok).toBe(true); + expect(probe.httpStatus).toBe(200); + expect(probe.contentType).toBe("application/json"); + expect(probe.body).toBe('{"a":1}'); + expect(probe.crossOriginRedirect).toBe(false); + expect(probe.error).toBeNull(); + }); + + it("refuses a non-public URL without ever fetching (SSRF guard)", async () => { + // routes({}) rejects ANY fetch, so reaching the network at all would surface as probe_fetch_failed. + for (const url of ["http://insecure.example/x", "https://127.0.0.1/x", "https://localhost/x", "not a url"]) { + const probe = await fetchSurfaceProbe(url, routes({})); + expect(probe.error).toBe("probe_url_not_public"); + expect(probe.ok).toBe(false); + } + }); + + it("re-applies the SSRF guard on every redirect hop (an https origin cannot 302 into loopback)", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: res({ status: 302, headers: { location: "https://127.0.0.1/admin" } }) })); + expect(probe.error).toBe("probe_url_not_public"); + expect(probe.ok).toBe(false); + }); + + it("follows a same-domain redirect without flagging a cross-origin hop", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ + [TARGET]: res({ status: 301, headers: { location: "https://docs.example.ai/v2" } }), + "https://docs.example.ai/v2": html("<p>hi</p>"), + })); + expect(probe.ok).toBe(true); + expect(probe.crossOriginRedirect).toBe(false); + }); + + it("flags a redirect that lands on a DIFFERENT registrable domain (bait-and-switch)", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ + [TARGET]: res({ status: 302, headers: { location: "https://elsewhere.test/landing" } }), + "https://elsewhere.test/landing": html("<p>hi</p>"), + })); + expect(probe.ok).toBe(true); + expect(probe.crossOriginRedirect).toBe(true); + }); + + it("gives up on a redirect with no Location header", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: res({ status: 302 }) })); + expect(probe.error).toBe("probe_redirect_without_location"); + expect(probe.httpStatus).toBe(302); + }); + + it("gives up on an unparseable Location header", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: res({ status: 302, headers: { location: "http://[bad" } }) })); + expect(probe.error).toBe("probe_redirect_without_location"); + }); + + it("gives up after too many redirect hops", async () => { + // A self-redirect loop: every hop is safe + parseable, so only the hop cap can stop it. + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: res({ status: 302, headers: { location: TARGET } }) })); + expect(probe.error).toBe("probe_too_many_redirects"); + }); + + it("reports a non-2xx as an http error carrying the status", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: res({ status: 404, body: "nope" }) })); + expect(probe.error).toBe("probe_http_error"); + expect(probe.httpStatus).toBe(404); + expect(probe.ok).toBe(false); + }); + + it("never throws when the fetch itself throws", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: "throw" })); + expect(probe.error).toBe("probe_fetch_failed"); + expect(probe.ok).toBe(false); + }); + + it("treats a 2xx whose body cannot be read as unreadable, NOT as an empty pass", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: res({ textThrows: true }) })); + expect(probe.error).toBe("probe_body_unreadable"); + expect(probe.ok).toBe(false); + }); + + it("truncates an oversized body to the probe cap", async () => { + const probe = await fetchSurfaceProbe(TARGET, routes({ [TARGET]: json("x".repeat(MAX_PROBE_BODY_CHARS + 500)) })); + expect(probe.body).toHaveLength(MAX_PROBE_BODY_CHARS); + }); +}); + +describe("probeToEvidence", () => { + const probe = (over: Partial<ReturnType<typeof probeToEvidence>> & { contentType?: string | null; body?: string; crossOriginRedirect?: boolean }) => ({ + ok: true, + httpStatus: 200, + contentType: over.contentType ?? null, + body: over.body ?? "", + crossOriginRedirect: over.crossOriginRedirect ?? false, + error: null, + }); + + it("extracts an HTML title and strips tags from the snippet", () => { + const ev = probeToEvidence(probe({ contentType: "text/html", body: "<html><title> Subnet 14

Hello

" })); + expect(ev.title).toBe("Subnet 14"); + expect(ev.snippet).toContain("Hello"); + expect(ev.snippet).not.toContain("

"); + }); + + it("drops script/style contents so bundled code cannot forge a grounding signal", () => { + const ev = probeToEvidence(probe({ contentType: "text/html", body: "

real

" })); + expect(ev.snippet).not.toContain("subnet 14"); + expect(ev.snippet).toContain("real"); + }); + + it("detects HTML from the body when the content-type does not say so", () => { + const ev = probeToEvidence(probe({ contentType: "text/plain", body: "T

x

" })); + expect(ev.title).toBe("T"); + expect(ev.snippet).not.toContain("

"); + }); + + it("uses a non-HTML body verbatim and reports no title", () => { + const ev = probeToEvidence(probe({ contentType: "application/json", body: '{"netuid":14}' })); + expect(ev.title).toBe(""); + expect(ev.snippet).toBe('{"netuid":14}'); + }); + + it("reports an empty title for HTML with no title tag, and for a blank title tag", () => { + expect(probeToEvidence(probe({ contentType: "text/html", body: "

x

" })).title).toBe(""); + expect(probeToEvidence(probe({ contentType: "text/html", body: "

x

" })).title).toBe(""); + }); + + it("propagates the cross-origin-redirect flag through to the grounding evidence", () => { + expect(probeToEvidence(probe({ crossOriginRedirect: true })).cross_origin_redirect).toBe(true); + expect(probeToEvidence(probe({ crossOriginRedirect: false })).cross_origin_redirect).toBe(false); + }); +}); + +// ── #8908: evidence-corroboration grounding is now actually consulted ───────────────────────── +describe("verifySurfaceEntry — grounding (#8908)", () => { + it("merges an entry whose fetched source corroborates the declared netuid", async () => { + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: html("

This repo powers subnet 14 on Bittensor.

") }), + }); + expect(v.grounding.outcome).toBe("pass"); + expect(v.grounding.detail).toContain("netuid"); + expect(v.disposition).toBe("merged"); + expect(v.summary).toBeNull(); + expect(v.reason).toBeNull(); + }); + + it("HOLDS (never merges) an entry whose fetched evidence corroborates nothing — the #8908 gap", async () => { + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

welcome

"), [SOURCE]: html("

an unrelated project

") }), + }); + expect(v.grounding.outcome).toBe("fail"); + expect(v.disposition).toBe("manual-review"); + expect(v.reason).toBe(GROUNDING_UNCONFIRMED_REASON); + expect(v.summary).toContain("does not corroborate"); + }); + + it("holds as INCONCLUSIVE — distinctly from a fail — when the source URL cannot be fetched", async () => { + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: "throw" }), + }); + expect(v.grounding.outcome).toBe("inconclusive"); + expect(v.disposition).toBe("manual-review"); + // The whole point of the tri-state: "we could not check" must never render as "we checked and disliked it". + expect(v.reason).toBe(GROUNDING_INCONCLUSIVE_REASON); + expect(v.reason).not.toBe(GROUNDING_UNCONFIRMED_REASON); + expect(v.summary).toContain("Could not verify"); + }); + + it("holds as inconclusive when the source URL returns a non-2xx, naming the status", async () => { + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: res({ status: 503 }) }), + }); + expect(v.reason).toBe(GROUNDING_INCONCLUSIVE_REASON); + expect(v.grounding.detail).toContain("HTTP 503"); + }); + + it("holds as inconclusive when the entry declares no fetchable source URL at all", async () => { + const v = await verifySurfaceEntry({ ...groundedEntry, source_url: undefined }, { fetchImpl: routes({ [TARGET]: html("

x

") }) }); + expect(v.reason).toBe(GROUNDING_INCONCLUSIVE_REASON); + expect(v.grounding.detail).toContain("probe_url_not_public"); + }); + + it("falls back to source_urls[0] when source_url is absent", async () => { + const v = await verifySurfaceEntry( + { netuid: 14, kind: "website", url: TARGET, source_urls: [SOURCE], public_safe: true }, + { fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: html("

netuid 14

") }) }, + ); + expect(v.grounding.outcome).toBe("pass"); + expect(v.disposition).toBe("merged"); + }); + + it("ignores a non-string source_urls[0] rather than fetching it", async () => { + const v = await verifySurfaceEntry( + { netuid: 14, kind: "website", url: TARGET, source_urls: [{ url: SOURCE }], public_safe: true }, + { fetchImpl: routes({ [TARGET]: html("

x

") }) }, + ); + expect(v.reason).toBe(GROUNDING_INCONCLUSIVE_REASON); + }); + + it("grounds from the TARGET page too, not only the source", async () => { + // The source body names neither netuid nor host; the target page names the netuid. + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

Docs for subnet 14

"), [SOURCE]: html("

readme

") }), + }); + expect(v.grounding.outcome).toBe("pass"); + }); + + it("still grounds a base-layer wss entry from its source alone (the target is not http-fetchable)", async () => { + const wssEntry = { netuid: 14, kind: "subtensor-wss", url: "wss://chain.example.ai", source_url: SOURCE, public_safe: true }; + const v = await verifySurfaceEntry(wssEntry, { fetchImpl: routes({ [SOURCE]: html("

archive for subnet 14

") }) }); + expect(v.grounding.outcome).toBe("pass"); + expect(v.functional.outcome).toBe("pass"); // not a functional kind + expect(v.disposition).toBe("merged"); + }); + + it("discounts a cross-domain redirect and says so when that tips grounding below the threshold", async () => { + // One positive signal (netuid named) minus the cross-origin penalty ⇒ strong 0 ⇒ below the minimum. + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ + [TARGET]: html("

docs

"), + [SOURCE]: res({ status: 302, headers: { location: "https://elsewhere.test/r" } }), + "https://elsewhere.test/r": html("

subnet 14

"), + }), + }); + expect(v.grounding.outcome).toBe("fail"); + expect(v.grounding.detail).toContain("cross-domain redirect"); + expect(v.reason).toBe(GROUNDING_UNCONFIRMED_REASON); + }); + + it("grounds on the claimed OWNER token alone, and names it", async () => { + // The source repo's owner ("acme") appears in the evidence; the netuid never does. + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: html("

Built and maintained by acme.

") }), + }); + expect(v.grounding.outcome).toBe("pass"); + expect(v.grounding.detail).toBe("corroborated by owner"); + expect(v.disposition).toBe("merged"); + }); + + it("grounds on the target HOST being referenced by the source body, and names every signal that fired", async () => { + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: html("

Docs live at sn14.example.ai today.

") }), + }); + expect(v.grounding.outcome).toBe("pass"); + // All three fire here, and deliberately so: the host string "sn14.example.ai" carries the owner token + // "sn14" AND satisfies netuidGroundingRegex(14) (the "sn"+"14" subnet-slug form it is built to recognize). + expect(v.grounding.detail).toBe("corroborated by netuid + owner + host"); + }); + + it("treats a blank source_url as absent and falls back to source_urls[0]", async () => { + const v = await verifySurfaceEntry( + { netuid: 14, kind: "website", url: TARGET, source_url: " ", source_urls: [SOURCE], public_safe: true }, + { fetchImpl: routes({ [TARGET]: html("

docs

"), [SOURCE]: html("

subnet 14

") }) }, + ); + expect(v.grounding.outcome).toBe("pass"); + }); + + it("pins the shipped threshold at one independent signal", () => { + expect(SURFACE_GROUNDING_MIN_STRONG).toBe(1); + }); +}); + +// ── #8909: functional-surface probing is now actually consulted ─────────────────────────────── +describe("verifySurfaceEntry — functional probing (#8909)", () => { + const OPENAPI = "https://api.example.ai/openapi.json"; + const openapiEntry = { netuid: 14, kind: "openapi", url: OPENAPI, source_url: SOURCE, public_safe: true }; + const groundingSource = html("

subnet 14 api

"); + + it("merges an openapi entry whose url really serves a spec", async () => { + const v = await verifySurfaceEntry(openapiEntry, { + fetchImpl: routes({ [OPENAPI]: json('{"openapi":"3.0.0","paths":{}}'), [SOURCE]: groundingSource }), + }); + expect(v.functional.outcome).toBe("pass"); + expect(v.disposition).toBe("merged"); + }); + + it("CLOSES an openapi entry whose url serves an HTML page instead — the #8909 gap", async () => { + const v = await verifySurfaceEntry(openapiEntry, { + fetchImpl: routes({ [OPENAPI]: html("

We support OpenAPI

"), [SOURCE]: groundingSource }), + }); + expect(v.functional.outcome).toBe("fail"); + expect(v.disposition).toBe("closed"); + expect(v.reason).toBe(FUNCTIONAL_NOT_SERVED_REASON); + expect(v.summary).toContain("does not serve the interface"); + }); + + it("CLOSES an sse entry that is not an event stream", async () => { + const SSE = "https://api.example.ai/stream"; + const v = await verifySurfaceEntry( + { netuid: 14, kind: "sse", url: SSE, source_url: SOURCE, public_safe: true }, + { fetchImpl: routes({ [SSE]: json("{}"), [SOURCE]: groundingSource }) }, + ); + expect(v.disposition).toBe("closed"); + expect(v.reason).toBe(FUNCTIONAL_NOT_SERVED_REASON); + }); + + it("merges an sse entry serving text/event-stream", async () => { + const SSE = "https://api.example.ai/stream"; + const v = await verifySurfaceEntry( + { netuid: 14, kind: "sse", url: SSE, source_url: SOURCE, public_safe: true }, + { fetchImpl: routes({ [SSE]: res({ headers: { "content-type": "text/event-stream" }, body: "data: x" }), [SOURCE]: groundingSource }) }, + ); + expect(v.disposition).toBe("merged"); + }); + + it("HOLDS — never closes and never passes — when the functional probe cannot reach the url", async () => { + const v = await verifySurfaceEntry(openapiEntry, { fetchImpl: routes({ [OPENAPI]: "throw", [SOURCE]: groundingSource }) }); + expect(v.functional.outcome).toBe("inconclusive"); + expect(v.disposition).toBe("manual-review"); + expect(v.reason).toBe(FUNCTIONAL_INCONCLUSIVE_REASON); + expect(v.summary).toContain("Could not confirm"); + }); + + it("holds as inconclusive on a non-2xx, naming the status rather than closing", async () => { + const v = await verifySurfaceEntry(openapiEntry, { fetchImpl: routes({ [OPENAPI]: res({ status: 502 }), [SOURCE]: groundingSource }) }); + expect(v.reason).toBe(FUNCTIONAL_INCONCLUSIVE_REASON); + expect(v.functional.detail).toContain("HTTP 502"); + }); + + it("holds as inconclusive when the url returns 2xx with an EMPTY body (a json content-type alone proves nothing)", async () => { + const v = await verifySurfaceEntry( + { netuid: 14, kind: "subnet-api", url: OPENAPI, source_url: SOURCE, public_safe: true }, + { fetchImpl: routes({ [OPENAPI]: json(" "), [SOURCE]: groundingSource }) }, + ); + expect(v.functional.outcome).toBe("inconclusive"); + expect(v.reason).toBe(FUNCTIONAL_INCONCLUSIVE_REASON); + expect(v.functional.detail).toContain("empty body"); + }); + + it("holds as inconclusive when a functional entry's url is not an https URL at all", async () => { + // Reachable only with sourceUrlValidation disabled upstream; the verifier must not pass an unprobed surface. + const v = await verifySurfaceEntry( + { netuid: 14, kind: "openapi", url: "http://api.example.ai/spec", source_url: SOURCE, public_safe: true }, + { fetchImpl: routes({ [SOURCE]: groundingSource }) }, + ); + expect(v.functional.outcome).toBe("inconclusive"); + expect(v.reason).toBe(FUNCTIONAL_INCONCLUSIVE_REASON); + expect(v.functional.detail).toContain("not a fetchable public HTTPS URL"); + }); + + it("skips the probe entirely for a kind that declares no functional surface", async () => { + const v = await verifySurfaceEntry(groundedEntry, { + fetchImpl: routes({ [TARGET]: html("

just a website

"), [SOURCE]: html("

subnet 14

") }), + }); + expect(v.functional.outcome).toBe("pass"); + expect(v.functional.detail).toContain("n/a"); + expect(v.disposition).toBe("merged"); + }); + + it("a confirmed functional failure OUTRANKS an inconclusive grounding check (fail-closed first)", async () => { + const v = await verifySurfaceEntry(openapiEntry, { fetchImpl: routes({ [OPENAPI]: html("

docs

"), [SOURCE]: "throw" }) }); + expect(v.grounding.outcome).toBe("inconclusive"); + expect(v.disposition).toBe("closed"); + expect(v.reason).toBe(FUNCTIONAL_NOT_SERVED_REASON); + }); + + it("an inconclusive functional probe outranks a failed grounding check", async () => { + const v = await verifySurfaceEntry(openapiEntry, { + fetchImpl: routes({ [OPENAPI]: "throw", [SOURCE]: html("

unrelated

") }), + }); + expect(v.grounding.outcome).toBe("fail"); + expect(v.disposition).toBe("manual-review"); + expect(v.reason).toBe(FUNCTIONAL_INCONCLUSIVE_REASON); + }); +}); + +describe("makeSurfaceEntryVerifier", () => { + it("returns null (the static merged assessment stands) for a clean entry", async () => { + const verify = makeSurfaceEntryVerifier({ fetchImpl: routes({ [TARGET]: html("

x

"), [SOURCE]: html("

subnet 14

") }) }); + expect(await verify(groundedEntry)).toBeNull(); + }); + + it("returns an overriding manual-review assessment for an unverified entry", async () => { + const verify = makeSurfaceEntryVerifier({ fetchImpl: routes({ [TARGET]: html("

x

"), [SOURCE]: html("

nothing

") }) }); + const assessment = await verify(groundedEntry); + expect(assessment?.verdict).toBe("manual-review"); + expect(assessment?.reason).toBe(GROUNDING_UNCONFIRMED_REASON); + expect(assessment?.candidate).toEqual(groundedEntry); + }); + + it("returns an overriding closed assessment for a functional surface that is not served", async () => { + const OPENAPI = "https://api.example.ai/openapi.json"; + const verify = makeSurfaceEntryVerifier({ fetchImpl: routes({ [OPENAPI]: html("

hi

"), [SOURCE]: html("

subnet 14

") }) }); + const assessment = await verify({ netuid: 14, kind: "openapi", url: OPENAPI, source_url: SOURCE, public_safe: true }); + expect(assessment?.verdict).toBe("closed"); + expect(assessment?.reason).toBe(FUNCTIONAL_NOT_SERVED_REASON); + }); + + it("returns null for a non-object entry rather than fetching anything", async () => { + const verify = makeSurfaceEntryVerifier({ fetchImpl: routes({}) }); + expect(await verify(null)).toBeNull(); + expect(await verify("nope")).toBeNull(); + }); +}); diff --git a/test/unit/content-lane-wire.test.ts b/test/unit/content-lane-wire.test.ts index e4ff17748e..c4b4faf775 100644 --- a/test/unit/content-lane-wire.test.ts +++ b/test/unit/content-lane-wire.test.ts @@ -836,3 +836,55 @@ describe("evaluateWithSurfaceLane (the processor seam helper)", () => { expect(advisory.findings).toEqual([]); }); }); + +// ── Live surface verification wiring (#8908, #8909) ─────────────────────────────────────────── +// The verifier's own checks are tested in content-lane-surface-verification.test.ts; what matters HERE is the +// flag gating — the shipped default must make no outbound probe and leave the verdict byte-identical. +describe("runRegistrySurfaceGate — LOOPOVER_REVIEW_SURFACE_VERIFICATION gating", () => { + const stub = { [`head:${SUBNET}`]: doc([existing, newEntry]), [`base:${SUBNET}`]: doc([existing]) }; + const files = [{ path: SUBNET, status: "modified" }]; + const run = (flagEnv: Record, verifyOverride?: SurfaceReviewInput["verifyEntry"]) => + runRegistrySurfaceGate( + flagEnv as unknown as Env, + METAGRAPHED_LANE_SPEC, + { installationId: 0, repoFullName: REPO, pr: { headSha: "HEAD", baseRef: "BASE" }, advisory: { findings: [] as AdvisoryFinding[] }, files }, + loader(stub), + verifyOverride, + ); + + it("flag OFF (the shipped default) makes NO outbound probe and merges exactly as before", async () => { + // Any real fetch would reject and surface as a hold/close; a clean success proves none was attempted. + vi.stubGlobal("fetch", () => Promise.reject(new Error("no network expected"))); + for (const env of [{}, { LOOPOVER_REVIEW_SURFACE_VERIFICATION: "false" }]) { + expect((await run(env))?.conclusion).toBe("success"); + } + }); + + it("flag ON runs verification, so an entry the verifier closes fails the gate", async () => { + const out = await run({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "true" }, () => + Promise.resolve({ verdict: "closed" as const, summary: "url does not serve the declared surface", candidate: null, reason: "functional-surface-not-served" }), + ); + expect(out?.conclusion).toBe("failure"); + expect(out?.blockers[0]?.code).toBe("surface_lane_reject"); + }); + + it("flag ON, an ungrounded entry HOLDS (neutral) rather than merging or closing", async () => { + const out = await run({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "true" }, () => + Promise.resolve({ verdict: "manual-review" as const, summary: "source does not corroborate", candidate: null, reason: "grounding-unconfirmed" }), + ); + expect(out?.conclusion).toBe("neutral"); + expect(out?.blockers).toEqual([]); + expect(out?.warnings[0]?.code).toBe("surface_lane_manual"); + }); + + it("flag ON with a real (unstubbed) verifier holds an entry whose URLs cannot be reached", async () => { + // No verifyEntry override: exercises the production makeSurfaceEntryVerifier path end-to-end. `newEntry` is + // a subnet-api (a FUNCTIONAL kind), so with every fetch failing the functional probe is inconclusive and + // outranks the equally-inconclusive grounding check — and an unreachable surface HOLDS, never merges. + vi.stubGlobal("fetch", () => Promise.reject(new Error("network down"))); + const out = await run({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "true" }); + expect(out?.conclusion).toBe("neutral"); + expect(out?.summary).toContain("Could not confirm the declared surface is served"); + expect(out?.warnings[0]?.code).toBe("surface_lane_manual"); + }); +}); diff --git a/worker-configuration.d.ts b/worker-configuration.d.ts index 486ab2b33e..19015af4aa 100644 --- a/worker-configuration.d.ts +++ b/worker-configuration.d.ts @@ -1,5 +1,5 @@ /* eslint-disable */ -// Generated by Wrangler by running `wrangler types` (hash: 12ab7fcab3ef47d0e7c428770fc3c36a) +// Generated by Wrangler by running `wrangler types` (hash: e0e315bc488417c1ee9bcfcfbe213400) // Runtime types generated with workerd@1.20260722.1 2026-05-28 nodejs_compat interface __BaseEnv_Env { REVIEW_AUDIT: R2Bucket; @@ -40,6 +40,7 @@ interface __BaseEnv_Env { LOOPOVER_REVIEW_CULTURE_PROFILE: "false"; LOOPOVER_REVIEW_MEMORY: "false"; LOOPOVER_REVIEW_CONTENT_LANE: "false"; + LOOPOVER_REVIEW_SURFACE_VERIFICATION: "false"; LOOPOVER_REVIEW_SELFTUNE: "false"; LOOPOVER_EXPERIMENTAL_GITTENSOR: "false"; LOOPOVER_MAINTAINER_RECAP: "false"; @@ -113,6 +114,7 @@ declare namespace NodeJS { | "LOOPOVER_REVIEW_SAFETY" | "LOOPOVER_REVIEW_SCREENSHOTS" | "LOOPOVER_REVIEW_SELFTUNE" + | "LOOPOVER_REVIEW_SURFACE_VERIFICATION" | "LOOPOVER_SKIP_AUTOMATION_BOT_PRS" | "LOOPOVER_SWEEP_WATCHDOG" | "PUBLIC_API_ORIGIN" diff --git a/wrangler.jsonc b/wrangler.jsonc index 809b046a6e..b0c67be631 100644 --- a/wrangler.jsonc +++ b/wrangler.jsonc @@ -163,6 +163,16 @@ // OFF (false): the processor takes no new branch + resolves no files, so the gate disposition is byte- // identical until deliberately enabled per-repo. See review/content-lane-wire. "LOOPOVER_REVIEW_CONTENT_LANE": "false", + // Convergence (surface LIVE VERIFICATION, #8908/#8909): when truthy AND the surface lane above is active for + // the repo, a surface entry that passes static validation is additionally checked against what its URLs + // actually serve — computeGrounding over the fetched source/target evidence (is the declared netuid/owner/ + // host corroborated at all?) and probeFunctionalSurface for the openapi/subnet-api/sse kinds (does the url + // serve the interface its kind claims?). A confirmed not-served functional surface CLOSES; unconfirmed + // grounding and any inconclusive probe HOLD for review — an inconclusive check never reads as a pass. + // Default OFF (false): no outbound probe is made and the surface verdict is byte-identical to today. Its own + // flag, separate from the lane, so the first outbound-fetch behavior in this lane can be rolled back alone. + // See review/content-lane/surface-verification. + "LOOPOVER_REVIEW_SURFACE_VERIFICATION": "false", // Convergence (self-improve / auto-tune): run the ported self-improvement loop on the cron tick over // loopover's own review-outcome data — compute tuning recommendations, SHADOW-SOAK any strictly- // tightening one, and AUTO-PROMOTE it to live ONLY after the soak window passes the gate; every action is