diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index bffdbd6fcb..4f68ef2c6c 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -11,6 +11,7 @@ import { z } from "zod"; import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js"; import { formatTable } from "../lib/format-table.js"; import { argsWantJson, describeCliError, reportCliFailure } from "../lib/cli-error.js"; +import { redactKnownLocalPaths, redactLocalPath } from "../lib/redact-local-path.js"; // Read name/version from this package's own package.json (always present in any install -- // global, npx, or local -- npm ships it regardless of the "files" allowlist) instead of hand-synced @@ -3707,17 +3708,10 @@ function parseDurationMs(value) { function sanitizeValidationText(value, maxLength = 240) { const text = String(value ?? "").replace(/[\r\n\t]+/g, " ").trim(); if (!text) return undefined; - const redacted = redactPrivateValidationMetrics(redactLocalValidationPaths(text)); + const redacted = redactPrivateValidationMetrics(redactLocalPath(text)); return redacted.length <= maxLength ? redacted : `${redacted.slice(0, maxLength - 3)}...`; } -function redactLocalValidationPaths(text) { - const pathSegment = "[^\\\\/\\s\"'`,;)]+(?:\\s+[^\\\\/\\s\"'`,;)]+)*(?=[\\\\/])"; - const pathTail = "[^\\\\/\\s\"'`,;)]+"; - const localPathPattern = new RegExp(`(^|[\\s"'\\\`=])((?:~[\\\\/]|[A-Za-z]:[\\\\/]|/)(?:${pathSegment}[\\\\/])*${pathTail})`, "g"); - return text.replace(localPathPattern, (_, prefix) => `${prefix}`); -} - function redactPrivateValidationMetrics(text) { return text.replace( /\b(?:wallet|hotkey|coldkey|mnemonic|raw[-_\s]?trust|private[-_\s]?reviewability|trust[-_\s]?score)\b(?:\s*[:=]\s*(?:"[^"]*"|'[^']*'|[^\s"'`,;)]+))?/gi, @@ -3943,7 +3937,7 @@ function isForbiddenCacheKey(key) { } function sanitizeCacheString(value) { - return redactPrivateValidationMetrics(redactLocalValidationPaths(sanitizeDiagnosticText(value))); + return redactPrivateValidationMetrics(redactLocalPath(sanitizeDiagnosticText(value))); } function decisionPackCacheFiles() { @@ -4032,30 +4026,16 @@ function findExecutable(name) { } function sanitizeDiagnosticText(value, extraPaths = []) { - if (value === undefined || value === null) return value; - let text = String(value); - const sensitiveValues = [ - process.env.LOOPOVER_API_TOKEN, - process.env.LOOPOVER_MCP_TOKEN, - process.env.LOOPOVER_TOKEN, - config.session?.token, - ...profileSessions(config).map((entry) => entry.session.token), - ].filter((candidate) => typeof candidate === "string" && candidate.length > 0); - for (const token of sensitiveValues) { - text = text.split(token).join("[redacted]"); - } - const localPaths = [ - configPath, - process.env.LOOPOVER_CONFIG_PATH, - process.env.LOOPOVER_CONFIG_DIR, - process.cwd(), - homedir(), - ...extraPaths, - ].filter((candidate) => typeof candidate === "string" && candidate.length > 1); - for (const localPath of localPaths.sort((left, right) => right.length - left.length)) { - text = text.split(localPath).join("[local-path]"); - } - return text; + return redactKnownLocalPaths(value, { + tokens: [ + process.env.LOOPOVER_API_TOKEN, + process.env.LOOPOVER_MCP_TOKEN, + process.env.LOOPOVER_TOKEN, + config.session?.token, + ...profileSessions(config).map((entry) => entry.session.token), + ], + paths: [configPath, process.env.LOOPOVER_CONFIG_PATH, process.env.LOOPOVER_CONFIG_DIR, process.cwd(), homedir(), ...extraPaths], + }); } function loadConfig() { diff --git a/packages/loopover-mcp/lib/local-branch.js b/packages/loopover-mcp/lib/local-branch.js index 11b225b972..1faa522417 100644 --- a/packages/loopover-mcp/lib/local-branch.js +++ b/packages/loopover-mcp/lib/local-branch.js @@ -3,8 +3,10 @@ import { realpathSync } from "node:fs"; import { dirname, isAbsolute, join, relative, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { isCodeFile, isTestPath as isTestFile } from "@loopover/engine/signals/test-evidence"; +import { redactLocalPath } from "./redact-local-path.js"; export { isCodeFile, isTestFile }; +export { redactLocalPath }; const packageRoot = join(dirname(fileURLToPath(import.meta.url)), ".."); @@ -211,14 +213,6 @@ export function referenceScorePreviewExample(kind = "metadata") { return `${interpreter} ./node_modules/@loopover/mcp/scripts/${script}`; } -export function redactLocalPath(value) { - const text = String(value ?? ""); - if (!text) return text; - return text - .replace(/(?:~\/|[A-Za-z]:\\)[^\s"'`,;)]+/g, "") - .replace(/(^|[\s"'`=])\/(?:[^\s"'`,;)]+(?:\/[^\s"'`,;)]+)*)/g, (_, prefix) => `${prefix}`); -} - export function redactScorerCommand(command) { const text = String(command ?? "").trim(); if (!text) return text; diff --git a/packages/loopover-mcp/lib/redact-local-path.js b/packages/loopover-mcp/lib/redact-local-path.js new file mode 100644 index 0000000000..ddd5745418 --- /dev/null +++ b/packages/loopover-mcp/lib/redact-local-path.js @@ -0,0 +1,55 @@ +// #6264: the one shared local-filesystem-path redactor for the MCP CLI. Three call sites used to +// carry their own copy of this logic (`redactLocalPath` here, `redactLocalValidationPaths` and +// `sanitizeDiagnosticText` in bin/loopover-mcp.js), so a single redaction fix had to be made — and +// kept in sync — three times. They are consolidated here so a future fix happens once. +// +// Two genuinely different mechanisms are needed, so both stay available as named functions rather +// than being forced into one: +// - `redactLocalPath` DETECTS an unknown absolute/home path in free text via a regex heuristic +// (stack traces, scorer stderr, pasted validation output) → ``. +// - `redactKnownLocalPaths` redacts KNOWN sensitive strings (session tokens, config dirs, cwd/home) +// supplied by the caller, by exact substring substitution → `[redacted]` / +// `[local-path]`. It cannot detect an arbitrary path; the heuristic cannot +// redact a token it was never told about. Each solves a distinct problem. + +/** + * Redact any absolute or home-anchored local path found in free text, replacing it with the + * `` placeholder. Heuristic (matches an unknown path by shape), so it never needs the + * concrete path in advance — the counterpart to the exact-match `redactKnownLocalPaths` below. + */ +export function redactLocalPath(value) { + const text = String(value ?? ""); + if (!text) return text; + // Both `/g` patterns are rebuilt per call so no `lastIndex` state carries between invocations. + // Delimiter-anchored roots (`~/`, `~\`, `C:\`, `C:/`, `/`) whose interior segments may contain + // spaces, e.g. `/Users/Alice Smith/project` — the anchoring prefix is preserved, only the path swaps. + const pathSegment = "[^\\\\/\\s\"'`,;)]+(?:\\s+[^\\\\/\\s\"'`,;)]+)*(?=[\\\\/])"; + const pathTail = "[^\\\\/\\s\"'`,;)]+"; + const rootedPath = new RegExp(`(^|[\\s"'\\\`=])((?:~[\\\\/]|[A-Za-z]:[\\\\/]|/)(?:${pathSegment}[\\\\/])*${pathTail})`, "g"); + return text + .replace(rootedPath, (_, prefix) => `${prefix}`) + // Home/Windows roots that appear mid-token with no leading delimiter (so the anchored pass skips + // them); run second so it only mops up what the anchored, space-aware pass could not claim. + .replace(/(?:~\/|[A-Za-z]:\\)[^\s"'`,;)]+/g, ""); +} + +/** + * Redact KNOWN sensitive strings from free text by exact substring substitution: every entry of + * `tokens` becomes `[redacted]` and every entry of `paths` becomes `[local-path]`. Non-string / + * empty entries are ignored; a token must be non-empty and a path longer than one character (a bare + * `/` is not a "known path"). Paths are applied longest-first so a nested path (e.g. cwd under home) + * is redacted before a shorter prefix would swallow its tail. `undefined`/`null` pass through + * untouched so callers can hand diagnostics straight in. + */ +export function redactKnownLocalPaths(value, { tokens = [], paths = [] } = {}) { + if (value === undefined || value === null) return value; + let text = String(value); + for (const token of tokens) { + if (typeof token === "string" && token.length > 0) text = text.split(token).join("[redacted]"); + } + const knownPaths = paths.filter((candidate) => typeof candidate === "string" && candidate.length > 1); + for (const localPath of knownPaths.sort((left, right) => right.length - left.length)) { + text = text.split(localPath).join("[local-path]"); + } + return text; +} diff --git a/packages/loopover-mcp/package.json b/packages/loopover-mcp/package.json index b7e2d9cd80..515f9ba59f 100644 --- a/packages/loopover-mcp/package.json +++ b/packages/loopover-mcp/package.json @@ -35,7 +35,7 @@ "CHANGELOG.md" ], "scripts": { - "build": "node --check bin/loopover-mcp.js && node --check lib/cli-error.js && node --check lib/local-branch.js && node --check lib/format-table.js && node --check scripts/gittensor-score-preview.mjs" + "build": "node --check bin/loopover-mcp.js && node --check lib/cli-error.js && node --check lib/local-branch.js && node --check lib/format-table.js && node --check lib/redact-local-path.js && node --check scripts/gittensor-score-preview.mjs" }, "dependencies": { "@loopover/engine": "^3.0.0", diff --git a/scripts/check-mcp-package.mjs b/scripts/check-mcp-package.mjs index a10884204c..60b46138b8 100644 --- a/scripts/check-mcp-package.mjs +++ b/scripts/check-mcp-package.mjs @@ -14,7 +14,7 @@ if (result.status !== 0) { const [pack] = JSON.parse(result.stdout); const files = pack.files.map((file) => file.path).sort(); -const allowed = [/^bin\/loopover-mcp\.js$/, /^lib\/cli-error\.js$/, /^lib\/local-branch\.js$/, /^lib\/format-table\.js$/, /^scripts\/gittensor-score-preview\.(mjs|py)$/, /^package\.json$/, /^README\.md$/, /^CHANGELOG\.md$/, /^LICENSE$/]; +const allowed = [/^bin\/loopover-mcp\.js$/, /^lib\/cli-error\.js$/, /^lib\/local-branch\.js$/, /^lib\/format-table\.js$/, /^lib\/redact-local-path\.js$/, /^scripts\/gittensor-score-preview\.(mjs|py)$/, /^package\.json$/, /^README\.md$/, /^CHANGELOG\.md$/, /^LICENSE$/]; const forbiddenPath = /(^|\/)(\.dev\.vars|\.env|\.npmrc|.*\.pem|.*private.*key.*|.*secret.*)$/i; const forbiddenContent = /(BEGIN (RSA |EC |OPENSSH )?PRIVATE KEY|github_pat_[A-Za-z0-9_]+|gh[pousr]_[A-Za-z0-9_]+|gts_[0-9a-f]{64}|[A-Z0-9_]*(TOKEN|SECRET|PRIVATE_KEY)=)/; const stalePackageText = /(private beta|zeronode\.workers\.dev|preview URL)/i; diff --git a/test/unit/redact-local-path.test.ts b/test/unit/redact-local-path.test.ts new file mode 100644 index 0000000000..1e988ea202 --- /dev/null +++ b/test/unit/redact-local-path.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; + +// #6264: the three former MCP redaction call sites now share packages/loopover-mcp/lib/redact-local-path.js. +// This is the single home for the redaction contract, so it is tested once here; the call-site tests +// (local-scorer-adapter.test.ts, mcp-cli-packets.test.ts) still assert the wired-up behavior end to end. +// @ts-expect-error package helper is plain JS because the local wrapper ships as a Node bin package. +const { redactLocalPath, redactKnownLocalPaths } = await import("../../packages/loopover-mcp/lib/redact-local-path.js"); + +describe("redactLocalPath (heuristic: detect an unknown path in free text)", () => { + it("redacts an absolute unix path, preserving any leading delimiter", () => { + expect(redactLocalPath("/home/user/proj/file.js")).toBe(""); + expect(redactLocalPath("failed under /home/user/proj/file.js")).toBe("failed under "); + expect(redactLocalPath('config="/etc/app/conf.d/main"')).toBe('config=""'); + }); + + it("redacts a path whose segments contain spaces (the space-aware validation shape)", () => { + expect(redactLocalPath("node /Users/Alice Smith/project/run.js")).toBe("node "); + }); + + it("redacts both Windows slash forms and a home-anchored path", () => { + expect(redactLocalPath("log=C:\\Users\\Alice Smith\\raw.log")).toBe("log="); + expect(redactLocalPath("C:/Users/bob/tmp/x")).toBe(""); + expect(redactLocalPath("~/secrets/key.pem")).toBe(""); + expect(redactLocalPath("~\\AppData\\Local\\thing")).toBe(""); + }); + + it("still redacts a home/Windows root that appears mid-token without a leading delimiter", () => { + const redacted = redactLocalPath("see~/private/notes here"); + expect(redacted).toContain(""); + expect(redacted).not.toContain("~/private"); + }); + + it("leaves text with no local path untouched, including a bare slash", () => { + expect(redactLocalPath("just some text, version 1.2.3")).toBe("just some text, version 1.2.3"); + expect(redactLocalPath("pass --flag / or | here")).toBe("pass --flag / or | here"); + }); + + it("coerces nullish and empty input to an empty string", () => { + expect(redactLocalPath(undefined)).toBe(""); + expect(redactLocalPath(null)).toBe(""); + expect(redactLocalPath("")).toBe(""); + }); +}); + +describe("redactKnownLocalPaths (exact substitution: redact a KNOWN token/path)", () => { + it("replaces known tokens with [redacted] and known paths with [local-path]", () => { + expect(redactKnownLocalPaths("token abc123 at /home/me/app", { tokens: ["abc123"], paths: ["/home/me/app"] })).toBe( + "token [redacted] at [local-path]", + ); + }); + + it("applies the longest known path first so a shorter prefix cannot swallow the tail", () => { + expect(redactKnownLocalPaths("/home/me/app/src/x", { paths: ["/home/me", "/home/me/app/src"] })).toBe("[local-path]/x"); + }); + + it("ignores empty tokens, one-character paths, and non-string entries", () => { + expect(redactKnownLocalPaths("keep / and x", { tokens: ["", 123 as unknown as string], paths: ["/", null as unknown as string] })).toBe( + "keep / and x", + ); + }); + + it("coerces a non-string value before substituting", () => { + expect(redactKnownLocalPaths(123, { tokens: ["2"] })).toBe("1[redacted]3"); + }); + + it("passes undefined/null through untouched and defaults its options", () => { + expect(redactKnownLocalPaths(undefined)).toBeUndefined(); + expect(redactKnownLocalPaths(null)).toBeNull(); + expect(redactKnownLocalPaths("plain text")).toBe("plain text"); + }); +});