Skip to content

Commit b09cd9c

Browse files
authored
feat(review): gate surface entries on live grounding and functional-surface checks (#8908, #8909) (#9255)
* feat(review): gate surface entries on live grounding and functional-surface checks (#8908, #8909) `computeGrounding` and `probeFunctionalSurface` have been fully implemented and unit-tested in registry-logic.ts since the metagraphed port, but neither had a single caller outside its own test file. The live surface-entry gate (assessSurfaceEntry -> runSurfaceReview) merged a submission having never once looked at what its URLs actually serve: it validated that `url`/`source_url` were well-formed public HTTPS/WSS URLs and stopped there. Add the missing half — the fetch plumbing and the gating decision — without touching either primitive: - surface-verification.ts: SSRF-guarded, redirect-following, truncation-tolerant probes of the entry's `source_url` and `url`; evidence extraction (title + tag-stripped text, script/style dropped so bundled code cannot forge a signal); and the close-vs-hold policy. - orchestrator: one optional injected `verifyEntry` hook, applied per appended entry, only to entries that already passed static validation. The orchestrator stays pure and domain-agnostic — any registry can supply its own verifier. - content-lane-wire: builds the verifier when the new flag is on. Policy. A CONFIRMED functional failure (a 2xx whose body demonstrably is not the declared surface — an `openapi` url serving HTML) CLOSES: an objective, reproducible, trivially-fixable fact, in the same class as the shape violations the entry validator already closes on. Unconfirmed grounding HOLDS rather than closes — it is a heuristic over page text, and a legitimate surface can fail it, so closing on it would one-shot-close good contributions. It no longer merges, which is the actual #8908 gap. Three-state, never fail-open. Each check resolves to pass / fail / inconclusive and these stay strictly distinct: an unreachable probe, a 2xx with an unreadable or empty body, a missing source, or a throwing verifier all HOLD with their own reason code, never a pass. A check that silently passes when it could not run launders "we didn't look" into "we verified". Flag-gated and OFF by default, on its own flag rather than the content lane's: this is the lane's first outbound request to submitter-controlled URLs and it can hold or close submissions that merge today, so both need independent rollback. Flag-off, no probe is made and the verdict is byte-identical. * docs(selfhost): document the surface-verification flag in tuning and privacy-security * test(engine): cover the surface-verification flag in the engine's own node:test suite The host vitest test for isSurfaceVerificationEnabled/isContentLaneEnabled (test/unit/content-lane-flag.test.ts) doesn't count toward the engine package's own Codecov flag -- that's measured from packages/loopover-engine's node:test suite via c8 (#9064), which had no equivalent test for this file.
1 parent d04d0fe commit b09cd9c

15 files changed

Lines changed: 1139 additions & 6 deletions

apps/loopover-ui/content/docs/privacy-security.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ LOOPOVER_REVIEW_OPS="true" # read-only anomaly scan + outcome stats endpoint
6868
LOOPOVER_REVIEW_SELFTUNE="true" # self-tightening tuning loop, never loosens
6969
LOOPOVER_REVIEW_PARITY_AUDIT="true" # shadow-record gate-decision parity readiness
7070
LOOPOVER_REVIEW_CONTENT_LANE="true" # dedicated content/registry-repo review lane
71+
LOOPOVER_REVIEW_SURFACE_VERIFICATION="true" # fetch + verify registry surface entries (needs the content lane too)
7172
LOOPOVER_REVIEW_DRAFT="true" # public draft-submission (contributor fork PR) flow`}
7273
/>
7374

apps/loopover-ui/content/docs/tuning.mdx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,15 @@ any per-PR feature to run on a given repo. So a per-PR feature activates only wh
165165
registries) through the dedicated content lane — duplicate detection, source-evidence
166166
reachability, security scanning, scope classification, registry grounding — instead of the
167167
code gate. Global.
168+
- `LOOPOVER_REVIEW_SURFACE_VERIFICATION` — live verification of registry surface
169+
entries, on top of the content lane above (both must be on). An entry that passes the
170+
static shape/safety checks is additionally fetched: its declared source is checked for
171+
evidence corroborating the claimed netuid, owner, or host, and an `openapi` /
172+
`subnet-api` / `sse` entry is probed to confirm its URL really serves the interface its
173+
`kind` declares. A confirmed not-served surface **closes**; unverified corroboration and
174+
any **inconclusive** probe **hold for review** — an inconclusive check is never reported
175+
as a pass. This is the only content-lane capability that makes outbound requests to
176+
submitter-supplied URLs, so it is a separate flag you can roll back on its own. Global.
168177
- `LOOPOVER_REVIEW_DRAFT` — the public draft-submission flow (the
169178
`/v1/drafts` endpoints: contributor draft → GitHub OAuth → fork PR). With the
170179
flag off every draft endpoint 404s. Requires the

packages/loopover-engine/src/review/content-lane/flag.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,27 @@
1515
export interface ContentLaneEnv {
1616
/** When truthy ("1"/"true"/"on"/"yes"), the content lane is enabled. Default OFF. */
1717
LOOPOVER_REVIEW_CONTENT_LANE?: string;
18+
/** When truthy, a surface entry that passes STATIC validation is ADDITIONALLY verified against live content
19+
* (#8908 evidence grounding, #8909 functional-surface probing) before it can merge. Default OFF. */
20+
LOOPOVER_REVIEW_SURFACE_VERIFICATION?: string;
1821
}
1922

2023
/** Is the content lane enabled? Default OFF — only a recognized truthy flag turns it on. */
2124
export function isContentLaneEnabled(env: ContentLaneEnv | undefined | null): boolean {
2225
if (!env) return false;
2326
return /^(1|true|yes|on)$/i.test((env.LOOPOVER_REVIEW_CONTENT_LANE ?? "").trim());
2427
}
28+
29+
/**
30+
* Is LIVE surface-entry verification enabled (#8908, #8909)? Default OFF, and deliberately its OWN flag rather
31+
* than riding on LOOPOVER_REVIEW_CONTENT_LANE: it is the first thing in this lane that makes OUTBOUND requests
32+
* to submitter-controlled URLs, and it can HOLD or CLOSE submissions that merge today. Both properties need to
33+
* be rollable back on their own without taking the whole (already-cutover) content lane down with them.
34+
*
35+
* The caller ANDs this with the lane's existing activation, so verification runs only where a RegistryLaneSpec
36+
* already resolved — i.e. it inherits the per-repo scoping for free and needs no config-surface of its own.
37+
*/
38+
export function isSurfaceVerificationEnabled(env: ContentLaneEnv | undefined | null): boolean {
39+
if (!env) return false;
40+
return /^(1|true|yes|on)$/i.test((env.LOOPOVER_REVIEW_SURFACE_VERIFICATION ?? "").trim());
41+
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { test } from "node:test";
2+
import assert from "node:assert/strict";
3+
4+
import { isContentLaneEnabled, isSurfaceVerificationEnabled } from "../dist/review/content-lane/flag.js";
5+
6+
test("isContentLaneEnabled is OFF by default (unset / empty / undefined env)", () => {
7+
assert.equal(isContentLaneEnabled(undefined), false);
8+
assert.equal(isContentLaneEnabled(null), false);
9+
assert.equal(isContentLaneEnabled({}), false);
10+
assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: "" }), false);
11+
});
12+
13+
test("isContentLaneEnabled is ON for recognized truthy values (case/whitespace insensitive)", () => {
14+
for (const v of ["1", "true", "on", "yes", "TRUE", " On ", "Yes"]) {
15+
assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: v }), true);
16+
}
17+
});
18+
19+
test("isContentLaneEnabled is OFF for non-truthy strings", () => {
20+
for (const v of ["0", "false", "off", "no", "enabled", "maybe"]) {
21+
assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: v }), false);
22+
}
23+
});
24+
25+
// #8908/#8909: live surface verification is its OWN flag, independent of the lane's, so the first
26+
// outbound-probe behavior in this lane can be rolled back without taking the whole lane down.
27+
test("isSurfaceVerificationEnabled is OFF by default (unset / empty / undefined env)", () => {
28+
assert.equal(isSurfaceVerificationEnabled(undefined), false);
29+
assert.equal(isSurfaceVerificationEnabled(null), false);
30+
assert.equal(isSurfaceVerificationEnabled({}), false);
31+
assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "" }), false);
32+
});
33+
34+
test("isSurfaceVerificationEnabled is ON for recognized truthy values (case/whitespace insensitive)", () => {
35+
for (const v of ["1", "true", "on", "yes", "TRUE", " On ", "Yes"]) {
36+
assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: v }), true);
37+
}
38+
});
39+
40+
test("isSurfaceVerificationEnabled is OFF for non-truthy strings", () => {
41+
for (const v of ["0", "false", "off", "no", "enabled", "maybe"]) {
42+
assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: v }), false);
43+
}
44+
});
45+
46+
test("isSurfaceVerificationEnabled is independent of the content-lane flag in BOTH directions", () => {
47+
assert.equal(isSurfaceVerificationEnabled({ LOOPOVER_REVIEW_CONTENT_LANE: "true" }), false);
48+
assert.equal(isContentLaneEnabled({ LOOPOVER_REVIEW_SURFACE_VERIFICATION: "true" }), false);
49+
});

src/env.d.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,16 @@ declare global {
516516
* gate disposition byte-identical. AI-FREE (pure structured-data adjudication), so independent of the AI
517517
* reviewer; a generic hard blocker (e.g. a committed secret) is always preserved over a surface "merge". */
518518
LOOPOVER_REVIEW_CONTENT_LANE?: string;
519+
/** Convergence (surface LIVE VERIFICATION, #8908/#8909): when truthy *AND* the surface lane above is active
520+
* for the repo, a surface entry that passes STATIC validation is additionally checked against what its URLs
521+
* actually serve — `computeGrounding` over the fetched source/target evidence (is the declared netuid /
522+
* owner / host corroborated at all?) and `probeFunctionalSurface` for the openapi/subnet-api/sse kinds (does
523+
* the url serve the interface its `kind` claims?). A CONFIRMED not-served functional surface CLOSES;
524+
* unconfirmed grounding and any INCONCLUSIVE probe HOLD for review — an inconclusive check is never reported
525+
* as a pass. Default OFF: unset/false makes no outbound probe and leaves the surface verdict byte-identical.
526+
* Its own flag, separate from the lane's, so the first outbound-fetch behavior in this lane can be rolled
527+
* back on its own — see review/content-lane/surface-verification. */
528+
LOOPOVER_REVIEW_SURFACE_VERIFICATION?: string;
519529
/** Convergence (self-improve / auto-tune): when truthy, the ported self-improvement loop
520530
* (src/review/auto-tune.ts + auto-apply.ts) runs on the cron tick over loopover's OWN review-outcome
521531
* data — it computes tuning recommendations, SHADOW-SOAKS any STRICTLY-TIGHTENING recommendation in the

src/review/content-lane-wire.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,11 @@
3737
// genuinely critical finding, or one of these other configured gates, still wins outright).
3838
import { AI_JUDGMENT_BLOCKER_CODES, type GateCheckEvaluation, isAiJudgmentOnlyFailure, isDuplicateOnlyFailure } from "../rules/advisory";
3939
import { LOOPOVER_GATE_CHECK_NAME } from "./check-names";
40-
import { isContentLaneEnabled } from "./content-lane/flag";
40+
import { isContentLaneEnabled, isSurfaceVerificationEnabled } from "./content-lane/flag";
4141
import { runSurfaceReview, type SurfaceReviewInput, type SurfaceReviewResult } from "./content-lane/orchestrator";
4242
import type { RegistryLaneSpec } from "./content-lane/registry-logic";
4343
import { registeredValidatorIds, resolveRegistryLaneSpec, unregisteredValidatorId } from "./content-lane/spec-resolver";
44+
import { makeSurfaceEntryVerifier } from "./content-lane/surface-verification";
4445
import { makeGithubFileFetcher } from "./grounding-wire";
4546
import { MAX_FETCH_CHARS } from "./review-grounding";
4647
import type { FocusManifest } from "../signals/focus-manifest";
@@ -185,6 +186,7 @@ export async function runRegistrySurfaceGate(
185186
files: { path: string; status?: string | null | undefined }[];
186187
},
187188
loadFileOverride?: SurfaceReviewInput["loadFile"],
189+
verifyEntryOverride?: SurfaceReviewInput["verifyEntry"],
188190
): Promise<GateCheckEvaluation | null> {
189191
let fetcherPromise: ReturnType<typeof makeGithubFileFetcher> | null = null;
190192
const githubLoad = async (path: string, ref: "head" | "base"): Promise<string | null> => {
@@ -219,10 +221,15 @@ export async function runRegistrySurfaceGate(
219221
if (ref === "base" && content === null && statusByPath.get(path) === "modified") deferUnreadable = true;
220222
return content;
221223
};
224+
// #8908/#8909: live verification is its OWN flag on top of the lane's activation, so it can be rolled back
225+
// without taking the (already-cutover) surface lane down. Flag-OFF, `verifyEntry` is undefined, the
226+
// orchestrator runs no verification and makes no outbound probe, and the verdict is byte-identical to today.
227+
const verifyEntry = verifyEntryOverride ?? (isSurfaceVerificationEnabled(env) ? makeSurfaceEntryVerifier() : undefined);
222228
const result = await runSurfaceReview(spec, {
223229
changedFiles: args.files.map((file) => file.path),
224230
loadFile,
225231
opts: { secretsScan: true, sourceUrlValidation: true },
232+
...(verifyEntry ? { verifyEntry } : {}),
226233
});
227234
if (result === null) return null; // not a registry submission → the generic gate applies
228235
if (deferUnreadable) return null; // a fetch blip on a file that must be readable → defer, never auto-close

src/review/content-lane/index.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,20 @@ export {
116116
type ProviderLike,
117117
type Verdict,
118118
} from "./registry-logic";
119+
export {
120+
fetchSurfaceProbe,
121+
makeSurfaceEntryVerifier,
122+
probeToEvidence,
123+
verifySurfaceEntry,
124+
FUNCTIONAL_INCONCLUSIVE_REASON,
125+
FUNCTIONAL_NOT_SERVED_REASON,
126+
GROUNDING_INCONCLUSIVE_REASON,
127+
GROUNDING_UNCONFIRMED_REASON,
128+
MAX_PROBE_BODY_CHARS,
129+
SURFACE_GROUNDING_MIN_STRONG,
130+
type SurfaceCheckOutcome,
131+
type SurfaceCheckResult,
132+
type SurfaceEntryVerification,
133+
type SurfaceProbe,
134+
} from "./surface-verification";
119135
export { runSurfaceReview, diffAppendedSurfaceEntries, type SurfaceReviewInput, type SurfaceReviewResult } from "./orchestrator";

src/review/content-lane/orchestrator.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ export interface SurfaceReviewInput {
3030
/** Loads decoded file content at a ref; injected so unit tests need no network. Returns null when absent. */
3131
loadFile: (path: string, ref: "head" | "base") => Promise<string | null>;
3232
opts?: { secretsScan?: boolean; sourceUrlValidation?: boolean };
33+
/** Optional LIVE content verification for an entry that already passed static validation (#8908, #8909) —
34+
* injected I/O, exactly like `loadFile`, so the orchestrator stays pure and domain-agnostic and any registry
35+
* can supply its own verifier (metagraphed's is `makeSurfaceEntryVerifier` in ./surface-verification).
36+
* Returns an OVERRIDING Assessment when verification downgrades the entry (hold or close), or `null` when it
37+
* confirms it (the static assessment stands). Omitted ⇒ no verification runs and no fetch is made, so the
38+
* verdict is byte-identical to the pre-#8908 lane. */
39+
verifyEntry?: (entry: unknown) => Promise<Assessment | null>;
3340
}
3441

3542
export interface SurfaceReviewResult {
@@ -192,6 +199,42 @@ function pickAggregateAssessment(assessments: Assessment[]): Assessment {
192199
return first as Assessment;
193200
}
194201

202+
/**
203+
* Apply the injected live-content verifier (#8908, #8909) to every appended entry that passed STATIC validation,
204+
* and return the per-entry assessments with any downgrade folded in.
205+
*
206+
* Only "merged" entries are verified, for two reasons: an entry already closing or held has its verdict decided
207+
* (verification could only ever confirm it), and skipping them means an invalid submission — the common bad case
208+
* — pays for no network I/O at all. The surviving entries are verified CONCURRENTLY (independent URLs, no data
209+
* dependency), mirroring how this function's caller already parallelizes its GitHub reads.
210+
*
211+
* A verifier that THROWS must never take the review down or, worse, be swallowed into a pass: the entry is held
212+
* for review instead. `makeSurfaceEntryVerifier` is itself written not to throw, so this is a belt-and-braces
213+
* guard against a future/third-party verifier that is less careful.
214+
*/
215+
async function verifyMergedEntries(
216+
staticAssessments: Assessment[],
217+
appendedEntries: readonly unknown[],
218+
verifyEntry: SurfaceReviewInput["verifyEntry"],
219+
): Promise<Assessment[]> {
220+
if (!verifyEntry) return staticAssessments;
221+
return await Promise.all(
222+
staticAssessments.map(async (assessment, idx) => {
223+
if (assessment.verdict !== "merged") return assessment;
224+
try {
225+
return (await verifyEntry(appendedEntries[idx])) ?? assessment;
226+
} catch {
227+
return {
228+
verdict: "manual-review" as const,
229+
summary: "Live verification of this surface entry could not be completed — routing to review rather than accepting an unverified entry.",
230+
candidate: assessment.candidate,
231+
reason: "verification-error",
232+
};
233+
}
234+
}),
235+
);
236+
}
237+
195238
/**
196239
* Adjudication policy (deterministic, DECISIVE): the overwhelming majority of outcomes are merge or close —
197240
* 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
279322
return { verdict: "manual", summary: NO_VALIDATOR_ENTRY_SUMMARY };
280323
}
281324
const headDoc = safeParseJson(headRaw);
282-
const assessment = pickAggregateAssessment(
283-
appendedEntries.map((appendedEntry) => assessEntry(headDoc, { ...input.opts, appendedEntry })),
284-
);
325+
const staticAssessments = appendedEntries.map((appendedEntry) => assessEntry(headDoc, { ...input.opts, appendedEntry }));
326+
const assessment = pickAggregateAssessment(await verifyMergedEntries(staticAssessments, appendedEntries, input.verifyEntry));
285327
if (companionProviderFile !== null) {
286328
// Guaranteed non-null: the no-validator short-circuit above already returned when this spec lacks one.
287329
const assessProvider = spec.assessProviderEntry as NonNullable<RegistryLaneSpec["assessProviderEntry"]>;

0 commit comments

Comments
 (0)