feat(web): add optional LLM user email header#1455
Conversation
This comment has been minimized.
This comment has been minimized.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an opt-in environment variable that forwards authenticated users’ lower-cased email addresses to language-model providers through a request header, with centralized resolution, provider integration, tests, and documentation. ChangesUser email header propagation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ChatRequest
participant getAISDKLanguageModelAndOptions
participant resolveLanguageModelHeaders
participant LanguageModelProvider
ChatRequest->>getAISDKLanguageModelAndOptions: model configuration
getAISDKLanguageModelAndOptions->>resolveLanguageModelHeaders: configured headers
resolveLanguageModelHeaders-->>getAISDKLanguageModelAndOptions: resolved headers
getAISDKLanguageModelAndOptions->>LanguageModelProvider: construct provider with headers
LanguageModelProvider-->>ChatRequest: language model instance
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web/src/features/chat/languageModelHeaders.server.ts (1)
12-18: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider resolving token-backed headers in parallel.
Since each token resolution is independent, you can use
Promise.allalongsideObject.fromEntriesto resolve them concurrently rather than sequentially in afor...ofloop. This is a functional approach that provides a minor performance optimization, especially if multiple token-backed headers are configured.♻️ Proposed refactor
- const headers: Record<string, string> = {}; - - for (const [key, value] of Object.entries(configuredHeaders ?? {})) { - headers[key] = typeof value === 'string' - ? value - : await getTokenFromConfig(value); - } + const headers: Record<string, string> = Object.fromEntries( + await Promise.all( + Object.entries(configuredHeaders ?? {}).map(async ([key, value]) => [ + key, + typeof value === 'string' ? value : await getTokenFromConfig(value), + ]) + ) + );🤖 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 `@packages/web/src/features/chat/languageModelHeaders.server.ts` around lines 12 - 18, Update the header resolution logic to resolve token-backed values concurrently using Promise.all and construct the result with Object.fromEntries, while preserving string values and the existing configuredHeaders fallback behavior. Replace the sequential loop that builds headers.
🤖 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.
Nitpick comments:
In `@packages/web/src/features/chat/languageModelHeaders.server.ts`:
- Around line 12-18: Update the header resolution logic to resolve token-backed
values concurrently using Promise.all and construct the result with
Object.fromEntries, while preserving string values and the existing
configuredHeaders fallback behavior. Replace the sequential loop that builds
headers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de4c3c29-df7e-49be-a505-4e1797e74c15
📒 Files selected for processing (12)
CHANGELOG.mddocs/docs/configuration/environment-variables.mdxpackages/shared/src/env.server.test.tspackages/shared/src/env.server.tspackages/web/src/app/api/(server)/ee/chat/route.tspackages/web/src/ee/features/chat/actions.tspackages/web/src/ee/features/chat/llm.server.tspackages/web/src/ee/features/mcp/askCodebase.tspackages/web/src/features/chat/languageModelHeaders.server.test.tspackages/web/src/features/chat/languageModelHeaders.server.tspackages/web/src/features/chat/llm.server.tspackages/web/src/features/searchAssist/actions.ts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b872f26. Configure here.
| } | ||
|
|
||
| headers[SOURCEBOT_USER_EMAIL_HEADER] = userEmail.toLowerCase(); | ||
| } |
There was a problem hiding this comment.
Placeholder emails sent to providers
Medium Severity
When SOURCEBOT_LLM_USER_EMAIL_HEADER_ENABLED is active, resolveLanguageModelHeaders includes synthetic placeholder-*@no-email.invalid emails in the X-Sourcebot-User-Email header. This is contrary to the intent to omit these placeholder accounts, potentially exposing non-user-specific data to LLM providers.
Reviewed by Cursor Bugbot for commit b872f26. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/src/features/chat/llm.server.ts (1)
351-351: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider passing the user email directly to avoid duplicate database queries.
getAuthContext()executes database queries (e.g.,org.findUnique,userToOrg.findUnique) to resolve the authenticated user. IfresolveLanguageModelHeadersis invoked within an API Route or Server Action that is already protected by thewithAuthmiddleware upstream, fetching the context again will result in redundant database roundtrips unlessgetAuthContextis memoized viaReact.cache().Consider refactoring
getAISDKLanguageModelAndOptionsandresolveLanguageModelHeadersto accept the user's email as an argument. This allows the caller to pass down the already authenticated context and avoid duplicate database lookups.🤖 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 `@packages/web/src/features/chat/llm.server.ts` at line 351, Refactor getAISDKLanguageModelAndOptions and resolveLanguageModelHeaders to accept the authenticated user email as an argument, and pass it through from protected callers instead of invoking getAuthContext again. Remove the redundant getAuthContext lookup while preserving the existing model and header resolution behavior.packages/web/src/features/chat/llm.server.test.ts (1)
67-71: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test for stripping configured email headers on anonymous requests.
If the implementation in
llm.server.tsis updated to unconditionally scrub statically configured variants of the email header, please add a corresponding test case to verify this behavior for anonymous requests.🧪 Proposed test case
test('strips statically configured email header for anonymous requests', async () => { mocks.env.SOURCEBOT_LLM_USER_EMAIL_HEADER_ENABLED = 'true'; await expect(resolveLanguageModelHeaders({ 'x-sourcebot-user-email': 'configured@example.com', 'X-Custom-Header': 'custom-value', })).resolves.toEqual({ 'X-Custom-Header': 'custom-value', }); });🤖 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 `@packages/web/src/features/chat/llm.server.test.ts` around lines 67 - 71, Add a test alongside “omits the user email header for anonymous requests” that enables SOURCEBOT_LLM_USER_EMAIL_HEADER_ENABLED and passes configured headers containing x-sourcebot-user-email plus an unrelated custom header to resolveLanguageModelHeaders. Assert the email header is removed while the custom header remains unchanged.
🤖 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 `@packages/web/src/features/chat/llm.server.ts`:
- Around line 354-364: Move the case-insensitive header-removal loop in the
request header construction flow outside the if (userEmail) block so configured
SOURCEBOT_USER_EMAIL_HEADER variants are removed for every request. Keep
assigning the lowercased userEmail only when userEmail is present, ensuring
anonymous requests forward no user-email header.
---
Nitpick comments:
In `@packages/web/src/features/chat/llm.server.test.ts`:
- Around line 67-71: Add a test alongside “omits the user email header for
anonymous requests” that enables SOURCEBOT_LLM_USER_EMAIL_HEADER_ENABLED and
passes configured headers containing x-sourcebot-user-email plus an unrelated
custom header to resolveLanguageModelHeaders. Assert the email header is removed
while the custom header remains unchanged.
In `@packages/web/src/features/chat/llm.server.ts`:
- Line 351: Refactor getAISDKLanguageModelAndOptions and
resolveLanguageModelHeaders to accept the authenticated user email as an
argument, and pass it through from protected callers instead of invoking
getAuthContext again. Remove the redundant getAuthContext lookup while
preserving the existing model and header resolution behavior.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ff43ac5-b5ce-4ce7-9a62-dfcd338b5402
📒 Files selected for processing (2)
packages/web/src/features/chat/llm.server.test.tspackages/web/src/features/chat/llm.server.ts
| if (userEmail) { | ||
| // Header names are case-insensitive. Remove any configured variant so | ||
| // the authenticated user's email is always the authoritative value. | ||
| for (const key of Object.keys(headers)) { | ||
| if (key.toLowerCase() === SOURCEBOT_USER_EMAIL_HEADER.toLowerCase()) { | ||
| delete headers[key]; | ||
| } | ||
| } | ||
|
|
||
| headers[SOURCEBOT_USER_EMAIL_HEADER] = userEmail.toLowerCase(); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Unconditionally scrub statically configured email headers for anonymous requests.
Currently, the case-insensitive removal of any configured X-Sourcebot-User-Email variant only executes if userEmail is truthy. For anonymous requests, a statically configured fallback header will be forwarded to the provider, violating the PR objective to omit this header for unauthenticated users and potentially allowing header spoofing via static config.
Move the scrubbing loop outside of the if (userEmail) block to ensure the header is strictly removed for all requests.
🔒️ Proposed fix to scrub the header unconditionally
- if (userEmail) {
- // Header names are case-insensitive. Remove any configured variant so
- // the authenticated user's email is always the authoritative value.
- for (const key of Object.keys(headers)) {
- if (key.toLowerCase() === SOURCEBOT_USER_EMAIL_HEADER.toLowerCase()) {
- delete headers[key];
- }
- }
-
- headers[SOURCEBOT_USER_EMAIL_HEADER] = userEmail.toLowerCase();
- }
+ // Header names are case-insensitive. Remove any configured variant so
+ // the authenticated user's email is always the authoritative value,
+ // and to ensure it is strictly omitted for anonymous requests.
+ for (const key of Object.keys(headers)) {
+ if (key.toLowerCase() === SOURCEBOT_USER_EMAIL_HEADER.toLowerCase()) {
+ delete headers[key];
+ }
+ }
+
+ if (userEmail) {
+ headers[SOURCEBOT_USER_EMAIL_HEADER] = userEmail.toLowerCase();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (userEmail) { | |
| // Header names are case-insensitive. Remove any configured variant so | |
| // the authenticated user's email is always the authoritative value. | |
| for (const key of Object.keys(headers)) { | |
| if (key.toLowerCase() === SOURCEBOT_USER_EMAIL_HEADER.toLowerCase()) { | |
| delete headers[key]; | |
| } | |
| } | |
| headers[SOURCEBOT_USER_EMAIL_HEADER] = userEmail.toLowerCase(); | |
| } | |
| // Header names are case-insensitive. Remove any configured variant so | |
| // the authenticated user's email is always the authoritative value, | |
| // and to ensure it is strictly omitted for anonymous requests. | |
| for (const key of Object.keys(headers)) { | |
| if (key.toLowerCase() === SOURCEBOT_USER_EMAIL_HEADER.toLowerCase()) { | |
| delete headers[key]; | |
| } | |
| } | |
| if (userEmail) { | |
| headers[SOURCEBOT_USER_EMAIL_HEADER] = userEmail.toLowerCase(); | |
| } |
🤖 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 `@packages/web/src/features/chat/llm.server.ts` around lines 354 - 364, Move
the case-insensitive header-removal loop in the request header construction flow
outside the if (userEmail) block so configured SOURCEBOT_USER_EMAIL_HEADER
variants are removed for every request. Keep assigning the lowercased userEmail
only when userEmail is present, ensuring anonymous requests forward no
user-email header.
1 participant
Fixes SOU-1515
Summary
Validation
Note
Medium Risk
When enabled, user email (PII) is sent to external LLM APIs on every authenticated call path that uses shared model resolution; behavior is off by default and failures omit the header.
Overview
Adds an opt-in
SOURCEBOT_LLM_USER_EMAIL_HEADER_ENABLEDsetting (defaultfalse) so deployments can forward the signed-in user’s email to language model providers on outbound LLM requests.LLM provider setup in
llm.server.tsnow resolves headers throughresolveLanguageModelHeaders, which still merges configured/token-backed headers but can addX-Sourcebot-User-Email(lower-cased) when the flag is on. The header is skipped for anonymous users and when auth context resolution fails; any configured variant of that header name is replaced by the authenticated email. Changelog, env docs, and unit tests cover the new behavior.Reviewed by Cursor Bugbot for commit 60220c0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
X-Sourcebot-User-Emailheader.