Skip to content

Conversation

rkuo-danswer
Copy link
Contributor

Description

Fixes https://linear.app/danswer/issue/DAN-2010/print-sanitized-api-key-in-embedding-exceptions-to-help

How Has This Been Tested?

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

Copy link

vercel bot commented May 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2025 6:14pm

@rkuo-danswer rkuo-danswer marked this pull request as ready for review May 23, 2025 18:12
@rkuo-danswer rkuo-danswer requested a review from a team as a code owner May 23, 2025 18:12
Copy link
Contributor

@greptile-apps greptile-apps 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 Summary

Added API key sanitization in error messages to help with troubleshooting embedding errors while maintaining security.

  • Modified backend/model_server/encoders.py to include sanitized API key (first 4 and last 4 chars visible) in embedding error messages
  • Ensures sensitive API key information is protected while providing enough context for debugging
  • Consider adding a test case to verify the API key sanitization logic works as expected

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

self.timeout = timeout
self.http_client = httpx.AsyncClient(timeout=timeout)
self._closed = False
self.sanitized_api_key = api_key[:4] + "********" + api_key[-4:]
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider moving API key sanitization to a separate utility function for reuse and consistent sanitization across the codebase

Copy link
Contributor

@raunakab raunakab left a comment

Choose a reason for hiding this comment

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

Looks solid!

@raunakab raunakab added this pull request to the merge queue May 23, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 23, 2025
@Weves Weves added this pull request to the merge queue May 24, 2025
Merged via the queue into main with commit 23ff347 May 24, 2025
11 checks passed
@Weves Weves deleted the feature/print-sanitized-key branch May 24, 2025 23:39
ferdinandl007 pushed a commit to ferdinandl007/onyx that referenced this pull request May 27, 2025
Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
aronszanto pushed a commit to aronszanto/onyx that referenced this pull request May 27, 2025
Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
ZhipengHe pushed a commit to ZhipengHe/onyx that referenced this pull request Jun 6, 2025
Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025
Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants