Skip to content

fix(producer): bound existing font-face recognition scans - #3718

Merged
jrusso1020 merged 1 commit into
mainfrom
fix/security-font-face-scan
Sep 5, 2026
Merged

fix(producer): bound existing font-face recognition scans#3718
jrusso1020 merged 1 commit into
mainfrom
fix/security-font-face-scan

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

What

Fix CodeQL #751: malformed font-face input can make existing-font detection spend quadratic time retrying overlapping matches before font fallback decisions.

Why

All three scanner examples exceeded two seconds with 100,000 repeated prefixes, spaces or family declarations. Existing-font detection runs before every deterministic font injection, so this can stall compilation.

How

Scan the font-face opener, family declaration, semicolon and closing brace in order. Skip immediately empty declarations as before; stop once a required suffix is absent, since no later opener could complete it. Successful scans advance past the closing brace. Normalize the captured family using the existing function.

Preserve existing recognition and fallback decisions, including first-family ordering, whitespace-only values and malformed-input matching. This remains the existing recognition heuristic; it does not introduce a CSS parser or change fetch/retry/system-font behavior. Two files changed; no workflow changes.

Test plan

  • All 90 deterministic-font tests pass, including 12 new cases for authored fonts, fallback decisions and the three adversarial inputs.
  • All five font-embedding tests pass with Vitest after building workspace dependencies.
  • 200,000 generated inputs match the original ordered normalized family sets exactly.
  • The three isolated adversarial scans complete in under a millisecond locally; public injection tests also finish promptly.
  • Parser/core/lint/Studio-server builds, producer typecheck, formatting, lint and complexity/duplication audit pass.
  • Independent Magi review and CI/CodeQL remain landing gates. Verify main-branch alert closure after merge.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at exact head 2c6c2ed3c969bc9486310a0b943a53d37df49a7e. No blockers.

The staged scan at packages/producer/src/services/deterministicFonts.ts:439-462 is linear. The opening and family regex cursors only advance; semicolon/closer searches cover the remainder once for a successful block, and a missing required suffix ends the scan because the old whole-region matcher could also cross intervening braces/openers—if that suffix does not exist in the remainder, no later opener can succeed. Successful matches move opening.lastIndex beyond the first closer, matching global-regex consumption.

The subtle empty cases are preserved correctly. An immediately empty font-family:; is skipped because the old ([^;]+) could not match it and would continue to the next declaration. A whitespace-only value is not skipped: it normalizes empty and consumes the block, matching the old \s* backtracking one whitespace into +; therefore a later declaration in that same block remains ignored. Non-empty declarations preserve first-family ordering and feed the same normalized ordered Set, which remains the single input to the authored-face/fallback decision at deterministicFonts.ts:1424-1435.

The public-injection table at deterministicFonts.test.ts:8-45 pins these recognition choices via actual fallback fetch attempts rather than a duplicate private parser.

Independent exact-head verification:

  • 90/90 deterministic-font tests passed
  • 5/5 font-embedding tests passed
  • changed-file formatting passed
  • ordered normalized family sets matched main across 500,010 generated/adversarial inputs
  • independent 100k-input scans completed in 0.01–1.21 ms; the public long cases also completed without timeout

CI has no completed failure; JavaScript CodeQL, Windows, typecheck/build, and regression jobs are still running and remain landing gates.

Verdict: APPROVE
Reasoning: The ordered state machine preserves the old recognition/fallback semantics—including the non-obvious empty/whitespace split—while giving every successful or failed suffix a bounded scan.

— Magi

@jrusso1020
jrusso1020 merged commit 3610d94 into main Sep 5, 2026
55 checks passed
@jrusso1020
jrusso1020 deleted the fix/security-font-face-scan branch September 5, 2026 21:17
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.

2 participants