fix: CodeQL alerts (bounded regexes, null-prototype export groups, exact site URL match)

.changeset/long-token-values.md

@@ -0,0 +1,7 @@
+---
+'onsystem': patch
+---
+
+A long length or color no longer slows onsystem down. A class with an arbitrary length thousands of digits long, such as `p-[999…9x]`, took a second to check at 20,000 characters and about 25 seconds at 100,000, in `check`, the MCP `check_ui` tool, the ESLint plugin and the GitHub Action: the pattern that reads a length tried every way to split the digits. It now takes a few milliseconds. The same goes for token values: a length, a `var()` with a fallback, or bare HSL or RGB channels in the design system's stylesheets or Tailwind config, and typography, spacing and radius values in `onsystem export`. A `var()` whose fallback holds `calc()` or another `var()` now has the fallback's value, inside `calc()` too: `calc(2 * var(--gap, var(--space-2)))` is read as a length, and `calc(var(--gap, var(--space-2)) + 1px)` no longer drops the `+ 1px`.
+
+`onsystem export --format dtcg` keeps a token whose path has a `__proto__` part, such as `--color-__proto__`, under a `"__proto__"` key of its group. It was left out, and a name such as `--__proto__.polluted` added a `polluted` property to every object in the process that ran the export.

site/lib/site.ts

@@ -133,7 +133,9 @@ export function docsHref(section: string): string {
  * GitHub, and an anchor to the section on this page.
  */
 export function resolveDocHref(href: string): string {
-  if (href.startsWith(SITE_URL)) return href.slice(SITE_URL.length) || '/';
+  if (href === SITE_URL || href.startsWith(`${SITE_URL}/`)) {
+    return href.slice(SITE_URL.length) || '/';
+  }
   if (/^https?:\/\//.test(href) || href.startsWith('/') || href.startsWith('#')) return href;
   const doc = /^(?:\.\/)?([\w-]+\.md)(#[\w-]*)?$/.exec(href);
   const page = doc?.[1] ? DOCS_FILES[doc[1]] : undefined;

site/test/markdown.test.ts

@@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest';
 
 import { InlineMarkdown } from '@/components/markdown';
 import { MarkdownBlocks, parseBlocks } from '@/components/markdown-blocks';
+import { resolveDocHref } from '@/lib/site';
 
 const inline = (text: string) => renderToStaticMarkup(createElement(InlineMarkdown, { text }));
 const text = (html: string) => html.replace(/<[^>]+>/g, '');
@@ -98,3 +99,15 @@ describe('MarkdownBlocks', () => {
     );
   });
 });
+
+describe('resolveDocHref', () => {
+  it("turns the site's own URLs into paths, and no other host's", () => {
+    expect(resolveDocHref('https://onsystem.vercel.app')).toBe('/');
+    expect(resolveDocHref('https://onsystem.vercel.app/')).toBe('/');
+    expect(resolveDocHref('https://onsystem.vercel.app/docs/ci')).toBe('/docs/ci');
+    // A host that only starts with the site's stays a link to that host.
+    expect(resolveDocHref('https://onsystem.vercel.app.example.com/x')).toBe(
+      'https://onsystem.vercel.app.example.com/x',
+    );
+  });
+});

src/tokens/color.ts

@@ -34,7 +34,7 @@ export type { Oklch };
  * colors, to be used as `hsl(var(--border))`.
  */
 const BARE_HSL =
-  /^(-?\d*\.?\d+)(deg)?(?:\s*,\s*|\s+)(\d*\.?\d+)%(?:\s*,\s*|\s+)(\d*\.?\d+)%(?:\s*\/\s*(\d*\.?\d+%?))?$/;
+  /^(-?(?:\d+(?:\.\d+)?|\.\d+))(deg)?(?:\s*,\s*|\s+)(\d+(?:\.\d+)?|\.\d+)%(?:\s*,\s*|\s+)(\d+(?:\.\d+)?|\.\d+)%(?:\s*\/\s*((?:\d+(?:\.\d+)?|\.\d+)%?))?$/;
 
 export function isBareHsl(value: string): boolean {
   return BARE_HSL.test(value.trim());
@@ -47,7 +47,7 @@ export function isBareHsl(value: string): boolean {
  * known to be a color: one the Tailwind config reads with `rgb()`.
  */
 const BARE_RGB =
-  /^(\d{1,3})(?:\s*,\s*|\s+)(\d{1,3})(?:\s*,\s*|\s+)(\d{1,3})(?:\s*\/\s*(\d*\.?\d+%?))?$/;
+  /^(\d{1,3})(?:\s*,\s*|\s+)(\d{1,3})(?:\s*,\s*|\s+)(\d{1,3})(?:\s*\/\s*((?:\d+(?:\.\d+)?|\.\d+)%?))?$/;
 
 export function isBareRgb(value: string): boolean {
   const match = BARE_RGB.exec(value.trim());

src/tokens/export.ts

@@ -43,7 +43,7 @@ export function toDtcg(tokens: readonly Token[]): Exported {
   const paths = assignPaths(tokens, warnings);
   const lookup = varLookup(tokens);
   const byName = new Map(tokens.map((t) => [t.name, t]));
-  const tree: JsonObject = {};
+  const tree: JsonObject = Object.create(null) as JsonObject;
   let count = 0;
   for (const category of TOKEN_CATEGORIES) {
     for (const token of tokens) {
@@ -149,7 +149,7 @@ function insert(tree: JsonObject, path: string[], node: JsonObject): void {
     if (isObject(next)) {
       group = next;
     } else {
-      const created: JsonObject = {};
+      const created: JsonObject = Object.create(null) as JsonObject;
       group[key] = created;
       group = created;
     }
@@ -285,7 +285,7 @@ function colorObject(css: string): JsonObject | undefined {
   return out;
 }
 
-const PLAIN_LENGTH = /^(-?(?:\d+\.?\d*|\.\d+))(px|rem)$/;
+const PLAIN_LENGTH = /^(-?(?:\d+(?:\.\d*)?|\.\d+))(px|rem)$/;
 
 /** `0.5rem` → `{ value: 0.5, unit: "rem" }`; a `calc()` of tokens → its px value. */
 function dimensionValue(value: string, lookup: Lookup): Typed | undefined {
@@ -306,8 +306,8 @@ function plainDimension(value: string): JsonObject | undefined {
 type TypographyKind =
   'fontFamily' | 'fontWeight' | 'fontSize' | 'lineHeight' | 'letterSpacing' | 'composite';
 
-const COMPOSITE = /^[\w-]+\s*:\s*[^;]+(?:;\s*[\w-]+\s*:\s*[^;]+)*;?$/;
-const NUMBER = /^-?(?:\d+\.?\d*|\.\d+)$/;
+const COMPOSITE = /^[\w-]+\s*:[^;]+(?:;\s*[\w-]+\s*:[^;]+)*;?$/;
+const NUMBER = /^-?(?:\d+(?:\.\d*)?|\.\d+)$/;
 
 /** What a typography token sets, from its name and Tailwind class (`font-sans`, `text-sm`). */
 function typographyKind(token: Token): TypographyKind | undefined {
@@ -573,7 +573,7 @@ function frontMatterValue(
 }
 
 /** A DESIGN.md dimension: px, em or rem. */
-const DESIGN_DIMENSION = /^-?(?:\d+\.?\d*|\.\d+)(?:px|rem|em)$/;
+const DESIGN_DIMENSION = /^-?(?:\d+(?:\.\d*)?|\.\d+)(?:px|rem|em)$/;
 
 /**
  * A typography token as a DESIGN.md typography level: `{ fontFamily: "Inter" }` for

src/tokens/units.ts

@@ -1,7 +1,7 @@
 /** Root font size used to convert rem/em to px for comparisons. */
 export const ROOT_FONT_SIZE = 16;
 
-const LENGTH = /^(-?(?:\d+\.?\d*|\.\d+))(px|rem|em)?$/i;
+const LENGTH = /^(-?(?:\d+(?:\.\d*)?|\.\d+))(px|rem|em)?$/i;
 
 /** `"0.5rem"` → 8, `"12px"` → 12, `"4"` → 4. Returns undefined for anything else. */
 export function lengthToPx(value: string | number): number | undefined {
@@ -36,13 +36,30 @@ export function evaluateLength(value: string, resolve: Resolve, depth = 0): numb
 function expandVars(value: string, resolve: Resolve, depth: number): string | undefined {
   let out = '';
   let last = 0;
-  for (const match of value.matchAll(/var\(\s*(--[\w-]+)\s*(?:,\s*([^)]+))?\)/g)) {
-    const [whole, name = '', fallback] = match;
+  // Each var() runs to its matching parenthesis, so a fallback may hold calc() or another var(). A
+  // var() that is never closed ends the scan: the value has no px value, and no later `var(` is
+  // scanned to the end again, so an unclosed run like `var(--a,var(--a,(...` stays linear.
+  for (let at = value.indexOf('var(', last); at !== -1; at = value.indexOf('var(', last)) {
+    let level = 0;
+    let comma = -1;
+    let end = -1;
+    for (let i = at + 3; i < value.length; i++) {
+      const c = value[i];
+      if (c === '(') level++;
+      else if (c === ')' && --level === 0) {
+        end = i;
+        break;
+      } else if (c === ',' && level === 1 && comma === -1) comma = i;
+    }
+    if (end === -1) return undefined;
+    const name = value.slice(at + 4, comma === -1 ? end : comma).trim();
+    if (!/^--[\w-]+$/.test(name)) return undefined;
+    const fallback = comma === -1 ? undefined : value.slice(comma + 1, end);
     const resolved = resolve(name) ?? fallback;
     const px = resolved === undefined ? undefined : evaluateLength(resolved, resolve, depth + 1);
     if (px === undefined) return undefined;
-    out += `${value.slice(last, match.index)}${px}px`;
-    last = match.index + whole.length;
+    out += `${value.slice(last, at)}${px}px`;
+    last = end + 1;
   }
   return out + value.slice(last);
 }

test/export.test.ts

@@ -344,6 +344,107 @@ describe('export --format design-md', () => {
   });
 });
 
+const tok = (name: string, category: Token['category'], value: string): Token => ({
+  name,
+  category,
+  value,
+  usage: [],
+  source: { file: 'a.css', line: 1 },
+});
+
+describe('export of long and unusual values', () => {
+  it('types the numbers, lengths, composites and color channels it typed before', () => {
+    const tokens = [
+      tok('font-size-sm', 'typography', '0.875rem'),
+      tok('line-height-tight', 'typography', '1.25'),
+      tok('line-height-snug', 'typography', '1.'),
+      tok('line-height-loose', 'typography', '24px'),
+      tok('tracking-tight', 'typography', '-.025em'),
+      tok('type-body', 'typography', 'fontFamily: Inter; fontSize: 1rem; lineHeight: 1.5;'),
+      tok('type-caption', 'typography', 'fontSize : .75rem;lineHeight:  16px'),
+      tok('space-2', 'spacing', '.5rem'),
+      tok('radius-sm', 'radius', '2.px'),
+      tok('border', 'color', '214.3 31.8% 91.4%'),
+      tok('ring', 'color', '221.2deg, 83.2%, 53.3% / .5'),
+      tok('success', 'color', '22 163 74 / 50%'),
+    ];
+    const json = JSON.parse(toDtcg(tokens).text) as Tree;
+    expect(json.typography?.['font-size-sm']?.$value).toEqual({ value: 0.875, unit: 'rem' });
+    expect(json.typography?.['line-height-tight']).toMatchObject({ $type: 'number', $value: 1.25 });
+    expect(json.typography?.['line-height-snug']).toMatchObject({ $type: 'number', $value: 1 });
+    expect(json.typography?.['line-height-loose']?.$value).toEqual({ value: 24, unit: 'px' });
+    expect(json.typography?.['tracking-tight']?.$value).toBe('-.025em');
+    expect(json.typography?.['type-body']).toMatchObject({
+      $type: 'typography',
+      $value: { fontFamily: 'Inter', fontSize: { value: 1, unit: 'rem' }, lineHeight: 1.5 },
+    });
+    expect(json.typography?.['type-caption']?.$value).toEqual({
+      fontSize: { value: 0.75, unit: 'rem' },
+      lineHeight: { value: 16, unit: 'px' },
+    });
+    expect(json.spacing?.['space-2']?.$value).toEqual({ value: 0.5, unit: 'rem' });
+    expect(json.radius?.sm?.$value).toEqual({ value: 2, unit: 'px' });
+    expect(json.color?.ring?.$value).toEqual({
+      colorSpace: 'hsl',
+      components: [221.2, 83.2, 53.3],
+      alpha: 0.5,
+    });
+    expect(json.color?.success?.$extensions).toMatchObject({
+      'com.github.dgesteves.onsystem': { channels: 'rgb' },
+    });
+
+    const md = toDesignMd(tokens, { name: 'Same' }).text;
+    expect(md).toContain(
+      'colors:\n  border: hsl(214.3 31.8% 91.4%)\n  ring: hsl(221.2deg, 83.2%, 53.3% / .5)\n  success: rgb(22 163 74 / 50%)\n',
+    );
+    expect(md).toContain('  line-height-snug:\n    lineHeight: 1\n');
+    expect(md).toContain('  tracking-tight:\n    letterSpacing: -.025em\n');
+    expect(md).toContain('  type-caption:\n    fontSize: .75rem\n    lineHeight: 16px\n');
+    expect(md).toContain('rounded:\n  sm: 2.px\nspacing:\n  space-2: .5rem\n');
+  });
+
+  it('exports long values in time that grows with their length', () => {
+    const digits = `9${'99'.repeat(50_000)}!`;
+    const nines = '9'.repeat(100_000);
+    const tokens = [
+      tok('font-size-huge', 'typography', digits),
+      tok('line-height-huge', 'typography', digits),
+      tok('space-huge', 'spacing', digits),
+      tok('radius-huge', 'radius', digits),
+      tok('type-n', 'typography', `lineHeight: ${digits}`),
+      tok('type-a', 'typography', `-:${' '.repeat(100_000)};!`),
+      // 126 characters, which took 3.6 s: each `-: :` could split two ways.
+      tok('type-b', 'typography', `-::;-:${' :;-:'.repeat(24)}`),
+      tok('color-hsl', 'color', nines),
+      tok('color-hsl-alpha', 'color', `9 9% 9%/${nines}!`),
+      tok('color-rgb-alpha', 'color', `9 9 9/${nines}!`),
+    ];
+    const started = performance.now();
+    const dtcg = toDtcg(tokens);
+    const md = toDesignMd(tokens, { name: 'Long' });
+    // Seconds per token before; a few milliseconds for all of them now.
+    expect(performance.now() - started).toBeLessThan(200);
+    expect(dtcg.count).toBe(tokens.length);
+    expect(md.text).toContain('# Long');
+  });
+
+  it('keeps a token named __proto__ in its group, and leaves Object.prototype alone', () => {
+    const { tokens } = parseCssTokens(
+      ':root { --__proto__.polluted: #ffffff; } @theme { --color-__proto__: #111111; }',
+      'a.css',
+    );
+    try {
+      const { text, count } = toDtcg(tokens);
+      expect(({} as Record<string, unknown>).polluted).toBeUndefined();
+      expect(count).toBe(2);
+      const group = (JSON.parse(text) as Tree).color?.['__proto__'];
+      expect(Object.keys(group ?? {})).toEqual(['$root', 'polluted']);
+    } finally {
+      delete (Object.prototype as Record<string, unknown>).polluted;
+    }
+  });
+});
+
 describe('onsystem export', () => {
   it('prints the export on stdout', async () => {
     const { code, stdout } = await run(['export', '--format', 'design-md', '--no-cache']);

test/tokens.test.ts

@@ -3,7 +3,14 @@ import path from 'node:path';
 
 import { describe, expect, it } from 'vitest';
 
-import { colorDistance, findColorLiterals, isTinted, parseColor } from '../src/tokens/color.js';
+import {
+  colorDistance,
+  findColorLiterals,
+  isBareHsl,
+  isBareRgb,
+  isTinted,
+  parseColor,
+} from '../src/tokens/color.js';
 import {
   loadTokens,
   mergeTokens,
@@ -473,6 +480,73 @@ describe('units and colors', () => {
     expect(parseColor('214.3 31.8% 91.4%')).toBeUndefined();
   });
 
+  it('reads the lengths and channels it read before', () => {
+    for (const [value, px] of [
+      ['1.', 1],
+      ['.5rem', 8],
+      ['-2px', -2],
+      ['1.5EM', 24],
+      ['10', 10],
+    ] as const) {
+      expect([value, lengthToPx(value)]).toEqual([value, px]);
+    }
+    for (const value of ['.', '1.2.3', '1 px', '-', 'px']) {
+      expect([value, lengthToPx(value)]).toEqual([value, undefined]);
+    }
+    const resolve = (name: string) => (name === '--gap' ? '0.5rem' : undefined);
+    expect(evaluateLength('var(--missing, 4px)', resolve)).toBe(4);
+    expect(evaluateLength('var( --missing ,  0.5rem )', resolve)).toBe(8);
+    expect(evaluateLength('calc(var(--gap) + var(--missing,2px))', resolve)).toBe(10);
+    expect(evaluateLength('var(--missing, )', resolve)).toBeUndefined();
+    // A fallback may hold calc() or another var(), inside calc() too.
+    expect(evaluateLength('var(--missing, calc(1px + 2px))', resolve)).toBe(3);
+    expect(evaluateLength('var(--missing, var(--gap))', resolve)).toBe(8);
+    expect(evaluateLength('calc(2 * var(--missing, var(--gap)))', resolve)).toBe(16);
+    expect(evaluateLength('calc(var(--missing, var(--gap)) + 1px)', resolve)).toBe(9);
+    expect(evaluateLength('var(--missing, var(--gap)', resolve)).toBeUndefined();
+    expect(evaluateLength('var(gap, 4px)', resolve)).toBeUndefined();
+
+    expect(isBareHsl('214.3 31.8% 91.4%')).toBe(true);
+    expect(isBareHsl('-30deg, 50%, .5% / .5')).toBe(true);
+    expect(isBareHsl('0 0% 100% / 50%')).toBe(true);
+    expect(isBareHsl('1. 2% 3%')).toBe(false);
+    expect(isBareHsl('1 2 3')).toBe(false);
+    expect(isBareRgb('22 163 74')).toBe(true);
+    expect(isBareRgb('22,163,74 / 0.5')).toBe(true);
+    expect(isBareRgb('255 255 255 / 50%')).toBe(true);
+    expect(isBareRgb('22 163 74 / 1.')).toBe(false);
+    expect(isBareRgb('256 0 0')).toBe(false);
+    expect(parseColor('22 163 74 / .5', { bareRgb: true })).toMatchObject({ alpha: 0.5 });
+  });
+
+  it('reads a long length, var() or color channels in time that grows with its length', () => {
+    const fast = <T>(read: () => T): T => {
+      const started = performance.now();
+      const value = read();
+      expect(performance.now() - started).toBeLessThan(100);
+      return value;
+    };
+    const digits = '9'.repeat(100_000);
+    const spaces = ' '.repeat(100_000);
+    // Each took 14 to 25 s before: the regex tried every way to split the run of digits or
+    // spaces between two of its parts.
+    expect(fast(() => lengthToPx(`9${digits}x`))).toBeUndefined();
+    expect(fast(() => evaluateLength(`var(---,${spaces}x`, () => undefined))).toBeUndefined();
+    // 7.4 s at 20,000 repeats with a fallback regex: each `var(` scanned to the end for its `)`.
+    expect(
+      fast(() => evaluateLength(`var(---,${'var(---,('.repeat(20_000)}`, () => undefined)),
+    ).toBeUndefined();
+    // Nested fallbacks are read again at each level, at most 16 deep, so this stays linear.
+    const nested = `${'var(--missing,'.repeat(2_000)}1px${')'.repeat(2_000)}`;
+    expect(fast(() => evaluateLength(nested, () => undefined))).toBeUndefined();
+    expect(fast(() => isBareHsl(digits))).toBe(false);
+    expect(fast(() => isBareHsl(`9 9% 9%/${digits}!`))).toBe(false);
+    expect(fast(() => isBareRgb(`9 9 9/${digits}!`))).toBe(false);
+    const bare = { bareHsl: true, bareRgb: true };
+    expect(fast(() => parseColor(digits, bare))).toBeUndefined();
+    expect(fast(() => parseColor(`9 9 9/${digits}!`, bare))).toBeUndefined();
+  });
+
   it('finds color literals inside CSS values', () => {
     expect(
       findColorLiterals('1px solid #ccc, 0 0 0 2px rgb(0 0 0 / 0.5)').map((m) => m.text),