Repository navigation
🔑 fix: Require Colon in API-Key Header Detection - #16803
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused regex correction preserves valid detection and includes appropriate regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Requires a colon in api-key header detection, preventing prose from being misclassified as credentials.
Changes:
- Tightens the
api_key_headerregex. - Adds regression coverage for the reported false positive.
| File | Description |
|---|---|
packages/api/src/protection/detectors/pattern.ts |
Requires the header colon. |
packages/api/src/protection/runtime.spec.ts |
Tests descriptive prose remains allowed. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Pull Request
Summary
The built-in
api_key_headerdetector currently treats the colon afterapi-keyas optional. This makes ordinary prose such as “contains an api-key header” match as though it contained a credential and can cause LibreChat's own content-filter error text to be rejected on a subsequent inspection.This change requires the literal header delimiter, so
api-key: secretremains detected while descriptive prose without a colon is allowed.How it works
Focused diff:
Type of change
Testing
Added a regression test using the exact false-positive text:
Submitted content contains an api-key header. Remove it and try again.The existing starter-pattern tests continue to confirm that an actual
api-key:header is detected.Tested environments/configuration:
api_key_headerstarter patternAutomated tests:
CI=true npm test --workspace=@librechat/api -- --runInBand --coverage=false src/protection/runtime.spec.tsnode scripts/sort-imports.mts <changed files>node scripts/sort-imports.mts --check <changed files>npx prettier --check <changed files>npx eslint <changed files> --pass-on-unpruned-suppressionsScreenshots / recordings
No visual UI change; this is backend pattern-matching behavior.
Risk / compatibility
The detector now follows HTTP-style header syntax by requiring a colon. Existing
api-key: <value>detection remains unchanged.Checklist