fix(db): escape regex metacharacters in group model patterns (#11311)

Validated on a 17-PR combined board: group-model-pattern-regex-escape within the board's 287/287, typecheck:core clean. matchesModelPattern() only substituted * before compiling to RegExp — every other metacharacter kept its regex meaning, so a malformed group pattern (unbalanced parens/brackets) threw uncaught and broke EVERY request for keys in that group, not just the malformed rule (isModelAllowedForKey has no try/catch and runs on the chat completion path and the /v1/models catalog). Thank you @ntdat812!

changelog.d/fixes/11311-group-model-pattern-regex-escape.md

@@ -0,0 +1 @@
+- **fix(db):** group model patterns escape regex metacharacters, so `gpt-4.1*` no longer matches `gpt-4o1-preview` and a pattern like `gpt-4(*` no longer throws `SyntaxError` out of the completion and `/v1/models` paths ([#11311](https://github.com/diegosouzapw/OmniRoute/pull/11311))

src/lib/db/apiKeyGroups.ts

@@ -304,11 +304,35 @@ export function checkKeyModelAccess(
   return { allowed: false, matchedRules: permissions, deniedBy: null };
 }
 
+/**
+ * Compile a group model pattern.
+ *
+ * `*` is the only wildcard this syntax has, so every other regex
+ * metacharacter must be escaped before the pattern is compiled. Interpolating
+ * it raw made an operator's pattern behave as a regex in two ways:
+ *
+ *   - `gpt-4.1*` matched `gpt-4o1-preview`, because `.` is "any character".
+ *     On a deny rule that blocks unrelated models; on an allow rule it grants
+ *     models the pattern was never meant to cover.
+ *   - `gpt-4(*`, `claude-3[*` and `*+*` threw `SyntaxError` (unterminated
+ *     group / unterminated character class / nothing to repeat) out of
+ *     `checkKeyModelAccess()`, which runs on the completion and /v1/models
+ *     paths — one malformed pattern broke every request for keys in that
+ *     group.
+ *
+ * Escaping keeps the semantics this function already had (case-sensitive,
+ * `*`-only) and matches how the rest of the repo compiles operator patterns
+ * (`globToRegex`, `matchesWildcardPattern`).
+ */
+function modelPatternToRegex(pattern: string): RegExp {
+  const escaped = pattern.replace(/[.+^${}()|[\]\\?]/g, "\\$&").replace(/\*/g, ".*");
+  return new RegExp(`^${escaped}$`);
+}
+
 function matchesModelPattern(pattern: string, model: string): boolean {
   if (pattern === "*") return true;
   if (pattern.includes("*")) {
-    const regex = new RegExp("^" + pattern.replace(/\*/g, ".*") + "$");
-    return regex.test(model);
+    return modelPatternToRegex(pattern).test(model);
   }
   return pattern === model;
 }

tests/unit/group-model-pattern-regex-escape.test.ts

@@ -0,0 +1,156 @@
+// A group model pattern is operator text, but `matchesModelPattern()` compiled
+// it into a RegExp with only `*` substituted, so every other metacharacter kept
+// its regex meaning. Measured on the pre-fix build, through the real
+// `checkKeyModelAccess()` (deny rule, key in the group):
+//
+//   "gpt-4.1*"   vs "gpt-4o1-preview" -> DENIED   ('.' matched 'o')
+//   "gpt-4(*"    vs "gpt-4o"          -> THROW SyntaxError: Unterminated group
+//   "claude-3[*" vs "claude-3-opus"   -> THROW SyntaxError: Unterminated character class
+//   "*+*"        vs "anything"        -> THROW SyntaxError: Nothing to repeat
+//
+// The throw is not contained: `isModelAllowedForKey()` calls this helper with no
+// try/catch, and that runs on the completion path (`src/sse/handlers/chat.ts`)
+// and on the /v1/models catalog, so one malformed pattern breaks every request
+// for keys in that group.
+import test from "node:test";
+import assert from "node:assert/strict";
+
+process.env.API_KEY_SECRET = "test-secret-key-for-unit-tests-123456789";
+
+import * as apiKeys from "../../src/lib/db/apiKeys";
+import * as apiKeyGroups from "../../src/lib/db/apiKeyGroups";
+
+let counter = 0;
+
+/**
+ * A fresh key in a fresh group carrying the rule under test.
+ *
+ * A deny rule is paired with `allow *`, because group membership alone is
+ * deny-by-default: with no matching allow rule `checkKeyModelAccess()` returns
+ * false for everything, which would hide whether the deny pattern matched.
+ */
+async function keyWithRule(pattern: string, accessType: "allow" | "deny"): Promise<string> {
+  const label = `pattern-escape-${counter++}`;
+  const key = await apiKeys.createApiKey(label, `machine-${label}`);
+  assert.ok(key, "test key must be created");
+  const group = apiKeyGroups.createKeyGroup(label);
+  apiKeyGroups.addKeyToGroup(key.id, group.id);
+  apiKeyGroups.addGroupPermission(group.id, pattern, accessType);
+  if (accessType === "deny") {
+    apiKeyGroups.addGroupPermission(group.id, "*", "allow");
+  }
+  return key.id;
+}
+
+test("a deny pattern's '.' is a literal, not any-character", async () => {
+  const keyId = await keyWithRule("gpt-4.1*", "deny");
+
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "gpt-4.1-mini").allowed,
+    false,
+    "the model the operator meant to deny must still be denied"
+  );
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "gpt-4o1-preview").allowed,
+    true,
+    "'.' must not match 'o' — an unrelated model was being denied"
+  );
+});
+
+test("an allow pattern's '.' does not widen the grant", async () => {
+  // Same defect in the direction that matters more: an allow rule that matches
+  // more models than it names hands out access the operator never granted.
+  const keyId = await keyWithRule("claude-3.5*", "allow");
+
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "claude-3.5-sonnet").allowed,
+    true,
+    "the model the operator meant to allow must still be allowed"
+  );
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "claude-3x5-internal").allowed,
+    false,
+    "'.' must not match 'x' — an unnamed model was being granted"
+  );
+});
+
+test("patterns that are not valid regexes no longer throw", async () => {
+  // Each of these threw SyntaxError out of the request path before the fix.
+  for (const pattern of ["gpt-4(*", "claude-3[*", "*+*", "a{2*", "gpt-4\\*"]) {
+    const keyId = await keyWithRule(pattern, "deny");
+    assert.doesNotThrow(
+      () => apiKeyGroups.checkKeyModelAccess(keyId, "gpt-4o"),
+      `pattern ${JSON.stringify(pattern)} must not throw`
+    );
+  }
+});
+
+test("the throw also escaped through isModelAllowedForKey", async () => {
+  // The end-to-end path: this is the helper the completion handler and the
+  // /v1/models catalog call, and it has no try/catch around the group check.
+  const label = `pattern-escape-e2e-${counter++}`;
+  const key = await apiKeys.createApiKey(label, `machine-${label}`);
+  assert.ok(key);
+  const group = apiKeyGroups.createKeyGroup(label);
+  apiKeyGroups.addKeyToGroup(key.id, group.id);
+  apiKeyGroups.addGroupPermission(group.id, "gpt-4(*", "deny");
+  apiKeyGroups.addGroupPermission(group.id, "*", "allow");
+
+  const allowed = await apiKeys.isModelAllowedForKey(key.key, "openai/gpt-4o");
+  assert.equal(
+    allowed,
+    true,
+    "a malformed pattern must not deny — and must not throw — on the request path"
+  );
+});
+
+test("literal metacharacters in a pattern match themselves", async () => {
+  // Model ids do carry dots and plus signs, so the escape has to make the
+  // literal reading work, not merely stop the throw.
+  const keyId = await keyWithRule("qwen2.5+vl*", "deny");
+
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "qwen2.5+vl-7b").allowed,
+    false,
+    "the literal pattern must match the literal model id"
+  );
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "qwen2X5vvl-7b").allowed,
+    true,
+    "and must not match the regex reading of itself"
+  );
+});
+
+test("plain wildcard semantics are unchanged", async () => {
+  const keyId = await keyWithRule("gpt-4*", "deny");
+
+  for (const model of ["gpt-4", "gpt-4o", "gpt-4-turbo"]) {
+    assert.equal(
+      apiKeyGroups.checkKeyModelAccess(keyId, model).allowed,
+      false,
+      `${model} must still be denied by gpt-4*`
+    );
+  }
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(keyId, "gpt-3.5-turbo").allowed,
+    true,
+    "an unrelated model must still be allowed"
+  );
+});
+
+test("'*' and exact matches keep their fast paths", async () => {
+  const denyAll = await keyWithRule("*", "deny");
+  assert.equal(apiKeyGroups.checkKeyModelAccess(denyAll, "anything/at-all").allowed, false);
+
+  const exact = await keyWithRule("gpt-4.1-mini", "deny");
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(exact, "gpt-4.1-mini").allowed,
+    false,
+    "an exact pattern still matches exactly"
+  );
+  assert.equal(
+    apiKeyGroups.checkKeyModelAccess(exact, "gpt-4X1-mini").allowed,
+    true,
+    "an exact pattern was never a regex and must stay literal"
+  );
+});