Skip to content

fix: strip raw HTML <img> and <p> tags in sanitize_markdown#73

Open
besologic wants to merge 5 commits intoplanningcenter:mainfrom
besologic:bl/fix-sanitize-markdown-img-tags
Open

fix: strip raw HTML <img> and <p> tags in sanitize_markdown#73
besologic wants to merge 5 commits intoplanningcenter:mainfrom
besologic:bl/fix-sanitize-markdown-img-tags

Conversation

@besologic
Copy link
Copy Markdown

@besologic besologic commented Mar 18, 2026

Problem

GitHub PR bodies can contain raw HTML — most commonly <img> tags for embedded screenshots (pasted or dragged in). sanitize_markdown strips markdown-syntax images (![...](...)) but passes raw HTML <img> tags through unchanged.

When the resulting string is sent to Asana's html_notes field, the API returns 400 Bad Request because:

  • Asana only supports <img> for Asana-hosted attachments with data-asana-gid attributes — external GitHub CDN URLs are always rejected
  • <p> is not in Asana's supported tag list at all (literal newlines are the correct alternative)

Fix

  • Strip raw <img> tags entirely — there's no meaningful fallback for external images
  • Strip <p> open tags and convert </p> to newlines — preserves paragraph breaks without the unsupported tag

Reference for Asana's empirically verified supported tag list: <strong>, <em>, <u>, <s>, <h1>, <h2>, <ul>, <ol>, <li>, <blockquote>, <code>, <pre>, <hr/>, <a href>, <table>, <tr>, <td>, and <img> (Asana-hosted only).

Tests

Added a full describe('sanitize_markdown') block — there were no existing tests for this helper.

Reproducing the original bug

Apply the create-asana-task label to any PR whose body contains a GitHub screenshot embed like:

<img width="2964" height="1284" alt="image" src="https://github.com/user-attachments/assets/..." />

The workflow will fail with Asana API error: 400 Bad Request.

I verified that removing the image allows the workflow to run completely: https://github.com/planningcenter/people/pull/14325

GitHub PR bodies can contain raw HTML tags (e.g. screenshot embeds as
<img> tags) that pass through sanitize_markdown unchanged. When sent to
Asana's html_notes field, unsupported tags cause a 400 Bad Request.

- Strip <img> tags entirely — Asana only supports Asana-hosted images
  with data-asana-gid attributes; external GitHub screenshot URLs are
  always rejected
- Strip <p> open tags and convert </p> to newlines — <p> is not in
  Asana's supported tag list (use literal newlines instead)
- Add a full describe('sanitize_markdown') test block covering the new
  behaviour and existing transforms

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 18, 2026 16:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Handlebars sanitize_markdown helper to better sanitize GitHub PR bodies for Asana compatibility, specifically preventing unsupported raw HTML from being sent to Asana’s html_notes (which can cause 400 Bad Request).

Changes:

  • Strip raw HTML <img> tags from sanitize_markdown output.
  • Strip <p> open tags and convert </p> close tags into newlines to preserve paragraph breaks.
  • Add a dedicated describe('sanitize_markdown') test block covering the new behavior and existing sanitization behaviors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/expression/helpers.ts Extends sanitize_markdown with additional HTML stripping/replacement for Asana compatibility.
__tests__/expression/helpers.test.ts Adds test coverage for sanitize_markdown, including raw HTML <img> and <p> handling.

Review concern (blocking): the new regex .replace(/<p[^>]*>/gi, "") will also match and remove tags that start with <p (e.g. <pre>, <param>, <picture>, etc.), which can corrupt content and potentially leave behind unmatched closing tags (e.g. </pre>). This should be tightened (e.g., using a word boundary like <p\b...>), and the <img...> regex should similarly use <img\b...> to avoid matching tag names like <imgur>.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

<p[^>]*> would also match <pre>, <picture>, <param> etc.
<img[^>]*> could match <imgur> or similar tag names.

Use \b after the tag name to match only the exact tag.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@besologic besologic marked this pull request as ready for review March 18, 2026 16:50
@besologic besologic requested a review from Copilot March 18, 2026 16:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the sanitize_markdown Handlebars helper so GitHub PR bodies containing raw HTML (notably <img> and <p>) don’t cause Asana html_notes / notes validation failures.

Changes:

  • Strip raw HTML <img> tags during markdown sanitization.
  • Strip <p> opening tags and convert </p> to newlines to preserve paragraph breaks without unsupported tags.
  • Add a dedicated describe('sanitize_markdown') test suite covering the new behavior and existing sanitization behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/expression/helpers.ts Extends sanitize_markdown to remove <img> tags and handle <p> tags in an Asana-compatible way.
__tests__/expression/helpers.test.ts Adds coverage for sanitize_markdown, including raw HTML image/paragraph cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

besologic and others added 3 commits March 18, 2026 10:22
- Tighten </p> regex to allow optional whitespace before > (</p\s*>)
- Hoist `raw` triple-stache helper to describe-block scope to avoid duplication
- Add explicit \n assertion to </p> conversion test
- Add test for </p > with whitespace before closing angle bracket

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The \b word boundaries defended against fictional tags like <imgur>
that no browser, markdown parser, or GitHub PR body would produce.
Remove them along with the <imgur> and <pre> tests to keep things
simple. The </p\s*> whitespace fix and newline assertion remain.

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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Handlebars sanitize_markdown helper to better sanitize GitHub PR bodies for Asana by removing raw HTML that Asana rejects (notably <img> and <p>), and adds unit tests to lock in the expected behavior.

Changes:

  • Strip raw HTML <img> tags during sanitize_markdown.
  • Strip <p> opening tags and convert </p> to newlines to preserve paragraph breaks without unsupported tags.
  • Add a dedicated sanitize_markdown test suite covering the new sanitization behavior and existing transformations.

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.

File Description
src/expression/helpers.ts Extends sanitize_markdown to remove raw <img> and <p> tags for Asana compatibility.
dist/index.js Updates bundled output to include the new sanitization logic, but also includes substantial unrelated bundle churn.
__tests__/expression/helpers.test.ts Adds coverage for sanitize_markdown, including <img>/<p> handling and newline normalization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

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