[PR #3251] Fix unescaped regex metacharacters in PATH rule matching causing request failures #33555

New Issue

GiteaMirror · 2026-06-15T10:57:49-05:00

GiteaMirror commented

2026-06-15 10:57:49 -05:00

📋 Pull Request Information

Original PR: https://github.com/fosrl/pangolin/pull/3251
Author: @kshitijshresth
Created: 6/12/2026
Status: 🔄 Open

Base: main ← Head: fix-path-rule-regex-escaping

📝 Commits (1)

b136bd2 Escape regex metacharacters in PATH rule wildcard matching

📊 Changes

3 files changed (+193 additions, -207 deletions)

View changed files

➕ server/lib/pathMatch.ts (+74 -0)
📝 server/routers/badger/verifySession.test.ts (+117 -70)
📝 server/routers/badger/verifySession.ts (+2 -137)

📄 Description

By creating this pull request, I grant the project maintainers an unlimited,
perpetual license to use, modify, and redistribute these contributions under any terms they
choose, including both the AGPLv3 and the Fossorial Commercial license terms. I
represent that I have the right to grant this license for all contributed content.

Summary

isPathAllowed (used by checkRules in verifySession.ts to evaluate PATH resource rules) converts wildcard pattern segments to regular expressions without escaping regex metacharacters. isValidUrlGlobPattern in validators.ts explicitly permits - . _ ~ ! $ & ' ( ) * + , ; # = @ : in rule values, so rules that pass API validation can produce invalid or semantically wrong regexes at request time.

Impact

Unhandled exception on every request. A PATH rule value like /(api* is accepted by validation, but the matcher builds new RegExp("^(api.*$"), which throws SyntaxError: Invalid regular expression: missing ). checkRules has no exception handling, so the error propagates to the catch-all in verifyResourceSession and every request to that resource fails with 500 Failed to verify session until the rule is deleted.

Incorrect matching for literal characters. Since PATH rules commonly gate auth bypass (ACCEPT) or denial (DROP), both directions matter:

*.png matched imageXpng (. acted as regex any-char)
a+b* matched aaabc and failed to match a+bc (+ acted as a quantifier)
$ref* could never match anything ($ acted as an anchor)

Changes

Extracted isPathAllowed from verifySession.ts into pathMatch.ts as a pure module (no logger/config/db imports) so it can be unit tested in isolation; verifySession.ts imports and re-exports it, keeping its public surface unchanged.
Regex metacharacters are now escaped before the * → .* and ? → . substitutions. The escape set deliberately excludes * and ? so wildcard semantics are unchanged.
Compiled segment regexes are cached in a module-level Map. Keys are admin-defined rule pattern segments (a bounded set), not request paths, so the cache cannot grow with traffic. This avoids recompiling regexes on every proxied request, which sits on the hot path.
As part of the extraction, the per-recursion-step logger.debug calls (~10 string interpolations per segment comparison, evaluated eagerly on every request regardless of log level) were removed.

Tests

verifySession.test.ts previously tested a stale inline duplicate of isPathAllowed rather than the real implementation, so regressions in the production code could never fail the suite. It now imports the real function from pathMatch.ts.

All pre-existing assertions are unchanged and pass, demonstrating no behavioral change for patterns without metacharacters.
Added assertions covering ( ) [ ] { } | \ . + ^ $ | as literals, a no-throw guarantee for all characters accepted by isValidUrlGlobPattern, preserved ? single-character wildcard behavior, and a deep-path sanity check.

Verified against the unfixed logic first: the new tests fail with Invalid regular expression: /^(api.*$/: Unterminated group, and pass after the fix.

npx tsx server/routers/badger/verifySession.test.ts   # all tests pass
npx tsc --noEmit                # clean
npx prettier --check <changed files>   # clean

Reproduction (before this fix)

Create a resource with rules enabled.
Add a PATH rule with value /(api* (accepted by validation).
Every request to the resource now returns 500 (Invalid regular expression: /^(api.*$/: Unterminated group in logs).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/fosrl/pangolin/pull/3251 **Author:** [@kshitijshresth](https://github.com/kshitijshresth) **Created:** 6/12/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix-path-rule-regex-escaping` --- ### 📝 Commits (1) - [`b136bd2`](https://github.com/fosrl/pangolin/commit/b136bd2246349da061067c3731ba8ff593f8c5c2) Escape regex metacharacters in PATH rule wildcard matching ### 📊 Changes **3 files changed** (+193 additions, -207 deletions) <details> <summary>View changed files</summary> ➕ `server/lib/pathMatch.ts` (+74 -0) 📝 `server/routers/badger/verifySession.test.ts` (+117 -70) 📝 `server/routers/badger/verifySession.ts` (+2 -137) </details> ### 📄 Description By creating this pull request, I grant the project maintainers an unlimited, perpetual license to use, modify, and redistribute these contributions under any terms they choose, including both the AGPLv3 and the Fossorial Commercial license terms. I represent that I have the right to grant this license for all contributed content. ## Summary `isPathAllowed` (used by `checkRules` in `verifySession.ts` to evaluate PATH resource rules) converts wildcard pattern segments to regular expressions without escaping regex metacharacters. `isValidUrlGlobPattern` in `validators.ts` explicitly permits `- . _ ~ ! $ & ' ( ) * + , ; # = @ :` in rule values, so rules that pass API validation can produce invalid or semantically wrong regexes at request time. ## Impact **Unhandled exception on every request.** A PATH rule value like `/(api*` is accepted by validation, but the matcher builds `new RegExp("^(api.*$")`, which throws `SyntaxError: Invalid regular expression: missing )`. `checkRules` has no exception handling, so the error propagates to the catch-all in `verifyResourceSession` and every request to that resource fails with `500 Failed to verify session` until the rule is deleted. **Incorrect matching for literal characters.** Since PATH rules commonly gate auth bypass (ACCEPT) or denial (DROP), both directions matter: * `*.png` matched `imageXpng` (`.` acted as regex any-char) * `a+b*` matched `aaabc` and failed to match `a+bc` (`+` acted as a quantifier) * `$ref*` could never match anything (`$` acted as an anchor) ## Changes * Extracted `isPathAllowed` from `verifySession.ts` into `pathMatch.ts` as a pure module (no logger/config/db imports) so it can be unit tested in isolation; `verifySession.ts` imports and re-exports it, keeping its public surface unchanged. * Regex metacharacters are now escaped before the `*` → `.*` and `?` → `.` substitutions. The escape set deliberately excludes `*` and `?` so wildcard semantics are unchanged. * Compiled segment regexes are cached in a module-level `Map`. Keys are admin-defined rule pattern segments (a bounded set), not request paths, so the cache cannot grow with traffic. This avoids recompiling regexes on every proxied request, which sits on the hot path. * As part of the extraction, the per-recursion-step `logger.debug` calls (~10 string interpolations per segment comparison, evaluated eagerly on every request regardless of log level) were removed. ## Tests `verifySession.test.ts` previously tested a stale inline duplicate of `isPathAllowed` rather than the real implementation, so regressions in the production code could never fail the suite. It now imports the real function from `pathMatch.ts`. * All pre-existing assertions are unchanged and pass, demonstrating no behavioral change for patterns without metacharacters. * Added assertions covering `( ) [ ] { } | \ . + ^ $ |` as literals, a no-throw guarantee for all characters accepted by `isValidUrlGlobPattern`, preserved `?` single-character wildcard behavior, and a deep-path sanity check. Verified against the unfixed logic first: the new tests fail with `Invalid regular expression: /^(api.*$/: Unterminated group`, and pass after the fix. ```bash npx tsx server/routers/badger/verifySession.test.ts # all tests pass npx tsc --noEmit # clean npx prettier --check <changed files> # clean ``` ## Reproduction (before this fix) 1. Create a resource with rules enabled. 2. Add a PATH rule with value `/(api*` (accepted by validation). 3. Every request to the resource now returns 500 (`Invalid regular expression: /^(api.*$/: Unterminated group` in logs). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-06-15 10:57:49 -05:00

Sign in to join this conversation.

Branches Tags

main

crowdin_dev

dev

dependabot/npm_and_yarn/npm-dependencies-338850a190

dependabot/go_modules/install/go-install-dependencies-3804ca7238

dependabot/docker/docker-dependencies-4faa477378

dependabot/github_actions/github-actions-dependencies-6d79802a48

api-improvements

feat/remember-last-idp-on-smart-login-form

refactor/batch-status-requests

dependabot/npm_and_yarn/multi-5f1280885e

fix/non-semver-version-error

private-resource-page

resource-launcher

backhaul

exit-node-reconnect

feat/command-palette

ssh

delete-account

msg-delivery

org-only-idp

cicd

patch

site-targets-auto-login

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/pangolin#33555