fix(security): harden prompt injection and plugin storage isolation#114
Merged
qnbs merged 4 commits intoJun 12, 2026
Merged
Conversation
Security fixes for critical vulnerabilities:
1. Plugin Storage Namespacing (HIGH)
- Enforce plugin:${id}: prefix for all storageRead/storageWrite calls
- Prevents cross-plugin data access and potential IP exfiltration
- services/pluginRegistry.ts: validatePluginStorageKey()
2. Prompt Injection Validation (HIGH)
- Reject null bytes and invalid UTF-8 in AI-generated content
- Applied before manuscript insertion via applyReviewEditsToSection
- services/proForge/applyReviewEdits.ts: validateProposedText()
3. DOMPurify Hardening (MEDIUM)
- Remove span from ALLOWED_TAGS to prevent style injection
- Explicitly set ALLOWED_ATTR to [class] only
- components/copilot/CopilotMessageList.tsx
4. Update plugin examples for namespaced keys
- services/plugins/sceneAppender.plugin.ts
- plugins/example-word-counter/index.ts
Tests: Add validation tests for all security boundaries
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| type: 'proseEdit', | ||
| severity: 'info', | ||
| description: 'Copilot suggested replacement', | ||
| original: sectionContent, |
There was a problem hiding this comment.
Suggestion: Whole-section replacement now fails when the current section is empty. In that case original is set to sectionContent (an empty string), and applyReviewEditsToSection treats empty original as non-anchorable (if (item.original) is false), so it skips the edit and returns applied = 0. Use an explicit full-range edit (for example range: { start: 0, end: sectionContent.length } with original omitted) so empty sections can still be replaced. [logic error]
Severity Level: Critical 🚨
- ❌ Copilot cannot populate empty manuscript sections via suggestions.
- ⚠️ Apply-last-suggestion flow fails on new empty chapters.Steps of Reproduction ✅
1. Locate the whole-section replacement path in `services/copilot/actionApplier.ts:19-41`,
where `applyTextEdit(sectionContent, original, proposed)` checks `if (!original.trim())`
and, for this case, calls `applyReviewEditsToSection(sectionContent, [{ original:
sectionContent, proposed, ... }])`.
2. From any runtime or test (e.g. mirroring
`tests/unit/copilot/actionApplier.test.ts:5-41`), call `applyTextEdit('', '', 'new chapter
text')`, so `sectionContent` is `''` and `original` is `''`, causing the whole-section
branch to construct a `ReviewItem` with `original: ''` and no `range`.
3. Inside `services/proForge/applyReviewEdits.ts:96-147`, `applyReviewEditsToSection`
processes this item: `item.proposed` is defined and validated, `item.range` is `undefined`
so Strategy 1 is skipped, and Strategy 2's fallback `if (item.original)` at line 119
evaluates false for `''`, so `nearestFreeOccurrence` is never called and `skipped++` is
executed with no planned edits.
4. `applyReviewEditsToSection` returns `{ content: '', applied: 0, skipped: 1 }`, so
`applyTextEdit` also returns `applied = 0`; when called through the UI path in
`hooks/useGlobalCopilot.ts:258-32` (where `applyLastSuggestion` invokes
`applyTextEdit(existingContent, '', codeBlock)` and only updates the manuscript if
`result.applied > 0`), attempting to apply a Copilot suggestion to an empty section leaves
the section unchanged and surfaces an error state instead of inserting the proposed text.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** services/copilot/actionApplier.ts
**Line:** 35:35
**Comment:**
*Logic Error: Whole-section replacement now fails when the current section is empty. In that case `original` is set to `sectionContent` (an empty string), and `applyReviewEditsToSection` treats empty `original` as non-anchorable (`if (item.original)` is false), so it skips the edit and returns `applied = 0`. Use an explicit full-range edit (for example `range: { start: 0, end: sectionContent.length }` with `original` omitted) so empty sections can still be replaced.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+78
to
+80
| if (text.includes('\0')) { | ||
| throw new Error('Invalid UTF-8 in proposed text'); | ||
| } |
There was a problem hiding this comment.
Suggestion: The implementation says it rejects control characters, but it only checks for null bytes, so other control characters (\u0001–\u001F, \u007F) still pass through into manuscript content. If control-char rejection is required for hardening, expand this validation to cover the full control-character set (except any explicitly allowed whitespace). [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Non-null control characters can be saved in manuscripts.
- ⚠️ Control-character rejection does not match hardening comment.Steps of Reproduction ✅
1. The security-hardening comment above `validateProposedText` in
`services/proForge/applyReviewEdits.ts:71-75` states: "reject control characters and
validate UTF-8 to prevent injection attacks via AI-generated content," indicating an
intent to filter the broader control-character set, not just null bytes.
2. The actual implementation at `services/proForge/applyReviewEdits.ts:76-88` only checks
`if (text.includes('\0')) { throw new Error('Invalid UTF-8 in proposed text'); }` (lines
78-80). No checks are performed for other control characters such as `\u0001`–`\u001F` or
`\u007F`.
3. When `applyReviewEditsToSection` (`services/proForge/applyReviewEdits.ts:96-129`) is
invoked—either directly in tests or indirectly via `planAcceptedManuscriptEdits` from
`ProForgeOrchestrator.submitReview`—each `ReviewItem`'s `proposed` text is passed through
`validateProposedText` at line 104. If a `ReviewItem` has `proposed: 'foo\u0001bar'` (or
any non-null control code), `text.includes('\0')` is false, so no error is thrown and the
string is accepted.
4. `planAcceptedManuscriptEdits` (`services/proForge/applyReviewEdits.ts:154-185`) then
applies these validated proposals into `project.manuscript`, and
`ProForgeOrchestrator.submitReview` (`services/proForge/proForgeOrchestrator.ts:32-46`)
persists the updated sections and creates snapshots. As a result, manuscript content can
contain control characters other than null despite the comment promising broader
control-character rejection, leaving the hardening incomplete and potentially allowing
undesirable control codes into stored prose.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** services/proForge/applyReviewEdits.ts
**Line:** 78:80
**Comment:**
*Incomplete Implementation: The implementation says it rejects control characters, but it only checks for null bytes, so other control characters (`\u0001`–`\u001F`, `\u007F`) still pass through into manuscript content. If control-char rejection is required for hardening, expand this validation to cover the full control-character set (except any explicitly allowed whitespace).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+82
to
+86
| try { | ||
| const encoded = new TextEncoder().encode(text); | ||
| new TextDecoder().decode(encoded); | ||
| } catch { | ||
| throw new Error('Invalid UTF-8 in proposed text'); |
There was a problem hiding this comment.
Suggestion: The UTF-8 check is ineffective: encoding a JavaScript string with TextEncoder and immediately decoding it with TextDecoder will always succeed for normal JS strings, so this does not actually detect malformed UTF-8 input. This creates a false security guarantee; validate raw byte input (if available) or remove this check and enforce the actual constraints you can verify at string level. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ UTF-8 validation guard never trips for any string input.
- ⚠️ Security hardening comment overstates actual validation performed.Steps of Reproduction ✅
1. Locate `validateProposedText` in `services/proForge/applyReviewEdits.ts:76-88`. The
function tries to reject invalid UTF-8 by encoding the input string with `new
TextEncoder().encode(text)` and immediately decoding those bytes with `new
TextDecoder().decode(encoded)` inside the `try` block shown at lines 82-86 of the PR hunk.
2. Observe that the only place this validator is used is in `applyReviewEditsToSection`
(`services/proForge/applyReviewEdits.ts:96-129`), where each `ReviewItem` with a
`proposed` value calls `const proposed = validateProposedText(item.proposed);` before
planning edits. This runs for every accepted edit used by both direct calls (unit tests)
and by the orchestrator via `planAcceptedManuscriptEdits`.
3. In the existing unit tests at `tests/unit/proForge/applyReviewEdits.test.ts:149-155`, a
proposed value containing a null byte (`'slow\0malicious'`) causes
`applyReviewEditsToSection` to throw `Invalid UTF-8`, but this exception is raised by the
null-byte check (`text.includes('\0')`) at `services/proForge/applyReviewEdits.ts:77-80`,
not by the `TextEncoder`/`TextDecoder` block at lines 82-86, demonstrating that the
encoder/decoder path is not needed to satisfy the test.
4. Because JavaScript strings are already sequences of Unicode scalar values,
`TextEncoder.encode(string)` followed by `TextDecoder.decode(encoded)` cannot fail for any
normal JS string; the decoder is fed exactly the bytes it just produced. Therefore the
`try`/`catch` at lines 82-86 can never throw for any `string` value passed through
`validateProposedText`, meaning that the "Validate UTF-8" step does not detect malformed
UTF-8 and provides a false sense of additional validation beyond the null-byte check.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** services/proForge/applyReviewEdits.ts
**Line:** 82:86
**Comment:**
*Incomplete Implementation: The UTF-8 check is ineffective: encoding a JavaScript string with `TextEncoder` and immediately decoding it with `TextDecoder` will always succeed for normal JS strings, so this does not actually detect malformed UTF-8 input. This creates a false security guarantee; validate raw byte input (if available) or remove this check and enforce the actual constraints you can verify at string level.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (item.proposed === undefined) continue; | ||
| const proposed = item.proposed; | ||
| // QNBS-v3: Validate proposed text before insertion to prevent injection attacks. | ||
| const proposed = validateProposedText(item.proposed); |
There was a problem hiding this comment.
Suggestion: Validating item.proposed by throwing inside the main edit loop makes one bad suggestion abort the entire section apply operation, so valid edits in the same batch are never processed. This can break submitReview flows that call this function without local handling for per-item failures; treat invalid proposals as skipped edits instead of terminating the whole batch. [logic error]
Severity Level: Critical 🚨
- ❌ One malformed edit aborts full ProForge stage submission.
- ⚠️ Other valid accepted edits for that stage never apply.Steps of Reproduction ✅
1. In the UI, `PipelineReviewPanel`'s `handleSubmit`
(`components/proForge/PipelineReviewPanel.tsx:132-142`) builds `decisions` from all
`activeStageResult.reviewItems` and calls `await submitReview(stage, decisions);` without
any try/catch around this call.
2. `submitReview` from `useProForgeOrchestrator`
(`hooks/useProForgeOrchestrator.ts:134-143`) obtains the orchestrator instance and calls
`await orchestrator.submitReview(stage, decisions, { advance: true });`, directly
forwarding rejections from `ProForgeOrchestrator.submitReview`.
3. In `ProForgeOrchestrator.submitReview`
(`services/proForge/proForgeOrchestrator.ts:25-92`), for editing stages
(`isEditingStage(stage)`), the method pulls `acceptedItems` from the current run and calls
`planAcceptedManuscriptEdits(project.manuscript, acceptedItems)` at lines 43-46 to apply
edits before snapshotting and dispatching `submitStageReview`.
4. `planAcceptedManuscriptEdits` (`services/proForge/applyReviewEdits.ts:154-185`) loops
by section and, for each, calls `applyReviewEditsToSection(section.content ?? '', items)`
(line 177). Inside `applyReviewEditsToSection`
(`services/proForge/applyReviewEdits.ts:96-129`), each item with a `proposed` value is
first validated via `const proposed = validateProposedText(item.proposed);` at line 104.
If any accepted `ReviewItem` contains a null byte in its `proposed` text (as in the unit
test `rejects proposed text containing null bytes` at
`tests/unit/proForge/applyReviewEdits.test.ts:149-155`), `validateProposedText` throws,
causing `applyReviewEditsToSection` to throw, which propagates up through
`planAcceptedManuscriptEdits` and `ProForgeOrchestrator.submitReview` to `handleSubmit`.
Because there is no error handling at any layer, this single invalid proposal aborts the
entire submission for that stage: no other valid edits are applied, no snapshot is
created, and the stage submission fails instead of treating the bad proposal as a skipped
edit.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** services/proForge/applyReviewEdits.ts
**Line:** 104:104
**Comment:**
*Logic Error: Validating `item.proposed` by throwing inside the main edit loop makes one bad suggestion abort the entire section apply operation, so valid edits in the same batch are never processed. This can break `submitReview` flows that call this function without local handling for per-item failures; treat invalid proposals as skipped edits instead of terminating the whole batch.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
24
to
+36
| if (!original.trim()) { | ||
| // QNBS-v3: whole-section replacement — used when the AI rewrites the full chapter. | ||
| return { content: proposed, applied: 1, skipped: 0 }; | ||
| // Validation happens in applyReviewEditsToSection via the ReviewItem path. | ||
| // For whole-section replacement, we still need to validate. | ||
| return applyReviewEditsToSection(sectionContent, [ | ||
| { | ||
| id: 'copilot-apply', | ||
| stage: 'copyEdit', | ||
| type: 'proseEdit', | ||
| severity: 'info', | ||
| description: 'Copilot suggested replacement', | ||
| original: sectionContent, | ||
| proposed, |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
Whole-section replacement now routes through applyReviewEditsToSection, but when the current section content is empty (sectionContent === ''), the generated ReviewItem has an empty original and no range, so it cannot be anchored and is always skipped, preventing Copilot from applying output into a blank chapter.
Suggestion: Special-case whole-section replacements on empty content so they unconditionally replace the section (e.g. bypass applyReviewEditsToSection or pass an explicit { start: 0, end: 0 } range) instead of relying on text anchoring that fails when original is empty.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** services/copilot/actionApplier.ts
**Line:** 24:36
**Comment:**
*HIGH: Whole-section replacement now routes through applyReviewEditsToSection, but when the current section content is empty (`sectionContent === ''`), the generated ReviewItem has an empty `original` and no `range`, so it cannot be anchored and is always skipped, preventing Copilot from applying output into a blank chapter.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
…njection & plugin isolation
- services/proForge/applyReviewEdits.ts:
- Reject C0 controls (except tab/lf/cr), null bytes, lone surrogates.
- Remove fake TextEncoder/TextDecoder UTF-8 round-trip.
- Per-item graceful skip; invalid edits no longer abort the batch.
- Allow empty original ('') as valid anchor with explicit range.
- Add MAX_PROPOSED_LENGTH (1 MiB) DoS guard.
- Add structured logging for rejected edits.
- services/copilot/actionApplier.ts:
- Whole-section replacement now passes explicit range {0, length} + original,
fixing empty-section rewrite failure.
- services/pluginRegistry.ts:
- Storage key length limit (256), allowed suffix chars, anti-traversal (..).
- Serialized value size cap (2 MiB).
- Extra plugin-id safety guard.
- Structured logging for isolation violations.
- components/copilot/CopilotMessageList.tsx:
- Harden DOMPurify: ALLOW_DATA_ATTR=false, FORBID_ATTR style, SANITIZE_DOM=true.
- Clarify sanitization comment.
- services/proForge/proForgeCapabilityCore.ts / proForgeOrchestrator.ts:
- Propagate new 'invalid' edit count through capability layer and orchestrator logs.
- Tests expanded for empty sections, control chars, lone surrogates, oversized
proposals, mixed valid/invalid batches, and plugin storage validation edge cases.
- docs: Update .github/SECURITY.md, docs/SECURITY-THREAT-MODEL.md, CHANGELOG.md.
…x with code-point loop
…curity contracts - Update sceneAppender test expectations to namespaced storage key plugin:storecraft.scene-appender:run-count. - Add createLogger mock in proForgeOrchestrator.test.ts because applyReviewEdits.ts now imports the logger factory.
qnbs
deleted the
fix/security-hardening-prompt-injection-plugin-isolation
branch
June 12, 2026 13:22
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Security Fixes
Critical Vulnerabilities Addressed
Plugin Storage Namespacing (HIGH)
plugin:${id}:prefix for allstorageRead/storageWritecallsservices/pluginRegistry.tsPrompt Injection Validation (HIGH)
applyReviewEditsToSectionservices/proForge/applyReviewEdits.tsDOMPurify Hardening (MEDIUM)
spanfromALLOWED_TAGSto prevent style injection vectorsALLOWED_ATTRto[class]onlycomponents/copilot/CopilotMessageList.tsxTest Coverage
Risk Assessment
These fixes address the highest-priority security findings from the audit:
CodeAnt-AI Description
Harden plugin storage, copilot text insertion, and message rendering against injection
What Changed
plugin:<plugin-id>:namespace, preventing one plugin from reading or writing another plugin’s dataspantags in rendered markdown, reducing the risk of style-based injectionImpact
✅ Fewer cross-plugin data leaks✅ Safer AI-generated manuscript edits✅ Lower risk of message rendering abuse💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.