Skip to content

fix(review): sanitize REES enrichment prompts - #1554

Merged
JSONbored merged 3 commits into
mainfrom
codex/fix-rees-untrusted-prompt-injection-vulnerability
Jun 27, 2026
Merged

fix(review): sanitize REES enrichment prompts#1554
JSONbored merged 3 commits into
mainfrom
codex/fix-rees-untrusted-prompt-injection-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The external REES enrichment brief was being spliced into reviewer prompts verbatim, allowing untrusted promptSection and systemSuffix text to reach the model and bypass existing prompt-injection/public-safety controls.
  • Treat REES output as untrusted advisory context to avoid giving an external service instruction-level influence over reviewer decisions or public outputs.

Description

  • Defang and sanitize REES promptSection with neutralizePromptInjection and sanitizePublicComment, and cap the returned prompt section to MAX_ENRICHMENT_PROMPT_SECTION_CHARS (8000 chars) before splicing. (src/review/enrichment-wire.ts)
  • Replace direct use of REES-provided systemSuffix with a fixed local verification suffix ENRICHMENT_SYSTEM_SUFFIX so the external service cannot inject system-level instructions. (src/review/enrichment-wire.ts)
  • Add sanitizeEnrichmentPromptSection helper that enforces type, trimming, prompt-injection neutralization, public-safety filtering, and length capping. (src/review/enrichment-wire.ts)
  • Update and extend unit tests to assert sanitization, length capping, rejection of non-public-safe briefs, and that the system prompt contains the fixed untrusted advisory suffix rather than the raw REES string. (test/unit/enrichment-wire.test.ts, test/unit/ai-review.test.ts)

Testing

  • Ran tsc --noEmit (typecheck) and it passed.
  • Ran the updated unit tests with npx vitest run test/unit/enrichment-wire.test.ts test/unit/ai-review.test.ts and both files passed (all related tests green).
  • Ran git diff --check and it reported no whitespace/conflict issues.
  • Attempted the full local gate via npm run test:ci and npm audit --audit-level=moderate; test:ci execution hit unrelated transient network/DNS failures during actionlint setup and some long-running existing suite shards timed out in this environment, and npm audit returned 403 Forbidden from the registry audit endpoint, so the full gate/audit could not be completed here.

Codex Task

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1554   +/-   ##
=======================================
  Coverage   95.36%   95.37%           
=======================================
  Files         197      197           
  Lines       21464    21472    +8     
  Branches     7759     7761    +2     
=======================================
+ Hits        20470    20478    +8     
  Misses        416      416           
  Partials      578      578           
Files with missing lines Coverage Δ
src/review/enrichment-wire.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 — scores a 0.05x multiplier. label Jun 26, 2026
@JSONbored JSONbored self-assigned this Jun 26, 2026
@loopover-orb

loopover-orb Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review — blocked

3 files · 1 AI reviewers · no blockers · readiness 48/100 · CI failing · unknown

🛑 Blocked

Review summary
The change correctly closes a prompt-injection path where REES-returned `promptSection` and `systemSuffix` were spliced verbatim into reviewer prompts. `sanitizeEnrichmentPromptSection` chains `neutralizePromptInjection` → `sanitizePublicComment` → length-cap before use, and the REES `systemSuffix` is fully replaced with the fixed `ENRICHMENT_SYSTEM_SUFFIX` constant rather than being forwarded from the external service. Tests are updated to assert sanitization, length enforcement, public-safety rejection, and that the fixed suffix — not the raw REES string — appears in output; imports of both sanitization utilities are confirmed present in the full file.

Nits (4)

  • In `buildReviewEnrichment` (`enrichment-wire.ts`, ~line 111), `ENRICHMENT_SYSTEM_SUFFIX` is only returned when REES provides a non-empty `systemSuffix`; a REES response with a valid `promptSection` but no `systemSuffix` splices sanitized-but-untrusted content without the 'treat as untrusted' advisory framing, silently weakening the defense-in-depth.
  • The new `it` block in `enrichment-wire.test.ts` reassigns `globalThis.fetch` twice within a single test to cover two distinct scenarios; splitting into two `it` blocks would give clearer failure attribution and avoid implicit state sharing between sub-cases.
  • In `buildReviewEnrichment` (`enrichment-wire.ts`), set `systemSuffix: ENRICHMENT_SYSTEM_SUFFIX` unconditionally whenever `promptSection` is non-empty, removing the REES-provided-`systemSuffix`-presence gate — the advisory framing guards against the enrichment content itself and should not be gated on a signal from the untrusted external service.
  • Consider returning `undefined` explicitly in `sanitizeEnrichmentPromptSection` when `sanitizePublicComment` produces an empty string (rather than relying on the caller's falsy guard on the return value), making the 'unsafe brief → no enrichment' contract visible in the helper itself (`enrichment-wire.ts`, `sanitizeEnrichmentPromptSection`).

CI checks failing

  • codecov/patch — 90.90% of diff hit (target 97.00%)
Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Review load ❌ 8/20 Readiness component derived from cached public PR metadata and labels; size label size:M.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 26 open PR(s), 9 likely reviewable, 17 unlinked.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 81 PR(s), 297 issue(s).
Gate result ✅ Passing No configured blocker found.
Nits — 2 non-blocking
  • Repository config was not parsed
  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 81 PR(s), 297 issue(s).
  • Related work: Titles/paths share 6 meaningful terms. (PR #1492)
  • Related work: Titles/paths share 6 meaningful terms. (PR #1537)
  • Related work: Titles/paths share 6 meaningful terms. (PR #1545)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Review top overlaps.
  • Add scope summary.
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The change correctly closes a prompt-injection path where REES-returned `promptSection` and `systemSuffix` were spliced verbatim into reviewer prompts. `sanitizeEnrichmentPromptSection` chains `neutralizePromptInjection` → `sanitizePublicComment` → length-cap before use, and the REES `systemSuffix` is fully replaced with the fixed `ENRICHMENT_SYSTEM_SUFFIX` constant rather than being forwarded from the external service. Tests are updated to assert sanitization, length enforcement, public-safety rejection, and that the fixed suffix — not the raw REES string — appears in output; imports of both sanitization utilities are confirmed present in the full file.

Nits (4)

  • In `buildReviewEnrichment` (`enrichment-wire.ts`, ~line 111), `ENRICHMENT_SYSTEM_SUFFIX` is only returned when REES provides a non-empty `systemSuffix`; a REES response with a valid `promptSection` but no `systemSuffix` splices sanitized-but-untrusted content without the 'treat as untrusted' advisory framing, silently weakening the defense-in-depth.
  • The new `it` block in `enrichment-wire.test.ts` reassigns `globalThis.fetch` twice within a single test to cover two distinct scenarios; splitting into two `it` blocks would give clearer failure attribution and avoid implicit state sharing between sub-cases.
  • In `buildReviewEnrichment` (`enrichment-wire.ts`), set `systemSuffix: ENRICHMENT_SYSTEM_SUFFIX` unconditionally whenever `promptSection` is non-empty, removing the REES-provided-`systemSuffix`-presence gate — the advisory framing guards against the enrichment content itself and should not be gated on a signal from the untrusted external service.
  • Consider returning `undefined` explicitly in `sanitizeEnrichmentPromptSection` when `sanitizePublicComment` produces an empty string (rather than relying on the caller's falsy guard on the return value), making the 'unsafe brief → no enrichment' contract visible in the helper itself (`enrichment-wire.ts`, `sanitizeEnrichmentPromptSection`).

🟩 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

@dosubot dosubot Bot added the size:M label Jun 27, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored
JSONbored merged commit 74ce025 into main Jun 27, 2026
18 checks passed
@JSONbored
JSONbored deleted the codex/fix-rees-untrusted-prompt-injection-vulnerability branch June 27, 2026 01:24
andriypolanski pushed a commit to andriypolanski/gittensory that referenced this pull request Jun 27, 2026
…SONbored#1138)

THE dominant contributor-flood failure (robustness audit). Two disposition bugs killed/stranded good PRs:

1. FORK FALSE-CLOSE: ciState='unverified' (a fork whose Actions await maintainer approval, or unreadable
   check-runs) made reviewGood=false, so a CONTRIBUTOR with a passing gate was one-shot CLOSED citing 'CI
   could not be verified' — and re-closed every push. FIX: willClose now fires ONLY on a REAL adverse signal
   — a confirmed gate FAILURE, a red required CI (ciFailed), or a base CONFLICT. Unverified / not-yet-mergeable
   is HELD for review, never killed. (Owner/automation still never close; guarded paths still held.)

2. NEUTRAL SILENT-STUCK: a non-confirmed contributor PR with any advisory blocker gets conclusion='neutral',
   and the planner returned [] immediately — no label, no disposition — so the PR looked 'unreviewed forever'
   (the metagraphed JSONbored#1551/JSONbored#1554 class). FIX: only SKIPPED short-circuits; a NEUTRAL gate now FLOWS to the
   disposition so the PR is surfaced with a label + held (never silently undecided). Neutral is NOT auto-merged
   (that trust/policy decision is deferred, not bundled here).

Full suite green (3589). Updated agent-actions + queue tests that asserted the old (harmful) close/silent behavior.
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.

1 participant