Skip to content

refactor(mcp): consolidate local-path redaction into one shared module - #6339

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-6264
Jul 16, 2026
Merged

refactor(mcp): consolidate local-path redaction into one shared module#6339
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
nghetienhiep:fix/issue-6264

Conversation

@nghetienhiep

Copy link
Copy Markdown
Contributor

Summary

  • Consolidates the three separately-maintained local-path-redaction implementations in the MCP CLI into one shared module, packages/loopover-mcp/lib/redact-local-path.js, so a future redaction fix (e.g. the sibling (-prefix bug in fix(mcp): local-path redaction regex misses the most common stack-trace shape, risking a real path leak upstream #6258) only has to happen once.
  • Both mechanisms the call sites actually need stay available as named functions in the one module: redactLocalPath (regex heuristic that DETECTS an unknown path in free text → <local-path>) and redactKnownLocalPaths (exact substring substitution that redacts KNOWN tokens/paths → [redacted] / [local-path]). They solve different problems, so neither is forced onto the other.
  • All three former call sites now import from the shared module:
    • lib/local-branch.js re-exports and uses redactLocalPath (was a local copy).
    • bin/loopover-mcp.js sanitizeValidationText / sanitizeCacheString use redactLocalPath (the former local redactLocalValidationPaths is removed).
    • bin/loopover-mcp.js sanitizeDiagnosticText is now a thin wrapper over redactKnownLocalPaths, passing the same session tokens and config/cwd/home paths it gathered before.

The consolidated heuristic is the strict superset of the two former regex variants (the space-aware, both-slash validation pattern plus the mid-token home/Windows-root pass), so no call site redacts less than it did before — it only ever redacts the same or more. Behavior at each of the three sites is preserved; the (-prefix leak fix itself is out of scope here and left for #6258, which now becomes a one-line change in this single module.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #6264).

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage — the changed source lives entirely in Codecov-ignored paths (packages/loopover-mcp/**, scripts/**, test/**; coverage include is src/**, packages/loopover-engine/src/**, packages/loopover-miner/lib/**), so this diff has no instrumented lines and no codecov/patch obligation. Verified behavior instead by running the full unit suite: 16739 passed. The only failures (15, in miner-discover-cli, miner-live-issue-snapshot, selfhost-ams-reporting) are pre-existing and environment-only (sqlite3 binary / GitHub token absent locally) — reproduced identically with the change stashed, and none of those files reference the changed code.
  • npm run test:workers — not applicable; no worker (src/**) code changed.
  • npm run build:mcp
  • npm run test:mcp-pack (new lib/redact-local-path.js added to the package allowlist)
  • npm run ui:openapi:check — not applicable; no API/schema change.
  • npm run ui:lint / ui:typecheck / ui:build — not applicable; no UI change.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • New or changed behavior has unit tests for new branches, fallback paths, and sanitizer boundaries — test/unit/redact-local-path.test.ts covers both functions (unix/home/Windows roots, spaced segments, mid-token roots, nullish/empty input, longest-first path ordering, non-string coercion, token/path filtering). The existing call-site tests (local-scorer-adapter.test.ts, mcp-cli-packets.test.ts) still pass unchanged against the consolidated module.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized and low-noise.
  • Auth/session-adjacent change (token/path redaction) keeps its negative-path coverage: the sanitizer boundary tests assert both the redacted output and that raw tokens/paths are gone.
  • MCP behavior is preserved and tested.

Notes

  • The two mechanisms are kept separate on purpose (per the issue's requirement): the heuristic cannot redact a token it was never told about, and the exact-substitution redactor cannot detect an arbitrary path by shape. redactPrivateValidationMetrics (economic/identity-term redaction, not path redaction) is intentionally left in bin/ — it is a different concern and out of this issue's scope.

Closes #6264

The MCP CLI carried three separately-maintained local-path redactors —
`redactLocalPath` (lib/local-branch.js), `redactLocalValidationPaths` and
`sanitizeDiagnosticText` (bin/loopover-mcp.js) — so a single redaction fix had
to be repeated and kept in sync three times.

Consolidate them into packages/loopover-mcp/lib/redact-local-path.js, keeping
both mechanisms the call sites need as named functions: `redactLocalPath` (regex
heuristic that detects an unknown path in free text) and `redactKnownLocalPaths`
(exact substring substitution of known tokens/paths). The heuristic is the strict
superset of the two former regex variants, so no call site redacts less than
before. All three former call sites now import from the shared module; behavior is
preserved. A future redaction fix now happens in one place.

Closes JSONbored#6264
@nghetienhiep
nghetienhiep requested a review from JSONbored as a code owner July 16, 2026 02:43
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 02:52:19 UTC

6 files · 1 AI reviewer · no blockers · readiness 80/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a clean, mechanical consolidation of three previously-separate local-path-redaction implementations into one shared module (`redact-local-path.js`), preserving both mechanisms callers need (`redactLocalPath` heuristic-detect vs `redactKnownLocalPaths` exact-substitution). Tracing each call site: `local-branch.js`'s old regex is exactly reproduced as the new module's mid-token fallback pass, `sanitizeValidationText`'s old `redactLocalValidationPaths` is byte-for-byte reproduced as the new anchored `rootedPath` regex, and `sanitizeDiagnosticText`'s token/path-gathering and longest-first substitution logic is preserved verbatim inside `redactKnownLocalPaths`. The combined two-pass heuristic (anchored+space-aware, then unanchored mid-token) is a genuine superset of what any one call site redacted before, and the new test file plus package.json/check-mcp-package.mjs allowlist updates keep the build/pack contract in sync with the new file.

Nits — 5 non-blocking
  • packages/loopover-mcp/lib/redact-local-path.js:1-13 — the module-level and per-function comment blocks are unusually long multi-paragraph docstrings; consider trimming to the one non-obvious fact each (why two functions exist, why paths sort longest-first).
  • The consolidated pass order (anchored `rootedPath` first, then unanchored mid-token fallback) is the reverse of `local-branch.js`'s prior order (mid-token first, then anchored); worth a one-line comment noting this reorder was verified not to change observable output, since a future reader may assume order-independence.
  • test/unit/redact-local-path.test.ts:1-4 — the file-level comment duplicates detail already in the source module's docstring; one is enough.
  • Consider a lint rule or a shared regex-export so `local-branch.js`'s re-export of `redactLocalPath` (lib/local-branch.js) can eventually be dropped once external callers migrate directly to the shared module.
  • In redact-local-path.js, the two regex passes are rebuilt on every call for `rootedPath`; the mid-token fallback regex could be hoisted to a module-level constant since it has no captured state, for a minor perf/readability win.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6264
Related work ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 112 registered-repo PR(s), 64 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor nghetienhiep; Gittensor profile; 112 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR creates one shared module (redact-local-path.js) exposing redactLocalPath (regex heuristic) and redactKnownLocalPaths (exact substitution), and migrates all three original call sites (local-branch.js, sanitizeValidationText/sanitizeCacheString, sanitizeDiagnosticText) to use it while removing the duplicated local implementations, matching the issue's explicit requirement to preserve both me

Review context
  • Author: nghetienhiep
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 112 PR(s), 0 issue(s).
  • Related work: Titles/paths share 8 meaningful terms. (issue #6264, issue #6258)
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit bc7993c into JSONbored:main Jul 16, 2026
14 checks passed
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (b1d7cd1) to head (308103a).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6339   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         596      596           
  Lines       47140    47140           
  Branches    15008    15008           
=======================================
  Hits        45068    45068           
  Misses       1290     1290           
  Partials      782      782           
Flag Coverage Δ
shard-1 44.13% <ø> (ø)
shard-2 36.52% <ø> (ø)
shard-3 32.41% <ø> (ø)
shard-4 34.58% <ø> (-0.01%) ⬇️
shard-5 31.58% <ø> (-0.11%) ⬇️
shard-6 44.87% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions github-actions Bot mentioned this pull request Jul 16, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(mcp): consolidate 3 separately-maintained local-path-redaction implementations

1 participant