Skip to content

Remove style attributes from markdown-rendered HTML and enhance security#86

Merged
ValwareIRC merged 3 commits into
mainfrom
fix/markdown
Oct 16, 2025
Merged

Remove style attributes from markdown-rendered HTML and enhance security#86
ValwareIRC merged 3 commits into
mainfrom
fix/markdown

Conversation

@ValwareIRC
Copy link
Copy Markdown
Contributor

@ValwareIRC ValwareIRC commented Oct 16, 2025

This PR enhances security for markdown rendering by:

  • Keeping forced styling on images with max-height constraint (150px)
  • Removing dangerous HTML tags (style, link, meta) from markdown output
  • Improving HTML sanitization to prevent CSS injection and other security risks

Changes Made

  • Modified src/lib/ircUtils.tsx to enhance markdown rendering security
  • Maintained image size constraints while removing other style attributes
  • Added sanitization for dangerous HTML tags

Testing

  • All 250 tests passing
  • Linting clean
  • No breaking changes

Summary by CodeRabbit

  • Bug Fixes
    • Tightened HTML sanitization when rendering formatted content: dangerous elements and event handlers are removed, only a whitelist of safe tags is allowed, and inline styles are stripped. Image heights are capped to control layout and appearance. No public API changes.

- Keep forced styling on images with max-height constraint
- Remove dangerous HTML tags (style, link, meta) from markdown output
- Improve security sanitization for markdown rendering
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 16, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

renderMarkdown in src/lib/ircUtils.tsx now applies a post-generation whitelist: it strips dangerous elements (script, iframe, object, embed, style, link, meta), removes event handler attributes, strips all style attributes, removes tags not in the allowed set, and enforces a max-height style for img tags.

Changes

Cohort / File(s) Summary
Markdown HTML sanitization
src/lib/ircUtils.tsx
Reworked renderMarkdown sanitization: remove dangerous tags (script, iframe, object, embed, style, link, meta), strip event-handler attributes and all style attributes, enforce img max-height, and drop any tags not in the allowed whitelist. No public API changes.

Sequence Diagram(s)

sequenceDiagram
    participant MD as Markdown Renderer
    participant Sanitizer as Sanitizer (old)
    participant PostFilter as Post-filter (new)

    MD->>Sanitizer: produce HTML
    Sanitizer-->>MD: (existing sanitization)
    MD->>PostFilter: sanitized HTML
    PostFilter->>PostFilter: remove dangerous tags (script, iframe, object, embed, style, link, meta)
    PostFilter->>PostFilter: strip event-handler attributes
    PostFilter->>PostFilter: remove all style attributes
    PostFilter->>PostFilter: enforce img max-height
    PostFilter->>MD: whitelisted, final HTML
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • matheusfillipe

Poem

🐰 A nibble, a scrub, a tidy little cheer,
I hop through the HTML, making things clear.
No scripts or styles to clutter the view,
Just safe, trimmed content — fresh as morning dew! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Remove style attributes from markdown-rendered HTML and enhance security" is directly related to the main changes in the changeset. The implementation adds sanitization to remove <style>, <link>, and <meta> elements from markdown-rendered HTML, which aligns with both the title's focus on removing style attributes and enhancing security. The title is concise, clear, and specific enough that a teammate scanning the repository history would understand this PR involves security-focused HTML sanitization improvements. While the title could be slightly more precise by mentioning all removed elements (style, link, meta), it appropriately captures the primary intent and main point of the change without vague terminology or misleading descriptions.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05232f2 and ab62c65.

📒 Files selected for processing (1)
  • src/lib/ircUtils.tsx (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@matheusfillipe matheusfillipe left a comment

Choose a reason for hiding this comment

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

Approving for now but dont think this is the proper way to deal with this to begin with.

@github-actions
Copy link
Copy Markdown

Pages Preview
Preview URL: https://fix-markdown.obsidianirc.pages.dev

Automated deployment preview for the PR in the Cloudflare Pages.

- Force all img tags to have style='max-height: 150px;' regardless of source
- Override any user-provided styles on images
- Maintain consistent image sizing across markdown and raw HTML
@ValwareIRC ValwareIRC merged commit b33b814 into main Oct 16, 2025
3 checks passed
zocram4cc pushed a commit to zocram4cc/ObsidianIRC that referenced this pull request Feb 17, 2026
…ity (obbyworld#86)

* Remove style attributes from markdown-rendered HTML and enhance security

- Keep forced styling on images with max-height constraint
- Remove dangerous HTML tags (style, link, meta) from markdown output
- Improve security sanitization for markdown rendering

* Ensure all img tags have controlled max-height styling

- Force all img tags to have style='max-height: 150px;' regardless of source
- Override any user-provided styles on images
- Maintain consistent image sizing across markdown and raw HTML

* lint
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