fix(prompt-filter): clarify risk labels and usage totals - #488
Merged
james-6-23 merged 4 commits intoAug 8, 2026
Conversation
📝 WalkthroughWalkthroughThe change adds prompt-review API-key listing and deletion with stable masked identifiers. It also adds API-key account-usage reconciliation metrics for grouped, ungrouped, duplicate, and multi-group accounts, with corresponding frontend displays and localization. ChangesPrompt-review API-key management
API-key account-usage reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
admin/api_key_account_usage_test.go (1)
41-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover
AccountBilledreconciliation.The test validates reconciliation only for
UserBilled. The aggregation also returnsAccountBilled, and the panel displays it. Add grouped, ungrouped, duplicate, and reconciledAccountBilledassertions.Proposed test additions
- if reconciliation.GroupedTotal.Accounts != 3 || reconciliation.GroupedTotal.Requests != 8 || reconciliation.GroupedTotal.TotalTokens != 500 || math.Abs(reconciliation.GroupedTotal.UserBilled-1.5) > 1e-9 { + if reconciliation.GroupedTotal.Accounts != 3 || reconciliation.GroupedTotal.Requests != 8 || reconciliation.GroupedTotal.TotalTokens != 500 || math.Abs(reconciliation.GroupedTotal.AccountBilled-1.0) > 1e-9 || math.Abs(reconciliation.GroupedTotal.UserBilled-1.5) > 1e-9 { - if reconciliation.Ungrouped.Accounts != 1 || reconciliation.Ungrouped.TotalTokens != 50 || math.Abs(reconciliation.Ungrouped.UserBilled-0.15) > 1e-9 { + if reconciliation.Ungrouped.Accounts != 1 || reconciliation.Ungrouped.Requests != 1 || reconciliation.Ungrouped.TotalTokens != 50 || math.Abs(reconciliation.Ungrouped.AccountBilled-0.1) > 1e-9 || math.Abs(reconciliation.Ungrouped.UserBilled-0.15) > 1e-9 { - if reconciliation.Duplicate.Accounts != 1 || reconciliation.Duplicate.Requests != 3 || reconciliation.Duplicate.TotalTokens != 200 || math.Abs(reconciliation.Duplicate.UserBilled-0.6) > 1e-9 { + if reconciliation.Duplicate.Accounts != 1 || reconciliation.Duplicate.Requests != 3 || reconciliation.Duplicate.TotalTokens != 200 || math.Abs(reconciliation.Duplicate.AccountBilled-0.4) > 1e-9 || math.Abs(reconciliation.Duplicate.UserBilled-0.6) > 1e-9 { + reconciledAccountBilled := reconciliation.GroupedTotal.AccountBilled + reconciliation.Ungrouped.AccountBilled - reconciliation.Duplicate.AccountBilled + if math.Abs(reconciledAccountBilled-summary.AccountBilled) > 1e-9 { + t.Fatalf("reconciled account billed = %f, summary = %f", reconciledAccountBilled, summary.AccountBilled) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@admin/api_key_account_usage_test.go` around lines 41 - 52, Extend the reconciliation assertions in the test to validate AccountBilled for reconciliation.GroupedTotal, reconciliation.Ungrouped, and reconciliation.Duplicate, using the same floating-point tolerance as UserBilled. Add a reconciled AccountBilled calculation and compare it with summary.AccountBilled, preserving the existing UserBilled checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin/prompt_filter.go`:
- Around line 113-122: Update maskPromptReviewAPIKey so keys of length 5 through
7 return the fully masked value instead of exposing overlapping prefix and
suffix characters. Preserve the existing masking behavior for longer keys and
add tests covering each length from 5 through 7.
- Around line 711-713: Update the handler around
SetPromptFilterConfigWithAdvancedRaw so a runtime configuration failure after
the compare-and-swap restores the previously persisted prompt-filter key state
before returning the error. Prefer rolling back the persisted change using the
prior configuration, or make both updates atomic, and add a test covering this
failure path.
In `@frontend/src/pages/PromptFilter.tsx`:
- Around line 2898-2909: Update the key-deletion flow around
deletePromptReviewAPIKey so the prompt_filter_review_api_key draft is cleared or
reconciled after deleting keyID, preventing promptFilterSavePayload from
submitting the deleted key on a later save. Preserve the existing
configured-key, form-count, and review-test result updates.
---
Nitpick comments:
In `@admin/api_key_account_usage_test.go`:
- Around line 41-52: Extend the reconciliation assertions in the test to
validate AccountBilled for reconciliation.GroupedTotal,
reconciliation.Ungrouped, and reconciliation.Duplicate, using the same
floating-point tolerance as UserBilled. Add a reconciled AccountBilled
calculation and compare it with summary.AccountBilled, preserving the existing
UserBilled checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18b46bd9-b390-426d-8d01-742861cb5efd
📒 Files selected for processing (12)
admin/api_key_account_usage_test.goadmin/handler.goadmin/prompt_filter.goadmin/prompt_filter_test.godatabase/prompt_filter_review_keys.godatabase/prompt_filter_review_keys_test.gofrontend/src/api.tsfrontend/src/components/APIKeyTokenUsagePanel.tsxfrontend/src/locales/en.jsonfrontend/src/locales/zh.jsonfrontend/src/pages/PromptFilter.tsxfrontend/src/types.ts
Comment on lines
+113
to
+122
| func maskPromptReviewAPIKey(key string) string { | ||
| key = strings.TrimSpace(key) | ||
| if len(key) <= 4 { | ||
| return "••••" | ||
| } | ||
| prefix := "" | ||
| if len(key) >= 3 { | ||
| prefix = key[:3] | ||
| } | ||
| return prefix + "••••" + key[len(key)-4:] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Prevent short key disclosure.
For keys with 5 to 7 characters, the three-character prefix and four-character suffix overlap. The response then exposes every character of the key.
Return a fully masked value when the visible prefix and suffix would overlap. Add tests for keys with lengths 5 through 7.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@admin/prompt_filter.go` around lines 113 - 122, Update maskPromptReviewAPIKey
so keys of length 5 through 7 return the fully masked value instead of exposing
overlapping prefix and suffix characters. Preserve the existing masking behavior
for longer keys and add tests covering each length from 5 through 7.
Comment on lines
+711
to
+713
| if err := h.store.SetPromptFilterConfigWithAdvancedRaw(runtimeCfg, h.store.GetPromptFilterAdvancedConfig()); err != nil { | ||
| writeError(c, http.StatusInternalServerError, "审查 Key 已保存，但运行时配置更新失败") | ||
| return |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Keep persisted and runtime key state consistent.
If SetPromptFilterConfigWithAdvancedRaw fails after the compare-and-swap succeeds, the database no longer contains the key but the running service continues to use it. This is critical when the deletion revokes a compromised credential.
Roll back the compare-and-swap before returning an error, or make the persistence and runtime update one atomic operation. Add a failure-path test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@admin/prompt_filter.go` around lines 711 - 713, Update the handler around
SetPromptFilterConfigWithAdvancedRaw so a runtime configuration failure after
the compare-and-swap restores the previously persisted prompt-filter key state
before returning the error. Prefer rolling back the persisted change using the
prior configuration, or make both updates atomic, and add a test covering this
failure path.
Comment on lines
+2898
to
+2909
| const result = await api.deletePromptReviewAPIKey(keyID) | ||
| setConfiguredReviewKeys(result.items) | ||
| setForm((current) => ({ | ||
| ...current, | ||
| prompt_filter_review_api_key_configured: result.count > 0, | ||
| prompt_filter_review_api_key_count: result.count, | ||
| })) | ||
| setReviewTestResult((current) => current ? { | ||
| ...current, | ||
| key_count: result.count, | ||
| results: current.results?.filter((item) => item.key_id !== keyID), | ||
| } : null) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Prevent a stale key draft from restoring a deleted key.
If an operator edits prompt_filter_review_api_key, deletes one saved key, and then saves the form, the unchanged draft can write the deleted key back. promptFilterSavePayload includes this field whenever it is non-empty.
After deletion, clear or reconcile the draft before a later save. Alternatively, block deletion until the operator saves or discards the key draft.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/PromptFilter.tsx` around lines 2898 - 2909, Update the
key-deletion flow around deletePromptReviewAPIKey so the
prompt_filter_review_api_key draft is cleared or reconciled after deleting
keyID, preventing promptFilterSavePayload from submitting the deleted key on a
later save. Preserve the existing configured-key, form-count, and review-test
result updates.
james-6-23
added a commit
that referenced
this pull request
Aug 8, 2026
- Preserve the review key column on full-settings saves that did not change it (PreservePromptFilterReviewAPIKey, mirroring the custom-patterns guard), so a stale snapshot from another instance can no longer resurrect a deleted key. - Compare the review key CAS against the raw stored value: SQL TRIM only strips spaces while strings.TrimSpace also strips newlines/tabs, so a padded legacy value made deletion fail with a permanent 409. - Mask review keys on rune boundaries and only reveal prefix/suffix for keys of 12+ characters; short keys were fully reconstructible from the overlapping fragments and multi-byte keys were sliced mid-character. - Cover the new DELETE route in IsSensitiveEndpoint and log successful deletions (masked key, client IP, remaining count). - Bump the api-key-accounts cache namespace to v2 so pre-deploy Redis entries without the reconciliation field are not served as zeros. - Add the new review-key and moderation-category strings to zh-TW and fall back to the raw category name for unmapped moderation labels.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
The production operational cutover script no longer blocks releases on external review-key availability; that change is maintained on the production host separately from the application PR.
Summary by CodeRabbit
New Features
Improvements