Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/adapter-vite/__tests__/static-serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,23 @@ Deno.test('tryStatic serves files and refuses path escape', async () => {
}
});

Deno.test('tryStatic treats a directory at a candidate path as a miss (#1281, CodeQL file-system-race)', async () => {
// The candidate check is read-and-fallback instead of existsSync/statSync
// guard-then-read (check-then-act TOCTOU): a directory named like a file
// candidate must fall through exactly like a missing file.
const root = await Deno.makeTempDir();
try {
await Deno.mkdir(join(root, 'dir.html'));
await Deno.writeTextFile(join(root, 'real.html'), '<h1>real</h1>');
assertEquals(tryStatic(root, '/dir.html'), null);
const real = tryStatic(root, '/real.html');
assert(real);
assertEquals(await real.text(), '<h1>real</h1>');
} finally {
await Deno.remove(root, { recursive: true });
}
});

Deno.test('malformed percent-encoding is a defined 400, never a crash (#823)', async () => {
// decodeURIComponent throws URIError on input like /%zz; the serving layer
// converts it to a 400 so `start` and the fixture server stay alive.
Expand Down
14 changes: 11 additions & 3 deletions packages/adapter-vite/src/internal/ssg/ssg-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ export function renderStandaloneServerModule(): string {
// reverse proxy: honor X-Forwarded-Proto/Host for the request URL — never
// trusted by default)
import { createServer } from 'node:http';
import { existsSync, readFileSync, statSync } from 'node:fs';
import { readFileSync } from 'node:fs';
import { extname, join, resolve, sep } from 'node:path';
import { fileURLToPath } from 'node:url';
import process from 'node:process';
Expand Down Expand Up @@ -405,13 +405,21 @@ function tryStatic(pathname) {
for (const candidate of new Set(candidates)) {
const filePath = resolve(join(root, candidate));
if (!filePath.startsWith(root + sep)) continue;
if (!existsSync(filePath) || !statSync(filePath).isFile()) continue;
// Read-and-fallback, mirroring internal/static-serve.ts (#1281): no
// existsSync/statSync guard-then-read TOCTOU race; a vanished or
// non-regular candidate fails the read and falls through like a miss.
let body;
try {
body = readFileSync(filePath);
} catch {
continue;
}
const headers = {
'content-type': MIME[extname(filePath).toLowerCase()] || 'application/octet-stream',
};
const cacheControl = cacheControlFor(filePath);
if (cacheControl) headers['cache-control'] = cacheControl;
return new Response(readFileSync(filePath), {
return new Response(body, {
status: 200,
headers,
});
Expand Down
14 changes: 11 additions & 3 deletions packages/adapter-vite/src/internal/static-serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* this module.
*/

import { existsSync, readFileSync, statSync } from 'node:fs';
import { readFileSync } from 'node:fs';
import { extname, join, resolve, sep } from 'node:path';
import { pathToFileURL } from 'node:url';
import type { IncomingMessage, ServerResponse } from 'node:http';
Expand Down Expand Up @@ -107,8 +107,16 @@ export function tryStatic(distDir: string, pathname: string): Response | null {
for (const candidate of candidates) {
const filePath = resolve(join(root, candidate));
if (!filePath.startsWith(root + sep)) continue;
if (!existsSync(filePath) || !statSync(filePath).isFile()) continue;
const body = readFileSync(filePath);
// Read directly instead of existsSync/statSync guard-then-read: a
// check-then-act pair is a TOCTOU race (CodeQL #1281). A vanished,
// unreadable, or non-regular candidate (EISDIR) simply fails the read and
// falls through to the next candidate, exactly like a miss.
let body: ReturnType<typeof readFileSync>;
try {
body = readFileSync(filePath);
} catch {
continue;
}
const headers: Record<string, string> = { 'content-type': contentTypeFor(filePath) };
const cacheControl = cacheControlFor(filePath);
if (cacheControl) headers['cache-control'] = cacheControl;
Expand Down
2 changes: 1 addition & 1 deletion packages/app/__tests__/spa-projection-guard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ async function mountAndCaptureHost(
loader: () => Promise<unknown>,
): Promise<{ host: Record<string, unknown>; baseline: object }> {
const hosts: Record<string, unknown>[] = [];
let baseline: object = Object.prototype;
let baseline: object;
const root = {
innerHTML: '',
addEventListener() {},
Expand Down
50 changes: 50 additions & 0 deletions packages/element/__tests__/html-escape.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,53 @@ Deno.test('wrapInDocument: strips slash-delimited script tags from headExtras',
assertEquals(out.includes('evil.example'), false);
assertEquals(out.includes('<meta name="ok" content="1">'), true);
});

Deno.test('wrapInDocument: script end tags with attributes close the strip precisely (#1281, CodeQL bad-tag-filter)', () => {
// Browsers accept `</script\t\n bar>` as a script end tag (attributes on end
// tags are ignored), so the stripper must match it — and must stop there
// instead of falling back to the strip-to-EOF pass that eats later markup.
const out = wrapInDocument('x', {
headExtras: '<script>alert(1)</script\t\n bar><meta name="ok" content="1">',
});
assertEquals(out.includes('alert(1)'), false);
assertEquals(out.includes('<meta name="ok" content="1">'), true);
});

Deno.test('wrapInDocument: strips re-formed script tags to a fixed point (#1281, CodeQL incomplete sanitization)', () => {
// Removing the inner pair of a nested fragment re-forms a live outer
// `<script>...</script>`; the strip must consume it precisely instead of
// falling back to strip-to-EOF, which would eat the trailing <meta>.
const out = wrapInDocument('x', {
headExtras: '<scri<script></script>pt>alert(1)</scri</script>pt><meta name="ok" content="1">',
});
assertEquals(out.includes('<script'), false);
assertEquals(out.includes('alert(1)'), false);
assertEquals(out.includes('<meta name="ok" content="1">'), true);
});

Deno.test('wrapInDocument: strips on* handlers exposed by an earlier strip (#1281, CodeQL incomplete sanitization)', () => {
// Removing ` onx='y'` concatenates the leftover ` o` prefix with the
// `nclick=...` suffix, re-forming a live `onclick` handler that a
// single-pass strip emits into the document. The strip must repeat until
// no handler pattern remains.
const out = wrapInDocument('x', {
headExtras: `<a o onx='y'nclick=alert(1)>text</a>`,
});
assertEquals(out.includes('onclick'), false);
assertEquals(out.includes('alert(1)'), false);
assertEquals(out.includes('text'), true);
});

Deno.test('wrapInDocument: --!> counts as a comment close in the balance check (#1281, CodeQL bad-tag-filter)', () => {
const warnings: string[] = [];
const originalWarn = console.warn;
console.warn = (msg: unknown) => warnings.push(String(msg));
try {
wrapInDocument('x', { headExtras: '<!-- ok --!>' });
wrapInDocument('x', { headExtras: '<!-- unclosed' });
} finally {
console.warn = originalWarn;
}
const unbalanced = warnings.filter((w) => w.includes('unbalanced HTML comments'));
assertEquals(unbalanced.length, 1);
});
45 changes: 32 additions & 13 deletions packages/element/src/internal/core/html-escape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,24 @@ function sanitizeHeadExtras(
): string {
if (allowHeadExtrasScripts || !headExtras) return headExtras;
// Strip <script> tags and their content. The delimiter class includes `/`
// (`<script/src=...>` is still a script tag to the browser), and a second
// pass strips an unclosed `<script ...>` to end-of-input (browsers treat
// the rest of the document as script raw text).
let safeHeadExtras = headExtras
.replace(/<script[\s>/][\s\S]*?<\/script\s*>/gi, '')
.replace(/<script[\s>/][\s\S]*$/gi, '');
// (`<script/src=...>` is still a script tag to the browser). The end-tag
// pattern accepts attributes/whitespace because browsers ignore them on end
// tags (`</script\t\n bar>` still ends the script raw-text element). Both
// properties hold only when the strip runs to a fixed point: removing an
// inner pair can re-form a live outer `<script>...</script>`, so the block
// pass repeats until stable before the final backstop strips an unclosed
// `<script ...>` to end-of-input (browsers treat the rest of the document
// as script raw text). Fixed-point first keeps that backstop from eating
// legitimate trailing markup (CodeQL #1281).
const SCRIPT_BLOCK_RE = /<script[\s>/][\s\S]*?<\/script(?=[\s/>])[^>]*>/gi;
const SCRIPT_OPEN_TO_EOF_RE = /<script[\s>/][\s\S]*$/gi;
let safeHeadExtras = headExtras;
for (;;) {
const stripped = safeHeadExtras.replace(SCRIPT_BLOCK_RE, '');
if (stripped === safeHeadExtras) break;
safeHeadExtras = stripped;
}
safeHeadExtras = safeHeadExtras.replace(SCRIPT_OPEN_TO_EOF_RE, '');
if (safeHeadExtras !== headExtras) {
warnOnce(
'headExtrasScripts',
Expand All @@ -194,12 +206,17 @@ function sanitizeHeadExtras(
warnScope,
);
}
// Strip on* event handler attributes (strong XSS indicator)
// Strip on* event handler attributes (strong XSS indicator). Also to a
// fixed point: a match ending at a quoted value can leave a concatenated
// `on...=` sequence that only becomes strippable once the earlier match is
// removed (CodeQL #1281).
if (/\s+on\w+\s*=/i.test(safeHeadExtras)) {
safeHeadExtras = safeHeadExtras.replace(
/\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi,
'',
);
const EVENT_HANDLER_ATTR_RE = /\s+on\w+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi;
for (;;) {
const stripped = safeHeadExtras.replace(EVENT_HANDLER_ATTR_RE, '');
if (stripped === safeHeadExtras) break;
safeHeadExtras = stripped;
}
log.warn(
'headExtras contained on* event handler attributes which were stripped for security.',
);
Expand All @@ -214,9 +231,11 @@ function sanitizeHeadExtras(
*/
function validateHeadExtrasBalance(headExtras: string): void {
if (!headExtras) return;
// Check for unclosed HTML comments: <!-- without matching -->
// Check for unclosed HTML comments: <!-- without a matching close. The
// HTML standard also accepts `--!>` as an (abrupt) comment close, so both
// forms count (CodeQL #1281).
const commentOpens = (headExtras.match(/<!--/g) || []).length;
const commentCloses = (headExtras.match(/-->/g) || []).length;
const commentCloses = (headExtras.match(/--!?>/g) || []).length;
if (commentOpens !== commentCloses) {
log.warn(
'headExtras has unbalanced HTML comments (<!-- vs -->). ' +
Expand Down
23 changes: 23 additions & 0 deletions tools/check-public-docs-integrity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import { assert, assertEquals } from '@std/assert';
import {
findIntegrationSpecifierFailures,
packageSurfaceSpecifiers,
staleCurrentClaims,
} from './check-public-docs-integrity.ts';
import { PREVIOUS_PACKAGE_VERSION } from './project-constants.ts';
import { escapeRegExp } from './lib/text.ts';

const surfaceText = `<!-- package-surface-map
{
Expand Down Expand Up @@ -100,3 +103,23 @@ Deno.test('integration specifiers: real repo docs stay inside the current packag
assert(docs.length > 0);
assertEquals(findIntegrationSpecifierFailures(read, docs), []);
});

Deno.test('stale "active release target" guard binds the previous prerelease tag as a literal (#1281)', () => {
// The tag is interpolated into a RegExp via escapeRegExp (CodeQL incomplete
// sanitization). The extraction regex constrains the tag to
// `[a-zA-Z]+\.\d+`, where escapeRegExp is byte-identical to the historical
// dot-only escaping — this pins that parity so the guard's matching
// contract provably did not change.
const previousTag = PREVIOUS_PACKAGE_VERSION.match(/-([a-zA-Z]+\.\d+)$/u)?.[1];
const guard = staleCurrentClaims.find((re) => re.source.startsWith('active release target'));
if (!previousTag) {
// Stable previous line: the guard is intentionally absent (#727).
assertEquals(guard, undefined);
return;
}
assert(guard);
assert(guard.source.includes(escapeRegExp(previousTag)));
assert(guard.test(`the active release target is v0.44.0-${previousTag}`));
// A `.` in the tag must be matched literally, never as a regex wildcard.
assert(!guard.test(`the active release target is v0x44x0-${previousTag.replace('.', 'X')}`));
});
4 changes: 2 additions & 2 deletions tools/check-public-docs-integrity.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { formatError } from '@openelement/element';
import { PREVIOUS_PACKAGE_VERSION } from './project-constants.ts';
import { staleCurrencyClaimPatterns } from './check-strategic-docs.ts';
import { MOJIBAKE_CHARS } from './lib/text.ts';
import { escapeRegExp, MOJIBAKE_CHARS } from './lib/text.ts';
import { STALE_HISTORY_CLAIM_PATTERNS } from './lib/stale-claims.ts';

// Prerelease tag of the superseded line (e.g. "alpha.9"). The
Expand Down Expand Up @@ -60,7 +60,7 @@ export const staleCurrentClaims: RegExp[] = [
// Bound to the superseded line's prerelease tag (see top of file) so a
// future current anchor never trips this guard (#727).
...(previousPrereleaseTag
? [new RegExp(`active release target.*${previousPrereleaseTag.replace(/\./g, '\\.')}`, 'i')]
? [new RegExp(`active release target.*${escapeRegExp(previousPrereleaseTag)}`, 'i')]
: []),
/alpha\.13 was\s+the prior recovery train/i,
// Currency claims ("published as X", "completed implementation anchor X")
Expand Down
28 changes: 28 additions & 0 deletions www/__tests__/article-body.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { assertEquals } from '@std/assert';
import { prepareArticle } from '../app/site-ui/article-body.ts';

Deno.test('prepareArticle: outline label drops a partial tag fragment (#1281, CodeQL incomplete sanitization)', () => {
// A heading body can contain `<script` with no closing `>`; the tag
// pattern `<[^>]+>` cannot match it, so the label must not carry the
// leftover fragment into the rail outline.
const { outline } = prepareArticle('<h2>configure <script</h2>');
assertEquals(outline.length, 1);
assertEquals(outline[0].label.includes('<'), false);
assertEquals(outline[0].label.includes('script'), true);
});

Deno.test('prepareArticle: outline label cannot retain angle brackets from nested fragments', () => {
const { outline } = prepareArticle('<h2>a <<script>script> b</h2>');
assertEquals(outline.length, 1);
assertEquals(outline[0].label.includes('<'), false);
assertEquals(outline[0].label.includes('>'), false);
assertEquals(outline[0].label, 'a script b');
});

Deno.test('prepareArticle: ordinary heading labels keep their text', () => {
const { outline, html } = prepareArticle('<h2 id="old">Getting <em>started</em> now</h2>');
assertEquals(outline.length, 1);
assertEquals(outline[0].label, 'Getting started now');
assertEquals(outline[0].id, 'getting-started-now');
assertEquals(html.includes('id="getting-started-now"'), true);
});
12 changes: 11 additions & 1 deletion www/app/site-ui/article-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,17 @@ export function prepareArticle(html: string): { html: string; outline: ArticleOu
const withIds = html.replace(
/<h([23])([^>]*)>([\s\S]*?)<\/h\1>/gi,
(_match, depth, attrs, body) => {
const label = String(body).replace(/<[^>]+>/g, '').replace(/&[^;]+;/g, ' ').trim();
// Strip tags to a fixed point, then any angle bracket the tag pattern
// could not match (e.g. a `<script` fragment with no closing `>`), so
// the plain-text label can never carry a partial tag into the rail
// outline (issue 1281).
let label = String(body);
for (;;) {
const stripped = label.replace(/<[^>]+>/g, '');
if (stripped === label) break;
label = stripped;
}
label = label.replace(/[<>]/g, '').replace(/&[^;]+;/g, ' ').trim();
const stem = label.toLowerCase().normalize('NFKD').replace(/[^\p{L}\p{N}]+/gu, '-').replace(
/(^-|-$)/g,
'',
Expand Down
Loading