Repository navigation
fix(ai): strip provider keys verbatim at the completer and embed seams, and redact header-style key echoes - #1350
Merged
Merged
Conversation
…s, and redact header-style key echoes Every Completer call and embed_adaptive's result now strip the stored key and base-url secrets verbatim on the error path, keeping the error variant and full text. The shared redactor learns header and JSON echoes (x-goog-api-key, x-api-key, *-key/*-token headers, any Authorization scheme, bare long Bearer tokens) without touching the context-length retry's keywords. Closes #1348 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RwZFadYd3YUadtn2ik5TmT
…t json key echoes, scan every pipeline provider call The seam test built its errors through real requests on the shared pooled client, which left keep-alive connections that made the ollama timeout tests flaky; it now builds them with friendly_api_error directly. The redactor catches compact JSON and glued header forms. The wiring guard now requires every self.provider call under src/pipeline to sit inside strip_secrets unless it is a listed non-io method. Refs #1348 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RwZFadYd3YUadtn2ik5TmT
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 37932390 | Triggered | Bearer Token | 9f963de | apps/desktop/src-tauri/src/observability/tests/header_echoes.rs | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
github-actions
Bot
added
app/desktop
Contributor
Coverage Report
File CoverageNo changed files found. |
Contributor
🦀 Rust Coverage
Per-file coverage |
Owner
Author
|
🎉 This is included in version 0.157.0. |
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
1 participant
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
This follows #1347 and #1349. The verbatim stripping of stored keys and base-URL secrets now happens at the two seams every provider call passes through, so it covers every generation path, not just four command edges. The shared redactor also catches keys echoed back in headers and JSON.
Type of change
fixChanges
error_map.rs):strip_secrets_in_placerebuilds the sameAppErrorvariant with the stripped message and applies no length cap. The match is exhaustive with no wildcard arm, so adding a variant is a compile error.provider_secretscollects the stored key (raw and trimmed), the base URL's userinfo password and its query values. Secrets shorter than 8 characters are skipped, and longer secrets are replaced first.redact_provider_errorandfinish_provider_resultnow reuse these helpers, with unchanged behaviour.Completer:strip_secretswrapsstream,stream_complete,complete,chat_with_tools,structured_calland the research methods. On the error path only, it reads the key from the same credential slot the adapter used. Keyless providers return before any keychain access.embed_text: the strip is applied toembed_adaptive's result after the halving retry, so the retry still sees the context-length wording.CompleterasErr.ai_generateandgenerate_pipelinepass that text toemit_stream_error/job_fail, so it is stripped before either of them sees it.observability/header_echo.rs): this is a single linear pass, case-insensitive. It handles:x-goog-api-key:andx-api-key:*-key:,*-token:or*-secret:header, including the_and glued formsAuthorization:with theBearer,Basic,TokenorKeyschemeBearer <token>{"Authorization":"Bearer gsk_…"}max_tokens: 4096and thex-ratelimit-*-tokensheadersBearer authentication failedis_context_length_errorkeyword, across 5 providers' error shapesTests
AIzaSy…key echoed by the upstream is stripped.is_empty_answer_length_cutandretriable()are unaffected.wiring_guard.rs):src/pipelineand requires eachself.provider.<m>(call to sit inside astrip_secrets(argument. Comments are stripped first, so a comment cannot satisfy it. The only exceptions are the listed non-IO methods.embed_textstrips theembed_adaptiveresult.complete_structuredunwrapped, the wiring guard fails.Review
tauri-security-reviewerraised one HIGH in the first draft: the seam test made real requests through the process-global pooled HTTP client. The keep-alive connections it left behind made theollama::local_chattimeout tests flaky:That test now builds its errors directly, and
cargo test --lib -j 1 commands::ai_providerpasses 10 of 10. CI uses nextest, which runs each test in its own process, so it would not have shown this flake.Verified by the reviewer:
translation.rsis the exception, but it is gated to keyless local providers.Its compact-JSON and wiring-guard suggestions are included in this PR.
Testing
--all-targets --all-features -D warningscommands::ai_provider10× (633 passed each run),pipeline(401),observability,commands::ai(663), architecture (26)Closes #1348
🤖 Generated with Claude Code