Skip to content

Commit 55bd549

Browse files
sametbasbugclaude
andcommitted
Assert what the escaper must guarantee, not one spelling of one tag
CodeQL opened a high-severity js/bad-tag-filter alert on the feed escaping test: `assert.doesNotMatch(xml, /<script>/u)` does not match `<SCRIPT>`. There is no vulnerability behind it. escapeXml works character by character — every `<` becomes `&lt;` whatever follows it — so an upper case tag is escaped exactly like a lower case one. What CodeQL found was the test, and it was right about that: an assertion that hunts for one spelling of one tag name would have stayed green while a real tag-name filter let `<SCRIPT>` through. So the assertion now states the invariant the escaper actually owes us: nothing an agent writes comes back out of a description as markup. Not one tag name — not a single angle bracket. Because it never looks at a tag name, the case question disappears rather than being answered. Measured rather than assumed. Against output from a filter with exactly that flaw — `<script>` escaped, `<SCRIPT>` passed through verbatim — the old assertion passes and the new one fails. Attempting the same proof by weakening escapeXml itself was refused by the sandbox, correctly; the comparison was made against a constructed string instead, and rss.ts was never left broken in the tree. Two smaller things the strict equality caught while writing it: the channel carries its own `<description>` ahead of every item, so the first match in the document is the wrong one, and the seed now carries both cases so the expected string has to spell out what escaping both of them produces. Local proof: 228 D1 tests pass, npm run check reports 0 errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 775391a commit 55bd549

1 file changed

Lines changed: 29 additions & 3 deletions

File tree

‎scripts/orbit-public-page-tests.ts‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,16 +296,42 @@ describe('Orbit dynamic public pages', () => {
296296
assert.match(xml, new RegExp(`<link>${link}</link><guid isPermaLink="true">${link}</guid>`, 'u'));
297297
});
298298

299+
/* Bu test `assert.doesNotMatch(xml, /<script>/u)` ile yazılmıştı ve CodeQL
300+
* onu bir HTML filtresi sanıp yüksek seviye uyarı açtı (js/bad-tag-filter).
301+
* Bulduğu şey doğruydu: küçük harfli etiketi arayan bir kalıp `<SCRIPT>`
302+
* geçse sessizce yeşil kalırdı. Zafiyet değildi — escapeXml karakter bazlı
303+
* çalışır, `<` her hâlde `&lt;` olur — zayıf olan testin kendisiydi.
304+
*
305+
* Artık iddia edilen şey gerçekten önemli olan değişmez: ajanın yazdığı
306+
* hiçbir şey description'dan markup olarak geri dönmez. Tek bir etiket adı
307+
* değil, tek bir açı parantezi bile. Etiket adına bakmadığı için hangi
308+
* harfle yazıldığının da önemi kalmıyor. */
299309
test('escapes XML metacharacters in feed summaries', async () => {
310+
const summary = 'Ajan & <script>alert("x")</script> <SCRIPT>alert("y")</SCRIPT> özeti';
300311
const response = await serveDynamicPublicPage(
301312
new Request('https://orbit.example/feed.xml'),
302313
assets,
303-
new FakePublicRepository([record({ summary: 'Ajan & <script>alert("x")</script> özeti' })]),
314+
new FakePublicRepository([record({ summary })]),
304315
);
305316
assert.ok(response);
306317
const xml = await response.text();
307-
assert.doesNotMatch(xml, /<script>/u);
308-
assert.match(xml, /Ajan &amp; &lt;script&gt;alert\(&quot;x&quot;\)&lt;\/script&gt; özeti/u);
318+
319+
/* Kanalın kendi description'ı item'lardan önce geliyor; sabit bir metin
320+
* olduğu için ilk eşleşme testi sessizce yanlış yerden geçirir. */
321+
const item = xml.indexOf('<item>');
322+
assert.ok(item >= 0, 'feed carries an item');
323+
const opening = '<description>';
324+
const start = xml.indexOf(opening, item);
325+
const end = xml.indexOf('</description>', start);
326+
assert.ok(start >= 0 && end > start, 'feed item carries a description');
327+
const description = xml.slice(start + opening.length, end);
328+
329+
assert.doesNotMatch(description, /[<>]/u);
330+
assert.equal(
331+
description,
332+
'Ajan &amp; &lt;script&gt;alert(&quot;x&quot;)&lt;/script&gt;'
333+
+ ' &lt;SCRIPT&gt;alert(&quot;y&quot;)&lt;/SCRIPT&gt; özeti',
334+
);
309335
});
310336

311337
test('answers HEAD for the feed without a body', async () => {

0 commit comments

Comments
 (0)