fix: address security and UX issues in request filters

[miraserver]

Summary

This PR addresses 4 bugs identified by Codex AI code review in PR #487 for the request filters feature (originally from PR #484).

Context: PR #484 was merged before these security fixes were pushed. This PR applies the fixes to dev branch.

Bugs Fixed

1. 🔴 CRITICAL — Provider API Keys Leak to Browser (Security)

Issue: Full Provider objects (including secret key field) were passed to client component, causing Next.js to serialize API keys into browser bundle.

Files:

• src/app/[locale]/settings/request-filters/page.tsx
• src/app/[locale]/settings/request-filters/_components/filter-table.tsx

Fix: Only pass {id, name} to client component, remove Provider type import.

Risk: HIGH — Upstream provider API keys exposed to end users

---

2. 🟠 HIGH — ReDoS Vulnerability in Pre-compiled Regex (Security)

Issue: compiledRegex was created without safeRegex() validation, allowing potentially malicious regex patterns from database to execute in proxy hot path.

File: src/lib/request-filter-engine.ts

Fix:

• Add safeRegex() check before compiling regex in reload()
• Add safeRegex() check in setFiltersForTest()
• Skip unsafe patterns with warning log

Risk: MEDIUM — ReDoS attack possible if admin adds malicious regex pattern

---

3. 🟡 MEDIUM — Double-toggle in Provider Multi-select (UX)

Issue: Clicking checkbox triggers both onSelect and onCheckedChange handlers, causing double-toggle (results in no-op but visible flicker).

File: src/app/[locale]/settings/request-filters/_components/provider-multi-select.tsx

Fix: Remove duplicate onCheckedChange handler, keep only onSelect on CommandItem.

---

4. 🟡 MEDIUM — Double-toggle in Group Multi-select (UX)

Issue: Same double-toggle issue as provider multi-select.

File: src/app/[locale]/settings/request-filters/_components/group-multi-select.tsx

Fix: Remove duplicate onCheckedChange handler.

---

Testing

• All 28 request filter binding tests pass
• Typecheck passes
• Lint passes (Biome auto-format applied)

Files Changed

src/app/[locale]/settings/request-filters/
  ├─ page.tsx                          (+3 lines)
  └─ _components/
      ├─ filter-table.tsx              (-1 import, +1 inline type)
      ├─ provider-multi-select.tsx     (-1 onCheckedChange)
      └─ group-multi-select.tsx        (-1 onCheckedChange)

src/lib/request-filter-engine.ts       (+14 lines: safeRegex checks)

Total: 5 files, +29/-15 lines

Related PRs

• PR feat: add provider/group binding for request filters #484: Original feature (already merged to dev)
• PR release v0.3.39 #487: Release PR (dev → main) — will include these fixes after merge

---

🤖 Generated with Claude Code

Greptile Summary

This PR addresses 4 critical security and UX bugs identified in the request filters feature from PR #484. The changes are focused, well-tested, and production-ready.

Security Fixes:

• Provider API Key Leak (CRITICAL): Fixed Next.js serializing full Provider objects to client bundle by filtering provider data to only {id, name} before passing to client components. The Provider.key field containing upstream API keys is no longer exposed in browser bundles.
• ReDoS Vulnerability (HIGH): Added safeRegex() validation before compiling regex patterns in both reload() and setFiltersForTest() methods. Unsafe patterns are now skipped with warning logs, preventing Regular Expression Denial of Service attacks via malicious regex in the database.

UX Fixes:

• Double-toggle in multi-select components (MEDIUM): Removed duplicate onCheckedChange handlers from both ProviderMultiSelect and GroupMultiSelect components. The CommandItem wrapper's onSelect handler is sufficient, eliminating the double-toggle bug that caused flickering.

All fixes are minimal, surgical changes that don't affect other functionality. The PR description accurately describes each issue and fix.

Confidence Score: 5/5

• This PR is safe to merge immediately - it fixes critical security vulnerabilities with minimal, well-tested changes
• Score reflects the critical security fixes (API key leak and ReDoS prevention) that are correctly implemented, the minimal and surgical nature of changes, proper validation using the well-established safe-regex library, comprehensive test coverage mentioned in PR description (28 tests pass), and clear alignment with the PR description
• No files require special attention - all changes are straightforward security and UX fixes

Important Files Changed

Filename	Overview
src/app/[locale]/settings/request-filters/page.tsx	Fixed critical security issue by filtering provider data to only pass id and name to client
src/app/[locale]/settings/request-filters/_components/filter-table.tsx	Updated type signature to accept filtered provider data without sensitive fields
src/lib/request-filter-engine.ts	Added safeRegex validation before compiling regex patterns in both reload() and setFiltersForTest()

Sequence Diagram

sequenceDiagram
    participant User
    participant Page as page.tsx (Server)
    participant DB as Database
    participant FilterTable as FilterTable (Client)
    participant MultiSelect as Multi-Select (Client)

    User->>Page: Load Request Filters Page
    Page->>DB: findAllProviders()
    DB-->>Page: Full Provider objects (with key field)
    
    Note over Page: SECURITY FIX:<br/>Filter sensitive data
    Page->>Page: providers.map(p => ({id, name}))
    
    Page->>FilterTable: Pass filtered providers<br/>(id, name only)
    Note over FilterTable: API keys NOT exposed<br/>to client bundle
    
    FilterTable->>MultiSelect: Open provider selection
    MultiSelect->>MultiSelect: Click checkbox
    Note over MultiSelect: UX FIX:<br/>Single onSelect handler<br/>(no double-toggle)
    MultiSelect-->>FilterTable: Provider IDs selected
    
    FilterTable->>DB: Save filter with provider IDs
    DB->>DB: Compile regex with<br/>safeRegex() validation
    Note over DB: SECURITY FIX:<br/>ReDoS prevention
    DB-->>FilterTable: Filter saved

Loading

[gemini-code-assist]

Summary of Changes

Hello @miraserver, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses several critical security and user experience issues within the request filters feature, identified through an AI code review. It prevents sensitive API keys from being exposed to the client, mitigates a potential ReDoS vulnerability by validating regular expressions, and resolves UI flickers in multi-select components. These changes enhance the security posture and improve the overall user experience of the request filtering functionality.

Highlights

• Provider API Key Leak Prevention: Full Provider objects, including secret key fields, were inadvertently passed to client components, leading to API keys being serialized into the browser bundle. This has been fixed by ensuring only {id, name} are passed to the client component, removing the Provider type import where full objects were previously used.
• ReDoS Vulnerability Mitigation: A potential Regular Expression Denial of Service (ReDoS) vulnerability existed because compiledRegex was created without safeRegex() validation. This allowed potentially malicious regex patterns from the database to execute in the proxy hot path. The fix adds safeRegex() checks before compiling regex in both reload() and setFiltersForTest() methods, skipping unsafe patterns with a warning log.
• UX Double-toggle Fix (Provider Multi-select): A user experience issue caused a visible flicker due to a double-toggle effect when clicking checkboxes in the provider multi-select. This was because both onSelect and onCheckedChange handlers were triggered. The duplicate onCheckedChange handler has been removed, keeping only onSelect on CommandItem.
• UX Double-toggle Fix (Group Multi-select): Similar to the provider multi-select, a double-toggle issue was present in the group multi-select, causing a visible flicker. The duplicate onCheckedChange handler has been removed to resolve this UX problem.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses several important issues, including a critical security vulnerability related to API key leakage and a high-severity ReDoS vulnerability. The changes are well-structured and clearly explained in the description. The fix to prevent passing full provider objects to client components is correctly implemented, and the addition of safeRegex checks provides a necessary layer of security. The UX fixes for the multi-select components are also straightforward and correct. I have one minor suggestion in src/lib/request-filter-engine.ts to improve logging consistency in a test helper, but overall this is an excellent set of fixes.

[gemini-code-assist]

For consistency with the reload method, it would be beneficial to log a warning if the RegExp compilation fails, even within this test helper. Silently ignoring errors can make debugging tests more difficult if a regex passes the safeRegex check but is still syntactically invalid.

          } catch (error) {
            logger.warn("[RequestFilterEngine] Failed to compile regex in test", {
              filterId: f.id,
              target: f.target,
              error,
            });
          }

[github-actions]

Code Review Summary

This PR effectively addresses 4 bugs from PR #487, including 2 security issues (API key leak and ReDoS vulnerability) and 2 UX fixes (double-toggle in multi-select components). The changes are minimal, focused, and follow established patterns in the codebase.

PR Size: S

• Lines changed: 49 (30 additions, 19 deletions)
• Files changed: 5

Issues Found

Category	Critical	High	Medium	Low
Logic/Bugs	0	0	0	0
Security	0	0	0	0
Error Handling	0	0	0	0
Types	0	0	0	0
Comments/Docs	0	0	0	0
Tests	0	0	0	0
Simplification	0	0	0	0

Review Coverage

• Logic and correctness - Clean
• Security (OWASP Top 10) - Clean
• Error handling - Clean
• Type safety - Clean
• Documentation accuracy - Clean
• Test coverage - Adequate (existing test covers unsafe regex scenario)
• Code clarity - Good

Notable Changes

Security Fix 1 - Provider API Key Leak (page.tsx, filter-table.tsx)

The PR correctly removes the Provider type from the client component and changes the prop type to Array<{ id: number; name }>. This prevents the key field from being serialized into the browser bundle.

Security Fix 2 - ReDoS Protection (request-filter-engine.ts)

The PR adds safeRegex() validation before compiling regex patterns, following the established pattern from error-rule-detector.ts. Unsafe patterns are logged and skipped with appropriate warning messages.

UX Fix 3 & 4 - Double-toggle Removal

The duplicate onCheckedChange handlers are correctly removed from both multi-select components, fixing the double-toggle behavior.

All changes are well-tested, follow the project's coding standards, and maintain backward compatibility.

---

Automated review by Claude AI