Skip to content

fix(signals): centralize public local-path redaction across drifted surfaces - #1748

Merged
JSONbored merged 4 commits into
JSONbored:mainfrom
GildardoDev:fix/centralize-local-path-redaction
Jun 29, 2026
Merged

fix(signals): centralize public local-path redaction across drifted surfaces#1748
JSONbored merged 4 commits into
JSONbored:mainfrom
GildardoDev:fix/centralize-local-path-redaction

Conversation

@GildardoDev

Copy link
Copy Markdown
Contributor

What

src/signals/redaction.ts is the canonical public/private boundary, but several other public surfaces carried their own copy of the local-path denylist and had drifted. Most were missing /root/ and several were missing /var/, so a contributor or operator running from a /root/... or /var/... path could leak an absolute local path through those surfaces even though the canonical boundary already blocks them. This centralizes the local-path roots in one source so the surfaces cannot drift again.

How

redaction.ts now exports the local-path root vocabulary as a single source, PUBLIC_LOCAL_PATH_INLINE, plus a shared .replace() scrubber (PUBLIC_LOCAL_PATH_SCRUB_PATTERN) and an anchored prefix pattern (PUBLIC_LOCAL_PATH_PREFIX_PATTERN). PUBLIC_UNSAFE_PATTERN is recomposed from that source with identical behavior.

Every drifted surface now composes from that source while keeping its own trailing and anchoring semantics, so behavior on the roots that already worked is unchanged and only the missing /root/ and /var/ (and the forward-slash Windows form C:/Users/) are added:

  • signals/focus-manifest.ts isFocusManifestPublicSafe: split into its existing term vocabulary plus a path check composed from the shared source, which adds /var/.
  • services/control-panel-roles.ts sanitizeRoleText and services/weekly-value-report.ts sanitizeReportText: use the shared scrub pattern, which adds /root/ and /var/.
  • services/agent-action-explanation-card.ts TOKEN_OR_PATH_PATTERN: keeps its token alternatives and composes the path alternatives from the shared source, adding /root/, /var/, and C:/Users/.
  • services/miner-dashboard-recommendations.ts LOCAL_PATH: rebuilt from the shared source, preserving its trailing class and case sensitivity.
  • db/repositories.ts product-usage scrubber: aliases the shared scrub pattern, unifying the Windows form.
  • signals/local-branch.ts safeRepoPath: uses the shared anchored prefix pattern, which adds /var/.

The context-specific phrase vocabularies in agent-action-explanation-card.ts and miner-dashboard-recommendations.ts, and the richer term vocabulary in focus-manifest.ts, stay local by design. pr-body-draft.ts keeps its generic local-path matcher (out of scope per the issue).

Tests

Extended the existing unit test for each wired surface to prove /root/, /var/, and the forward-slash C:/Users/ form are now redacted or blocked, with no regression on the previously handled roots.

Closes #1418

@GildardoDev
GildardoDev requested a review from JSONbored as a code owner June 29, 2026 14:53
@loopover-orb

loopover-orb Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-06-29 22:37:05 UTC

15 files · 1 AI reviewer · no blockers · readiness 55/100 · CI green · unknown

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change correctly centralizes the public local-path root vocabulary in `src/signals/redaction.ts` and rewires the drifted scrub/test surfaces to compose from it while preserving each caller's anchoring or trailing-match behavior. The notable edge case, lower-case Windows drive redaction in the case-sensitive dashboard scrubber, is covered by `[A-Za-z]` in the shared source and by a direct unit test, so the cited regression is not present in this diff. The implementation is safe enough to proceed; the remaining issue is a misleading comment in one consumer.

Nits — 4 non-blocking
  • nit: `src/services/miner-dashboard-recommendations.ts:44` says the Windows form is via `[A-Z]`, but the shared source now intentionally uses `[A-Za-z]`; update that comment so it does not contradict the actual lower-case-drive behavior.
  • `src/services/miner-dashboard-recommendations.ts:44` should say the case-sensitive `/g` scrubber relies on `PUBLIC_LOCAL_PATH_INLINE`'s `[A-Za-z]` drive-letter class, matching the comment already added in `test/unit/redaction.test.ts`.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #1418
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 (size label size:M; 1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR; address the blocker before review.
Contributor workload ✅ 10/10 Author activity: 82 registered-repo PR(s), 52 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor GildardoDev; Gittensor profile; 82 PR(s), 0 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
Contributor next steps
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Fix the blocker.
  • Triage stale or unlinked PRs.
  • Refresh registry data or choose a registered active repo.
  • Check active issues and PRs before submitting.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jun 29, 2026
@dosubot dosubot Bot added the size:M label Jun 29, 2026
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.59%. Comparing base (29da3b1) to head (3c621d2).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1748   +/-   ##
=======================================
  Coverage   95.59%   95.59%           
=======================================
  Files         204      204           
  Lines       22316    22321    +5     
  Branches     8067     8068    +1     
=======================================
+ Hits        21332    21337    +5     
  Misses        408      408           
  Partials      576      576           
Files with missing lines Coverage Δ
src/db/repositories.ts 96.16% <100.00%> (ø)
src/services/agent-action-explanation-card.ts 100.00% <100.00%> (ø)
src/services/control-panel-roles.ts 98.82% <ø> (ø)
src/services/miner-dashboard-recommendations.ts 100.00% <100.00%> (ø)
src/services/weekly-value-report.ts 100.00% <ø> (ø)
src/signals/focus-manifest.ts 99.17% <100.00%> (+<0.01%) ⬆️
src/signals/local-branch.ts 97.19% <ø> (ø)
src/signals/redaction.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/signals/redaction.ts
@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jun 29, 2026
@superagent-security superagent-security Bot removed the pr:flagged PR flagged for review by security analysis. label Jun 29, 2026
…l-path-redaction

# Conflicts:
#	src/signals/focus-manifest.ts
#	test/unit/focus-manifest.test.ts
@GildardoDev
GildardoDev force-pushed the fix/centralize-local-path-redaction branch from 1b25eab to 52aebf4 Compare June 29, 2026 22:28
@dosubot dosubot Bot added the lgtm label Jun 29, 2026
@JSONbored
JSONbored merged commit 791d95d into JSONbored:main Jun 29, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 29, 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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

fix(signals): centralize public local-path redaction across drifted surfaces

2 participants