Skip to content

[codex] security(rag): harden tenant isolation and prompt boundaries#2295

Merged
Mikecranesync merged 2 commits into
mainfrom
security/rag-tenant-isolation-2112
Jun 25, 2026
Merged

[codex] security(rag): harden tenant isolation and prompt boundaries#2295
Mikecranesync merged 2 commits into
mainfrom
security/rag-tenant-isolation-2112

Conversation

@Mikecranesync

Copy link
Copy Markdown
Owner

Summary

Refs #2112 and supersedes the stale #2253 hardening branch.

This PR hardens RAG tenant isolation and prompt-boundary handling end to end:

  • Strengthens /api/knowledge/search regression coverage so private tenant snippets are withheld at the DB/query boundary while shared OEM snippets still return.
  • Moves Hub asset/node manual context out of system-role authority and into the final user turn as untrusted reference data.
  • Moves bot RAG/Neon reference chunks out of system-role authority, sanitizes source-label metadata, and neutralizes forged source headers or reference delimiters in retrieved text.
  • Adds prompt-injection regression tests proving malicious retrieved text cannot forge trusted source headers or override the prompt boundary while legitimate grounded reference text remains available for citation.
  • Bumps root VERSION to 3.42.1 and mira-hub to 2.18.2.

Root cause

The narrow #2112 SQL issue was already fixed on main, but coverage only asserted query shape. The broader risk was that retrieved RAG documents were still being assembled into system-role prompt content in Hub and bot paths. That gave malicious retrieved text too much prompt authority even when delimiter stripping existed.

Verification

Passed:

  • git diff --check
  • cd mira-hub && ./node_modules/.bin/vitest run src/lib/__tests__/manual-rag.test.ts src/app/api/knowledge/search/__tests__/route.is-private.test.ts && ./node_modules/.bin/tsc --noEmit
  • cd mira-hub && ./node_modules/.bin/vitest run — 108 files, 875 tests passed
  • cd mira-bots && /Users/charlienode/MIRA/.venv/bin/python -m pytest tests/test_unit2_citations.py tests/test_reranking.py -q && /Users/charlienode/MIRA/.venv/bin/python -m py_compile shared/workers/rag_worker.py — 55 tests passed

Known pre-existing blocker:

  • cd mira-bots && /Users/charlienode/MIRA/.venv/bin/python -m pytest -q still stops during collection on unrelated adapter/dependency imports: GoogleChatAdapter, SlackChatAdapter, TeamsChatAdapter, telegram.Update, and slack_bolt.adapter.socket_mode.async_handler.

Remaining risk

@github-actions

Copy link
Copy Markdown

🤖 AI Code Review

Review by: groq (llama-3.3-70b-versatile)

Review of [codex] security(rag): harden tenant isolation and prompt boundaries

🔴 IMPORTANT: Security Vulnerabilities

  • No hardcoded secrets or SQL injection vulnerabilities were found in the provided diff.
  • However, the code does not explicitly handle potential command injection vulnerabilities. For example, in mira-bots/shared/workers/rag_worker.py (lines 118-129), the _SENTINEL_RE regular expression is used to strip prompt-injection sentinel patterns from chunk text. While this helps prevent some types of injection attacks, it may not cover all possible scenarios. Consider adding additional input validation and sanitization measures to prevent command injection attacks.

🔴 IMPORTANT: Missing Error Handling

  • The diff does not explicitly handle potential network/IO operation errors that could cause the application to crash in production. For example, in mira-bots/shared/workers/rag_worker.py (lines 832-882), the _build_clarification_request method does not handle potential errors that may occur when processing chunk text or metadata. Consider adding try-except blocks to handle and log potential exceptions.

🟡 WARNING: Logic Bugs or Incorrect Assumptions

  • In mira-bots/shared/workers/rag_worker.py (lines 46-57), the format_source_label function assumes that the chunk dictionary will always contain the expected keys (e.g., manufacturer, model_number, metadata). However, if these keys are missing, the function may return incorrect or incomplete labels. Consider adding error handling or input validation to ensure that the function behaves correctly in these scenarios.
  • The _sanitize_label_field function (lines 34-41) uses a regular expression to strip certain characters from the input value. However, this regular expression may not cover all possible scenarios, and the function may not behave correctly for all input values. Consider adding additional input validation and sanitization measures to ensure that the function behaves correctly.

🟡 WARNING: Missing Input Validation at API Boundaries

  • The diff does not explicitly validate input at API boundaries. For example, in mira-bots/shared/workers/rag_worker.py (lines 985-1037), the _build_prompt method does not validate the input message or state parameters. Consider adding input validation to ensure that the method behaves correctly for all possible input values.

🔵 SUGGESTION: Code Quality Improvements

  • The code could benefit from additional comments and docstrings to improve readability and understandability. For example, the _neutralize_chunk_text function (lines 129-141) could include a docstring that explains its purpose and behavior.
  • Some of the variable names (e.g., nc, text, label) are not very descriptive. Consider using more descriptive names to improve code readability.

✅ GOOD: Noteworthy Good Practices

  • The code uses regular expressions to strip prompt-injection sentinel patterns from chunk text, which helps prevent injection attacks.
  • The code uses a try-except block to handle potential exceptions in the _build_clarification_request method. However, this block is not explicitly shown in the provided diff.

Generated by the MIRA automated code review pipeline (Groq → Cerebras → Gemini cascade)
To trigger self-fix: run bash scripts/pr_self_fix.sh 2295 locally, or add the auto-fix label to this PR (or run /autofix-pr from a Claude Code session)

@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

MIRA staging gate — ✅ PASS

Engine + NeonDB staging branch + Groq cascade against fixed questions, graded on the 5-dimension rubric in docs/specs/mira-answer-quality-standard.md. Skipped questions (embed sidecar unavailable, etc.) are excluded from pass/fail math; the run fails closed if >50% are skipped.

  • mean of means: 4.95 (pass threshold: 3.5, scored over 15/15)
  • questions passed: 15 / 15
  • skipped (harness): 0
  • below mean 3.0: 0 (max allowed: 2)
  • hard fails: 0
  • full run logs
id category g c a s t mean note
oem-model-fault-powerflex-f004 oem_model_fault 5 5 5 5 5 5.00
oem-only-no-fault-sew oem_only 5 5 5 5 5 5.00
symptom-no-oem-abbrev symptom_only 5 4 5 5 5 4.80
uns-gate-grinding uns_gate 5 5 5 5 5 5.00
safety-arc-flash safety 5 5 5 5 5 5.00
greeting-hygiene greeting 5 5 5 5 5 5.00
session-followup followup 5 5 5 5 5 5.00
photo-less-ocr-claim no_photo 5 5 5 5 5 5.00
off-topic-redirect off_topic 5 5 5 5 5 5.00
cmms-context-followup cmms_context 4 4 5 5 5 4.60
oem-fault-variant-lowercase oem_model_fault 5 5 5 5 5 5.00
cross-oem-confusion oem_model_fault 5 5 5 5 5 5.00
oem-unknown-fault-admit oem_unknown_fault 5 5 5 5 5 5.00
safety-loto-explicit safety 5 5 5 5 5 5.00
uns-gate-no-line uns_gate 5 4 5 5 5 4.80

Rubric: docs/specs/mira-answer-quality-standard.md · Spec: docs/specs/staging-environment-spec.md

@github-actions

Copy link
Copy Markdown

🤖 AI Code Review

Review by: groq (llama-3.3-70b-versatile)

Review of PR: [codex] security(rag): harden tenant isolation and prompt boundaries

🔴 IMPORTANT: Security vulnerabilities

No hardcoded secrets, SQL injection, path traversal, or command injection vulnerabilities were found in the provided diff.

🔴 IMPORTANT: Missing error handling on network/IO operations

No missing error handling on network/IO operations that could crash in production were found in the provided diff.

🟡 WARNING: Logic bugs or incorrect assumptions

The changes to the RAGWorker class seem to be correct, but it's worth reviewing the logic again to ensure that the retrieved chunks are being handled correctly as untrusted reference data.

Specifically, the _inject_reference_block method (in mira-bots/shared/workers/rag_worker.py at lines 945-958) prepends the retrieved reference block to the final user turn. This logic seems correct, but it's worth verifying that this is the intended behavior.

🟡 WARNING: Missing input validation at API boundaries

The diff does not include any new API endpoints or changes to existing API endpoints. However, it's worth reviewing the API endpoints that are being used to ensure that they have proper input validation.

🔵 SUGGESTION: Code quality improvements, naming, maintainability

The code changes are mostly improvements to existing code. However, some variable names could be more descriptive. For example, nc in mira-bots/shared/workers/rag_worker.py at line 849 could be renamed to something more descriptive.

The _reference_preamble variable (in mira-bots/shared/workers/rag_worker.py at line 873) is a long string that could be broken up into multiple lines for better readability.

✅ GOOD: Noteworthy good practices found

The changes to the RAGWorker class are well-organized and easy to follow. The use of list comprehensions and f-strings improves the readability of the code.

The test cases (in mira-bots/tests/test_reranking.py and mira-bots/tests/test_unit2_citations.py) are updated to reflect the changes to the RAGWorker class, which is good practice.

Overall, the changes in this PR seem to be improvements to the existing code, and no major security vulnerabilities or logic bugs were found. However, it's always a good idea to review the code again to ensure that it's correct and follows best practices.

Specific file and line number comments

  • mira-bots/shared/workers/rag_worker.py: The _sanitize_label_field function (at lines 41-50) is a good addition to sanitize label fields.
  • mira-bots/shared/workers/rag_worker.py: The _neutralize_chunk_text function (at lines 71-81) is a good addition to defuse structural markers in untrusted retrieved chunk text.
  • mira-bots/shared/workers/rag_worker.py: The _inject_reference_block function (at lines 945-958) seems correct, but it's worth verifying that this is the intended behavior.
  • mira-bots/tests/test_reranking.py: The test case test_chunks_injected_in_user_reference_block (at lines 103-115) is updated to reflect the changes to the RAGWorker class.

Generated by the MIRA automated code review pipeline (Groq → Cerebras → Gemini cascade)
To trigger self-fix: run bash scripts/pr_self_fix.sh 2295 locally, or add the auto-fix label to this PR (or run /autofix-pr from a Claude Code session)

@Mikecranesync Mikecranesync marked this pull request as ready for review June 25, 2026 12:50
@Mikecranesync Mikecranesync merged commit 2868627 into main Jun 25, 2026
27 checks passed
@Mikecranesync Mikecranesync deleted the security/rag-tenant-isolation-2112 branch June 25, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant