Skip to content

fix: add explicit DOMPurify allowlist for ANSI log rendering#35

Merged
abhizipstack merged 3 commits into
mainfrom
fix/dompurify-explicit-allowlist
Apr 2, 2026
Merged

fix: add explicit DOMPurify allowlist for ANSI log rendering#35
abhizipstack merged 3 commits into
mainfrom
fix/dompurify-explicit-allowlist

Conversation

@abhizipstack

Copy link
Copy Markdown
Contributor

What

  • Add explicit allowlist config to DOMPurify sanitization for ANSI log renderer

Why

How

  • ALLOWED_TAGS: ["span", "br"] — only tags ansi-to-html generates
  • ALLOWED_ATTR: ["style"] — only attribute needed for ANSI colors

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No — ansi-to-html only generates <span style="color:..."> and <br> tags, both are in the allowlist

Database Migrations

  • None

Env Config

  • None

Notes on Testing

  • Open IDE → run transformation → verify logs render with colors

Checklist

I have read and understood the Contribution Guidelines.

Restrict allowed tags to span and br, attributes to style only.
Tighter than DOMPurify defaults — guards against future version
changes while preserving ANSI color spans.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@tahierhussain tahierhussain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@abhizipstack abhizipstack self-assigned this Apr 2, 2026
@greptile-apps

greptile-apps Bot commented Apr 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR tightens the DOMPurify sanitization config in the ANSI log renderer (parseLog) by replacing the default allowlist with an explicit one — ALLOWED_TAGS: ["span", "br"] and ALLOWED_ATTR: ["style"]. This matches exactly what ansi-to-html generates and reduces the attack surface against future DOMPurify default allowlist changes.

Changes:

  • frontend/src/ide/editor/no-code-model/no-code-model.jsx: Adds ALLOWED_TAGS and ALLOWED_ATTR options to the DOMPurify.sanitize call in parseLog, restricting output to only <span> and <br> tags with only the style attribute permitted.

The change is correct and well-scoped. ansi-to-html exclusively generates <span style="color:..."> and <br> tags for standard ANSI sequences, so no existing rendering will be broken. DOMPurify also applies its own CSS value sanitization within style attributes, guarding against CSS injection (e.g. url() data exfiltration attempts) even with style explicitly allowed.

Confidence Score: 5/5

  • Safe to merge — the change is a focused, correct security hardening with no behavioral regressions.
  • No P0 or P1 issues found. The explicit allowlist correctly matches the output of ansi-to-html, DOMPurify continues to sanitize CSS values within the permitted style attribute, and no existing rendering functionality is broken.
  • No files require special attention.

Important Files Changed

Filename Overview
frontend/src/ide/editor/no-code-model/no-code-model.jsx Adds explicit ALLOWED_TAGS and ALLOWED_ATTR config to DOMPurify.sanitize in parseLog — correct, minimal, and matches ansi-to-html output.

Sequence Diagram

sequenceDiagram
    participant LogSource as Log Source
    participant parseLog as parseLog()
    participant ansiToHtml as ansi-to-html
    participant DOMPurify as DOMPurify.sanitize()
    participant DOM as React innerHTML

    LogSource->>parseLog: "raw ANSI log string"
    parseLog->>ansiToHtml: "toHtml(log)"
    ansiToHtml-->>parseLog: "HTML with span[style] and br tags"
    parseLog->>DOMPurify: "sanitize(html, {ALLOWED_TAGS:[span,br], ALLOWED_ATTR:[style]})"
    DOMPurify-->>parseLog: "sanitized HTML - only span/br with style"
    parseLog-->>DOM: "safe HTML via dangerouslySetInnerHTML"
Loading

Reviews (2): Last reviewed commit: "fix: restore package-lock.json from main" | Re-trigger Greptile

abhizipstack and others added 2 commits April 2, 2026 16:27
npm ci was failing because lock file was out of sync with
dependency versions from recent dependabot merges.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previous regeneration caused version mismatches in CI.
This PR only changes a .jsx file — no lock file changes needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@abhizipstack
abhizipstack merged commit 6f17c7e into main Apr 2, 2026
3 of 4 checks passed
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.

3 participants