Skip to content

fix(web): escape memory type badges#137

Merged
tickernelz merged 1 commit into
mainfrom
fix/issue-135-memorytype-xss
Jun 28, 2026
Merged

fix(web): escape memory type badges#137
tickernelz merged 1 commit into
mainfrom
fix/issue-135-memorytype-xss

Conversation

@GraDea

@GraDea GraDea commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the stored XSS vector reported in #135 for memoryType badge rendering.

memory.memoryType was interpolated directly into Web UI HTML in both standalone memory cards and combined prompt-memory cards. This PR reuses the existing escapeHtml() render-boundary helper so malicious memory type values render as text instead of executable markup.

Changes

  • Escape memory.memoryType in renderMemoryCard()
  • Escape memory.memoryType in renderCombinedCard()
  • Add regression tests for both render paths using the confirmed <img onerror> payload

Verification

  • bun test tests/web-memorytype-xss.test.ts
  • bun test
  • bun run typecheck
  • bun run build

Scope

This PR intentionally only fixes the memoryType stored-XSS sink.

Other security concerns from #135, including auth/CORS hardening and broader prompt-injection behavior, should be handled in separate PRs.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@GraDea GraDea requested a review from tickernelz June 22, 2026 20:41
@tickernelz

Copy link
Copy Markdown
Owner

You're the maintainer now, you can merge and release by yourself

@tickernelz tickernelz left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅

Verification:

  • escapeHtml() helper already exists in src/web/app.js:1130 and the implementation is correct (DOM textContent→innerHTML).
  • Both sinks (renderCombinedCard line 186, renderMemoryCard line 284) now escape memory.memoryType.
  • Test harness uses a node:vm Script sandbox with a real document.createElement mock — not just a string assertion.
  • Local merge-test clean, bun test 156 pass, typecheck + build pass.

Scope:
This is correctly scoped — it only closes the XSS sink in memoryType. Memory content, tags, and displayName are already escaped via escapeHtml() / renderMarkdown() (DOMPurify-sanitized).

Nice catch on issue #135.

@tickernelz tickernelz merged commit 54283d9 into main Jun 28, 2026
1 check passed
@tickernelz

Copy link
Copy Markdown
Owner

Merged ✅

Verification on merged main:

Closes the stored-XSS sink in memoryType badge rendering reported in #135.

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.

2 participants