Skip to content

feat(page-context): collect page-type signals (jsonLdType, ogType, lang) - #2770

Merged
anpete merged 5 commits into
mainfrom
apeters/page-type-signals
Jun 17, 2026
Merged

feat(page-context): collect page-type signals (jsonLdType, ogType, lang)#2770
anpete merged 5 commits into
mainfrom
apeters/page-type-signals

Conversation

@anpete

@anpete anpete commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Add an optional pageTypeSignals field to page-context collection, gated behind the includePageTypeSignals feature setting (off by default). It extracts schema.org JSON-LD @type, Open Graph og:type, and the declared page language, riding the existing collectionResult message so consumers (e.g. Duck.ai sidebar shortcuts) get cheap page-type hints with no extra round-trip and no LLM.

  • Export extractPageTypeSignals(document) for unit testing (mirrors domToMarkdown); JSON-LD parsed defensively (malformed/oversized skipped), @type collected as string or array and from @graph, deduped, casing preserved, capped.
  • Add jasmine unit specs.

Asana Task/Github Issue: https://app.asana.com/1/137249556945/project/1209072953775241/task/1215655051685860

Description

Adds page signal collection to the page context collection script.

Testing Steps

Added unit tests

Checklist

Please tick all that apply:

  • I have tested this change locally
  • I have tested this change locally in all supported browsers
  • This change will be visible to users
  • I have added automated tests that cover this change
  • I have ensured the change is gated by config
  • This change was covered by a ship review
  • This change was covered by a tech design
  • Any dependent config has been merged

Note

Low Risk
Feature is config-gated off by default and only adds best-effort metadata to existing collection; JSON-LD handling includes explicit bounds to avoid breaking full page collection.

Overview
Adds optional pageTypeSignals to page-context collectionResult payload when includePageTypeSignals is enabled (default off), giving consumers cheap page-type hints without an extra round-trip.

New extractPageTypeSignals harvests JSON-LD @type (string/array, @graph, deduped, order preserved), og:type, and html lang. JSON-LD parsing is defensive: malformed/oversized blocks skipped, type count and block size capped via config (maxPageTypeSignals, maxJsonLdBlockLength), recursion bounded, case-insensitive script type, hasOwnProperty for @type/@graph.

Coverage includes JSDOM unit specs, a Playwright end-to-end spec with fixture HTML/config, and registration of that spec in the Windows Playwright project.

Reviewed by Cursor Bugbot for commit 911adcf. Bugbot is set up for automated code reviews on this repo. Configure here.

@anpete
anpete requested review from a team and noisysocks as code owners June 15, 2026 18:09
@github-actions
github-actions Bot requested a review from moon0326 June 15, 2026 18:10
@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

[Beta] Generated file diff

Time updated: Wed, 17 Jun 2026 07:28:03 GMT

Android
    - android/contentScope.js

File has changed

Apple
    - apple/contentScope.js

File has changed

Integration
    - integration/contentScope.js

File has changed

Windows
    - windows/contentScope.js

File has changed

daxtheduck
daxtheduck previously approved these changes Jun 15, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Injected PR Evaluation: Web Compatibility & Security

Web Compatibility Assessment

File Lines Severity Finding
injected/src/features/page-context.js 253–265 info Read-only DOM access (querySelector / querySelectorAll on <head> elements). No API overrides, prototype patches, or descriptor mutations — zero toString() / instanceof fidelity concerns.
injected/src/features/page-context.js 275–290 info collectJsonLdTypes only walks arrays, top-level @type, and @graph. Nested entities (e.g. mainEntity.@type) are intentionally skipped for a cheap signal. Consumers should not assume exhaustive schema.org coverage.
injected/src/features/page-context.js 593–595, 600–602 info pageTypeSignals is included in the same content cache as markdown body text. The MutationObserver watches document.body only, so head-only updates (late-injected JSON-LD / og:type changes) may not invalidate the cache until another trigger fires. Typical static <head> metadata is unaffected.
injected/src/features/page-context.js 724–728 info getPageTypeSignals() runs during collectPageContent() in init()-time collection paths — consistent with existing optional fields (includeHeadings, includeLinks, etc.). No load()-before-config hazard.
injected/unit-test/page-context-signals.spec.js all info Good unit coverage for malformed JSON, oversized blocks, deduping, and caps. Tests pass locally (13 specs, 0 failures).

No warning or error-level web-compat findings.

Security Assessment

File Lines Severity Finding
injected/src/features/page-context.js 260 info Uses page-reachable JSON.parse instead of captured JSONparse from captured-globals.js. A hostile page could substitute JSON.parse after injection; risk is limited here (parsed output is walked defensively, failures are caught, only @type strings are retained). Aligning with captured globals would be defense-in-depth consistency, not a blocker.
injected/src/features/page-context.js 243 info new Set() is uncaptured. Tampered Set could break deduping or throw; impact is localized to this optional, config-gated sub-field.
injected/src/features/page-context.js 282–289 info Bracket reads (obj['@type'], obj['@graph']) on JSON.parse output. Prototype-polluted Object.prototype could theoretically surface spurious types when own properties are absent; only trimmed strings are collected, limiting blast radius.
injected/src/features/page-context.js 593–595, 754–757 info New data flows through the existing sendContentResponsemessaging.notify('collectionResult', { serializedPageData }) path. No new nativeData fields, no bridge/origin changes, no postMessage usage. Appropriately gated behind includePageTypeSignals (default disabled).
injected/src/features/page-context.js 256–257, 726–727 info maxBlockLength (default 100k) and maxTypes (default 10) bound CPU/memory per collection. Remote config can raise these (same pattern as maxContentLength / upperLimit elsewhere in this feature).

No warning, error, or critical security findings.

Risk Level

Low Risk — Config-gated, read-only DOM/metadata extraction with bounded parsing; no API shims, messaging transport changes, or captured-globals modifications.

Recommendations

  1. (info) When enabling includePageTypeSignals in privacy-configuration, add an integration-test case under integration-test/test-pages/page-context/ to verify end-to-end collection (mirrors existing page-context IT coverage).
  2. (info) Consider import { JSONparse } from '../captured-globals.js' in extractJsonLdTypes for consistency with hardened features (optional hardening).
  3. (info) Document for consumers that jsonLdType is a shallow @type/@graph harvest, not a full JSON-LD graph walk.
  4. (info) If SPA head updates matter for your use case, consider invalidating cache on document.head mutations or re-collecting signals outside the body-only observer (not required for initial rollout).
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@anpete
anpete force-pushed the apeters/page-type-signals branch from b89b88d to 3a3341d Compare June 15, 2026 18:12
@daxtheduck
daxtheduck dismissed their stale review June 15, 2026 18:12

Dismissing stale approval — new commits pushed, awaiting Cursor re-review.

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Build Branch

Branch pr-releases/apeters/page-type-signals
Commit 59500c67d0
Updated June 17, 2026 at 7:27:42 AM UTC

Static preview entry points

QR codes (mobile preview)
Entry point QR code
Docs QR for docs preview
Static pages QR for static pages preview
Integration pages QR for integration pages preview

Integration commands

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#pr-releases/apeters/page-type-signals

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", branch: "pr-releases/apeters/page-type-signals")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/apeters/page-type-signals
git -C submodules/content-scope-scripts checkout origin/pr-releases/apeters/page-type-signals
Pin to exact commit

npm (Android / Extension):

npm i github:duckduckgo/content-scope-scripts#59500c67d0806767e2e44ce9b6a167bdede3c4de

Swift Package Manager (Apple):

.package(url: "https://github.com/duckduckgo/content-scope-scripts.git", revision: "59500c67d0806767e2e44ce9b6a167bdede3c4de")

git submodule (Windows):

git -C submodules/content-scope-scripts fetch origin pr-releases/apeters/page-type-signals
git -C submodules/content-scope-scripts checkout 59500c67d0806767e2e44ce9b6a167bdede3c4de

daxtheduck
daxtheduck previously approved these changes Jun 15, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Injected PR Evaluation: Web Compatibility & Security

Reviewed extractPageTypeSignals and its integration into page-context.js (+ unit tests). This is read-only DOM/metadata extraction behind a new config gate (includePageTypeSignals, default disabled). No API overrides, prototype patches, or messaging transport changes.

Web Compatibility Assessment

File Lines Severity Finding
injected/src/features/page-context.js 253 info querySelectorAll('script[type="application/ld+json"]') is case-sensitive for the attribute value. HTML type is case-insensitive; pages using APPLICATION/LD+JSON or mixed case may be missed. Low impact — signals are best-effort.
injected/src/features/page-context.js 275–290 info collectJsonLdTypes only walks arrays, top-level @type, and @graph. Nested entities (e.g. mainEntity: { "@type": "Recipe" }) are not traversed. Common on recipe/article pages; won't break sites but may under-collect.
injected/src/features/page-context.js 275–290 warning Recursive traversal has no depth cap. A page can embed deeply nested JSON-LD (within maxJsonLdBlockLength) that triggers RangeError during collection. Caught by handleContentCollectionRequest, but when includePageTypeSignals is enabled it can fail the entire content collection for that page.
injected/src/features/page-context.js 593–595 info Feature is config-gated with default disabled and follows the same optional-field pattern as includeMetaDescription / includeHeadings. Safe rollout path.
injected/unit-test/page-context-signals.spec.js all info Good unit coverage for happy paths, malformed JSON, caps, and deduping. No integration test for live DOM injection timing (acceptable for this scope).

Security Assessment

File Lines Severity Finding
injected/src/features/page-context.js 260 warning Uses uncaptured JSON.parse. captured-globals.js exports JSONparse for hostile-page hardening. A tampered JSON.parse could throw or behave unexpectedly; wrapped in try/catch so impact is limited, but inconsistent with security-sensitive injected patterns.
injected/src/features/page-context.js 243 warning Uses uncaptured new Set(). Captured Set is available from captured-globals.js. Low exploitability here, but a tampered Set could break dedup logic.
injected/src/features/page-context.js 282–289 warning Reads obj['@type'] and obj['@graph'] without own-property checks. Object.prototype pollution with @type / @graph keys can inject spurious signals into the native payload. Prefer hasOwnProperty from captured globals (see broker-protection / api-manipulation patterns).
injected/src/features/page-context.js 593–595, 754–757 info New data flows through existing serializedPageData: JSON.stringify(content) path. No new notify fields, no nativeData spread, no bridge changes.
injected/src/features/page-context.js 726–727 info maxPageTypeSignals / maxJsonLdBlockLength are read from config without upper-bound clamping. Misconfiguration could increase parse work; not a direct exploit but worth capping in code (e.g. Math.min(config, 50)).

Risk Level

Medium Risk — New injected-runtime code that reads and parses page-controlled JSON-LD, but no API shims or trust-boundary changes; properly config-gated with sensible defaults and bounds.

Recommendations

  1. (warning) Import JSONparse and Set from captured-globals.js instead of using page-mutable globals.
  2. (warning) Guard @type / @graph reads with captured hasOwnProperty.call(obj, key) before access.
  3. (warning) Add a recursion-depth limit to collectJsonLdTypes (e.g. 32–64) to prevent stack overflow from adversarial nesting when the feature is enabled.
  4. (info) Consider case-insensitive JSON-LD script discovery (script[type="application/ld+json" i] where supported, or normalize type in a manual loop).
  5. (info) Clamp config-driven limits to safe maximums in getPageTypeSignals().
  6. (info) Ensure includePageTypeSignals, maxPageTypeSignals, and maxJsonLdBlockLength are documented in privacy-configuration before enabling in production.

Overall this is a well-scoped, opt-in addition with solid unit tests. The main actionable items are captured-global hygiene, prototype-pollution guards, and recursion bounding before wide rollout.

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated

@moon0326 moon0326 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left one minor comment, but LGTM otherwise.

anpete added a commit that referenced this pull request Jun 16, 2026
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>
@daxtheduck
daxtheduck dismissed their stale review June 16, 2026 06:10

Dismissing stale approval — new commits pushed, awaiting Cursor re-review.

daxtheduck
daxtheduck previously approved these changes Jun 16, 2026
cursor[bot]

This comment was marked as outdated.

anpete added a commit that referenced this pull request Jun 16, 2026
…harvest

Follow-up to PR #2770 re-review:
- add() now returns early once maxTypes is reached so a single wide @graph stops iterating.
- Document that jsonLdType is a shallow top-level/@graph harvest (no nested mainEntity.@type).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@daxtheduck
daxtheduck dismissed their stale review June 16, 2026 06:30

Dismissing stale approval — new commits pushed, awaiting Cursor re-review.

daxtheduck
daxtheduck previously approved these changes Jun 16, 2026
cursor[bot]

This comment was marked as outdated.

@daxtheduck
daxtheduck dismissed their stale review June 16, 2026 06:49

Dismissing stale approval — new commits pushed, awaiting Cursor re-review.

daxtheduck
daxtheduck previously approved these changes Jun 16, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Web Compatibility & Security Review

Web Compatibility Assessment

File Lines Severity Finding
injected/src/features/page-context.js 267–330 info Read-only DOM queries (querySelector / querySelectorAll) with no API overrides or prototype mutation. Matches existing page-context patterns (getMetaDescription, getHeadings, etc.) and is unlikely to break third-party scripts.
injected/src/features/page-context.js 267 info Case-insensitive attribute selector ([type="application/ld+json" i]) is already used in web-compat.js for viewport meta tags, so this is an established repo pattern.
injected/src/features/page-context.js 220–228 info Shallow JSON-LD harvest (top-level + @graph only) is documented in JSDoc. Consumers must not treat jsonLdType as full schema.org graph coverage — this is a product contract note, not a runtime risk.
injected/src/features/page-context.js 549–557 info MutationObserver watches document.body only (pre-existing). Head metadata (og:type, JSON-LD scripts, lang) can be stale if injected into <head> after initial collection without a body mutation. Same limitation applies to includeMetaDescription; not introduced by this PR.
injected/integration-test/page-type-signals.spec.js info Integration test is registered only on the Windows Playwright project. Broader page-context coverage lives in pages.spec.js (chrome-mv3); consider adding a signal assertion there if cross-platform regression signal is desired.

No warning or error-level web-compat findings.


Security Assessment

File Lines Severity Finding
injected/src/features/page-context.js 9, 274, 300–309 info Sensitive operations use captured globals (JSONparse, Set, hasOwnProperty) rather than re-reading page-tamperable builtins.
injected/src/features/page-context.js 300–309 info hasOwnProperty.call() guards on @type / @graph prevent polluted Object.prototype from injecting spurious types. Unit test explicitly covers this.
injected/src/features/page-context.js 14, 255–281, 292–293 info DoS mitigations are solid: maxBlockLength skip before parse, MAX_JSON_LD_DEPTH (32) bounds recursion, maxTypes short-circuits iteration. Malformed JSON blocks are skipped without failing collection.
injected/src/features/page-context.js 613–615 info New functionality is config-gated behind includePageTypeSignals (default disabled), allowing remote rollback without deploy.
injected/src/features/page-context.js 788–791 info Signals flow through the existing serializedPageData: JSON.stringify(content) notify path. This is page-derived metadata sent to native — same trust model as title/content/headings. No new postMessage or bridge surface. nativeData is not included.
injected/src/features/page-context.js 258 info Non-string @type values are filtered, preventing object-coercion surprises in downstream consumers.

No warning, error, or critical security findings.


Risk Level

Medium Risk — New runtime signal-extraction logic in injected page-context, but no API shims, no messaging transport changes, config-gated off by default, and defensive bounds on hostile JSON-LD input.


Recommendations

  1. (info) Add includePageTypeSignals, maxPageTypeSignals, and maxJsonLdBlockLength to the privacy-configuration schema for pageContext so remote config authors have typed, documented settings.
  2. (info) Ensure downstream consumers understand the shallow JSON-LD scope (documented in code; may warrant a brief note in consumer docs).
  3. (optional) Extend chrome-mv3 pages.spec.js Page Context test to assert pageTypeSignals when the setting is enabled, for broader platform coverage.
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@anpete

anpete commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@moon0326 All review comments addressed.

@anpete
anpete enabled auto-merge June 16, 2026 06:57
Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated
Comment thread injected/src/features/page-context.js Outdated
@daxtheduck
daxtheduck dismissed their stale review June 16, 2026 17:35

Dismissing stale approval — new commits pushed, awaiting Cursor re-review.

daxtheduck
daxtheduck previously approved these changes Jun 16, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Injected PR Evaluation: Web Compatibility & Security

Re-evaluation after dc38d866 — prior warning-level findings are resolved.

Web Compatibility Assessment

File Lines Severity Finding
injected/src/features/page-context.js 264–327 info Read-only DOM access (querySelector / querySelectorAll on <head> elements). No API overrides, prototype patches, or descriptor mutations — zero toString() / instanceof fidelity concerns.
injected/src/features/page-context.js 264 info Case-insensitive script[type="application/ld+json" i] matches the established web-compat.js pattern (meta[name=viewport i]). On engines without Selectors Level 4 i, querySelectorAll may throw; impact is limited because includePageTypeSignals defaults to disabled and is remotely gateable.
injected/src/features/page-context.js 226–228, 289–307 info Shallow @type harvest (top-level + @graph only). Now documented in JSDoc — consumers must not assume full schema.org graph coverage.
injected/src/features/page-context.js 610–611, 617–618 info pageTypeSignals is cached with body markdown. MutationObserver watches document.body only, so head-only late injections (JSON-LD / og:type) may not invalidate the cache until another trigger fires. Typical static <head> metadata is unaffected.
injected/integration-test/page-type-signals.spec.js all info End-to-end integration test added; good coverage alongside 17 unit specs.

No warning or error-level web-compat findings.

Security Assessment

File Lines Severity Finding
injected/src/features/page-context.js 9, 252, 271 info Uses captured JSONparse, Set, and hasOwnProperty — addresses prior defense-in-depth feedback. Parsed output is walked defensively; only trimmed string @type values are retained.
injected/src/features/page-context.js 289–307 info hasOwnProperty.call before @type / @graph reads mitigates Object.prototype pollution. MAX_JSON_LD_DEPTH (32) bounds recursion. Oversized blocks skipped via maxBlockLength.
injected/src/features/page-context.js 254–260, 266 info recordType short-circuits once types.size >= maxTypes; outer loops also break early. Resolves prior perf concern about wide @graph iteration past the cap.
injected/src/features/page-context.js 757–759 info clampedNumericSetting() coerces non-finite config to fallback before Math.min, preventing NaN from disabling caps. Resolves prior config-hardening warning.
injected/src/features/page-context.js 610–611 info Gated behind includePageTypeSignals (default disabled) — required remote kill switch present.
injected/src/features/page-context.js 278 info Array.from(types) uses uncaptured global; Arrayfrom is available in captured-globals.js. Tampered Array.from could throw; impact is localized to this optional sub-field. Cosmetic consistency only.
injected/src/features/page-context.js 785–788 info pageTypeSignals flows into existing serializedPageData: JSON.stringify(content) notify path. No nativeData spread; no messaging-boundary changes.

No warning, error, or critical security findings.

Risk Level

Low Risk — Config-gated, read-only DOM metadata extraction with defensive bounds, captured globals for hostile-environment parsing, and no API shims or messaging trust-boundary changes.

Recommendations

  1. (info) Consider wrapping document.querySelectorAll('script[type="application/ld+json" i]') in try/catch so an unsupported selector on a legacy engine cannot take down the entire collectPageContent() path when the setting is enabled.
  2. (info) Optional: switch Array.from(types) → captured Arrayfrom(types) for consistency with repo hygiene patterns.
  3. (info) If consumers need fresh signals after SPA head mutations, consider extending cache invalidation to <head> changes (only if product requirements demand it).
Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

Comment thread injected/src/features/page-context.js
Comment thread injected/src/features/page-context.js
@anpete

anpete commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

@jonathanKingston Thanks, addressed feedback.

anpete and others added 5 commits June 17, 2026 09:26
Add an optional pageTypeSignals field to page-context collection, gated behind the includePageTypeSignals feature setting (off by default). It extracts schema.org JSON-LD @type, Open Graph og:type, and the declared page language, riding the existing collectionResult message so consumers (e.g. Duck.ai sidebar shortcuts) get cheap page-type hints with no extra round-trip and no LLM.

- Export extractPageTypeSignals(document) for unit testing (mirrors domToMarkdown); JSON-LD parsed defensively (malformed/oversized skipped), @type collected as string or array and from @graph, deduped, casing preserved, capped.
- Add jasmine unit specs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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>
…harvest

Follow-up to PR #2770 re-review:
- add() now returns early once maxTypes is reached so a single wide @graph stops iterating.
- Document that jsonLdType is a shallow top-level/@graph harvest (no nested mainEntity.@type).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…passing the cap

Malformed config (e.g. "foo") previously produced NaN from Math.min, disabling the type cap. Clamp via a finite-number check that still honors an explicit 0.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e Set

Address @jonathanKingston review: a Set already dedupes and preserves insertion
order, so it replaces the parallel types array + seen Set. Cap via .size, drop the
now-redundant slice (recordType short-circuits at the cap), and rename add -> recordType.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@anpete
anpete force-pushed the apeters/page-type-signals branch from dc38d86 to 911adcf Compare June 17, 2026 07:26
@daxtheduck
daxtheduck dismissed their stale review June 17, 2026 07:27

Dismissing stale approval — new commits pushed, awaiting Cursor re-review.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web Compatibility & Security Review (automation)

Reviewed 911adcff on apeters/page-type-signals. This adds config-gated, read-only page-type signal extraction (jsonLdType, ogType, lang) to the existing page-context feature. No API overrides, messaging transport changes, or prototype patches.

Web Compatibility Assessment

File Lines Severity Finding
injected/src/features/page-context.js 264–327 info Read-only querySelector / querySelectorAll on <script>, <meta>, and <html lang> — same DOM-read pattern as existing page-context collectors. No layout/DOM mutation risk.
injected/src/features/page-context.js 264 info Case-insensitive script[type="application/ld+json" i] selector improves discovery of non-canonical script types without affecting page scripts. Supported on all DDG WebView targets.
injected/src/features/page-context.js 226–228 info Shallow @type harvest (top-level + @graph only) is documented. Consumers must not treat jsonLdType as full schema.org graph coverage — acceptable contract limitation.
injected/src/features/page-context.js 741–745 info Signals are collected from the main document only; same-origin iframe content is not scanned (unlike markdown collection's includeIframes). Intentional scope boundary — document for downstream consumers.
injected/src/features/page-context.js 610–612 info Feature is opt-in via includePageTypeSignals (default disabled), so no compat impact until remotely enabled per-domain.

No web-compat warnings or errors identified.

Security Assessment

File Lines Severity Finding
injected/src/features/page-context.js 9, 271, 252, 297–306 info Correct use of captured JSONparse, Set, and hasOwnProperty for hostile-page hardening. Prior review feedback addressed.
injected/src/features/page-context.js 14, 268, 256, 290 info DoS mitigations are solid: MAX_JSON_LD_DEPTH=32, maxBlockLength (default 100 KB, ceiling 1 MB), maxTypes (default 10, ceiling 50), and per-block parse try/catch so one bad block cannot fail collection.
injected/src/features/page-context.js 757–759 info clampedNumericSetting() prevents malformed config from producing NaN and bypassing caps — good config-trust hygiene.
injected/src/features/page-context.js 278, 291, 299 info Array.from / Array.isArray are uncaptured globals. Low practical risk here (internal iteration only), and consistent with broader codebase patterns. Optional harden: import Arrayfrom from captured-globals.js.
injected/src/features/page-context.js 610–612, 785–788 info pageTypeSignals is added as an explicit field on the content object (not spread from untrusted input). Outbound path reuses existing collectionResult / serializedPageData channel — no new exfiltration surface beyond page-context's existing trust model. Signals are public page metadata only.
injected/src/features/page-context.js 297–306 info hasOwnProperty.call guards on @type / @graph prevent Object.prototype pollution from surfacing spurious types. Covered by unit test.

No security warnings, errors, or critical findings.

Risk Level

Low Risk — Config-gated, read-only DOM inspection with bounded JSON parsing; no API shims, messaging changes, or captured-global regressions in critical paths.

Recommendations

  1. (info) Consider importing Arrayfrom from captured-globals.js for consistency with the JSONparse/Set hardening — not blocking.
  2. (info) Ensure privacy-configuration schema documents includePageTypeSignals, maxPageTypeSignals, and maxJsonLdBlockLength with safe defaults before rollout.
  3. (done) Prior review items (captured globals, depth cap, prototype pollution, numeric clamping, case-insensitive discovery, tests) are addressed in commits c02ea1541911adcff3.

Verdict: Approve from a web-compat and security perspective. Well-bounded addition with appropriate hardening and test coverage (17 unit specs + windows integration test).

Open in Web View Automation 

Sent by Cursor Automation: Web compat and sec

@anpete
anpete added this pull request to the merge queue Jun 17, 2026
Merged via the queue into main with commit 71ce27f Jun 17, 2026
45 of 46 checks passed
@anpete
anpete deleted the apeters/page-type-signals branch June 17, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants