fix: t3chat prompt injection

[MaheshtheDev]

No description provided.

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	supermemory-app	1165834	Oct 21 2025, 07:01 PM

[MaheshtheDev]

• fix: t3chat prompt injection #505 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add the label Main to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Code Review: T3 Chat Prompt Injection Fix

Summary

This PR addresses a prompt injection vulnerability in the T3 chat integration by refactoring event handlers to conditionally intercept send actions only when supermemories are present. The approach prevents double-submission issues and improves code organization.

---

✅ Strengths

1. Security Improvement: The conditional check (hasMemories) prevents unnecessary interception when no memories are attached, reducing the attack surface.

2. Code Organization: Refactoring inline event listeners into named functions (handleT3SendButtonClick, handleT3EnterKey) improves readability and maintainability.

3. Better Event Handling: Using event.preventDefault() and event.stopPropagation() only when needed prevents unintended side effects.

---

⚠️ Issues & Concerns

1. Race Conditions with setTimeout (Critical)

Location: t3.ts:609-625, t3.ts:658-671

The use of hardcoded 100ms timeouts creates race conditions:

setTimeout(() => {
    const form = sendButton.closest("form")
    if (form) {
        form.requestSubmit()
    }
}, 100)

Issues:

• If captureT3PromptContent takes >100ms, the form submits with incomplete data
• On slow devices/networks, this could fail unpredictably
• No error handling if the async operation fails

Recommendation: Use await properly or implement a callback pattern:

await captureT3PromptContent("button click")
const form = sendButton.closest("form")
if (form) {
    form.requestSubmit()
} else {
    // fallback
}

2. Event Handler Re-registration Anti-pattern (High)

Location: t3.ts:619-623

document.removeEventListener("click", handleT3SendButtonClick, true)
sendButton.dispatchEvent(newEvent)
setTimeout(() => {
    document.addEventListener("click", handleT3SendButtonClick, true)
}, 100)

Issues:

• Creates a 100ms window where clicks are not captured
• Multiple rapid clicks could cause handler registration issues
• Unnecessarily complex - the handler should be idempotent

Recommendation: Use a flag to prevent re-entry:

let isProcessing = false
const handleT3SendButtonClick = async (event: Event) => {
    if (isProcessing) return
    // ... rest of logic
}

3. Memory Check Logic Duplication (Medium)

Location: t3.ts:588-598 and t3.ts:638-648

The same memory detection logic appears in both handlers. This violates DRY principles.

Recommendation: Extract to a shared function:

function hasMemoriesAttached(): boolean {
    const textareaElement = 
        document.querySelector("textarea") as HTMLTextAreaElement ||
        document.querySelector('div[contenteditable="true"]') as HTMLElement
    
    return !!(
        textareaElement?.dataset.supermemories ||
        document.querySelector('[id*="sm-t3-input-bar-element"]')?.dataset.memoriesData
    )
}

4. Missing Error Handling (Medium)

Location: Throughout both handlers

If captureT3PromptContent throws an error, the form never submits. Users would experience a broken send button with no feedback.

Recommendation: Add try-catch with fallback:

try {
    await captureT3PromptContent("button click")
} catch (error) {
    console.error("Failed to capture prompt, proceeding with submit:", error)
    // Still allow form submission
}

5. Synthetic Event Security (Low-Medium)

Location: t3.ts:614-618, t3.ts:663-668

Creating and dispatching synthetic events can bypass security checks on some platforms.

Issues:

• form.requestSubmit() is preferred but fallback creates new events
• The new KeyboardEvent doesn't include all properties of the original

Recommendation: Consider if the fallback is necessary or if it should log a warning instead.

---

🔒 Security Analysis

Positive Changes:

• ✅ Conditional interception reduces unnecessary data processing
• ✅ No obvious XSS vulnerabilities introduced
• ✅ Maintains existing validation in captureT3PromptContent

Remaining Concerns:

• ⚠️ The prompt injection vulnerability fix relies on client-side checks only. If the attacker can manipulate the DOM to add/remove dataset.supermemories, they could bypass this.
• ⚠️ No server-side validation mentioned for captured prompts
• ⚠️ The innerHTML manipulation at t3.ts:541 could be exploited if storedMemories contains malicious HTML

Recommendation: Ensure server-side validation and consider using textContent instead of innerHTML for memory injection.

---

🚀 Performance Considerations

1. Event Listener Registration: Using capture phase (true) on document-level listeners is correct but ensure cleanup on page unload to prevent memory leaks.

2. Repeated DOM Queries: Both handlers query for the same elements. Consider caching references if this code runs frequently.

3. Async in Event Handlers: The await in event handlers blocks the event loop. For better UX, consider showing a loading state.

---

🧪 Test Coverage

Critical Issue: No test files found for the browser extension.

Recommendations:

1. Add unit tests for:
   • hasMemoriesAttached() logic
   • Event handler behavior with/without memories
   • Race condition scenarios with setTimeout
2. Add integration tests for:
   • Full prompt capture flow
   • Form submission with synthetic events
3. Add E2E tests for:
   • User interaction with T3 chat when memories are attached
   • Regression tests for the original prompt injection bug

---

📝 Minor Issues

1. Inconsistent naming: handleT3SendButtonClick vs handleT3EnterKey - consider handleT3SendButtonClick → handleT3ButtonSubmit for consistency.

2. Version bump: 6.0.101 → 6.0.102 in wxt.config.ts - should this be a patch bump given the security fix nature?

3. Comments: The inline comments "No memories present, let the button click proceed normally" are helpful, but more comprehensive JSDoc comments would improve maintainability.

---

🎯 Recommendations Summary

Must Fix (Before Merge):

1. Replace setTimeout with proper async/await flow
2. Add error handling with fallback to form submission
3. Remove the event listener re-registration anti-pattern

Should Fix (Soon After):

1. Extract duplicate memory detection logic
2. Add comprehensive test coverage
3. Implement server-side validation for prompt injection

Nice to Have:

1. Add JSDoc comments
2. Consider showing loading states during async operations
3. Add DOM reference caching for performance

---

Overall Assessment

This PR moves in the right direction for fixing the prompt injection issue, but the implementation has critical race condition and error handling problems that could break user experience. The lack of test coverage is concerning for security-sensitive code.

Recommendation: Request changes to address the race conditions and error handling before merging.

---

Review generated following the repository's CLAUDE.md conventions for the supermemory monorepo.

[graphite-app]

Merge activity

• Oct 21, 6:52 PM UTC: Dhravya added this pull request to the Graphite merge queue.
• Oct 21, 6:56 PM UTC: Merged by the Graphite merge queue.

[graphite-app]

[claude]

Code Review: Fix T3 Chat Prompt Injection

Overall Assessment

This PR addresses a prompt injection vulnerability by intercepting and controlling the form submission flow when memories are present. The approach is sound, but there are several areas that need attention.

---

🔴 Critical Issues

1. Race Condition with Event Re-dispatch (apps/browser-extension/entrypoints/content/t3.ts:611-620)

The current implementation uses setTimeout with arbitrary delays (100ms) to re-dispatch events after capturing:

setTimeout(() => {
  document.removeEventListener("click", handleT3SendButtonClick, true)
  sendButton.dispatchEvent(newEvent)
  setTimeout(() => {
    document.addEventListener("click", handleT3SendButtonClick, true)
  }, 100)
}, 100)

Issues:

• The 100ms delay is arbitrary and may be too short or unnecessarily long
• If captureT3PromptContent takes longer than expected, the form might submit before capture completes
• Nested setTimeout makes the flow harder to reason about

Recommendation:

await captureT3PromptContent("button click")
// Remove listener to prevent recursion
document.removeEventListener("click", handleT3SendButtonClick, true)

const form = sendButton.closest("form")
if (form) {
  form.requestSubmit()
} else {
  sendButton.click()
}

// Re-add listener after next tick
requestAnimationFrame(() => {
  document.addEventListener("click", handleT3SendButtonClick, true)
})

2. Inconsistent Memory Check Logic (apps/browser-extension/entrypoints/content/t3.ts:587-595, 634-642)

The memory presence check is duplicated in both handlers with identical logic:

const hasMemories =
  textareaElement?.dataset.supermemories ||
  (document.querySelector('[id*="sm-t3-input-bar-element"]') as HTMLElement)?.dataset.memoriesData

Issues:

• Code duplication violates DRY principle
• DOM query inside event handler may impact performance
• Logic is repeated in two places, increasing maintenance burden

Recommendation:

function hasMemoriesPresent(): boolean {
  const textareaElement =
    (document.querySelector("textarea") as HTMLTextAreaElement) ||
    (document.querySelector('div[contenteditable="true"]') as HTMLElement)
  
  const inputBarElement = document.querySelector('[id*="sm-t3-input-bar-element"]') as HTMLElement
  
  return !!(textareaElement?.dataset.supermemories || inputBarElement?.dataset.memoriesData)
}

---

⚠️ Potential Bugs

3. Textarea Element Re-querying (apps/browser-extension/entrypoints/content/t3.ts:587-589)

Both handlers query for the textarea element even though event.target should already reference it in the Enter key handler:

const textareaElement =
  (document.querySelector("textarea") as HTMLTextAreaElement) ||
  (document.querySelector('div[contenteditable="true"]') as HTMLElement)

Issue: In handleT3EnterKey, the target is already the textarea/contenteditable div, so re-querying is unnecessary and could reference the wrong element if multiple textareas exist.

Recommendation:

// In handleT3EnterKey, use the target directly
const hasMemories =
  target.dataset.supermemories ||
  (document.querySelector('[id*="sm-t3-input-bar-element"]') as HTMLElement)?.dataset.memoriesData

4. Missing Error Handling

The async handlers don't have try-catch blocks. If captureT3PromptContent throws an error, the form submission will be blocked permanently.

Recommendation:

try {
  await captureT3PromptContent("button click")
} catch (error) {
  console.error("Failed to capture T3 prompt, proceeding with submission:", error)
  // Proceed with form submission even if capture fails
}

---

🟡 Code Quality & Best Practices

5. Missing Cleanup for Event Listeners

The event listeners are added but never cleaned up. If setupT3PromptCapture is called multiple times (e.g., on route changes), listeners will accumulate.

Recommendation:

// Store references for cleanup
let clickCleanup: (() => void) | null = null
let keydownCleanup: (() => void) | null = null

function setupT3PromptCapture() {
  // Clean up existing listeners
  clickCleanup?.()
  keydownCleanup?.()
  
  // ... rest of setup ...
  
  document.addEventListener("click", handleT3SendButtonClick, true)
  document.addEventListener("keydown", handleT3EnterKey, true)
  
  // Return cleanup functions
  clickCleanup = () => document.removeEventListener("click", handleT3SendButtonClick, true)
  keydownCleanup = () => document.removeEventListener("keydown", handleT3EnterKey, true)
}

6. TypeScript Type Assertions

Multiple type assertions (as HTMLElement, as HTMLTextAreaElement) could be replaced with proper type guards.

Recommendation:

function isTextInputElement(element: unknown): element is HTMLTextAreaElement | HTMLElement {
  return element instanceof HTMLTextAreaElement || 
         (element instanceof HTMLElement && element.contentEditable === "true")
}

---

🔵 Performance Considerations

7. DOM Queries in Event Handlers

Every click/keydown event triggers DOM queries even when not needed:

• document.querySelector("textarea")
• document.querySelector('[id*="sm-t3-input-bar-element"]')

Recommendation: Cache these queries or use event delegation more effectively. Consider querying only when memories are detected initially.

---

✅ Security Assessment

Prompt Injection Fix

The core fix is effective - by intercepting events when memories are present and ensuring captureT3PromptContent runs first, the prompt injection vulnerability is addressed. The approach of:

1. Preventing default behavior
2. Capturing content with memories
3. Re-submitting the form

This successfully ensures the injected memory context is captured before submission.

However: The security is somewhat fragile due to the timing issues mentioned in Critical Issue #1.

---

📝 Test Coverage

Current State: No tests found for this module.

Recommendation: Add integration tests covering:

1. Form submission with memories present
2. Form submission without memories (should not intercept)
3. Enter key submission with memories
4. Error handling when capture fails
5. Memory data cleanup after submission

Example test structure:

describe('T3 Prompt Injection Fix', () => {
  it('should intercept form submission when memories are present', async () => {
    // Setup: Add memories to textarea
    // Action: Click send button
    // Assert: captureT3PromptContent called before submission
  })
  
  it('should allow normal submission when no memories present', async () => {
    // Setup: No memories
    // Action: Click send button
    // Assert: Form submits without interception
  })
})

---

🎯 Summary

Strengths:

• Addresses the security vulnerability effectively
• Separates handlers into named functions (better than inline)
• Checks for memory presence before intercepting

Must Fix:

1. Race condition with setTimeout (Critical Issue Gemini api #1)
2. Missing error handling (Potential Bug New UI #4)
3. Event listener cleanup (Code Quality New UI #5)

Should Fix:
4. Extract duplicate memory check logic (Critical Issue #2)
5. Optimize DOM queries in event handlers (Performance #7)
6. Add test coverage

Approval Status: ⏸️ Request Changes - The race condition and missing error handling should be addressed before merging.