Skip to content

release v0.3.39#487

Merged
ding113 merged 23 commits intomainfrom
dev
Dec 31, 2025
Merged

release v0.3.39#487
ding113 merged 23 commits intomainfrom
dev

Conversation

@ding113
Copy link
Owner

@ding113 ding113 commented Dec 30, 2025

Summary

Release v0.3.39 merges feature branch dev into main, bringing provider management enhancements, request filter improvements, and important bug fixes.

Problem

This release addresses several user-requested improvements and bug fixes:

  • 优先级、权重是否可在列表页直接配置 #456: Users needed quick access to edit provider priority, weight, and cost multiplier without opening the full edit dialog
  • Request filters lacked granular control - they could only be applied globally to all providers
  • HTTP status text was incorrectly formatted (e.g., HTTP/1.1 200 200 instead of HTTP/1.1 200 OK)
  • Gemini GET/HEAD requests were incorrectly serializing request bodies

Solution

Provider Management Enhancements (#486)

Added inline editing capability for provider configuration fields:

  • InlineEditPopover component with real-time validation and keyboard shortcuts (Enter to save, Escape to cancel)
  • Supports editing: priority, weight, and cost multiplier directly from the provider list
  • Admin-only access enforced via canEdit permission check
  • Weight minimum changed from 0 to 1 (range: 1-100) to avoid semantic overlap with is_enabled=false
  • Comprehensive test coverage added for validation rules

Request Filter Provider/Group Binding (#484)

Extended request filters with granular binding capabilities:

  • Three binding types: global (all requests), providers (specific provider IDs), groups (provider group tags)
  • Two-phase application: applyGlobal() runs before provider selection, applyForProvider() runs after
  • Database schema: Added binding_type, provider_ids, group_tags columns with indexed lookups
  • New UI components: Multi-select components for providers and groups in filter dialog
  • Performance optimizations: Pre-compiled regex caching, Set-based O(1) provider/group matching
  • Comprehensive testing: 28 new unit tests covering all binding scenarios

HTTP and Gemini Fixes (#481)

Fixed two separate bugs in the proxy forwarder:

  • StatusText fix: Changed from String(statusCode) to STATUS_CODES[statusCode] using Node.js's built-in map
  • Gemini fix: Added method check before JSON serialization to skip body for GET/HEAD requests

Changes

Core Changes

Area Files Description
Schema src/drizzle/schema.ts, drizzle/0041_sticky_jackal.sql Request filter binding columns and index
Engine src/lib/request-filter-engine.ts Two-phase filter application with optimizations
Proxy src/app/v1/_lib/proxy/guard-pipeline.ts, provider-request-filter.ts, forwarder.ts, request-filter.ts Provider filter pipeline step and HTTP fixes
UI Components src/app/[locale]/settings/providers/_components/inline-edit-popover.tsx, provider-rich-list-item.tsx Inline editing for provider fields
UI Components src/app/[locale]/settings/request-filters/_components/* Multi-select dialogs and improved table layout
Actions src/actions/request-filters.ts Validation and helper actions for bindings
Validation src/lib/validation/schemas.ts, schemas.test.ts Weight range update and test coverage

Supporting Changes

  • i18n: Updated messages/{en,ja,ru,zh-CN,zh-TW}/settings.json for new UI strings
  • Repository: src/repository/request-filters.ts added binding query methods
  • Tests: tests/unit/request-filter-binding.test.ts with 28 comprehensive tests

Breaking Changes

Change Impact Migration
Weight minimum changed from 0 to 1 Existing providers with weight=0 will need update Weight=0 providers should use is_enabled=false instead
Request filter schema extended New columns binding_type, provider_ids, group_tags Auto-migration on startup (migration 0041)

Testing

Automated Tests

  • All existing tests pass
  • 28 new tests for request filter binding types
  • New validation tests for provider fields

Manual Testing

  1. Inline editing: Click priority/weight/cost values in provider list, verify popover opens, Enter saves, Escape cancels
  2. Provider-bound filter: Create filter with specific provider IDs, verify it only applies to those providers
  3. Group-bound filter: Create filter with group tags, verify it applies to all providers with matching tags
  4. HTTP response: Verify status line format is correct (e.g., HTTP/1.1 200 OK)
  5. Gemini requests: Test GET/HEAD requests work without body serialization errors

Related Issues/PRs


🤖 Generated with Claude Code

Greptile Summary

This release implements advanced request filter binding with three binding types: global (all providers), provider-specific, and group-based filters. The feature includes:

Major Changes:

  • Database migration adds binding_type, provider_ids, and group_tags columns to request_filters table with proper indexing
  • Request filter engine refactored to split global and provider-specific filter execution with performance optimizations (pre-compiled regex, Set-based lookups, memory leak fix via event listener cleanup)
  • New two-phase filtering pipeline: global filters execute before provider selection, provider/group filters execute after
  • New ProxyProviderRequestFilter guard component integrated into both CHAT and COUNT_TOKENS pipelines

UI Enhancements:

  • Filter dialog extended with binding type selector and provider/group multi-select components
  • Filter table displays binding type with icons (Globe/Package/Tags)
  • New reusable InlineEditPopover component for inline editing of numeric provider fields
  • Complete i18n coverage across 5 languages (en, ja, ru, zh-CN, zh-TW)

Performance & Quality:

  • Comprehensive test suite with 645+ lines covering all binding scenarios
  • Pre-compiled regex caching for text_replace operations
  • O(1) provider/group lookups using Set data structures
  • Memory leak fix with proper event listener cleanup
  • Validation logic ensures binding type constraints are enforced

Bug Fixes:

  • Fixed HTTP status text using STATUS_CODES instead of numeric string in forwarder
  • Improved Gemini request body handling to skip body for GET/HEAD requests

Confidence Score: 5/5

  • This PR is safe to merge with high confidence
  • Score reflects comprehensive testing (645-line test suite), well-structured architecture with proper separation of concerns, performance optimizations throughout, complete i18n coverage, and proper database migration. The changes are backward-compatible with deprecated apply() method maintained.
  • No files require special attention

Important Files Changed

Filename Overview
drizzle/0041_sticky_jackal.sql Adds database columns for request filter binding types with proper indexing
src/lib/request-filter-engine.ts Major refactor: splits filters into global/provider-specific with performance optimizations (regex caching, Set lookups, memory leak fix)
src/app/v1/_lib/proxy/guard-pipeline.ts Adds providerRequestFilter step after provider selection in both CHAT and COUNT_TOKENS pipelines
src/app/v1/_lib/proxy/provider-request-filter.ts New guard component to apply provider-specific filters with fail-open error handling
src/actions/request-filters.ts Added binding type validation logic and new actions for provider/group listing
src/app/[locale]/settings/providers/_components/inline-edit-popover.tsx New reusable component for inline editing of numeric provider fields with validation
src/app/[locale]/settings/request-filters/_components/filter-dialog.tsx Extended UI to support binding type selection with provider/group multi-selects
tests/unit/request-filter-binding.test.ts Comprehensive test suite covering global, provider-specific, and group-based filter scenarios

Sequence Diagram

sequenceDiagram
    participant Client
    participant Pipeline as Guard Pipeline
    participant GlobalFilter as Request Filter (Global)
    participant ProviderSelector as Provider Selector
    participant ProviderFilter as Provider Request Filter
    participant Engine as Request Filter Engine
    participant Provider as Upstream Provider

    Client->>Pipeline: API Request
    Pipeline->>GlobalFilter: Apply Global Filters
    GlobalFilter->>Engine: applyGlobal(session)
    Note over Engine: Apply filters with bindingType='global'<br/>to headers and body
    Engine-->>GlobalFilter: Request Modified
    GlobalFilter-->>Pipeline: Continue
    
    Pipeline->>ProviderSelector: Select Provider
    Note over ProviderSelector: Choose provider based on<br/>routing logic, weights, etc.
    ProviderSelector-->>Pipeline: Provider Selected
    
    Pipeline->>ProviderFilter: Apply Provider-Specific Filters
    ProviderFilter->>Engine: applyForProvider(session)
    Note over Engine: Apply filters with bindingType='providers'<br/>matching session.provider.id<br/>OR bindingType='groups'<br/>matching session.provider.groupTag
    Engine-->>ProviderFilter: Request Modified
    ProviderFilter-->>Pipeline: Continue
    
    Pipeline->>Provider: Forward Request
    Provider-->>Pipeline: Response
    Pipeline-->>Client: Response
Loading

NieiR and others added 13 commits December 30, 2025 01:07
- Fix statusText bug: was `HTTP/1.1 200 200`, now `HTTP/1.1 200 OK`
  (use STATUS_CODES from node:http instead of String(statusCode))
- Fix Gemini request body handling: skip JSON.stringify for GET/HEAD
  requests which should not have a body
Allow Request Filters to be applied globally, to specific providers, or to provider groups.

## Key Changes

### Database Schema
- Add `bindingType` field ('global' | 'providers' | 'groups')
- Add `providerIds` field (array of provider IDs)
- Add `groupTags` field (array of group tags)
- Migration: 0041_sticky_jackal.sql

### Backend Logic
- Split filter engine into global and provider-specific filters
- `applyGlobal()`: runs BEFORE provider selection
- `applyForProvider()`: runs AFTER provider selection
- Provider matching: checks provider IDs or group tags

### Guard Pipeline
- Add new `providerRequestFilter` guard step
- CHAT_PIPELINE order: [..., provider, providerRequestFilter, messageContext]
- COUNT_TOKENS_PIPELINE includes provider filters

### UI Components
- New: `ProviderMultiSelect` - multi-select for providers
- New: `GroupMultiSelect` - multi-select for group tags
- Updated: `FilterDialog` - binding type selector moved after Name field
- Updated: `FilterTable` - compact layout with icon-only display
- Binding display: Global (icon only), Providers (icon + count), Groups (icon + count)

### Localization
- Add translations for 5 languages (en, ru, ja, zh-CN, zh-TW)
- Column name changed from "Apply To" to "Apply"

### Bug Fixes
- Fix binding type change validation (use explicit undefined check instead of ??)
- Prevent stale binding data when switching types

### Other Improvements
- Compact last 4 columns in filter table (Priority, Apply, Status, Actions)
- Improved UX: binding configuration at top of dialog

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add truncate to name, target, and replacement columns to keep table rows single-line
- Add tooltips on Apply column showing provider names or group tags on hover
- Fetch provider list to display names instead of IDs in tooltips
- Optimize provider lookup with useMemo Map (O(1) access)
- Add i18n translations for tooltip labels (en, ja, ru, zh-CN, zh-TW)
- Optimize request filter engine with caching and early exits

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Set max-w-2xl width for consistency with error rules dialog
- Add max-h-80vh height limit to prevent overflow
- Restructure layout with fixed header and footer
- Make only content area scrollable when form has many fields
- Ensures Cancel/Save buttons and close button always accessible

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add 28 unit tests for RequestFilterEngine binding functionality
- Cover global, provider-specific, and group-specific filters
- Test combined filters and edge cases
- Fix setFiltersForTest to sort by priority (matching reload behavior)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Iterate over provider tags (typically 1-3) instead of filter group tags
for faster set intersection check.

Suggested by: @gemini-code-assist

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…r fields (#456)

- Add InlineEditPopover component with Popover + Input + save/cancel buttons
- Integrate inline editing in ProviderRichListItem for admin users
- Add i18n strings for 5 locales (en/ja/ru/zh-CN/zh-TW)
- Allow weight minimum value to be 0 (PROVIDER_LIMITS.WEIGHT.MIN)
- Add keyboard focus styles and upper bound validation
- Add unit tests for schema validation rules
Weight=0 previously meant "skip this provider in weighted selection",
which was confusing. Now enforce 1-100 range; disable the provider
via is_enabled flag instead.

- Update PROVIDER_LIMITS.WEIGHT.MIN to 1
- Add Chinese error messages to Zod schemas
- Update all 5 locale error messages
- Adjust unit tests accordingly
- Consolidate handleSavePriority, handleSaveWeight, handleSaveCostMultiplier
  into a single createSaveHandler factory function
- Fix incorrect error message key in catch block (deleteError → unknownError)
…ting

Fix #456: Add inline editing for provider priority, weight and cost multiplier
feat: add provider/group binding for request filters
fix: correct HTTP statusText and Gemini GET/HEAD handling
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @ding113, I'm Gemini Code Assist1! 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 delivers a major upgrade to the request filtering capabilities, moving beyond global-only filters to support granular control at the provider and provider group levels. This change empowers administrators with more precise tools for managing request flows and provider interactions. Alongside this core feature, the PR introduces convenient inline editing for provider settings and includes several performance enhancements to the request filtering engine, ensuring efficient operation of the new functionalities.

Highlights

  • Provider/Group-Specific Request Filters: Implemented the ability to define request filters that apply only to specific providers or provider groups, greatly enhancing flexibility and control over request modification.
  • UI for Filter Binding Management: Added new UI components and fields in the request filter dialog and table to allow users to select the binding type (global, specific providers, or provider groups) and manage the associated providers or group tags.
  • Inline Editing for Provider Properties: Introduced an inline editing popover for provider priority, weight, and cost multiplier directly within the provider list, improving usability for quick adjustments.
  • Request Filter Engine Optimizations: Refactored the request filter engine to separate global and provider-specific filters, pre-compile regular expressions, and use Set data structures for faster lookups, leading to performance improvements.
  • Gemini Request Body Handling & HTTP Status Text Improvement: Fixed an issue where Gemini request bodies were always stringified, now only for non-GET/HEAD requests. Also, improved HTTP response statusText to use standard STATUS_CODES.
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.

@github-actions
Copy link
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@github-actions github-actions bot added enhancement New feature or request area:UI area:i18n area:core area:Error Rule size/XL Extra Large PR (> 1000 lines) labels Dec 30, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a major feature allowing request filters to be bound to specific providers or provider groups, enhancing the flexibility of request modification. The implementation is comprehensive, touching the database schema, backend engine, server actions, and frontend UI. Key highlights include a significant performance-oriented refactoring of the request-filter-engine, the addition of new UI components for provider/group selection, and an inline editing feature for provider settings. The introduction of extensive unit tests for the new logic is also a great addition.

I've identified a critical bug in the request filter engine that could cause crashes and a medium-severity issue related to handling HTTP status text. My review includes suggestions to fix these issues. Overall, this is a high-quality contribution with well-thought-out architecture and implementation.

let providerTagsSet: Set<string> | null = null;
if (this.hasGroupBasedFilters) {
const providerGroupTag = session.provider.groupTag;
providerTagsSet = new Set(providerGroupTag?.split(",").map((t) => t.trim()) ?? []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There's a potential TypeError here. If providerGroupTag is null or undefined, providerGroupTag?.split(",") will evaluate to undefined. Calling .map() on undefined will then throw an exception, crashing the request processing for any provider that doesn't have a group tag when group-based filters are active.

The nullish coalescing operator ?? should be used to provide a default empty value before calling .split() to ensure the code is robust against null or undefined values.

Suggested change
providerTagsSet = new Set(providerGroupTag?.split(",").map((t) => t.trim()) ?? []);
providerTagsSet = new Set((providerGroupTag ?? "").split(",").map((t) => t.trim()));

return new Response(bodyStream, {
status: undiciRes.statusCode,
statusText: String(undiciRes.statusCode),
statusText: STATUS_CODES[undiciRes.statusCode] ?? "OK",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using "OK" as a fallback for statusText can be misleading for non-2xx status codes. If undiciRes.statusCode is a non-standard code not present in STATUS_CODES, the status text will incorrectly be "OK". For example, a response with status: 499 would have statusText: "OK".

It's safer to provide undefined or an empty string and let the Response constructor or the client use the default status text for the given status code. By removing the fallback, undefined will be passed if the code is not found, which is handled correctly by the Response constructor.

Suggested change
statusText: STATUS_CODES[undiciRes.statusCode] ?? "OK",
statusText: STATUS_CODES[undiciRes.statusCode],

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This is a well-structured release (v0.3.39) implementing request filter provider/group binding, provider inline editing, and important bug fixes. The code quality is high with comprehensive testing and proper error handling throughout.

PR Size: XL

  • Lines changed: 4128 additions, 83 deletions
  • Files changed: 26

Note: For future XL PRs, consider splitting into smaller focused PRs to improve reviewability and reduce merge risk.

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 Summary

No significant issues identified. This PR demonstrates:

Strengths:

  • Excellent test coverage: 28 comprehensive unit tests covering all binding scenarios (645+ lines)
  • Proper error handling: All catch blocks log errors and implement fail-open behavior appropriately
  • Performance optimizations: Pre-compiled regex caching, Set-based O(1) provider/group lookups, early exit optimizations
  • Memory leak fix: Event listener cleanup added in request-filter-engine.ts
  • Type safety: Proper TypeScript types throughout with discriminated unions
  • Security: ReDoS validation for regex patterns in both create and update operations
  • Good architecture: Clean separation of applyGlobal() and applyForProvider() methods

Notable Changes:

  1. RequestFilterEngine: Two-phase filtering with performance optimizations
  2. ProxyProviderRequestFilter: New guard step for provider-specific filtering
  3. GuardPipeline: Provider filter step added after provider selection
  4. ProxyForwarder: HTTP statusText fix using STATUS_CODES map, Gemini GET/HEAD fix

Minor Observations (Informational):

  • The destroy() method on RequestFilterEngine is defined but no explicit cleanup call was found in lifecycle hooks (this is acceptable as Node.js process cleanup handles this)
  • Some Russian comments mixed with Chinese comments in request-filter-engine.ts (cosmetic, doesn't affect functionality)
  • Weight minimum changed from 0 to 1 - this is a breaking change but well-documented in PR description

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean (ReDoS protection present)
  • Error handling - Clean (fail-open with proper logging)
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Excellent (28 new tests)
  • Code clarity - Good

Automated review by Claude AI

const [filters, providers] = await Promise.all([listRequestFilters(), findAllProviders()]);

return <FilterTable filters={filters} />;
return <FilterTable filters={filters} providers={providers} />;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] [SECURITY-VULNERABILITY] Provider secret keys can be leaked to the client via providers prop

File: src/app/[locale]/settings/request-filters/page.tsx:30

This passes providers into a "use client" component:
return <FilterTable filters={filters} providers={providers} />;

findAllProviders() returns full Provider rows including key (see src/repository/provider.ts:185 selecting key: providers.key). Next.js will serialize the entire providers array into the client payload, exposing upstream API keys in the browser.

Suggested fix:

// src/app/[locale]/settings/request-filters/page.tsx
const [filters, providers] = await Promise.all([listRequestFilters(), findAllProviders()]);
const providerOptions = providers.map((p) => ({ id: p.id, name: p.name }));

return <FilterTable filters={filters} providers={providerOptions} />;

And tighten the client component prop type to prevent regressions:

// src/app/[locale]/settings/request-filters/_components/filter-table.tsx
interface Props {
  filters: RequestFilter[];
  providers: Array<{ id: number; name: string }>;
}


interface Props {
filters: RequestFilter[];
providers: Provider[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] [SECURITY-VULNERABILITY] Client prop type providers: Provider[] makes it easy to leak secrets

File: src/app/[locale]/settings/request-filters/_components/filter-table.tsx:23

This is a "use client" component, and Provider includes key: string (src/types/provider.ts:24). If a caller passes full Provider objects (as page.tsx does in this PR), Next.js will serialize the secret key into the browser payload.

Suggested fix:

// src/app/[locale]/settings/request-filters/_components/filter-table.tsx
interface Props {
  filters: RequestFilter[];
  providers: Array<{ id: number; name: string }>;
}

Then ensure callers pass only { id, name } objects.

<CommandItem
key={provider.id}
value={`${provider.name} ${provider.id}`}
onSelect={() => toggleProvider(provider.id)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [LOGIC-BUG] Provider selection can double-toggle when clicking the checkbox

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

CommandItem toggles selection on onSelect, and the nested Checkbox toggles again on onCheckedChange:

  • onSelect={() => toggleProvider(provider.id)}
  • onCheckedChange={() => toggleProvider(provider.id)}

Because the checkbox click bubbles to the CommandItem, both handlers can run, causing a double-toggle (often a no-op/flicker).

Suggested fix (keep row-click support, remove the duplicate handler):

<CommandItem onSelect={() => toggleProvider(provider.id)}>
  <Checkbox checked={selectedProviderIds.includes(provider.id)} className="mr-2" />
  {/* ... */}
</CommandItem>

<CommandItem
key={groupTag}
value={groupTag}
onSelect={() => toggleGroup(groupTag)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [LOGIC-BUG] Group selection can double-toggle when clicking the checkbox

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

Same double-toggle pattern as ProviderMultiSelect: the item toggles on onSelect, and the nested Checkbox toggles again on onCheckedChange. A checkbox click can bubble and execute both handlers, resulting in a no-op/flicker.

Suggested fix (keep row-click support, remove the duplicate handler):

<CommandItem onSelect={() => toggleGroup(groupTag)}>
  <Checkbox checked={selectedGroupTags.includes(groupTag)} className="mr-2" />
  {/* ... */}
</CommandItem>

// Optimization #2: Pre-compile regex for text_replace
if (f.matchType === "regex" && f.action === "text_replace") {
try {
cached.compiledRegex = new RegExp(f.target, "g");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] [SECURITY-VULNERABILITY] compiledRegex path bypasses the existing safeRegex (ReDoS) guard

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

There is an explicit ReDoS guard in the fallback regex path:
if (!safeRegex(target)) { ... return input; } (src/lib/request-filter-engine.ts:92-95)

But when compiledRegex is present, the engine does:
return input.replace(re, replacement); (src/lib/request-filter-engine.ts:84)

…and compiledRegex is populated at load time without any safeRegex check:
cached.compiledRegex = new RegExp(f.target, "g"); (src/lib/request-filter-engine.ts:176)

So an unsafe-but-valid regex pattern loaded from the DB could execute in the proxy hot path.

Suggested fix (validate once at load time, and mirror in setFiltersForTest):

if (f.matchType === "regex" && f.action === "text_replace") {
  if (!safeRegex(f.target)) {
    logger.warn("[RequestFilterEngine] Skip unsafe regex at load", {
      filterId: f.id,
      target: f.target,
    });
  } else {
    cached.compiledRegex = new RegExp(f.target, "g");
  }
}

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR introduces provider/group-bound request filters and related UI/proxy pipeline changes. There is a critical security regression where full Provider objects (including secret key) are passed into a client component and will be serialized to the browser. The new multi-select components can also double-toggle on checkbox clicks, and the request-filter engine’s compiled-regex path bypasses the existing ReDoS guard.

PR Size: XL

  • Lines changed: 4211
  • Files changed: 26
  • Split suggestion: Separate the Drizzle migration/meta snapshot changes from the request-filter engine/proxy pipeline changes, and split the admin UI/i18n updates into a dedicated PR to make review/rollback safer.

Issues Found

Category Critical High Medium Low
Logic/Bugs 0 2 0 0
Security 1 1 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

Critical Issues (Must Fix)

  • (Confidence 100) Secret provider key is exposed to the browser by passing full providers into a client component: src/app/[locale]/settings/request-filters/page.tsx:30 and src/app/[locale]/settings/request-filters/_components/filter-table.tsx:23.

High Priority Issues (Should Fix)

  • (Confidence 90) Checkbox click can trigger a double-toggle (row onSelect + checkbox onCheckedChange) in provider selector: src/app/[locale]/settings/request-filters/_components/provider-multi-select.tsx:130.
  • (Confidence 90) Checkbox click can trigger a double-toggle (row onSelect + checkbox onCheckedChange) in group selector: src/app/[locale]/settings/request-filters/_components/group-multi-select.tsx:130.
  • (Confidence 88) compiledRegex execution bypasses the existing safeRegex guard (ReDoS risk) when loading regex filters: src/lib/request-filter-engine.ts:176.

Review Coverage

  • Logic and correctness
  • Security (OWASP Top 10)
  • Error handling
  • Type safety
  • Documentation accuracy
  • Test coverage
  • Code clarity

Automated review by Codex AI

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • PR #487 labeled size/XL (4211 lines changed, 26 files).
  • Posted 5 inline review comments covering:
    • Critical security leak: passing full Provider objects (incl. secret key) into a client component (src/app/[locale]/settings/request-filters/page.tsx:30, src/app/[locale]/settings/request-filters/_components/filter-table.tsx:23)
    • High logic bugs: checkbox click can double-toggle in both multi-selects (src/app/[locale]/settings/request-filters/_components/provider-multi-select.tsx:130, src/app/[locale]/settings/request-filters/_components/group-multi-select.tsx:130)
    • High security/perf risk: compiledRegex path bypasses the existing safeRegex guard (src/lib/request-filter-engine.ts:176)
  • Submitted the required review summary via gh pr review --comment.

John Doe and others added 2 commits December 30, 2025 21:36
Address 4 bugs identified in code review (Codex AI):

1. CRITICAL: Prevent provider API keys leak to browser
   - Only pass {id, name} to client component FilterTable
   - Remove Provider type import, use inline type
   - Files: page.tsx, filter-table.tsx

2. HIGH: Add ReDoS validation for pre-compiled regex
   - Check safeRegex() before compiling in reload()
   - Check safeRegex() in setFiltersForTest()
   - Skip unsafe patterns with warning log
   - File: request-filter-engine.ts

3. MEDIUM: Fix double-toggle in provider multi-select
   - Remove duplicate onCheckedChange handler
   - Keep only onSelect on CommandItem
   - File: provider-multi-select.tsx

4. MEDIUM: Fix double-toggle in group multi-select
   - Remove duplicate onCheckedChange handler
   - Keep only onSelect on CommandItem
   - File: group-multi-select.tsx

All 28 unit tests pass after fixes.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@miraserver
Copy link
Contributor

⚠️ Security Fixes Required Before Merge

Code review (Codex AI) identified 4 bugs in request filters feature:

  • 🔴 1 CRITICAL (API keys leak)
  • 🟠 1 HIGH (ReDoS vulnerability)
  • 🟡 2 MEDIUM (UX double-toggle)

Root cause: PR #484 was merged before security fixes were pushed.

Resolution: Created PR #488 with all fixes:
#488

Action needed:

  1. Review and merge PR fix: address security and UX issues in request filters #488 into dev
  2. This PR (release v0.3.39 #487) will automatically include the fixes
  3. Then safe to merge to main

Fixes in PR #488:

✅ Prevent provider API keys from leaking to browser
✅ Add ReDoS validation for pre-compiled regex
✅ Fix double-toggle in provider/group multi-selects

All tests pass (28/28).

John Doe and others added 2 commits December 30, 2025 21:41
Add explicit error logging when regex compilation fails in test helper
for consistency with reload() method and easier test debugging.

Suggested by: @gemini-code-assist

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
fix: address security and UX issues in request filters
@github-actions
Copy link
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

BlackHole1 and others added 4 commits December 31, 2025 11:41
重构 webhook 通知系统,支持飞书和微信两种平台。

- 引入 StructuredMessage 抽象层实现多平台渲染
- 添加飞书卡片消息渲染器
- 统一消息模板(熔断、成本、排行榜)
- 支持指数退避重试

BREAKING CHANGE: WeChatBot API 已替换为 WebhookNotifier

Signed-off-by: Kevin Cui <bh@bugs.cc>
Signed-off-by: Kevin Cui <bh@bugs.cc>
Signed-off-by: Kevin Cui <bh@bugs.cc>
@github-actions
Copy link
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

feat(notification): add Feishu webhook support
@github-actions
Copy link
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

…splay

- Fix i18n key: checkInterval -> interval for cost alert settings
- Add webhook type auto-detection based on URL hostname (WeCom/Feishu)
- Display platform badge next to webhook URL inputs
- Show warning for unrecognized webhook platforms
- Add i18n translations for webhook type labels (5 locales)

Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

🧪 测试结果

测试类型 状态
代码质量
单元测试
集成测试
API 测试

总体结果: ✅ 所有测试通过

@ding113 ding113 merged commit 486156f into main Dec 31, 2025
13 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in Claude Code Hub Roadmap Dec 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core area:Error Rule area:i18n area:UI enhancement New feature or request size/XL Extra Large PR (> 1000 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

优先级、权重是否可在列表页直接配置

5 participants

Comments