Skip to content

fix(signals): redact /root/ local paths on the public-safety boundary#1376

Merged
JSONbored merged 4 commits into
JSONbored:mainfrom
kiannidev:fix/signals-redact-root-local-path
Jun 26, 2026
Merged

fix(signals): redact /root/ local paths on the public-safety boundary#1376
JSONbored merged 4 commits into
JSONbored:mainfrom
kiannidev:fix/signals-redact-root-local-path

Conversation

@kiannidev

@kiannidev kiannidev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

The signals public/private boundary treats /Users/, /home/, and /tmp/ (and Windows …\Users\…) as local filesystem paths that must never reach a public GitHub surface, but it missed /root/ — the root user's home directory. A contributor running local branch analysis from a /root/... working tree (common in containers, CI, and devcontainers) could leak that absolute local path onto public surfaces.

The project already treats /root/ as a local path to redact in src/services/miner-dashboard-recommendations.ts:

const LOCAL_PATH = /(?:\/(?:Users|home|root|tmp|var)\/[^\s,;:)]+|[A-Za-z]:\\Users\\[^\s,;:)]+)/g;

This PR aligns the canonical boundary primitive and the changed-file-path redactor with that established intent.

Changes

  • src/signals/redaction.ts — add /root/ to PUBLIC_UNSAFE_PATTERN, the canonical isPublicSafeText boundary governing PR/issue comments, check annotations, notifications, badge, and extension payloads.
  • src/signals/local-branch.ts — add /root/ to safeRepoPath, which redacts changed file paths rendered into the public PR packet's Changed Paths section (the most likely place a /root/... path appears).
  • Tests for the new case in test/unit/redaction.test.ts and test/unit/local-branch.test.ts.

Behavior-preserving for every existing input — it only adds /root/ detection.

Security / privacy notes

This strengthens the public/private boundary: an absolute /root/... path is now redacted (isPublicSafeText returns false; changed-file paths render as [local path hidden]) before reaching any public GitHub surface, consistent with the existing /Users/, /home/, /tmp/ handling. No public output that was previously sanitized changes; only previously-leaking /root/... paths are now caught.

Scope

Narrow, scoped to the canonical signals boundary primitive plus the changed-file-path redactor. Other surfaces that keep their own context-specific path denylists (control-panel-roles.ts, weekly-value-report.ts, db/repositories.ts, agent-action-explanation-card.ts, focus-manifest.ts) can be aligned in a follow-up.

Validation

Run from the repo root (Node 22, after npm ci):

git diff --check
npm run actionlint
npm run db:migrations:check
npm run typecheck
npm run test:coverage
npm run test:workers
npm run ui:openapi:check

All pass locally. npm ci reports found 0 vulnerabilities. New behavior is covered by the added unit tests (changed source lines are exercised; safeRepoPath's changed line is additionally covered behaviorally via the PR-packet test).

Linked issue

Fixes #1375

The signals public/private boundary treats /Users/, /home/, and /tmp/ (and
Windows ...\Users\...) as local filesystem paths that must not reach public
GitHub surfaces, but missed /root/ — the root user's home directory. A
contributor running local branch analysis from a /root/... working tree
(common in containers/CI/devcontainers) could leak that absolute path.

The project already treats /root/ as a local path in
miner-dashboard-recommendations.ts; this aligns the canonical boundary
primitive (redaction.ts PUBLIC_UNSAFE_PATTERN) and the changed-file-path
redactor (local-branch.ts safeRepoPath) with that intent.

Behavior-preserving for all existing inputs; only adds /root/ detection.

Fixes JSONbored#1375

Co-authored-by: Cursor <cursoragent@cursor.com>
@kiannidev kiannidev requested a review from JSONbored as a code owner June 25, 2026 14:22
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jun 25, 2026
@superagent-security

Copy link
Copy Markdown

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

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.37%. Comparing base (de4f460) to head (0ccdd2e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1376   +/-   ##
=======================================
  Coverage   95.37%   95.37%           
=======================================
  Files         192      192           
  Lines       20857    20857           
  Branches     7542     7542           
=======================================
  Hits        19892    19892           
  Misses        383      383           
  Partials      582      582           
Files with missing lines Coverage Δ
src/signals/local-branch.ts 97.17% <ø> (ø)
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.

@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix - worth 0.5x multiplier. label Jun 26, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Correct, first-filed, and complete — patches both boundary arms, which is right since either alone leaves a leak path. /root/ was the obvious omission next to the /Users/,/home/,/tmp/ family, and a CI/container tree lives exactly there. Green. Merge.

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 - worth 0.5x multiplier. lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(signals): public-safety boundary does not redact /root/ local paths

2 participants