Skip to content

fix(chat): redact private lane signals from Q&A - #5149

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-chat-command-exposing-private-information
Jul 12, 2026
Merged

fix(chat): redact private lane signals from Q&A#5149
JSONbored merged 1 commit into
mainfrom
codex/fix-chat-command-exposing-private-information

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent private reward/lane economics (e.g. Maintainer cut, direct PR lane share, split lane, issue-discovery) from being included in the LLM grounding or published in public PR comments via the @gittensory chat Q&A path.

Description

  • Add a PRIVATE_LANE_SIGNAL_PATTERN to detect lane/reward phrasing and redact those matches from grounding text before building the LLM prompt in src/services/ai-chat-qa.ts.
  • Extend the model-output forbidden-text guard (PUBLIC_FORBIDDEN_TEXT_PATTERN) to treat these lane-signal phrases as forbidden so generated answers that repeat them are withheld.
  • Wire the new pattern into redactGroundingText so action.why lines containing lane signals are replaced with private readiness context before prompting the provider.
  • Add regression tests in test/unit/ai-chat-qa.test.ts to assert prompt redaction, output rejection when the provider repeats lane signals, and helper-level sanitization behavior.

Testing

  • Ran npx vitest run test/unit/ai-chat-qa.test.ts and the suite passed (29 tests) against the modified code.
  • Ran npm run typecheck and tsc --noEmit which completed successfully on the changes.
  • Ran scoped coverage for the file (npm run test:coverage targeted at the unit file) which passed for the modified tests, but a full repository coverage run still fails the global thresholds (the repo-wide coverage requirement is unchanged and requires broader test coverage than this focused fix).
  • Attempting the full gate npm run test:ci failed at cf-typegen:check due to stale generated worker types (the repo reports worker-configuration.d.ts needs regeneration) and npm audit --audit-level=moderate could not complete due to an audit endpoint 403 in this environment.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.14%. Comparing base (cd1a905) to head (b89c02b).
⚠️ Report is 56 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5149   +/-   ##
=======================================
  Coverage   94.13%   94.14%           
=======================================
  Files         466      466           
  Lines       39573    39574    +1     
  Branches    14436    14436           
=======================================
+ Hits        37254    37255    +1     
  Misses       1664     1664           
  Partials      655      655           
Flag Coverage Δ
shard-1 46.50% <100.00%> (-0.02%) ⬇️
shard-2 33.44% <100.00%> (-0.12%) ⬇️
shard-3 31.60% <100.00%> (+0.25%) ⬆️
shard-4 32.70% <100.00%> (+0.01%) ⬆️
shard-5 33.19% <100.00%> (-0.24%) ⬇️
shard-6 45.14% <100.00%> (+0.20%) ⬆️

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

Files with missing lines Coverage Δ
src/services/ai-chat-qa.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.

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

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-12 09:01:29 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · unstable

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
The AI review returned blocking findings for this change but did not include a separate narrative summary. Review the blockers below before deciding this PR.

Blockers

  • src/services/ai-chat-qa.ts:68 leaves bare `direct PR` out of `PUBLIC_FORBIDDEN_TEXT_PATTERN` even though src/services/ai-chat-qa.ts:63 redacts it as private grounding, so a provider response like `direct PR 1 is the useful lane` returns `ok` instead of `unsafe`; add `direct PR` to the output forbidden pattern or remove it from the private lane signal list with a reason.
Nits — 5 non-blocking
  • The bare `direct PR` alternative in PRIVATE_LANE_SIGNAL_PATTERN (src/services/ai-chat-qa.ts:60-61) will redact any grounding text mentioning a plain direct PR (unrelated to lane economics), which is broader than the stated intent — consider requiring `lane`/`share` context to avoid over-redacting benign phrasing.
  • PUBLIC_FORBIDDEN_TEXT_PATTERN (src/services/ai-chat-qa.ts:66) omits the bare `direct PR` term that PRIVATE_LANE_SIGNAL_PATTERN redacts on input, so a model that independently generates the bare phrase `direct PR` (not from grounding) would not be caught by containsPublicForbiddenText — worth confirming this asymmetry is intentional.
  • The PR description doesn't reference an open issue number (e.g. `Fixes #NNNN`) that this change closes, which this repo's contribution policy requires for external PRs.
  • Consider tightening the `direct PR` alternative to require accompanying lane/share/percentage context so plain mentions of a direct PR aren't unnecessarily rewritten to "private readiness context".
  • Add a code comment cross-referencing PRIVATE_LANE_SIGNAL_PATTERN and the lane-related terms folded into PUBLIC_FORBIDDEN_TEXT_PATTERN so future edits keep the two patterns in sync (mirroring the existing comment style for PRIVATE_DECISION_BLOCKER_PATTERN).

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 37 merged, 409 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 409 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 45 PR(s), 409 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

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

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory 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/gittensory-commands

🟩 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 the manual-review Gittensor contributor context label Jul 12, 2026
@JSONbored
JSONbored merged commit b349784 into main Jul 12, 2026
18 checks passed
@JSONbored
JSONbored deleted the codex/fix-chat-command-exposing-private-information branch July 12, 2026 09:03
JSONbored added a commit that referenced this pull request Jul 12, 2026
#5149 added PRIVATE_LANE_SIGNAL_PATTERN and correctly wired it into
redactGroundingText, applied to objective/summary/publicSafeSummary/
freshnessWarnings -- the only fields compactChatSignalBundle() ever
reads. But its own new integration test overrode action.why instead,
a field that function never reads at all (by design: raw action
rationale/blockedBy is omitted entirely from the chat grounding
bundle, not merely redacted -- see the comment above
PRIVATE_DECISION_BLOCKER_PATTERN). The override never reached the
prompt, so neither assertion exercised the new pattern; it happened
to still report a false pass shape until CI caught the real failure
downstream. Point the override at publicSafeSummary, the field that
actually flows into the prompt, so the test validates what it claims to.

No src/ change: redactGroundingText and its regex were already correct,
proven by the adjacent pure-function unit test a few lines down in the
same file, which was passing throughout.
JSONbored added a commit that referenced this pull request Jul 12, 2026
…x, chat-qa test fix (#5201)

* docs(selfhost): document the claude code token secret file + fix stale screenshots note

Two drift fixes found while auditing the recent secrets-migration and
screenshot-allowlist changes:

- The security and AI-providers docs pages only described the inline
  .env path for CLAUDE_CODE_OAUTH_TOKEN; add the secrets/*.txt file
  alternative now that docker-compose.yml supports it.
- config/examples/gittensory.full.yml and .gittensory.yml.example both
  still called `screenshots` a "plain symmetric override" -- #4990 made
  it allowlist-required (force-off only outside the rollout allowlist),
  the same asymmetry as safety/grounding. Update both mirrored comment
  blocks to match.

* fix(ci): build gittensory-engine before UI typecheck on ui-only PR diffs

validate-code's "Build engine package" step only ran on push, or when
backend/engine changed -- not when only ui changed. But the very next
"UI typecheck" step (gated separately on ui) transitively needs that
build: apps/gittensory-ui's tsconfig pulls in src/mcp/local-write-tools.ts,
which imports @jsonbored/gittensory-engine, whose dist/ is gitignored
and only exists after this step runs. Any PR touching only apps/gittensory-ui/**
(a docs-only page edit, for example) hit TS2307 here with no code being
wrong -- confirmed live on PR #5201's own validate-code run. Add the
missing `|| needs.changes.outputs.ui == 'true'` condition.

* fix(test): exercise the real lane-signal redaction path in ai-chat-qa

#5149 added PRIVATE_LANE_SIGNAL_PATTERN and correctly wired it into
redactGroundingText, applied to objective/summary/publicSafeSummary/
freshnessWarnings -- the only fields compactChatSignalBundle() ever
reads. But its own new integration test overrode action.why instead,
a field that function never reads at all (by design: raw action
rationale/blockedBy is omitted entirely from the chat grounding
bundle, not merely redacted -- see the comment above
PRIVATE_DECISION_BLOCKER_PATTERN). The override never reached the
prompt, so neither assertion exercised the new pattern; it happened
to still report a false pass shape until CI caught the real failure
downstream. Point the override at publicSafeSummary, the field that
actually flows into the prompt, so the test validates what it claims to.

No src/ change: redactGroundingText and its regex were already correct,
proven by the adjacent pure-function unit test a few lines down in the
same file, which was passing throughout.
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant