fix(site): strip tags to a fixpoint when extracting llms-full.txt section titles - #62
Merged
Conversation
Title extraction for the llms-full.txt design-record filter matched the built page's <h1> and stripped its markup with a single `.replace(/<[^>]+>/g, '')` pass. A single pass over arbitrary HTML is not idempotent: removing one match can turn the surrounding text into something that looks like a tag too, so a heading with inline markup (a <code> span, an icon wrapper) could leave a partial tag in the extracted title and silently break the exact-string match this filter depends on. Loop the strip to a fixpoint instead. Also clears CodeQL alert js/incomplete-multi-character-sanitization on this line. The string is build-time output used only as a Set key, never re-emitted as HTML, so this was a correctness fix for title extraction rather than a security fix. Checked whether the title could come from a non-HTML source instead: `astro:build:done`'s `pages` list carries only `pathname`, and `astro:content` is not resolvable at that hook (Vite's module runner is already closed by the time it fires, confirmed by trying both a static and a dynamic import against a real build). So the built HTML remains the only source available here. Verified against a real site build: llms-full.txt is byte-for-byte identical before and after this change (spec/research sections still excluded, page separator still cleaned up, no guide/articles page dropped). Co-Authored-By: lsimons-bot <bot@leosimons.com> Assisted-by: Claude:claude-sonnet-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the one open CodeQL alert on this repo —
js/incomplete-multi-character-sanitizationin
site/src/plugins/exclude-design-record-from-llms-full.mjs.What the real defect is
Not a vulnerability, and this PR does not claim to be a security fix: the input is
the project's own build output, the code runs at build time in an
astro:build:donehook, and the extracted string is only ever used as aSetkeyto match section headings — it is never re-emitted as HTML.
The genuine defect underneath is that
replace(/<[^>]+>/g, '')is notidempotent: stripping one match can create a new one out of the surrounding
text, so a heading containing markup (inline code, a
<span>, an elementwrapping an emoji) can leave a partial tag behind and fail to match its
llms-full.txtsection. The site's own spec pages contain exactly that kind ofheading.
Why the title is still read out of HTML
The preferred fix was to stop parsing HTML at all and read the title from Astro.
That was ruled out empirically, not assumed:
astro:build:done's hook argument is{ pages: { pathname }[], dir, assets, logger }—no title, no frontmatter (checked against Astro's own type definitions).
astro:contentinside the hook fails both ways: statically atconfig-load time (
Cannot find module 'astro:content'), and dynamically athook-run time (
Vite module runner has been closed) — Astro tears the modulerunner down before this hook fires.
astro:routes:resolved's route type exposesentrypoint/pattern, notfrontmatter.
So the built HTML is the only source available at this point, and the fix is to
make the strip idempotent:
stripTagsToFixpoint()loops until the string stopschanging. No new dependency added.
The two pre-existing comment blocks recording why this plugin exists (the
upstream
excludeoption is not forwarded tollms-full.txt) and whyPAGE_SEPARATORreplaced a blank-line split (a guide page quoting file contentsin a fenced block produced a false page boundary) are left intact.
Verification
Real build, not logic in isolation. Built
llms-full.txt/llms-small.txtwiththe fix, then reverted the plugin and rebuilt with the original single-pass code,
and diffed: byte-for-byte identical output on the real corpus — 54 spec and
research pages excluded in both, 18 guide/article/get-involved pages surviving
with headings intact,
PAGE_SEPARATORfully cleaned from both files in bothcases.
astro check: 0 errors.mise run cigreen (unaffected — pure JS change).The alert should auto-close once code scanning re-runs on
main; no manualdismissal, since this is a fix rather than a suppression.
Co-Authored-By: lsimons-bot bot@leosimons.com