Read-modify-write through the MCP permanently destroys stored issue and PR body content

[arolling-sa]

Summary

Body sanitization is lossy and applied on read, so a client's in-memory copy of a body is already damaged before it edits anything. Editing a body means sending the whole body back, so an ordinary read-modify-write cycle writes the damage back and the sanitizer's deletions become permanent in GitHub's stored data.

The loop needs no misbehaviour:

1. A client reads an issue or PR body through a sanitizing read path.
2. It makes an unrelated edit — ticking a checklist box, appending a status line.
3. It writes the whole body back.

Everything the sanitizer removed in step 1 is now gone from the stored issue. This turns a response-fidelity problem into repository data loss.

Reproduction

Verified 2026-08-26. pkg/sanitize/sanitize.go at main (a00dc319) produces this behaviour.

Create an issue via REST, not the MCP, with this body:

## Checklist

- [ ] step one
- [ ] step two

## Probe 1

SENTINEL-1-BEFORE. The retry threshold is <n> by default. SENTINEL-1-AFTER.

## Probe 2

SENTINEL-2-BEFORE. Returns Promise<string> on success. SENTINEL-2-AFTER.

## Probe 3

SENTINEL-3-BEFORE. The style probe: `<style>`. SENTINEL-3-AFTER-SHOULD-SURVIVE.

Then:

1. GET /repos/{owner}/{repo}/issues/{n} — body intact, 492 bytes.
2. issue_read (get) — <n> and <string> are gone, and the body ends mid-sentence at the <style> probe, with SENTINEL-3-AFTER-SHOULD-SURVIVE absent.
3. issue_write (update), sending back exactly what step 2 returned, with - [ ] step one changed to - [x] step one. One character of intent.
4. GET again — 438 bytes.

The stored body, diffed against the baseline:

-- [ ] step one
+- [x] step one

-SENTINEL-1-BEFORE. The retry threshold is <n> by default. SENTINEL-1-AFTER.
+SENTINEL-1-BEFORE. The retry threshold is  by default. SENTINEL-1-AFTER.

-SENTINEL-2-BEFORE. Returns Promise<string> on success. SENTINEL-2-AFTER.
+SENTINEL-2-BEFORE. Returns Promise on success. SENTINEL-2-AFTER.

-SENTINEL-3-BEFORE. The style probe: `<style>`. SENTINEL-3-AFTER-SHOULD-SURVIVE.
+SENTINEL-3-BEFORE. The style probe: `

One intended change; 54 bytes destroyed, including the entire tail of the body after <style>.

Why this is filed separately from #2202

#2202 reports the read-side symptom, framed as angle-bracket loss inside code blocks. The fix is the same — stop mangling bodies — but the severity is not. A client that only reads gets a degraded response; a client that reads, edits and writes back destroys stored content, and the currently merged behaviour makes every such client destructive by default.

Two properties make it hard to catch:

• No marker. Deletions often splice the surrounding text together and read grammatically; truncation simply ends. A client comparing its intended edit against what it sent sees nothing wrong.
• The client is behaving correctly. Sending back the body it was given is the documented way to edit an issue.

The worst input: skip-content elements truncate everything after them

bluemonday keeps a set of elements whose content is discarded along with the tag — frame, frameset, iframe, noembed, noframes, noscript, nostyle, object, script, style, title (policy.go:979-990 in v1.0.27). The skip flag clears only on the matching end tag, so an opening tag with no closing tag — routine in a markdown body — drops everything to end-of-body.

Membership is a policy list, not an HTML category, and predicting from the category gets it backwards in both directions: iframe and object truncate without being raw-text elements, while textarea does not truncate despite being one. @franklinjavier set this out in detail on #2202.

Independent public reproduction with sentinels: flungo/claude-plugins#55 and its test comments B, C and D — one of which cut itself off at its own header.

Combined with the write loop, a single backticked <style> in a body means one checkbox tick discards everything below it, permanently.

Scope

Backticks and code fences offer no protection: the pass runs on the raw string before any markdown parsing. @ericknavarro's census on #2202 measured 177 bodies with confirmed mid-body loss out of 600 recent items in one repository — worst case 2,755 characters deleted from an 8,360-character body — and found the majority of the damage in prose rather than code.

Coverage widened considerably in 56bfeec0 (2026-08-19), which centralized sanitize.Sanitize into the shared convertToMinimal* converters — so comment bodies, review bodies, review comment bodies, releases and commit messages are now affected where previously they were not. minimal_types.go currently reads Body: sanitize.Sanitize(comment.GetBody()).

Suggested fix

@ericknavarro proposed the approach on #2202 on 2026-08-01: skip FilterHTMLTags for body fields entirely.

#3035 implements it, as a FilterBody that keeps the injection defences which matter for bodies — invisible characters, BiDi overrides, hidden code-fence info strings — and drops only the HTML pass, reserving full Sanitize for titles. It has had reviewer engagement, and predates the 2026-08-19 refactor of Sanitize, so it needs updating against current main.

Its three sibling PRs — #3039, #3040, #3043 — still apply full sanitize.Sanitize to bodies, and #3043 applies it to gist file content. Merging those as written would extend this problem rather than contain it.

If the HTML pass must stay for bodies, entity-escaping an unrecognized < rather than deleting the span would at least preserve meaning.

Related

• #2202 — primary read-side report, open since 2026-03-12
• #3106 — the coverage-consistency issue, which asked for "a content-safe strategy for source code and file contents"
• Fix attempts: #3035 (open), #2408 (open, code regions only), #2633 (closed), #2308 (closed)

🤖 Investigated and drafted by Claude Code (Claude Opus 5)

[arolling-sa]

Worth adding, because it bears on how this gets triaged: reading this issue through the MCP truncates it at the raw-text style token inside its own reproduction block. 1,248 of 6,598 bytes survive — the summary and the opening of the reproduction. The diff, the severity argument, the census figures and every suggested fix are absent, with nothing marking the cut. Apostrophes arrive as ' on the way.

The rest of the thread degrades the same way. #2202's before/after example reads identically through the MCP — both halves come back as let ptr: mut_raw_ptr = raw_new int; — so the report appears to demonstrate nothing, and flungo/claude-plugins#55 truncates outright. Agent-assisted triage here is reading a fragment, or an example that has lost its point, with no signal that either happened.

This comment deliberately avoids the angle-bracket form of that token. Written the natural way, 91% of the text above would be discarded on read, cutting off mid-sentence in the first line — so authors have to know the failure mode and write around it for a report to stay legible to the tool. That is part of the cost too.

🤖 Investigated and drafted by Claude Code (Claude Opus 5)

[arolling-sa]

Correcting the mechanism in the body above. It originally read — angle brackets removed here so this comment survives its own subject:

style and title are HTML raw-text and RCDATA elements, so the tokenizer consumes the remainder of the body as element content. With no closing tag, that is the rest of the document.

That attributes the truncation to HTML tokenization. The cause is bluemonday's skip-content set — policy.go:979-990, verified against v1.0.27 — set out precisely in @franklinjavier's comment on #2202, posted before this issue was filed. The two sets differ in both directions: iframe, object and noscript truncate without being raw-text elements, and textarea does not truncate despite being one.

The body has been corrected. Byte figures in my earlier comment describe it as first filed; the surviving 1,248 bytes are unchanged, the total is now 7,157.

🤖 Investigated and drafted by Claude Code (Claude Opus 5)