Defense-in-depth: webview/path/parser hardening and Skill Finder model switch

.vscodeignore

@@ -26,6 +26,18 @@ dist/**/*.map
 README.github.md
 SECURITY.md
 
+# Dev-only config & tooling — not needed at runtime
+.husky/**
+.devcontainer/**
+cspell.json
+knip.json
+playwright.config.ts
+.lockfile-lintrc.json
+.npmrc
+
+# Marketing assets are for the repo / store listing, not the shipped bundle
+marketing/**
+
 # Benchmark / perf outputs
 benchmark-*.json
 

src/core/dsl/safe-regex.test.ts

@@ -358,4 +358,34 @@ describe('isLikelySafe', () => {
   it('returns false for prefix-overlapping alternation under a quantifier', () => {
     expect(isLikelySafe('(a|aa)+')).toBe(false);
   });
+
+  it('returns false for a character-class branch overlapping a literal under a quantifier', () => {
+    expect(isLikelySafe('^([a]|a)+$')).toBe(false);
+  });
+
+  it('returns false for a dot branch overlapping a literal under a quantifier', () => {
+    expect(isLikelySafe('^(.|a)+$')).toBe(false);
+  });
+
+  it('returns false for a class-escape branch overlapping a literal under a quantifier', () => {
+    expect(isLikelySafe('^(\\w|a)+$')).toBe(false);
+  });
+
+  it('returns true for a disjoint character-class alternation under a quantifier', () => {
+    expect(isLikelySafe('^([a-z]|_)+$')).toBe(true);
+  });
+
+  it('rejects a class-overlapping alternation pattern through compileSafe', () => {
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+
+    // Pattern assembled at runtime so static analysis does not treat this
+    // rejected fixture as a live regex; compileSafe returns null before any
+    // RegExp is constructed.
+    const ch = String.fromCharCode(97);
+    const overlapping = `^([${ch}]|${ch})+$`;
+    expect(compileSafe(overlapping)).toBeNull();
+
+    expect(warn).toHaveBeenCalledTimes(1);
+    warn.mockRestore();
+  });
 });

src/core/dsl/safe-regex.ts

@@ -159,44 +159,81 @@ export function isLikelySafe(pattern: string): boolean {
   return maxStarHeight <= 2;
 }
 
-function hasOverlappingAlternation(body: string): boolean {
+const CLASS_ESCAPES = new Set(['w', 'W', 'd', 'D', 's', 'S']);
+
+function splitTopLevelBranches(body: string): string[] {
   const branches: string[] = [];
   let depth = 0;
   let start = 0;
   for (let i = 0; i < body.length; i++) {
     const ch = body[i];
     if (ch === '\\') { i++; continue; }
-    if (ch === '[') {
-      while (i < body.length && body[i] !== ']') {
-        if (body[i] === '\\') i++;
-        i++;
-      }
-      continue;
-    }
+    if (ch === '[') { i = skipCharacterClass(body, i); continue; }
     if (ch === '(') depth++;
     else if (ch === ')') depth--;
     else if (ch === '|' && depth === 0) {
       branches.push(body.slice(start, i));
       start = i + 1;
     }
   }
-  if (branches.length === 0) return false;
   branches.push(body.slice(start));
+  return branches;
+}
+
+// Alternation branches that can match the same first character cause
+// catastrophic backtracking under an unbounded quantifier (e.g. `(.|a)+`).
+function hasOverlappingAlternation(body: string): boolean {
+  const tokens = splitTopLevelBranches(body).map(firstToken);
+  if (tokens.length < 2) return false;
 
-  // If any two branches share a non-empty literal prefix, flag it.
-  for (let i = 0; i < branches.length; i++) {
-    for (let j = i + 1; j < branches.length; j++) {
-      const a = literalPrefix(branches[i]);
-      const b = literalPrefix(branches[j]);
-      if (a && b && (a === b || a.startsWith(b) || b.startsWith(a))) {
-        return true;
-      }
+  for (let i = 0; i < tokens.length; i++) {
+    for (let j = i + 1; j < tokens.length; j++) {
+      if (tokensOverlap(tokens[i], tokens[j])) return true;
     }
   }
   return false;
 }
 
-/** Return the leading literal-character run of a branch (no metachars). */
+// A branch's leading token: either a literal run or a single-char matcher
+// (`.`, a class escape, or a `[...]` class). `null` means indeterminate.
+type FirstToken =
+  | { set: false; literal: string }
+  | { set: true; source: string }
+  | null;
+
+function firstToken(branch: string): FirstToken {
+  const body = branch.startsWith('^') ? branch.slice(1) : branch;
+  if (body.length === 0) return null;
+
+  if (body[0] === '.') return { set: true, source: '.' };
+  if (body[0] === '[') return { set: true, source: body.slice(0, skipCharacterClass(body, 0) + 1) };
+  if (body[0] === '\\' && body.length > 1 && CLASS_ESCAPES.has(body[1])) {
+    return { set: true, source: body.slice(0, 2) };
+  }
+
+  const prefix = literalPrefix(body);
+  return prefix ? { set: false, literal: prefix } : null;
+}
+
+function tokensOverlap(a: FirstToken, b: FirstToken): boolean {
+  if (!a || !b) return false;
+  if (!a.set && !b.set) return a.literal.startsWith(b.literal) || b.literal.startsWith(a.literal);
+  if (a.set && b.set) return true;
+  const set = a.set ? a : (b as Extract<FirstToken, { set: true }>);
+  const literal = a.set ? (b as Extract<FirstToken, { set: false }>) : a;
+  return setMatchesChar(set.source, literal.literal[0]);
+}
+
+// Whether a single-char matcher source (e.g. `[a-z]`, `\w`, `.`) accepts `ch`.
+// An unparseable source is treated as a match (fail closed toward rejection).
+function setMatchesChar(source: string, ch: string): boolean {
+  try {
+    return new RegExp(`^(?:${source})$`).test(ch);
+  } catch {
+    return true;
+  }
+}
+
 function literalPrefix(branch: string): string {
   let out = '';
   for (let i = 0; i < branch.length; i++) {

src/core/parser-vscode-files.test.ts

@@ -199,6 +199,28 @@ describe('reconstructFromJsonl', () => {
       expect(mode.id).toBe('agent');
     });
   });
+
+  it('does not pollute Object.prototype via a __proto__ set path', () => {
+    const lines = [
+      JSON.stringify({ kind: 0, v: { ok: true } }),
+      JSON.stringify({ kind: 1, k: ['__proto__', 'polluted'], v: 'pwned' }),
+    ].join('\n');
+    withTempFile('proto-set.jsonl', lines, (filePath) => {
+      reconstructFromJsonl(filePath);
+      expect(({} as Record<string, unknown>).polluted).toBeUndefined();
+    });
+  });
+
+  it('does not pollute Object.prototype via a constructor.prototype append path', () => {
+    const lines = [
+      JSON.stringify({ kind: 0, v: { ok: true } }),
+      JSON.stringify({ kind: 2, k: ['constructor', 'prototype', 'tainted'], v: ['x'] }),
+    ].join('\n');
+    withTempFile('proto-append.jsonl', lines, (filePath) => {
+      reconstructFromJsonl(filePath);
+      expect(({} as Record<string, unknown>).tainted).toBeUndefined();
+    });
+  });
 });
 
 describe('parseWorkspaceName', () => {

src/core/parser-vscode-files.ts

@@ -107,6 +107,12 @@ type JsonValue = string | number | boolean | null | JsonValue[] | { [key: string
 type JsonObject = Record<string, JsonValue>;
 type PathKey = string | number;
 
+const FORBIDDEN_KEYS = new Set(['__proto__', 'constructor', 'prototype']);
+
+function isForbiddenKey(key: PathKey): boolean {
+  return typeof key === 'string' && FORBIDDEN_KEYS.has(key);
+}
+
 function isJsonObject(value: unknown): value is Record<string, unknown> {
   return typeof value === 'object' && value !== null && !Array.isArray(value);
 }
@@ -135,16 +141,18 @@ function setAtPath(obj: JsonValue, keys: PathKey[], value: JsonValue): void {
   let current = obj;
   for (let i = 0; i < keys.length - 1; i++) {
     const key = keys[i];
+    if (isForbiddenKey(key)) return;
     if (typeof key === 'number' && Array.isArray(current)) {
       while (current.length <= key) current.push(null);
       if (current[key] === null) current[key] = {};
       current = current[key]!;
     } else if (typeof current === 'object' && current !== null && !Array.isArray(current)) {
-      if (!(key as string in current)) (current as JsonObject)[key as string] = {};
+      if (!Object.prototype.hasOwnProperty.call(current, key as string)) (current as JsonObject)[key as string] = {};
       current = (current as JsonObject)[key as string];
     }
   }
   const last = keys[keys.length - 1];
+  if (isForbiddenKey(last)) return;
   if (Array.isArray(current)) {
     while (current.length <= (last as number)) current.push(null);
     current[last as number] = value;
@@ -156,10 +164,11 @@ function setAtPath(obj: JsonValue, keys: PathKey[], value: JsonValue): void {
 function appendAtPath(obj: JsonValue, keys: PathKey[], items: JsonValue): void {
   let target: JsonValue = obj;
   for (const key of keys) {
+    if (isForbiddenKey(key)) return;
     if (typeof key === 'number' && Array.isArray(target)) {
       target = target[key];
     } else if (typeof target === 'object' && target !== null && !Array.isArray(target)) {
-      if (!(key as string in target)) (target as JsonObject)[key as string] = [];
+      if (!Object.prototype.hasOwnProperty.call(target, key as string)) (target as JsonObject)[key as string] = [];
       target = (target as JsonObject)[key as string];
     }
   }

src/webview/fetch-utils.test.ts

@@ -0,0 +1,24 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See LICENSE in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { describe, expect, it } from 'vitest';
+import { readTextWithByteLimit } from './fetch-utils';
+
+describe('readTextWithByteLimit', () => {
+  it('reads content under the byte limit', async () => {
+    const response = new Response('hello', { headers: { 'content-length': '5' } });
+    await expect(readTextWithByteLimit(response, 5, 'too large')).resolves.toBe('hello');
+  });
+
+  it('rejects when content-length exceeds the limit', async () => {
+    const response = new Response('hello', { headers: { 'content-length': '6' } });
+    await expect(readTextWithByteLimit(response, 5, 'too large')).rejects.toThrow('too large');
+  });
+
+  it('rejects streamed content after the byte limit is exceeded', async () => {
+    const response = new Response('hello');
+    await expect(readTextWithByteLimit(response, 4, 'too large')).rejects.toThrow('too large');
+  });
+});

src/webview/fetch-utils.ts

@@ -0,0 +1,37 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See LICENSE in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+export async function readTextWithByteLimit(response: Response, maxBytes: number, tooLargeMessage: string): Promise<string> {
+  const declared = Number(response.headers.get('content-length'));
+  if (Number.isFinite(declared) && declared > maxBytes) {
+    throw new Error(tooLargeMessage);
+  }
+
+  if (!response.body) {
+    const text = await response.text();
+    if (new TextEncoder().encode(text).byteLength > maxBytes) {
+      throw new Error(tooLargeMessage);
+    }
+    return text;
+  }
+
+  const reader = response.body.getReader();
+  const decoder = new TextDecoder();
+  let bytes = 0;
+  let text = '';
+
+  while (true) {
+    const { done, value } = await reader.read();
+    if (done) break;
+    bytes += value.byteLength;
+    if (bytes > maxBytes) {
+      await reader.cancel();
+      throw new Error(tooLargeMessage);
+    }
+    text += decoder.decode(value, { stream: true });
+  }
+
+  return text + decoder.decode();
+}

src/webview/panel-catalog.ts

@@ -3,8 +3,12 @@
  *  Licensed under the MIT License. See LICENSE in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { readTextWithByteLimit } from './fetch-utils';
+
 export const CATALOG_BASE = 'https://awesome-copilot.github.com';
 
+const CATALOG_PAGE_MAX_BYTES = 5 * 1024 * 1024;
+
 export interface RawCatalogItem {
   kind: 'skill' | 'agent' | 'instruction' | 'hook';
   id: string;
@@ -31,9 +35,9 @@ function stripHtml(text: string): string {
 
 async function fetchCatalogPage(slug: string, kind: RawCatalogItem['kind']): Promise<RawCatalogItem[]> {
   const url = `${CATALOG_BASE}/${slug}/`;
-  const response = await fetch(url);
+  const response = await fetch(url, { redirect: 'error' });
   if (!response.ok) return [];
-  const html = await response.text();
+  const html = await readTextWithByteLimit(response, CATALOG_PAGE_MAX_BYTES, 'Catalog page too large');
 
   const items: RawCatalogItem[] = [];
   const articleRegex = /<article\s+class="resource-item"[^>]*data-path="([^"]*)"[^>]*>([\s\S]*?)<\/article>/g;