test(scenarios): grade the #8884 guard via its engine-src path

packages/loopover-engine/src/index.ts

@@ -919,3 +919,8 @@ export { generateAnonSecret, hmacAnonymize } from "./telemetry/anonymize.js";
 // Pure PR-target-key parser (#4882) -- parses `"<owner>/<repo>#<number>"` into its parts; extracted so the
 // D1-heavy repositories access layer no longer carries this stranded pure logic.
 export { parsePullRequestTargetKey } from "./parse-pull-request-target-key.js";
+
+// Local-branch scenario-input safety guard (#8884) -- shared by the backend scenario-input model and the
+// loopover-mcp production local-branch collector so both enforce the same forbidden-source-upload-key /
+// oversized-content, metadata-only contract on the real collection path.
+export { assertScenarioLocalBranchInputSafe } from "./scenario-input-safety.js";

packages/loopover-engine/src/scenario-input-safety.ts

@@ -0,0 +1,40 @@
+// Shared local-branch scenario-input safety guard. Lives in the engine so BOTH the LoopOver backend
+// (src/scenarios/input-model.ts re-exports it) and the loopover-mcp production local-branch collector
+// (packages/loopover-mcp/lib/local-branch.ts) enforce the exact same metadata-only contract -- the
+// forbidden-source-upload-key / oversized-content scan is the documented safety mechanism and must run
+// on the real collection path, not just its own unit test.
+const FORBIDDEN_SOURCE_UPLOAD_KEYS =
+  /^(?:sourceContent|sourceContents|fileContent|fileContents|rawSource|rawSourceContent|content|contents|diff|patch|rawDiff)$/i;
+
+export function assertScenarioLocalBranchInputSafe(payload: Record<string, unknown>): void {
+  if (/^(1|true|yes)$/i.test(String(process.env.LOOPOVER_UPLOAD_SOURCE ?? "false"))) {
+    throw new Error("LOOPOVER_UPLOAD_SOURCE=true is not supported; scenario inputs remain metadata-only.");
+  }
+  for (const key of Object.keys(payload)) {
+    if (FORBIDDEN_SOURCE_UPLOAD_KEYS.test(key)) {
+      throw new Error(`Refusing scenario local-branch field ${key}; source contents are never uploaded.`);
+    }
+  }
+  const changedFiles = payload.changedFiles;
+  // #8328: a present-but-non-array changedFiles (a plain object like { diff: "…source…" }, a string, a number)
+  // is not the documented array-of-entries shape, and the Array.isArray guard below would silently skip the
+  // entire forbidden-key/oversize scan for it — letting exactly the source content this validator exists to
+  // refuse slip through unchecked. Reject it outright; an omitted / undefined changedFiles stays allowed.
+  if (changedFiles !== undefined && !Array.isArray(changedFiles)) {
+    throw new Error("Refusing non-array changedFiles; an array of file entries is required.");
+  }
+  if (Array.isArray(changedFiles)) {
+    for (const entry of changedFiles) {
+      if (!entry || typeof entry !== "object") continue;
+      for (const nestedKey of Object.keys(entry as Record<string, unknown>)) {
+        if (FORBIDDEN_SOURCE_UPLOAD_KEYS.test(nestedKey)) {
+          throw new Error(`Refusing changedFiles.${nestedKey}; source contents are never uploaded.`);
+        }
+        const value = (entry as Record<string, unknown>)[nestedKey];
+        if (typeof value === "string" && value.length > 4000) {
+          throw new Error("Refusing oversized changedFiles payload; metadata-only paths are required.");
+        }
+      }
+    }
+  }
+}

packages/loopover-mcp/lib/local-branch.ts

@@ -2,6 +2,7 @@ import { execFileSync } from "node:child_process";
 import { realpathSync } from "node:fs";
 import { isAbsolute, join, relative, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
+import { assertScenarioLocalBranchInputSafe } from "@loopover/engine";
 import { isCodeFile, isTestPath as isTestFile } from "@loopover/engine/signals/test-evidence";
 import { redactLocalPath } from "./redact-local-path.js";
 
@@ -122,7 +123,10 @@ export function collectLocalDiff(cwd: string, baseRef: string, workspaceRoots?:
 }
 
 export function collectLocalBranchMetadata(input: CollectLocalBranchMetadataInput): LocalBranchMetadata {
-  assertSourceUploadDisabled();
+  // #8884: run the shared forbidden-source-upload-key / oversized-content scan on the real collection
+  // entry point (not just the old env-flag-only assertSourceUploadDisabled check) so any source-content
+  // field smuggled into the metadata-only scenario input is refused before anything is collected.
+  assertScenarioLocalBranchInputSafe(input);
   const workspace = resolveWorkspaceCwd(input);
   const cwd = workspace.cwd;
   const baseRef = input.baseRef ?? defaultBaseRef(cwd);
@@ -704,12 +708,6 @@ function splitCommand(command: unknown): string[] {
   return String(command).match(/(?:[^\s"]+|"[^"]*")+/g)?.map((part) => part.replace(/^"|"$/g, "")) ?? [];
 }
 
-function assertSourceUploadDisabled(): void {
-  if (/^(1|true|yes)$/i.test(process.env.LOOPOVER_UPLOAD_SOURCE ?? "false")) {
-    throw new Error("LOOPOVER_UPLOAD_SOURCE=true is not supported in v1; local MCP sends metadata only.");
-  }
-}
-
 // Word-boundary the closing keywords (as the server-side extractors in src/db/repositories.ts and
 // src/signals/engine.ts already do) so a keyword embedded in a longer word does not spuriously link an
 // issue: without \b, `hotfix 5` / `prefixes 12` matched the `fix`/`fixes` substring and captured the

src/scenarios/input-model.ts

@@ -1,6 +1,11 @@
 import { z } from "zod";
 import { sanitizePublicComment } from "../github/commands";
 
+// #8884: the local-branch scenario-input safety guard now lives in @loopover/engine so the loopover-mcp
+// production collector (packages/loopover-mcp/lib/local-branch.ts) can share the exact same scan instead
+// of a weaker env-flag-only check. Re-exported here to keep this module's public surface unchanged.
+export { assertScenarioLocalBranchInputSafe } from "@loopover/engine";
+
 export const SCENARIO_INPUT_VERSION = 1 as const;
 export const SCENARIO_MAX_REPO_FULL_NAME_CHARS = 200;
 export const SCENARIO_MAX_BRANCH_REF_CHARS = 200;
@@ -33,9 +38,6 @@ export type ScenarioSignalSource = (typeof scenarioSignalSources)[number];
 export const FORBIDDEN_PUBLIC_LANGUAGE =
   /wallet|hotkey|coldkey|mnemonic|seed phrase|payout|estimated[-\s]?rewards?|rewards?|reward[-\s]?estimate|rankings?|farming|raw trust|trust[-\s]?score|scoreability|private[-\s]?reviewability|public[-\s]?score[-\s]?(?:estimate|prediction)/i;
 
-const FORBIDDEN_SOURCE_UPLOAD_KEYS =
-  /^(?:sourceContent|sourceContents|fileContent|fileContents|rawSource|rawSourceContent|content|contents|diff|patch|rawDiff)$/i;
-
 const scenarioSignalEntrySchema = z
   .object({
     id: z.string().min(1).max(120),
@@ -187,39 +189,6 @@ export function buildScenarioInput(args: {
   });
 }
 
-export function assertScenarioLocalBranchInputSafe(payload: Record<string, unknown>): void {
-  if (/^(1|true|yes)$/i.test(String(process.env.LOOPOVER_UPLOAD_SOURCE ?? "false"))) {
-    throw new Error("LOOPOVER_UPLOAD_SOURCE=true is not supported; scenario inputs remain metadata-only.");
-  }
-  for (const key of Object.keys(payload)) {
-    if (FORBIDDEN_SOURCE_UPLOAD_KEYS.test(key)) {
-      throw new Error(`Refusing scenario local-branch field ${key}; source contents are never uploaded.`);
-    }
-  }
-  const changedFiles = payload.changedFiles;
-  // #8328: a present-but-non-array changedFiles (a plain object like { diff: "…source…" }, a string, a number)
-  // is not the documented array-of-entries shape, and the Array.isArray guard below would silently skip the
-  // entire forbidden-key/oversize scan for it — letting exactly the source content this validator exists to
-  // refuse slip through unchecked. Reject it outright; an omitted / undefined changedFiles stays allowed.
-  if (changedFiles !== undefined && !Array.isArray(changedFiles)) {
-    throw new Error("Refusing non-array changedFiles; an array of file entries is required.");
-  }
-  if (Array.isArray(changedFiles)) {
-    for (const entry of changedFiles) {
-      if (!entry || typeof entry !== "object") continue;
-      for (const nestedKey of Object.keys(entry as Record<string, unknown>)) {
-        if (FORBIDDEN_SOURCE_UPLOAD_KEYS.test(nestedKey)) {
-          throw new Error(`Refusing changedFiles.${nestedKey}; source contents are never uploaded.`);
-        }
-        const value = (entry as Record<string, unknown>)[nestedKey];
-        if (typeof value === "string" && value.length > 4000) {
-          throw new Error("Refusing oversized changedFiles payload; metadata-only paths are required.");
-        }
-      }
-    }
-  }
-}
-
 export function scenarioInputFromLocalBranchMetadata(args: {
   scenarioType: ScenarioType;
   login: string;

test/unit/local-branch.test.ts

@@ -1980,6 +1980,18 @@ describe("local MCP git metadata collection", () => {
     expect(() => collectLocalBranchMetadata({ cwd: tempDir, baseRef: "HEAD", login: "oktofeesh1" })).toThrow(/not supported/);
   });
 
+  it("refuses forbidden source-upload fields and oversized changedFiles on the production collector (#8884)", async () => {
+    const { collectLocalBranchMetadata } = await import("../../packages/loopover-mcp/lib/local-branch.js");
+    delete process.env.LOOPOVER_UPLOAD_SOURCE;
+    // The scan runs before any git access, so a bare forbidden-key input is rejected outright -- no repo needed.
+    expect(() => collectLocalBranchMetadata({ login: "oktofeesh1", sourceContent: "secret source" } as never)).toThrow(/never uploaded/i);
+    expect(() => collectLocalBranchMetadata({ login: "oktofeesh1", changedFiles: [{ path: "a.ts", diff: "code" }] } as never)).toThrow(/never uploaded/i);
+    expect(() => collectLocalBranchMetadata({ login: "oktofeesh1", changedFiles: [{ path: "a.ts", body: "x".repeat(5000) }] } as never)).toThrow(
+      /oversized changedFiles/i,
+    );
+    expect(() => collectLocalBranchMetadata({ login: "oktofeesh1", changedFiles: { diff: "x".repeat(5000) } } as never)).toThrow(/non-array changedFiles/i);
+  });
+
   it("selects and validates cwd from MCP roots without leaking local paths", async () => {
     const { collectLocalBranchMetadata, normalizeMcpWorkspaceRoots, resolveWorkspaceCwd } = await import("../../packages/loopover-mcp/lib/local-branch.js");
     tempDir = mkdtempSync(join(tmpdir(), "loopover-local-"));

test/unit/scenario-input-model.test.ts

@@ -4,7 +4,6 @@ import {
   SCENARIO_MAX_LINKED_ISSUE_NUMBERS,
   SCENARIO_MAX_REPO_FULL_NAME_CHARS,
   SCENARIO_MAX_SIGNAL_DETAIL_CHARS,
-  assertScenarioLocalBranchInputSafe,
   buildScenarioInput,
   createScenarioSignalEntry,
   normalizeScenarioInput,
@@ -13,6 +12,10 @@ import {
   serializeScenarioInputPrivate,
   serializeScenarioInputPublic,
 } from "../../src/scenarios/input-model";
+// Import the guard from its engine-src home (#8884) so v8 attributes coverage to
+// packages/loopover-engine/src/scenario-input-safety.ts rather than the built dist copy reached via the
+// @loopover/engine barrel. src/scenarios/input-model.ts re-exports the same symbol for the public surface.
+import { assertScenarioLocalBranchInputSafe } from "../../packages/loopover-engine/src/scenario-input-safety";
 
 const FORBIDDEN_PUBLIC_LANGUAGE =
   /wallet|hotkey|coldkey|mnemonic|seed phrase|payout|estimated[-\s]?rewards?|rewards?|reward[-\s]?estimate|rankings?|farming|raw trust|trust[-\s]?score|scoreability|private[-\s]?reviewability|public[-\s]?score[-\s]?(?:estimate|prediction)/i;