Skip to content

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

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.50from
ntdat812:fix/group-model-pattern-regex-escape
Aug 24, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.50from
ntdat812:fix/group-model-pattern-regex-escape

Conversation

@ntdat812

@ntdat812 ntdat812 commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

The bug

matchesModelPattern() in src/lib/db/apiKeyGroups.ts compiled an operator's group pattern into a RegExp with only * substituted:

const regex = new RegExp("^" + pattern.replace(/\*/g, ".*") + "$");

Every other metacharacter kept its regex meaning. Measured on release/v3.8.50 (3192eb88d) through the real checkKeyModelAccess(), with a deny rule and the key in the group:

pattern model result
gpt-4.1* gpt-4o1-preview DENIED — . matched o
gpt-4(* gpt-4o THROW SyntaxError: Invalid regular expression: /^gpt-4(.*$/: Unterminated group
claude-3[* claude-3-opus THROW Unterminated character class
*+* anything THROW Nothing to repeat

modelPattern reaches the DB as free text — POST /api/keys/groups/[id]/permissions validates it as z.string().trim().min(1) and nothing narrows it afterwards.

Why the throw matters

It is not contained. isModelAllowedForKey() (src/lib/db/apiKeys.ts:1563) calls the group check with no try/catch, and that helper runs on the completion path (src/sse/handlers/chat.ts:942) and on the /v1/models catalog (src/app/api/v1/models/catalogResponse.ts:209). Confirmed end to end:

deny  "gpt-4(*" -> THROW Invalid regular expression: /^gpt-4(.*$/: Unterminated group
allow "gpt-4(*" -> THROW Invalid regular expression: /^gpt-4(.*$/: Unterminated group

So one malformed pattern breaks every request for keys in that group, not just the rule that carries it.

The over-match is quieter but worse in one direction: on a deny rule it blocks unrelated models, on an allow rule it grants models the operator never named.

The fix

Escape the metacharacters before substituting *. This keeps the semantics the function already had — case-sensitive, *-only, no ? wildcard — so no existing pattern changes meaning except the ones that were being read as regexes. It also brings this call site in line with how the rest of the repo compiles operator patterns (globToRegex in src/shared/utils/globPattern.ts, matchesWildcardPattern in src/lib/db/apiKeys/modelPermissions.ts); this was the last unescaped one.

I deliberately did not switch to globToRegex: it is case-insensitive and treats ? as a wildcard, which would silently widen existing allow rules.

Tests

tests/unit/group-model-pattern-regex-escape.test.ts — 7 tests through the real checkKeyModelAccess() and isModelAllowedForKey(), no mocks.

They are load-bearing, not decorative. Removing the escape (keeping the rest of the fix) fails 5 of the 7:

not ok 1 - a deny pattern's '.' is a literal, not any-character
not ok 2 - an allow pattern's '.' does not widen the grant
not ok 3 - patterns that are not valid regexes no longer throw
not ok 4 - the throw also escaped through isModelAllowedForKey
not ok 5 - literal metacharacters in a pattern match themselves
ok 6 - plain wildcard semantics are unchanged
ok 7 - '*' and exact matches keep their fast paths
# pass 2  # fail 5

The two that still pass are the ones pinning unchanged behaviour, which is what they are for.

Verification

  • New file: 7 passed, 0 failed.
  • Neighbours (db-api-key-groups, group-provider-permission, model-catalog-policy-invalidation-8728, api-key-policy, api-key-scope-validation, api-key-lifecycle): 83 tests, 82 pass. The one failure is api-key-lifecycle — EBUSY: resource busy or locked, unlink '…\storage.sqlite' during teardown on Windows; all 11 of its assertions pass. It fails identically with src/lib/db/apiKeyGroups.ts restored from HEAD, so it is pre-existing and environment-specific, not this change.
  • npm run typecheck:core: clean. eslint on both files: clean.
  • prettier --check reports the same pre-existing formatting drift on apiKeyGroups.ts before and after this change (the file uses trailing commas the config would strip), so I left the rest of the file alone rather than shipping a whole-file reformat.

Found by audit while looking for unescaped dynamic RegExp construction, not from a reported incident — no user report is attached to it.


⚠️ base-red inherited: #9985 — No new ESLint warnings fails on this PR with

There are suppressions left that do not occur anymore.
Consider re-running the command with `--prune-suppressions`.

That is a stale entry in config/quality/eslint-suppressions.json, not a warning introduced here: config/quality/eslint-suppressions.json has no entry for src/lib/db/apiKeyGroups.ts, eslint on both changed files is clean locally, and the same job is red on the other open PRs against this base (#11307, #11308, #11309 — including the maintainer's own). Left alone rather than pruned from a contributor branch.

@ntdat812
ntdat812 requested a review from diegosouzapw as a code owner August 24, 2026 01:39
`matchesModelPattern()` compiled an operator's group pattern into a RegExp
with only `*` substituted, so every other metacharacter kept its regex
meaning:

  "gpt-4.1*"   vs "gpt-4o1-preview" -> denied ('.' matched 'o')
  "gpt-4(*"    vs "gpt-4o"          -> SyntaxError: Unterminated group
  "claude-3[*" vs "claude-3-opus"   -> SyntaxError: Unterminated character class
  "*+*"        vs "anything"        -> SyntaxError: Nothing to repeat

The throw is not contained: `isModelAllowedForKey()` calls the group check
with no try/catch, and it runs on the completion path and on the /v1/models
catalog, so one malformed pattern breaks every request for keys in that
group. The over-match is quieter but worse on an allow rule, which then
grants models the pattern never named.

Escape the metacharacters before substituting `*`, keeping the semantics
this function already had (case-sensitive, `*`-only) and matching how the
rest of the repo compiles operator patterns (`globToRegex`,
`matchesWildcardPattern`).
@diegosouzapw
diegosouzapw merged commit 6945bba into diegosouzapw:release/v3.8.50 Aug 24, 2026
14 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…uzapw#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!
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