Skip to content

fix(studio-server): simplify normalized group ID trimming - #3711

Merged
jrusso1020 merged 1 commit into
mainfrom
fix/security-group-id-regex
Sep 5, 2026
Merged

fix(studio-server): simplify normalized group ID trimming#3711
jrusso1020 merged 1 commit into
mainfrom
fix/security-group-id-regex

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

Addresses CodeQL alert #714 in group DOM ID generation. The preceding normalization already collapses every non-alphanumeric run into one hyphen, so the final leading/trailing hyphen repetitions are redundant. I did not find reachable quadratic behavior through the current normalization chain.

Trim one hyphen at each edge and document that invariant. Generated IDs, group labels, member order and collision suffixing remain unchanged; no rule suppression or alert dismissal is added.

Validation: all 101 source-mutation tests pass, including eight new grouping cases covering punctuation, Unicode, empty/default names, 200k-character hyphen runs and occupied base/base-2 IDs. A deterministic 100k-name comparison matched old/new normalized IDs. Parser/core builds, Studio-server typecheck, lint and format pass.

@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 cbc6d5c388003ebfae45eb0cb33552c68f72ee8d. No blockers.

The invariant at packages/studio-server/src/helpers/sourceMutation.ts:477-482 is sound: after lowercasing, /[^a-z0-9]+/g maps every maximal non-alphanumeric run to one hyphen, so either edge contains zero or one hyphen. Replacing ^-+|-+$ with ^-|-$ is therefore behavior-equivalent on every reachable intermediate string; this removes the analyzer's repeated quantifiers without adding another normalization owner.

The table at packages/studio-server/src/helpers/sourceMutationSplitAndGroup.test.ts:200-223 pins all downstream contracts together: normalized DOM ID, occupied base/base-2 collision suffixing to base-3, raw data-hf-group label preservation, and member order. The 200k edge-run case covers the adversarial input through the public mutation path.

Independent verification:

  • parser, lint, and core prerequisite builds passed
  • all 101 source-mutation tests passed
  • Studio-server typecheck and changed-file format check passed
  • old/new normalization matched across 299,596 exhaustive and long inputs

Fresh CI, Windows, and JavaScript CodeQL were still finishing when I submitted; they remain landing gates.

Verdict: APPROVE
Reasoning: The normalization proof is complete, downstream observable behavior is pinned, and exact-head local verification found no regression.

— Magi

@jrusso1020
jrusso1020 merged commit e9250fc into main Sep 5, 2026
47 checks passed
@jrusso1020
jrusso1020 deleted the fix/security-group-id-regex branch September 5, 2026 18:12
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