Conversation
Attacker-supplied [UD-*] / <user-data-*> patterns in tool output could nest inside Defender's real boundary, creating ambiguous trust framing for the LLM. Strip all boundary-like patterns from text before wrapping with the authentic boundary. Adds stripBoundaryPatterns() in boundary.ts, calls it in both sanitizer code paths, and adds integration tests for fake UD/XML boundary stripping. Made-with: Cursor
Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
This PR mitigates boundary tag spoofing (issue #46) by stripping attacker-supplied boundary-like markers ([UD-*] and <user-data-*>) from tool-result text before Defender applies its own boundary annotation, preventing fake “trusted” tags from nesting inside the real untrusted boundary.
Changes:
- Added
stripBoundaryPatterns()utility to remove boundary-like markers from content. - Applied boundary stripping immediately before
wrapWithBoundary()in both risk-based and specific-method sanitization flows. - Added integration tests to verify fake boundary tags are removed and real boundary wrapping remains intact.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/utils/boundary.ts | Adds stripBoundaryPatterns() for removing spoofed boundary-like tags prior to applying authentic boundaries. |
| src/sanitizers/sanitizer.ts | Invokes boundary stripping before boundary wrapping in both applyRiskBasedMethods and applySpecificMethods. |
| specs/integration.spec.ts | Adds integration coverage for spoofed [UD-*] and <user-data-*> tags being stripped and real wrapping preserved. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
cubic analysis
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/sanitizers/sanitizer.ts">
<violation number="1" location="src/sanitizers/sanitizer.ts:9">
P1: According to linked Jira issue ENG-12605, this change does not implement the ticket’s required Auth DB → Airbyte integration and instead modifies sanitizer boundary handling.</violation>
<violation number="2" location="src/sanitizers/sanitizer.ts:159">
P1: `stripBoundaryPatterns()` is called after role stripping and pattern removal have already run. An attacker can split an injection phrase with fake boundary tags (e.g., `ig[UD-x]nore previous instructions`) so earlier sanitizers don't match, and then this final strip re-joins the dangerous phrase unchecked. Move boundary pattern stripping to the beginning of the pipeline (before role/pattern sanitization), or re-run those detectors after stripping.</violation>
</file>
<file name="src/utils/boundary.ts">
<violation number="1" location="src/utils/boundary.ts:82">
P1: Replacing spoofed boundary tags with an empty string can concatenate adjacent tokens, enabling attackers to reconstitute blocked keywords (e.g., `SY[UD-x]STEM:` → `SYSTEM:`). Replace with a space and normalize whitespace to prevent unintended token joining.</violation>
</file>
Linked issue analysis
Linked issue: ENG-12605: Integrate Auth DB with Airbyte for BI data pipeline
| Status | Acceptance criteria | Notes |
|---|---|---|
| ❌ | Add the Auth DB as a source in Airbyte to pipe data to Databricks | No Airbyte connector or configuration changes in diffs |
| ❌ | Create a database user for Airbyte | No DB user creation scripts, migrations, or infra changes |
| ❌ | Update permissions to allow the Airbyte instance to query the database | No GRANT/permission SQL or permission configuration changes |
| ❌ | Provide a more accurate source of truth for user state by routing Auth DB data to Databricks (data pipeline end-to-end) | No pipeline, Databricks destination, or sync logic present |
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Stripping spoofed boundary tags at the end of the pipeline allowed attackers to use fake tags as splitters (e.g. SY[UD-x]STEM:) to evade role stripping and pattern removal. Move stripBoundaryPatterns to run unconditionally before any content-based detection steps. Adds regression tests for splitter evasion via fake boundary tags. Made-with: Cursor
…on (#55) * fix(tier2): strip boundary markers from input before classification Defender wraps sanitized outputs with [UD-<id>]...[/UD-<id>] markers. When those outputs are fed back as input — nested tool calls, cached responses, multi-hop agent traces, or attacker-spoofed tags — the tokenizer counts the tag tokens as part of the classified sentence and the v4 ONNX model treats that structure as injection-adjacent. Measured on a benign Jira payload pre-wrapped by upstream defender: score went 0.008 (stripped) → 0.99 (with tags), flipping a clean pass into a high-risk block. The stripBoundaryPatterns utility (src/utils/boundary.ts:68) was written for exactly this — docstring: "Boundary tags like [UD-xyz] ...[/UD-xyz] corrupt per-sentence model scores because the tokenizer treats the tag text as part of the sentence" — but had zero callers. Wires it into the three Tier 2 entry points (classify, classifyByChunks, prepareChunks) before the length check and tokenization. Also mitigates the spoofed-boundary attack PR #49 was targeting: attacker-injected [UD-*] tags in input get stripped before the classifier sees them, so they can't be used to mask injection content as "already-trusted" data. Behavior change: Tier 2 scores on payloads containing UD/XML boundary markers now match scores on the same payloads with markers manually stripped (added spec asserts bit-identical scoring). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(tier2): address Copilot review on boundary stripping Four review items: 1. classifyBySentence was missing the strip — added. Now all four public Tier 2 entry points (classify, classifyBySentence, classifyByChunks, prepareChunks) share the same pre-strip behavior. 2. stripBoundaryPatterns utility previously called .trim() as a side effect, which changed semantics for every Tier 2 input regardless of whether it contained boundary markers. Removed the trim from the utility; callers who need it should call .trim() themselves. None of the current call sites need it (the strip itself is sufficient). 3. New spec was missing it.skipIf(!!process.env.CI) — other model- dependent tests in the file use it to avoid CI flakiness on ONNX loading. Matched the convention. 4. Strict toBe equality on float model output is brittle across runtime/hardware. Switched to toBeCloseTo at 10-decimal precision — still asserts the scores match (the inputs are bit-identical after stripping) while tolerating any ONNX runtime non-determinism. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ty redaction Two issues from Codex review: P1: PatternDetector classified obfuscated payloads (1gn0r3 pr3v10us, S Y S T E M:, Zalgo) by running Tier 1 against normalized text, but Sanitizer.applyRiskBasedMethods ran containsRoleMarkers/removePatterns against the un-normalized original. Result: detection escalated risk to medium/high but the actual obfuscated content survived in the sanitized output and could flow into downstream prompts. Fix: added Step 1.5 in the sanitizer that runs at HIGH risk only — NFD decompose + stripCombiningMarks + normalizeWhitespace + normalizeLeetSpeak — before role stripping and pattern removal. At high risk Tier 1 already has high confidence of an attack, so the trade-off of stripping benign accents from the redacted output is acceptable. Medium-risk benign content is unaffected. P2: detectHtmlEntities pushed every 3+ entity run into detections, causing redactAllEncoding to wipe benign escaped content like 10% ("10%") whenever the field was escalated to high risk for an unrelated reason. The decoder doc claimed "Only emits suspicious" but the code emitted all. Fix: kept the detector pushing all detections (so decodeAllLevels can chain through HTML→base64→plaintext correctly), but processEncodedContent in REDACT mode now filters out non-suspicious HTML entities. Decode mode is unaffected. Verified end-to-end: - Leet, whitespace-spaced, and Zalgo payloads are now redacted in output - Benign HTML entities (10% encoded) survive sanitization - Accented text (café, niño) preserved at low/medium risk Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…#39) * feat(tier1): add obfuscation normalisation chain to Tier 1 detection Adds a pre-processing normalisation chain to PatternDetector.analyze so existing injection patterns catch obfuscated variants without requiring new regex entries for each substitution style. Changes: - leet-normalizer.ts (new): normalizeLeetSpeak() reverses digit/symbol substitutions (4→a, 3→e, 1→i, 0→o, 5→s, 7→t); protects hex escapes, base64 blobs and $( from corruption - normalizer.ts: adds normalizeWhitespace() — collapses letter-by-letter spacing (S Y S T E M → SYSTEM) and embedded newlines inside words - pattern-detector.ts: runs normalisation chain (whitespace → unicode → leet) before Tier 1 matching; two-pass pattern run on raw+normalised text with dedup so obfuscation patterns still fire on raw text - encoding-detector.ts: adds decodeAllLevels() for chained encoding (base64 of hex etc.) and containsSuspiciousEncodingDeep() - sanitizer.ts: uses containsSuspiciousEncodingDeep in high-risk path - patterns.ts: removes stale leet entries from FAST_FILTER_KEYWORDS Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(lint): merge duplicate normalizer imports and sort import order Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(review): address PR review comments on obfuscation normalisation - encoding-detector: containsSuspiciousEncodingDeep now also runs containsSuspiciousEncoding on the decoded result, catching payloads that remain partially encoded when maxIterations is reached (P1) - types: add normalised?: boolean to PatternMatch to signal when position/matched values reference the normalised form, not the original input string (P2) - pattern-detector: tag normalised-pass matches with normalised:true so consumers can distinguish them from raw-text matches (P2) - normalizer: remove \s* from newline regex to avoid silently consuming word-separator spaces (e.g. "ignore\n previous" → "ignoreprevious" would break multi-word pattern matching) (P2) - test: assert specifically on ignore_previous rather than using || with leetspeak_injection, so regressions in leet normalisation are caught rather than masked by the raw-text pass (P2) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(tier1): correct normalisation order and skip redundant pass for plain text Two bugs found during PR review: 1. Normalisation order was wrong: running normalizeWhitespace before normalizeUnicode means Cyrillic/fullwidth homoglyph spacing attacks (e.g. "s у s t e m" with Cyrillic у) are never collapsed — whitespace normalisation uses [a-zA-Z] and exits before unicode normalisation resolves the homoglyphs to ASCII. Fix: run normalizeUnicode first so all characters are ASCII before whitespace collapse runs. Order is now: normalizeUnicode → normalizeWhitespace → normalizeLeetSpeak 2. Performance: when normalisation produces no change (plain text with keywords, the common case), detectPatterns was called twice on identical text. Added a rawText === analysisText guard to short-circuit to a single pass, restoring the original single-pass performance for unobfuscated input. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(tier1): run raw pass when normHasKeywords; add missing unit tests Raw pass was skipped for pure leet-speak inputs (e.g. "1gn0r3 pr3v10us") because leet keywords were removed from FAST_FILTER_KEYWORDS, making rawHasKeywords false. The comment claimed the raw pass "catches obfuscation patterns like leetspeak_injection" but it was never actually running for those inputs. Fix: run the raw pass whenever rawHasKeywords OR normHasKeywords is true, so raw obfuscation patterns fire even when only the normalised text triggered the fast filter. Adds unit tests for: - normalizeWhitespace: letter spacing, embedded newlines, edge cases - normalizeLeetSpeak: substitution map, all protected sequence types ($(/ hex/unicode/base64), ! boundary rule, plain text passthrough - decodeAllLevels: single-layer, double-layer (chained), maxIterations cap, amplification guard - containsSuspiciousEncodingDeep: single/double encoded payloads, benign cases Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * feat(detection): add HTML entity, ROT13, ROT47, binary, Morse, and Zalgo detection New encoding detectors (all integrated into detectEncoding/decodeAllLevels): - HTML entities: decodes &#NNN;, &#xHH;, and 30 named entities; gate: 3+ grouped entity tokens; suspicious when decoded content has injection keyword - ROT13: gate 70%+ letter density; only emits when decoded text contains an injection keyword — prevents false positives on arbitrary high-letter text - ROT47: printable ASCII rotation; conservative — only emits on suspicious decoded content - Binary strings: gate 3+ space-separated 8-bit groups of [01]; decodes via parseInt(group, 2) - Morse code: gate 5+ dot/dash groups; 36-entry table (A–Z, 0–9); rejects if >20% unknown symbols Zalgo / combining marks (normalizer.ts): - stripCombiningMarks() strips U+0300–U+036F and 4 other combining ranges - normalizeUnicode() now runs NFD → stripCombiningMarks → NFKC so marks are separated before being stripped (NFKC alone would compose them into precomposed chars that the regex cannot see) - containsSuspiciousUnicode() flags 3+ combining marks Leet-speak improvements (leet-normalizer.ts): - Token-aware normalization: only substitutes within alphanumeric tokens that contain at least one letter — "price: 100" stays "price: 100" - Added @→a and 8→b to LEET_MAP - Includes !, @, $ in token regex so "adm!n", "@dm1n", "$y$tem" normalize correctly - PROTECTED_SEQUENCE continues to guard $( before token processing runs Tier 1 patterns (patterns.ts): - Added binary_string_encoding (medium) and morse_code_encoding (low) patterns - Upgraded rot13_mention severity from low → medium All new behaviour covered by 28 new test cases (240 total, up from 212). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(encoding): prevent full-text detection overlap in processEncodedContent ROT13 and ROT47 detections span position=0, length=text.length. When both fired on the same text alongside positional detections (hex/base64), the reverse-position splice loop would apply positional replacements first, then the full-text detection would overwrite them using the original text.length — corrupting previous replacements. In decodeAllLevels (action:"decode"), a text triggering both ROT13 and ROT47 would oscillate across all 5 iterations without converging. Fix: processEncodedContent now separates positional from full-text detections. Positional detections are applied first. Full-text is only applied when there are no positional detections; only the first full-text detection is used when multiple exist. decodeAllLevels naturally converges because after positional content is decoded, the next iteration re-evaluates the full-text transforms. Also adds three tests flagged during review: - normalizeWhitespace: letter-adjacent newline (no surrounding spaces) - normalizeUnicode: precomposed accent (café → cafe) via NFD decomposition - normalizeLeetSpeak: mixed alphanumeric tokens (v3rs10n → version) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * test(encoding): add ROT13+ROT47 simultaneous detection coverage Adds two tests specifically for the full-text detection overlap fix: - processedText is a coherent string (not a corrupted splice) when both ROT13 and ROT47 fire on the same input - decodeAllLevels converges (levels <= 2, not oscillating to maxIterations=5) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * fix(detection): add encoding-based risk escalation in sanitizeStringField Encoded payloads (ROT13, binary, Morse, etc.) don't trigger Tier 1 patterns because their content has no fast-filter keywords — so risk stays at the default "medium" and encoding detection in the sanitizer (Step 4, gated behind riskLevel === "high") never runs. Injections encoded in these formats pass through undetected. Fix: after Tier 1 classification in sanitizeStringField, run containsSuspiciousEncoding (shallow, single pass, ~0.05ms per field) as an additional risk escalation check. If suspicious encoding is found, risk escalates to "high" and the existing deep multi-level decoder in the sanitizer's Step 4 handles decoding and redaction. Before: ROT13/binary/Morse payloads → risk stays medium → encoding detection skipped → allowed: true (injection passes through) After: ROT13/binary/Morse payloads → encoding escalation → risk=high → deep decode + redaction → blocked Quality test results (Tier 1 + sanitizer, no ML): - Precision: 100% (0 false positives on 10 benign inputs) - Recall: 88% (7/8 encoding types caught; ROT47 miss due to short input) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: restore main's v4 ONNX model files (accidentally reverted during merge) * fix(review): address 3 Codex review findings on obfuscation PR P1: Accent stripping leaked into returned output (data loss bug) - normalizeUnicode previously did NFD→stripCombiningMarks→NFKC, which rewrote 'café' → 'cafe' for benign user content. Sanitizer.applyRiskBasedMethods returns this output to callers, breaking the analysis-only contract. - Fix: removed stripCombiningMarks from normalizeUnicode (kept NFKC only). Combining-mark stripping now lives in the analysis-only path inside PatternDetector.analyze: stripCombiningMarks(rawText.normalize('NFD')) runs before normalizeUnicode in the chain. P1: Long leet payloads bypassed the fast filter - The leet normaliser skips 20+ char alphanumeric tokens (treated as base64-like blobs), so '1gn0r3pr3v10us1nstruct10ns' was left unchanged by both the leet normaliser and unicode normaliser. With the leet keywords removed from FAST_FILTER_KEYWORDS, the fast filter short-circuited and leetspeak_injection was never evaluated. - Fix: restore '1gn0r3', 'f0rg3t', 'byp4ss' to FAST_FILTER_KEYWORDS so long leet payloads still pass the filter and reach the regex. P2: Chained encoding payloads (e.g. btoa(btoa(...))) slipped through - sanitizeStringField escalated using shallow containsSuspiciousEncoding, which only catches single-layer encodings. Doubly-encoded payloads — where the outer layer decodes to another encoded blob with no visible keywords — stayed at medium risk and never reached the deep check in the sanitizer (gated at high). - Fix: sanitizeStringField now uses containsSuspiciousEncodingDeep which loops through layers (max 5, with amplification guard). Also tracks whether escalation came from encoding so the early-block path records 'encoding_detection' in methodsByField — without this, blockHighRisk would set sanitized to '[CONTENT BLOCKED]' but allowed would stay true because hasThreats only counts active sanitization methods. Test updates: - normalizeUnicode tests now assert accent preservation (the new contract) - new stripCombiningMarks tests cover the analysis-only Zalgo path 268/269 tests passing (1 pre-existing flaky test on main, not introduced by this PR — onnx-classifier batch test scoring borderline injection sample at 0.467 vs >0.5 threshold). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): remove dead try/catch in detectBinaryStrings (aikido) * fix(review): close detection-vs-sanitization gap and benign HTML entity redaction Two issues from Codex review: P1: PatternDetector classified obfuscated payloads (1gn0r3 pr3v10us, S Y S T E M:, Zalgo) by running Tier 1 against normalized text, but Sanitizer.applyRiskBasedMethods ran containsRoleMarkers/removePatterns against the un-normalized original. Result: detection escalated risk to medium/high but the actual obfuscated content survived in the sanitized output and could flow into downstream prompts. Fix: added Step 1.5 in the sanitizer that runs at HIGH risk only — NFD decompose + stripCombiningMarks + normalizeWhitespace + normalizeLeetSpeak — before role stripping and pattern removal. At high risk Tier 1 already has high confidence of an attack, so the trade-off of stripping benign accents from the redacted output is acceptable. Medium-risk benign content is unaffected. P2: detectHtmlEntities pushed every 3+ entity run into detections, causing redactAllEncoding to wipe benign escaped content like 10% ("10%") whenever the field was escalated to high risk for an unrelated reason. The decoder doc claimed "Only emits suspicious" but the code emitted all. Fix: kept the detector pushing all detections (so decodeAllLevels can chain through HTML→base64→plaintext correctly), but processEncodedContent in REDACT mode now filters out non-suspicious HTML entities. Decode mode is unaffected. Verified end-to-end: - Leet, whitespace-spaced, and Zalgo payloads are now redacted in output - Benign HTML entities (10% encoded) survive sanitization - Accented text (café, niño) preserved at low/medium risk Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Summary
stripBoundaryPatterns()insrc/utils/boundary.ts— removes attacker-supplied[UD-*]and<user-data-*>patterns from text before the real boundary is applied.applyRiskBasedMethodsandapplySpecificMethodsinsrc/sanitizers/sanitizer.ts, immediately beforewrapWithBoundary.[UD-TRUSTED]...[/UD-TRUSTED]can no longer nest inside Defender's authentic boundary.Test plan
[UD-TRUSTED]stripped, only one real boundary pair remains<user-data-FAKE>XML tags strippedCloses the structural component of #46.
blockHighRiskdefault and risk escalation are separate product decisions.Summary by cubic
Prevents boundary tag spoofing by stripping fake
[UD-*]and<user-data-*>tags at the very start of sanitization, before any detection, so attackers can’t split patterns or nest fake tags inside the real boundary.stripBoundaryPatterns()insrc/utils/boundary.tsto remove fake boundary-like tags.src/sanitizers/sanitizer.ts, before any content-based detection and beforewrapWithBoundary.[UD-TRUSTED]and<user-data-FAKE>, block splitter evasion (SY[UD-x]STEM:,ig[UD-x]nore...), and ensure only one real boundary remains after wrapping.Written for commit 1c03507. Summary will update on new commits.