ποΈπ§οΌsee a description as a reader would - #1822
Merged
openinf-commit-queue[bot] merged 1 commit intoAug 15, 2026
Conversation
CodeQL had a second alert open on my code that I had not seen: stripping HTML comments with a single replacement leaves what surrounded one able to form another. `<!<!-- x -->-- sneaky -->` came out as `<!-- sneaky -->`, which a reader still cannot see -- so a pull request with no description at all satisfied the check for having one. Comments are now removed by scanning, repeatedly until nothing changes, capped so that a description written to be awkward cannot turn that into many passes. Each pass is linear and removes at least one comment. This is a heuristic for whether anything was written, not a sanitizer, but the pattern is worth not having in the codebase at all. Signed-off-by: Derek Lewis <DerekNonGeneric@inf.is> Assisted-by: Claude-Code:claude-opus-5
β Deploy Preview for gh-pages-openinf ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
DerekNonGeneric
added
the
commit-queue
openinf-commit-queue
Bot
removed
the
commit-queue
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
1 participant
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.
The second CodeQL alert,
js/incomplete-multi-character-sanitizationβ which was open and unnoticed while we were discussing whether alerts should block merging. It rather makes the case.The bypass
A single replacement removes the inner comment, and what surrounded it closes up into another one. The result is non-empty, so the check said the pull request had a description β while a reader would see nothing at all. Exactly the check's purpose, defeated.
My first attempt at a fix reproduced the same string, because building the output by concatenation re-forms the comment just as a replacement does. It now scans repeatedly until nothing changes, capped at eight passes so an awkward description cannot turn that into many. Each pass is linear and removes at least one comment.
Verified end to end: that body is now rejected.
This is a heuristic for whether anything was written, not a sanitizer, so the alert's severity overstates the risk here. The pattern is still not worth having in the codebase.
Verification
nps testβ 16/16 tasks, 72/72 testsvisibleText, including the adversarial input, an unterminated comment, and the pull request template on its own β which must come out empty, since that is the case the check exists for