Skip to content

fix(signals): filter public-unsafe wantedPaths/preferredLabels from manifest findings - #5967

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
claytonlin1110:fix/focus-manifest-public-safe-leak-5945
Jul 15, 2026
Merged

fix(signals): filter public-unsafe wantedPaths/preferredLabels from manifest findings#5967
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
claytonlin1110:fix/focus-manifest-public-safe-leak-5945

Conversation

@claytonlin1110

Copy link
Copy Markdown
Contributor

Summary

  • manifest_off_focus and manifest_missing_preferred_label interpolated manifest.wantedPaths/manifest.preferredLabels directly into contributor-facing finding detail text without the isFocusManifestPublicSafe filter already applied to testExpectations in the sibling manifest_missing_tests finding, letting a maintainer-authored public-unsafe term leak through LocalBranchAnalysis.localFindings (the /v1 API + MCP exposure path) to contributors.
  • Both findings now filter through isFocusManifestPublicSafe before interpolation, mirroring the existing safeExpectations pattern. manifest_off_focus drops the parenthetical when nothing is safe to show; manifest_missing_preferred_label falls back to a static "No maintainer-preferred label applied." sentence since its entire detail is built from the label list.
  • publicNextSteps.push(...) in both blocks is left unchanged since it is already protected by the downstream .filter(isFocusManifestPublicSafe) applied when safeNextSteps is constructed.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • 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 (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • This local Windows dev environment cannot run wrangler (cf-typegen:check fails with a pre-existing, change-unrelated spawnSync wrangler ENOENT), which blocks the chained npm run test:ci from reaching later steps locally. Each step was therefore run individually: git diff --check, actionlint, db:migrations:check, db:schema-drift:check, selfhost:env-reference:check, miner:env-reference:check, selfhost:validate-observability, engine build, typecheck, test:engine-parity, test:live-gate-parity, and test:driver-parity all pass clean on this branch. test:coverage was verified scoped to every test file that exercises the two changed source files (focus-manifest.ts, local-branch.ts), showing 100% coverage on every changed line/branch — the full unsharded run also hits unrelated pre-existing local flakiness (missing sqlite3 CLI on PATH, subprocess-timing tests) in ~51 files outside this diff's scope, none of which touch focus-manifest.ts/local-branch.ts.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below — N/A, no UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

UI Evidence

N/A — backend-only fix, no visible UI change.

Notes

…anifest findings

manifest_off_focus and manifest_missing_preferred_label interpolated
manifest.wantedPaths/preferredLabels directly into contributor-facing
finding details without the isFocusManifestPublicSafe filter already
applied to testExpectations, letting a maintainer-authored public-unsafe
term leak through localFindings to contributors via the /v1 API and MCP
tools.

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

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-15 04:57:42 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This closes a real information-leak gap: manifest_off_focus and manifest_missing_preferred_label previously interpolated raw maintainer-authored wantedPaths/preferredLabels straight into contributor-facing finding detail text without the isFocusManifestPublicSafe filter that the sibling manifest_missing_tests finding already applies to testExpectations. The fix filters both lists before interpolation and adds sensible zero-safe-entries fallbacks (dropped parenthetical vs. a static sentence), and both the guidance-builder and the full localFindings exposure path are covered by new tests. The change is narrowly scoped to the two vulnerable interpolation sites and mirrors an existing, proven pattern in the same function.

Nits — 4 non-blocking
  • src/signals/focus-manifest.ts: the publicNextSteps.push for manifest_missing_preferred_label still interpolates the raw, unfiltered manifest.preferredLabels.slice(0,3) rather than the new safePreferredLabels — the PR description claims this is safe because of a downstream `.filter(isFocusManifestPublicSafe)` applied when safeNextSteps is built, but that code isn't visible in this diff, so it's worth a comment or test asserting that downstream filter actually catches this specific string too.
  • The inline comments in focus-manifest.ts are quite long (3+ lines each) for a fairly mechanical filter-before-interpolate change; consider trimming to the non-obvious part (why the fallback differs between the two findings) since the pattern itself is already established by safeExpectations.
  • Add a direct test asserting the publicNextSteps entry for manifest_missing_preferred_label with an unsafe preferredLabels value doesn't leak the unsafe term, to make the 'downstream filter protects this' claim self-verifying rather than relying on unseen code.
  • Consider extracting the repeated `manifest.X.filter(isFocusManifestPublicSafe).slice(0, N)` pattern (now used in at least 3 places across this file) into a small named helper to reduce copy-paste drift risk for the next finding that needs it.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5945
Related work ⚠️ 2 scoped overlaps 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: 385 registered-repo PR(s), 260 merged, 103 issue(s).
Contributor context ✅ Confirmed Gittensor contributor claytonlin1110; Gittensor profile; 385 PR(s), 103 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff applies isFocusManifestPublicSafe filtering to wantedPaths and preferredLabels before interpolation in both manifest_off_focus and manifest_missing_preferred_label findings, gracefully omitting the parenthetical or falling back to a static sentence when nothing is safe, exactly mirroring the safeExpectations pattern cited in the issue. It also adds regression tests in both focus-manifest.

Review context
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 <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> 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

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.15%. Comparing base (c78b576) to head (ed30852).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5967   +/-   ##
=======================================
  Coverage   95.15%   95.15%           
=======================================
  Files         589      589           
  Lines       46695    46699    +4     
  Branches    14902    14904    +2     
=======================================
+ Hits        44434    44438    +4     
  Misses       1511     1511           
  Partials      750      750           
Flag Coverage Δ
shard-1 43.42% <50.00%> (-0.54%) ⬇️
shard-2 36.28% <0.00%> (+0.23%) ⬆️
shard-3 31.85% <75.00%> (-0.20%) ⬇️
shard-4 31.80% <0.00%> (-1.05%) ⬇️
shard-5 32.39% <50.00%> (+0.88%) ⬆️
shard-6 44.91% <100.00%> (+0.35%) ⬆️

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

Files with missing lines Coverage Δ
src/signals/focus-manifest.ts 99.65% <100.00%> (+<0.01%) ⬆️

@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 4076df1 into JSONbored:main Jul 15, 2026
15 checks passed
This was referenced Jul 15, 2026
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.

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

1 participant