Skip to content

Shrink screenshots that GitHub uploads as HTML img tags - #2

Merged
d4rken merged 3 commits into
mainfrom
feat/thumbnail-html-img
Aug 11, 2026
Merged

Shrink screenshots that GitHub uploads as HTML img tags#2
d4rken merged 3 commits into
mainfrom
feat/thumbnail-html-img

Conversation

@d4rken

@d4rken d4rken commented Aug 11, 2026

Copy link
Copy Markdown
Member

What changed

The thumbnail workflow only shrank Markdown-style images (![alt](url)). GitHub's uploader now inserts screenshots as HTML tags (<img width="3000" height="4000" alt="Image" src="…" />), which the action ignored — see d4rken-org/sdmaid-se#2632, where five oversized portrait screenshots stayed full size and the workflow run reported changed=false.

  • HTML <img> tags emitted by GitHub's uploader are now rewritten into the same clickable <a><img width="N"> thumbnails as Markdown images (portrait-only, min height 640, target rendered height 400).
  • The tag's own width/height attributes provide the dimensions, so these rewrites make no network requests; probing and the max-images cap stay Markdown-only.
  • Only the uploader's exact grammar is rewritten (src/width/height/alt, double-quoted, no duplicates). Hand-written tags with extra attributes are left alone rather than having those attributes silently dropped.
  • Guards keep tags untouched inside code fences and inline code, HTML comments, existing anchors (including multi-line ones), and Markdown link wrappers; GFM backslash escapes are respected, and containment between the two syntaxes is resolved in both directions. All scans stay bounded and linear on attacker-controlled text (adversarial perf cases included in the test suite).

Refs d4rken-org/sdmaid-se#2632

Deployment (required for this to take effect)

Pins are by commit SHA, so merging this alone changes nothing for callers:

  1. After this merges to main at SHA X: bump uses: d4rken-org/.github/actions/thumbnail-analyze@X in .github/workflows/thumbnail-images.yml (this repo) → merges at SHA Y.
  2. Bump caller stubs (e.g. sdmaid-se's .github/workflows/thumbnail-images.yml) to …/thumbnail-images.yml@Y.
  3. Optionally verify against SD Maid stopping banking app opening! sdmaid-se#2632 or a disposable test issue.

Validation

npm ci / npm test (48 tests) / npm run verify all pass in actions/thumbnail-analyze/; the dist/ bundle is rebuilt and committed in step.

d4rken added 3 commits August 10, 2026 07:56
GitHub's uploader no longer inserts Markdown images; it emits
<img width="3000" height="4000" alt="Image" src="..." /> instead, which
the Markdown-only matcher skipped entirely (real case: sdmaid-se#2632).

The tags are found with a bounded quote-aware scanner rather than a
regex: a pattern terminating on the first ">" mangles alt="a > b", and
one that models quoting cannot stay free of nested quantifiers. Only the
uploader's own grammar is rewritten (src/width/height/alt, all
double-quoted, no duplicates); anything else is a hand-written tag whose
extra attributes a rewrite would silently drop, so it is left alone but
still bounds where a Markdown image may be rewritten.

Tags declare their dimensions, so they need no probe: the max-images cap
and the probe loop stay Markdown-only. Guards keep tags inside code,
HTML comments, anchors and Markdown links untouched; the comment guard
now covers the Markdown path too, closing the same gap there.
The cross-line comment and anchor trackers scanned every line, so an
unclosed "<a" or "<!--" shown inside a code fence or inline code span
entered their carried state and suppressed every image below it. The
inline code spans are now computed once and shared with both trackers
and the hit loop. Comment openers on code lines are ignored, but a
comment that is genuinely open still closes anywhere, since Markdown is
not interpreted inside one. Anchor tokens are ignored on code lines,
inside inline code and inside comments unconditionally: an "</a>" shown
as a sample is text even while a real anchor is open.

The Markdown link guard required an "https?://" destination, so
"[<img …>](#details)" was not recognised as a link and the tag inside it
got an anchor nested into the author's own. The destination is now any
bounded run without whitespace, parens or quotes.

The anchor opener lookahead required a boundary character, missing a
"<a" that ends its line with the attributes on the next one.

Fixes review findings F1, F2, F3.
The comment and anchor trackers tested every candidate token against the
whole inline-code span list for that line, so a body of repeated `<a>` or
`<!--` code samples rescanned the list once per sample: ~5.5ms at 8KiB
against ~60ms at 64KiB. The module's bound is no quadratic scan on
attacker-controlled text.

Both token streams are produced in ascending index order, so a cursor
over the spans visits each range once. inlineCodeRanges emits a span when
its closer is found, which is not ascending by start when a short opener
nests inside a longer one, so each line's spans are sorted once where
rewrite() builds them.

The hit-loop guards keep using inSpan: their candidates are full-size
constructs, so candidate count times range count is already bounded.

Fixes review finding F4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant