Skip to content

fix(signals): buildFocusManifestGuidance leaks raw maintainer wantedPaths/preferredLabels to contributors via unfiltered findings #5945

Description

@JSONbored

Context

buildFocusManifestGuidance in src/signals/focus-manifest.ts builds findings entries surfaced to contributors about how their PR relates to a maintainer's focus manifest. Most detail strings that embed manifest text are filtered for public-safety first, but two are not:

// manifest_off_focus finding, ~line 692
detail: `No changed path matches the maintainer-wanted patterns (${manifest.wantedPaths.slice(0, 5).join(", ")}).`,

// manifest_missing_preferred_label finding, ~line 713
detail: `Maintainer prefers labels: ${manifest.preferredLabels.slice(0, 5).join(", ")}.`,

Both interpolate the raw, unfiltered manifest.wantedPaths/manifest.preferredLabels. Compare with the manifest_missing_tests finding later in the same function (~lines 740-741), which already applies the public-safety filter before interpolating:

const safeExpectations = manifest.testExpectations.filter(isFocusManifestPublicSafe).slice(0, 3);
const expectationDetail = safeExpectations.length > 0 ? ` Expected evidence: ${safeExpectations.join("; ")}.` : "";

That fix exists specifically to close a previously-reported leak: test/unit/focus-manifest.test.ts has a regression test titled "omits the 'Expected evidence' detail when every test expectation is public-unsafe (#3304)". The manifest_off_focus and manifest_missing_preferred_label findings never received the equivalent fix.

This is not cosmetic. findings is returned from buildFocusManifestGuidance unfiltered — unlike publicNextSteps, which is filtered via safeNextSteps = [...].filter(isFocusManifestPublicSafe) before being returned. src/signals/local-branch.ts (buildLocalBranchAnalysis, ~lines 318-324) maps manifestGuidance.findings verbatim into LocalBranchAnalysis.localFindings, which is exposed unfiltered through the /v1 API (src/api/routes.ts, ~line 3290) and the MCP analyzeLocalBranch/remediationPlan tools (src/mcp/server.ts, ~line 3937) — both authenticated as the contributor, not the maintainer. So a public-unsafe term present in a maintainer's wantedPaths or preferredLabels (the exact vocabulary isFocusManifestPublicSafe exists to block from contributor-facing surfaces) leaks verbatim to the contributor through this path.

Note local-branch.ts's own manifestFocusLines() helper (used to build the PR-packet markdown) already correctly reuses the filtered guidance.publicNextSteps rather than findings — proving the safe pattern was known and applied elsewhere in the same file, just missed for localFindings.

Requirements

  • Apply isFocusManifestPublicSafe filtering to the manifest.wantedPaths and manifest.preferredLabels values interpolated into the manifest_off_focus and manifest_missing_preferred_label finding detail strings, mirroring the existing safeExpectations pattern used for manifest_missing_tests in the same function.
  • Handle the case where filtering removes every entry (e.g. all wantedPaths are public-unsafe) the same way manifest_missing_tests does — omit the leaking detail fragment gracefully (adjust wording, e.g. drop the parenthetical) rather than emitting an empty ()/: . artifact.
  • Do not change the finding's code, severity, title, or action fields — only the detail string's content.
  • Do not change publicNextSteps (already correctly filtered) or any other finding in this function.

Deliverables

  • manifest_off_focus's detail interpolates only public-safety-filtered wantedPaths entries.
  • manifest_missing_preferred_label's detail interpolates only public-safety-filtered preferredLabels entries.
  • Both gracefully omit the offending fragment (not an empty artifact) when filtering leaves zero entries.
  • A regression test in test/unit/focus-manifest.test.ts asserting a manifest with a public-unsafe wantedPaths/preferredLabels entry does not leak that term into the corresponding finding's detail string, following the same shape as the existing #3304 regression test for manifest_missing_tests.
  • A test confirming src/signals/local-branch.ts's LocalBranchAnalysis.localFindings (which consumes these findings verbatim) no longer surfaces the unsafe term either, closing the actual contributor-facing exposure path.

Test Coverage Requirements

Aim for 99%+ Codecov patch coverage (100% including the new branch/invariant) on the touched lines in src/signals/focus-manifest.ts. This is a fix for a real contributor-facing information leak (public-unsafe maintainer text reaching an unauthenticated-relative-to-maintainer surface via API/MCP), so both regression tests above are required, not just incidental line coverage.

Expected Outcome

A public-unsafe term in a maintainer's wantedPaths or preferredLabels no longer reaches a contributor via the manifest_off_focus/manifest_missing_preferred_label finding details, the /v1 local-branch-analysis API, or the MCP analyzeLocalBranch/remediationPlan tools — matching the protection isFocusManifestPublicSafe already provides for publicNextSteps and the manifest_missing_tests finding.

Links & Resources

  • src/signals/focus-manifest.ts (buildFocusManifestGuidance, manifest_off_focus ~line 687-696, manifest_missing_preferred_label ~line 708-717, the already-fixed manifest_missing_tests pattern ~line 738-745)
  • src/signals/local-branch.ts (buildLocalBranchAnalysis, ~lines 318-324 — verbatim pass-through of findings into localFindings; manifestFocusLines() — the correct filtered-reuse precedent)
  • src/api/routes.ts (~line 3290 — /v1 exposure of localFindings)
  • src/mcp/server.ts (~line 3937 — MCP analyzeLocalBranch/remediationPlan exposure)
  • test/unit/focus-manifest.test.ts (existing #3304 regression test — the pattern to mirror for these two findings)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions