fix(page-context): harden page-type-signal collection per review

anpete · claude · anpete · commit 0c1ac30f5fed · 2026-06-16T08:10:30.000+02:00
Address PR #2770 review feedback: - Use `??` (not `||`) for maxPageTypeSignals/maxJsonLdBlockLength so an explicit 0 is honored (@moon0326). - Import captured JSONparse/Set/hasOwnProperty so a hostile page can't subvert parsing or dedup. - Guard @type/@graph reads with hasOwnProperty.call to avoid Object.prototype pollution surfacing spurious types. - Bound JSON-LD recursion (MAX_JSON_LD_DEPTH=32) so adversarial nesting can't fail the whole collection. - Clamp config-driven limits (Math.min) to safe ceilings. - Case-insensitive ld+json script discovery (APPLICATION/LD+JSON). - Add unit specs (depth cap, prototype pollution, casing, explicit 0) and a windows-project integration test (page + config) asserting end-to-end pageTypeSignals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/injected/integration-test/page-type-signals.spec.js b/injected/integration-test/page-type-signals.spec.js
@@ -0,0 +1,35 @@
+import { test, expect } from '@playwright/test';
+import { ResultsCollector } from './page-objects/results-collector.js';
+
+const HTML = '/page-context/pages/page-type-signals.html';
+const CONFIG = './integration-test/test-pages/page-context/config/page-type-signals.json';
+
+/**
+ * Return the page-type signals from the first collectionResult message that carries them.
+ * @param {ResultsCollector} collector
+ */
+async function collectedSignals(collector) {
+    const messages = await collector.waitForMessage('collectionResult', 1);
+    for (const message of messages) {
+        const params = /** @type {{ serializedPageData?: string }} */ (message.payload.params);
+        if (!params?.serializedPageData) continue;
+        const data = JSON.parse(params.serializedPageData);
+        if (data.pageTypeSignals) return data.pageTypeSignals;
+    }
+    return null;
+}
+
+test('page-context collects page-type signals end-to-end', async ({ page }, testInfo) => {
+    const collector = ResultsCollector.create(page, testInfo.project.use);
+    await collector.load(HTML, CONFIG);
+
+    const signals = await collectedSignals(collector);
+
+    // jsonLdType is collected in priority order (Recipe before the embedded VideoObject); the
+    // second block uses non-canonical casing to exercise case-insensitive discovery.
+    expect(signals).toStrictEqual({
+        jsonLdType: ['Recipe', 'NewsArticle', 'VideoObject'],
+        ogType: 'article',
+        lang: 'en',
+    });
+});
diff --git a/injected/integration-test/test-pages/page-context/config/page-type-signals.json b/injected/integration-test/test-pages/page-context/config/page-type-signals.json
@@ -0,0 +1,26 @@
+{
+    "readme": "Enables page-context collection with includePageTypeSignals to verify end-to-end signal extraction.",
+    "version": 1,
+    "features": {
+        "pageContext": {
+            "state": "enabled",
+            "exceptions": [],
+            "settings": {
+                "collectOnInit": "enabled",
+                "subscribeToLoad": "enabled",
+                "subscribeToCollect": "enabled",
+                "observeMutations": "enabled",
+                "bodyFallback": "enabled",
+                "trimBlankLinks": "enabled",
+                "includeIframes": "enabled",
+                "includePageTypeSignals": "enabled",
+                "maxContentLength": 9500,
+                "maxTitleLength": 100,
+                "maxDepth": 5000,
+                "cacheExpiration": 30000,
+                "recheckLimit": 5
+            }
+        }
+    },
+    "unprotectedTemporary": []
+}
diff --git a/injected/integration-test/test-pages/page-context/pages/page-type-signals.html b/injected/integration-test/test-pages/page-context/pages/page-type-signals.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width">
+    <title>Page Type Signals Test</title>
+    <meta name="description" content="Test page exercising page-type-signal collection">
+    <meta property="og:type" content="article">
+    <!-- Multi-@type block: priority order matters to consumers (Recipe before the embedded video). -->
+    <script type="application/ld+json">
+        { "@context": "https://schema.org", "@type": ["Recipe", "NewsArticle"] }
+    </script>
+    <!-- Non-canonical casing must still be discovered (case-insensitive selector). -->
+    <script type="APPLICATION/LD+JSON">
+        { "@graph": [{ "@type": "VideoObject" }] }
+    </script>
+    <link rel="stylesheet" href="../../shared/style.css">
+</head>
+<body>
+    <p><a href="../index.html">[Page Context]</a></p>
+
+    <article>
+        <h1>Page Type Signals Test</h1>
+        <p>This page exposes JSON-LD <code>@type</code>, <code>og:type</code>, and a declared language so
+        the page-context feature can collect normalized page-type signals.</p>
+    </article>
+</body>
+</html>
diff --git a/injected/playwright.config.js b/injected/playwright.config.js
@@ -21,6 +21,7 @@ export default defineConfig({
                 'integration-test/web-detection.spec.js',
                 'integration-test/web-events.spec.js',
                 'integration-test/web-interference-detection-events.spec.js',
+                'integration-test/page-type-signals.spec.js',
             ],
             use: { injectName: 'windows', platform: 'windows' },
         },
diff --git a/injected/src/features/page-context.js b/injected/src/features/page-context.js
@@ -5,8 +5,14 @@
 import ContentFeature from '../content-feature.js';
 import { getFaviconList } from './favicon.js';
 import { isDuckAi, isBeingFramed, getTabUrl } from '../utils.js';
+// eslint-disable-next-line no-redeclare
+import { JSONparse, Set, hasOwnProperty } from '../captured-globals.js';
 const MSG_PAGE_CONTEXT_RESPONSE = 'collectionResult';
 
+// Bail past this nesting depth when walking JSON-LD so adversarially deep blocks
+// can't blow the stack and fail the whole content collection.
+const MAX_JSON_LD_DEPTH = 32;
+
 export function checkNodeIsVisible(node) {
     try {
         const style = window.getComputedStyle(node);
@@ -250,14 +256,16 @@ function extractJsonLdTypes(document, maxTypes, maxBlockLength) {
         types.push(type);
     };
 
-    const blocks = document.querySelectorAll('script[type="application/ld+json"]');
+    // Case-insensitive attribute match ("i") so non-canonical casing (e.g. APPLICATION/LD+JSON)
+    // is still discovered.
+    const blocks = document.querySelectorAll('script[type="application/ld+json" i]');
     for (const block of blocks) {
         if (types.length >= maxTypes) break;
         const text = block.textContent || '';
         if (!text || text.length > maxBlockLength) continue;
         let data;
         try {
-            data = JSON.parse(text);
+            data = JSONparse(text);
         } catch (e) {
             continue;
         }
@@ -269,24 +277,30 @@ function extractJsonLdTypes(document, maxTypes, maxBlockLength) {
 
 /**
  * Walk a parsed JSON-LD value (object, array, or `@graph` container) and report each `@type`.
+ * Own-property checks keep a polluted `Object.prototype` from surfacing spurious `@type` /
+ * `@graph` keys, and recursion is bounded by {@link MAX_JSON_LD_DEPTH}.
  * @param {unknown} node
  * @param {(value: unknown) => void} add
+ * @param {number} [depth]
  */
-function collectJsonLdTypes(node, add) {
+function collectJsonLdTypes(node, add, depth = 0) {
+    if (depth > MAX_JSON_LD_DEPTH) return;
     if (Array.isArray(node)) {
-        for (const item of node) collectJsonLdTypes(item, add);
+        for (const item of node) collectJsonLdTypes(item, add, depth + 1);
         return;
     }
     if (!node || typeof node !== 'object') return;
     const obj = /** @type {Record<string, unknown>} */ (node);
-    const type = obj['@type'];
-    if (Array.isArray(type)) {
-        for (const value of type) add(value);
-    } else if (type != null) {
-        add(type);
+    if (hasOwnProperty.call(obj, '@type')) {
+        const type = obj['@type'];
+        if (Array.isArray(type)) {
+            for (const value of type) add(value);
+        } else if (type != null) {
+            add(type);
+        }
     }
-    if (obj['@graph']) {
-        collectJsonLdTypes(obj['@graph'], add);
+    if (hasOwnProperty.call(obj, '@graph')) {
+        collectJsonLdTypes(obj['@graph'], add, depth + 1);
     }
 }
 
@@ -722,9 +736,11 @@ export default class PageContext extends ContentFeature {
     }
 
     getPageTypeSignals() {
+        // `??` so an explicit 0 isn't silently replaced by the default; Math.min clamps remote
+        // config to a safe ceiling to bound parse work.
         return extractPageTypeSignals(document, {
-            maxTypes: this.getFeatureSetting('maxPageTypeSignals') || 10,
-            maxBlockLength: this.getFeatureSetting('maxJsonLdBlockLength') || 100000,
+            maxTypes: Math.min(this.getFeatureSetting('maxPageTypeSignals') ?? 10, 50),
+            maxBlockLength: Math.min(this.getFeatureSetting('maxJsonLdBlockLength') ?? 100000, 1000000),
         });
     }
 
diff --git a/injected/unit-test/page-context-signals.spec.js b/injected/unit-test/page-context-signals.spec.js
@@ -78,6 +78,37 @@ describe('page-context.js - extractPageTypeSignals', () => {
         expect(signals.jsonLdType).toEqual([]);
     });
 
+    it('collects nothing when maxTypes is explicitly 0', () => {
+        const dom = new JSDOM(pageWithHead(scriptLd({ '@type': 'Recipe' })));
+        expect(extractPageTypeSignals(dom.window.document, { maxTypes: 0 }).jsonLdType).toEqual([]);
+    });
+
+    it('discovers JSON-LD with non-canonical script type casing', () => {
+        const head = '<script type="APPLICATION/LD+JSON">' + JSON.stringify({ '@type': 'Recipe' }) + '</script>';
+        expect(signalsFor(pageWithHead(head)).jsonLdType).toEqual(['Recipe']);
+    });
+
+    it('bails out of deeply nested JSON-LD without collecting past the cap (a thrown RangeError fails this test)', () => {
+        let nested = /** @type {any} */ ({ '@type': 'TooDeep' });
+        for (let i = 0; i < 60; i++) nested = { '@graph': [nested] };
+        const head = scriptLd({ '@type': 'Shallow', '@graph': [nested] });
+        const signals = signalsFor(pageWithHead(head));
+        expect(signals.jsonLdType).toContain('Shallow');
+        expect(signals.jsonLdType).not.toContain('TooDeep');
+    });
+
+    it('ignores @type / @graph inherited from a polluted Object.prototype', () => {
+        // Aliased reference so the deliberate pollution doesn't trip the no-extend-native lint rule.
+        const proto = /** @type {Record<string, unknown>} */ (Object.prototype);
+        proto['@type'] = 'Polluted';
+        try {
+            const signals = signalsFor(pageWithHead(scriptLd({ name: 'no own @type' })));
+            expect(signals.jsonLdType).toEqual([]);
+        } finally {
+            delete proto['@type'];
+        }
+    });
+
     it('collects all three signals from a realistic page (recipe with embedded video)', () => {
         const head =
             '<meta property="og:type" content="article">' +
