fix(security): adjudicate open CodeQL alerts (#1281) - #1291
Merged
Conversation
Fixes (8 alerts): - element html-escape.ts (#4140/#4142/#4143/#4144): headExtras script strip now runs the block pass to a fixed point before the strip-to-EOF backstop (a single pass could emit a re-formed <script> or a live on* handler that the strip itself concatenated); the script end-tag pattern accepts attributes/whitespace per the HTML spec; the comment-balance check counts --!> as a close. Contract preserved: scripts/handlers are still stripped when allowHeadExtrasScripts is false; adversarial regression tests added. - adapter-vite static-serve.ts (#4150): tryStatic reads candidates directly with failure-as-miss fallback instead of existsSync/statSync guard-then-read (TOCTOU). Mirrored in the generated serve.mjs template (ssg-helpers.ts); directory-candidate parity pinned by test. - www site-ui article-body.ts (#4151): prepareArticle heading labels drop stray angle brackets the tag pattern cannot match, so a partial tag fragment can never reach the rail outline. - tools check-public-docs-integrity.ts (#4131): stale-claim guard embeds the previous prerelease tag via the shared escapeRegExp (repo-standard full metacharacter escape); parity pinned by test. - app spa-projection-guard.test.ts (#4170): drop the always-overwritten baseline initializer. Dismissed via gh api with recorded rationale (see issue #1281): - #4145 (used in tests): release.test.ts fixture intentionally embeds a template literal as text data mirroring project-constants.ts. - #4148/#4149 (false positive): sitemap/robots writes target the developer's own dist output dir with pinned mode 0o644; overwrite-on-rebuild is the intended contract; tmpdir dataflow exists only in test fixtures. No ADR-0122 frozen-semantics paths touched (freeze:semantics:check green).
Contributor
|
APIError: Insufficient Balance |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…issue ref from comment (#1281) CodeQL js/incomplete-multi-character-sanitization re-fired on the single-pass tag strip in prepareArticle (new alert on PR #1291); apply the documented fixpoint idiom, matching the html-escape.ts sanitizeHeadExtras structure. The www theme-token gate also read '(#1281)' on a line containing 'outline' as a hex color literal; reworded.
Deploying openelement with
|
| Latest commit: |
03c801f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://86358420.lessjs.pages.dev |
| Branch Preview URL: | https://fix-1281-codeql-alert-adjudi.lessjs.pages.dev |
Contributor
|
APIError: Insufficient Balance |
This was referenced Sep 3, 2026
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
2 participants
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.
Problem
Issue #1281: 11 CodeQL security alerts have been open on
mainsince Aug 2026 (they ship in stable 0.43.3; they surfaced as "new" during the Beta.1 dev→main promotion only because the promotion diff is the entire v0.44 train). Beta.2 hardening requires each alert to be either fixed with test evidence or dismissed with a technically defensible, externally visible rationale.Owner
Single-owner security-hygiene slice: static sanitizers, static-file serving, and docs-gate tooling. No second owner was involved because no runtime architecture, public API, or frozen-semantics path is touched (
deno task freeze:semantics:checkgreen — no ADR-0122 paths changed, so no ADR-0151 authorization was required).Per-alert adjudication
packages/element/src/internal/core/html-escape.ts<script>…</script>and the backstop then ate all trailing markup.<\/script(?=[\s/>])[^>]*>— browsers ignore attributes on end tags (</script\t\n bar>still ends the raw-text element).oprefix with annclick=…suffix into a liveonclickhandler.--!>(HTML-standard abrupt close) in addition to-->. Diagnostic heuristic only, not a security boundary.www/app/site-ui/article-body.tsprepareArticleheading labels now drop stray</>the tag pattern cannot match, so a partial-tag fragment (e.g.<scriptwith no>) can never reach the rail outline.packages/adapter-vite/src/internal/static-serve.tstryStaticreads each candidate directly with failure-as-miss fallback (try/catchreadFileSync) instead ofexistsSync/statSyncguard-then-read TOCTOU. Mirrored in the generatedserve.mjstemplate (ssg-helpers.ts) so the two servers cannot drift.packages/adapter-vite/src/internal/content/sitemap/generator.tsmode: 0o644pinned; truncate-overwrite is the required regeneration contract (O_EXCL would break rebuilds); the onlyos.tmpdir()dataflow is from test fixtures.tools/check-public-docs-integrity.tsescapeRegExp(tools/lib/text.ts) instead of a dot-only replace. Provably behavior-identical for the constrained tag shape[a-zA-Z]+\.\d+(parity pinned by test).tools/autoflow/__tests__/release.test.tsproject-constants.tslinePACKAGE_VERSION_TAG = `v${PACKAGE_VERSION}`as input tobumpProjectConstantsText; it must not interpolate. Rewriting it would reduce fixture fidelity.packages/app/__tests__/spa-projection-guard.test.tsbaseline = Object.prototypeinitializer.The 8 fixed alerts auto-close when the next CodeQL scan of
devno longer finds them; the 3 dismissals are already recorded on the alerts with these rationales.Before / After
sanitizeHeadExtrasthat (a) missed spec-valid script end tags with attributes, (b) could emit sanitizer-created live markup in adversarial nesting cases, and (c) a destructive strip-to-EOF fallback that ate legitimate trailing markup; check-then-read static file serving; a docs-gate RegExp interpolation with partial escaping; a useless assignment in a test.escapeRegExp; clean test code. The escape/sanitize security contract is unchanged in the safe direction only: everything the old code stripped is still stripped; the new code additionally strips residuals the old code missed.Why-not-second-owner
No frozen-semantics path (ADR-0122) is touched; no public API, protocol, or compiler contract changes.
html-escape.tsis not on the frozen list, and its changes are contract-preserving strengthenings proven against the existing security-projection suites plus new adversarial regression tests. A second owner would add review latency without a second architecture domain.Evidence
html-escapetests + 2 newarticle-bodytests fail on the old code, pass on the fix (packages/element/__tests__/html-escape.test.ts10/10,www/__tests__/article-body.test.ts3/3).packages/element/__tests__253/253 (incl.compiled-escape-parity,sanitize);deno task check:static-output-freeze -- --self-checkPASS (655 files byte-identical across runs). The full baseline-comparison variant errors on a pre-existing environment issue (the v0.41.2 baseline worktree cannot resolvenpm:sanitize-html); that failure is in the baseline build path and unrelated to this change.static-serve,ssg-helpers,serve-entry(boots the regeneratedserve.mjsover HTTP — dynamic + static channels green),sitemap-generator,head-injection,entry-renderer,ssg-admission-parity,start-request-containment: 123+ green; directory-candidate parity test added.tools/479/479;deno task docs:check-publicpassed.packages/app/__tests__99/99.deno task pack:dry-runexit 0;deno task package-artifacts:checkexit 0.deno task test: green (see checks).Scope
In: the 11 open alerts enumerated live via
gh api repos/open-element/openelement/code-scanning/alerts?state=open(matches the #1281 list exactly). Out: any other CodeQL rule tuning, sanitizer contract changes, frozen-semantics paths, release/tag work.Risk
Low. All behavior changes are in the safe direction (strip more residual adversarial markup; fail closed on vanished files). Normal-content parity is pinned by the pre-existing suites plus new tests (ordinary headings, meta preservation, directory-miss parity, fixture server and standalone server contract tests).
Closes #1281