refactor: use getClientApiBaseUrl in chat libs#1633
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThis PR removes the Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ApiOverrideSync
participant SessionStorage
participant getClientApiBaseUrl
participant API
Browser->>ApiOverrideSync: mount (reads searchParams)
ApiOverrideSync->>SessionStorage: set / remove API_OVERRIDE_STORAGE_KEY
Browser->>getClientApiBaseUrl: called by client code
getClientApiBaseUrl->>SessionStorage: read API_OVERRIDE_STORAGE_KEY (if window)
getClientApiBaseUrl-->>Browser: return resolved base URL
Browser->>API: fetch(`${base}/api/...`, Authorization)
API-->>Browser: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a25fb87b5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| throw new Error("Authentication token is missing. Please refresh and try again."); | ||
| } | ||
| return deleteChat(roomId, accessToken, apiOverride ?? undefined); | ||
| return deleteChat(roomId, accessToken); |
There was a problem hiding this comment.
useDeleteChat now calls deleteChat(roomId, accessToken) without reading useApiOverride, so this mutation only honors an override if sessionStorage was already populated elsewhere. On routes that render the sidebar/Recent Chats but do not mount useChatTransport (for example non-chat pages), visiting with ?api=<url> no longer redirects delete requests to the override backend and will hit NEW_API_BASE_URL instead.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/getConversations.tsx (1)
2-27:⚠️ Potential issue | 🟠 MajorPrevent Bearer-token exfiltration via URL override
At Line 27,
getClientApiBaseUrl()can honor?api=...overrides (perlib/api/getClientApiBaseUrl.ts), and this request still sendsAuthorization: Bearer ...(Line 34). That allows token-bearing requests to be redirected to an arbitrary origin via a crafted URL.Please restrict override behavior for authenticated requests (e.g., allowlist/internal env only, or a non-overridable resolver for auth-protected endpoints).
As per coding guidelines, "Implement built-in security practices for authentication and data handling."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/getConversations.tsx` around lines 2 - 27, getConversations currently calls getClientApiBaseUrl() (which honors ?api overrides) and then sends Authorization: Bearer ..., enabling token exfiltration if a malicious base URL is supplied; fix it by ensuring authenticated endpoints use a non-overridable base resolver or validate the resolved URL domain before attaching the Authorization header: change getConversations to call a secure resolver (e.g., getServerApiBaseUrl or add a noOverride flag to getClientApiBaseUrl) or perform an allowlist check on the returned url string and abort/return [] if it’s not in the trusted domains, and only attach the Authorization header to fetch if the url passes that validation.
🧹 Nitpick comments (3)
lib/artists/fetchArtists.ts (1)
31-31: Optional: avoid emitting a trailing?when no query params are set.Line 31 always appends
?, even whenorgIdis absent. It’s harmless in most cases, but building the URL conditionally keeps cache keys/logs cleaner.Optional cleanup diff
- const response = await fetch(`${getClientApiBaseUrl()}/api/artists?${params.toString()}`, { + const query = params.toString(); + const url = `${getClientApiBaseUrl()}/api/artists${query ? `?${query}` : ""}`; + const response = await fetch(url, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/artists/fetchArtists.ts` at line 31, The request URL in fetchArtists.ts always appends a trailing '?' even when params is empty; update the code that builds the fetch URL (the call using getClientApiBaseUrl() and params.toString()) to only append `?${params.toString()}` when params.toString() is non-empty (e.g., compute a baseUrl via getClientApiBaseUrl() and conditionally concat the query string) so response fetching logic and cache keys/logs don't include a stray '?'.lib/composio/api/disconnectConnectorApi.ts (1)
28-30: Consider enriching the error message with response context.The current error is generic, which can make debugging harder when failures occur. Including the HTTP status or a snippet of the response body would improve diagnosability without exposing sensitive details.
♻️ Suggested improvement
if (!response.ok) { - throw new Error("Failed to disconnect connector"); + throw new Error(`Failed to disconnect connector (status: ${response.status})`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/composio/api/disconnectConnectorApi.ts` around lines 28 - 30, The error thrown in disconnectConnectorApi (the branch that checks if !response.ok) is too generic; update the error to include response context such as HTTP status and a short response body or statusText to aid debugging. Modify the error construction in disconnectConnectorApi to await and include a safe snippet of response.text() or response.json() (truncated to a reasonable length) alongside response.status/response.statusText, and ensure no sensitive data is logged; keep the message concise and descriptive (e.g., "Failed to disconnect connector: status X - snippet...").hooks/useAddArtistToOrganization.ts (1)
15-37: NarrowuseCallbackdependency to be more explicit.The
addArtistToOrganizationcallback depends on the entireoptionsobject, but only usesonSuccessfrom it. ExtractonSuccessat the hook level and depend on it directly for a clearer, more precise dependency that documents the actual dependency and prevents unnecessary recreations if callers ever pass options without memoization.♻️ Proposed refactor
const useAddArtistToOrganization = (options?: UseAddArtistToOrganizationOptions) => { const [addingToOrgId, setAddingToOrgId] = useState<string | null>(null); + const onSuccess = options?.onSuccess; const addArtistToOrganization = useCallback( async (artistId: string, organizationId: string) => { setAddingToOrgId(organizationId); try { const response = await fetch(`${getClientApiBaseUrl()}/api/organizations/artists`, { method: "POST", body: JSON.stringify({ artistId, organizationId, }), headers: { "Content-Type": "application/json" }, }); if (response.ok) { - options?.onSuccess?.(organizationId); + onSuccess?.(organizationId); return true; } return false; } finally { setAddingToOrgId(null); } }, - [options] + [onSuccess] );Per coding guidelines: "hooks/**/*.ts: ... Use proper dependency arrays."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useAddArtistToOrganization.ts` around lines 15 - 37, The useCallback for addArtistToOrganization currently depends on the whole options object; destructure options to pull out onSuccess at the hook level (e.g., const { onSuccess } = options) and then change the dependency array to depend explicitly on onSuccess (and any other direct dependencies like getClientApiBaseUrl or setAddingToOrgId if they are not stable) so the callback only re-creates when needed; update the body of addArtistToOrganization to call onSuccess directly instead of options?.onSuccess?. and keep setAddingToOrgId and any stable helpers out of the array if they are stable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/sandboxes/deleteSandbox.ts`:
- Line 4: The fetch in deleteSandbox.ts uses getClientApiBaseUrl() which honors
?api= overrides and can redirect bearer-authenticated requests to attacker
origins; update the code that issues the authenticated fetch (the call building
the Authorization-bearing request in deleteSandbox.ts and similarly the fetch in
getSandboxes.ts) to first validate or replace the base URL: either call a
trusted server-side base URL helper that ignores query overrides or implement an
allowlist check that parses getClientApiBaseUrl() output and rejects/throws if
the origin is not an approved trusted origin (or strip any ?api override before
concatenation); ensure the code that builds the Authorization header (the
authenticated fetch) only runs after validation and add a clear error path when
the origin is invalid.
---
Outside diff comments:
In `@lib/getConversations.tsx`:
- Around line 2-27: getConversations currently calls getClientApiBaseUrl()
(which honors ?api overrides) and then sends Authorization: Bearer ..., enabling
token exfiltration if a malicious base URL is supplied; fix it by ensuring
authenticated endpoints use a non-overridable base resolver or validate the
resolved URL domain before attaching the Authorization header: change
getConversations to call a secure resolver (e.g., getServerApiBaseUrl or add a
noOverride flag to getClientApiBaseUrl) or perform an allowlist check on the
returned url string and abort/return [] if it’s not in the trusted domains, and
only attach the Authorization header to fetch if the url passes that validation.
---
Nitpick comments:
In `@hooks/useAddArtistToOrganization.ts`:
- Around line 15-37: The useCallback for addArtistToOrganization currently
depends on the whole options object; destructure options to pull out onSuccess
at the hook level (e.g., const { onSuccess } = options) and then change the
dependency array to depend explicitly on onSuccess (and any other direct
dependencies like getClientApiBaseUrl or setAddingToOrgId if they are not
stable) so the callback only re-creates when needed; update the body of
addArtistToOrganization to call onSuccess directly instead of
options?.onSuccess?. and keep setAddingToOrgId and any stable helpers out of the
array if they are stable.
In `@lib/artists/fetchArtists.ts`:
- Line 31: The request URL in fetchArtists.ts always appends a trailing '?' even
when params is empty; update the code that builds the fetch URL (the call using
getClientApiBaseUrl() and params.toString()) to only append
`?${params.toString()}` when params.toString() is non-empty (e.g., compute a
baseUrl via getClientApiBaseUrl() and conditionally concat the query string) so
response fetching logic and cache keys/logs don't include a stray '?'.
In `@lib/composio/api/disconnectConnectorApi.ts`:
- Around line 28-30: The error thrown in disconnectConnectorApi (the branch that
checks if !response.ok) is too generic; update the error to include response
context such as HTTP status and a short response body or statusText to aid
debugging. Modify the error construction in disconnectConnectorApi to await and
include a safe snippet of response.text() or response.json() (truncated to a
reasonable length) alongside response.status/response.statusText, and ensure no
sensitive data is logged; keep the message concise and descriptive (e.g.,
"Failed to disconnect connector: status X - snippet...").
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 143448b0-ad15-4696-9a28-5fbf21b332b7
📒 Files selected for processing (27)
hooks/useAccountOrganizations.tshooks/useAddArtistToOrganization.tshooks/useCreateChat.tsxhooks/useCreateOrganization.tshooks/useOrgSettings.tslib/api/artist/getArtistSocials.tslib/artists/fetchArtists.tslib/catalog/getCatalogSongs.tslib/catalog/postCatalogSongs.tslib/chat/validateHeaders.tslib/chats/updateChat.tslib/composio/api/authorizeConnectorApi.tslib/composio/api/disconnectConnectorApi.tslib/composio/api/fetchConnectorsApi.tslib/getConversations.tsxlib/keys/createApiKey.tslib/keys/deleteApiKey.tslib/keys/fetchApiKeys.tslib/pulse/getPulse.tslib/pulse/updatePulse.tslib/sandboxes/deleteSandbox.tslib/sandboxes/getFileContents.tslib/sandboxes/getSandboxes.tslib/sandboxes/uploadSandboxFiles.tslib/spotify/getSpotifyFollowers.tslib/tasks/getTaskRuns.tslib/tools/getArtistSegments.ts
✅ Files skipped from review due to trivial changes (2)
- hooks/useOrgSettings.ts
- lib/keys/fetchApiKeys.ts
|
|
||
| export async function deleteSandbox(accessToken: string): Promise<void> { | ||
| const response = await fetch(`${NEW_API_BASE_URL}/api/sandboxes`, { | ||
| const response = await fetch(`${getClientApiBaseUrl()}/api/sandboxes`, { |
There was a problem hiding this comment.
Block untrusted API overrides for bearer-authenticated requests
Line 4 now resolves the base URL from getClientApiBaseUrl(), which accepts ?api= overrides (lib/api/getClientApiBaseUrl.ts:8-18). That allows redirecting this Authorization header to an attacker-controlled origin and leaking access tokens. Enforce a trusted-origin check (or disable query-param override) before issuing authenticated fetch calls. This same risk also applies to lib/sandboxes/getSandboxes.ts Line 20.
Suggested hardening (example)
import { getClientApiBaseUrl } from "@/lib/api/getClientApiBaseUrl";
+import { NEW_API_BASE_URL } from "@/lib/consts";
export async function deleteSandbox(accessToken: string): Promise<void> {
- const response = await fetch(`${getClientApiBaseUrl()}/api/sandboxes`, {
+ const apiBaseUrl = getClientApiBaseUrl();
+ if (new URL(apiBaseUrl).origin !== new URL(NEW_API_BASE_URL).origin) {
+ throw new Error("Untrusted API base URL for authenticated request");
+ }
+
+ const response = await fetch(`${apiBaseUrl}/api/sandboxes`, {
method: "DELETE",
headers: {
Authorization: `Bearer ${accessToken}`,
},
});As per coding guidelines, "Implement built-in security practices for authentication and data handling."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandboxes/deleteSandbox.ts` at line 4, The fetch in deleteSandbox.ts uses
getClientApiBaseUrl() which honors ?api= overrides and can redirect
bearer-authenticated requests to attacker origins; update the code that issues
the authenticated fetch (the call building the Authorization-bearing request in
deleteSandbox.ts and similarly the fetch in getSandboxes.ts) to first validate
or replace the base URL: either call a trusted server-side base URL helper that
ignores query overrides or implement an allowlist check that parses
getClientApiBaseUrl() output and rejects/throws if the origin is not an approved
trusted origin (or strip any ?api override before concatenation); ensure the
code that builds the Authorization header (the authenticated fetch) only runs
after validation and add a clear error path when the origin is invalid.
| import { NEW_API_BASE_URL } from "@/lib/consts"; | ||
|
|
||
| const API_OVERRIDE_STORAGE_KEY = "recoup_api_override"; | ||
| function normalizeApiBaseUrl(url: string): string { |
There was a problem hiding this comment.
SRP - new lib function file for normalizeApiBaseUrl.
| import { getClientApiBaseUrl } from "@/lib/api/getClientApiBaseUrl"; | ||
|
|
||
| export const PULSE_API_URL = `${NEW_API_BASE_URL}/api/pulses`; | ||
| export function getPulseApiUrl(): string { |
There was a problem hiding this comment.
SRP - new lib file for getPulseApiUrl.
Browser Spot-Check:
|
| # | Endpoint | Lib / Caller | No override (default host) | With override (?api=...) |
Status |
|---|---|---|---|---|---|
| 1 | GET /api/chats |
lib/getConversations.tsx |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 2 | GET /api/chats/:id/artist |
lib/chats/getChatArtist.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 3 | GET /api/chats/:id/messages |
lib/messages/getChatMessages.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 4 | GET /api/organizations |
hooks/useAccountOrganizations.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 5 | GET /api/artists |
lib/artists/fetchArtists.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 6 | GET /api/tasks |
lib/tasks/getTasks.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 7 | GET /api/tasks/runs |
lib/tasks/getTaskRuns.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 8 | GET /api/connectors |
lib/composio/api/fetchConnectorsApi.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 9 | GET /api/keys |
lib/keys/fetchApiKeys.ts |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
| 10 | GET /api/chats (sidebar) |
lib/getConversations.tsx |
test-recoup-api.vercel.app ✅ |
Override host ✅ | PASS |
Notes
- 10/10 endpoints pass — all correctly use the default API host when
?api=is absent, and the override host when?api=is present. - The
?api=query param is read fromwindow.location.searchat call time (viagetClientApiBaseUrl()), so it works on any page as long as the param is in the URL. - Important caveat: Client-side navigations (e.g., clicking sidebar links) strip the
?api=param from the URL, causing subsequent API calls to fall back to the default host. This is expected behavior since the olduseApiOverridehook persisted tosessionStorage, while the new implementation reads directly from the URL. For testing, navigating directly with?api=in the URL works correctly. - 404s on
/api/chats/:id/artistand/api/chats/:id/messagesappear in both override and non-override — this is a data mismatch (chat IDs differ between backends), not an override bug.
🤖 Generated with Claude Code using Chrome DevTools MCP
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@providers/ApiOverrideSync.tsx`:
- Around line 10-27: The useEffect currently accepts any parseable URL for the
?api= override (variables: searchParams.get("api"), normalizedApiParam,
API_OVERRIDE_STORAGE_KEY), which is a security risk; update the handler so after
parsing the URL you validate its origin against a whitelist (e.g., require
protocol === "https:" and host === window.location.host or a configured
ALLOWED_API_HOSTS list) and only persist normalizedApiParam when it matches
those constraints; on any validation failure or exception
removeItem(API_OVERRIDE_STORAGE_KEY) (fail closed) instead of silently ignoring,
and ensure the try/catch logs or triggers the same clear behavior so a crafted
or invalid value cannot remain in session storage.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dbb8f3db-8aaf-43f3-8a81-06d350d4b7e5
📒 Files selected for processing (6)
lib/api/getClientApiBaseUrl.tslib/consts.tslib/pulse/getPulse.tslib/pulse/updatePulse.tsproviders/ApiOverrideSync.tsxproviders/Providers.tsx
✅ Files skipped from review due to trivial changes (1)
- lib/consts.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/pulse/updatePulse.ts
- lib/pulse/getPulse.ts
- lib/api/getClientApiBaseUrl.ts
| useEffect(() => { | ||
| const apiParam = searchParams.get("api"); | ||
|
|
||
| try { | ||
| if (apiParam === "clear") { | ||
| window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY); | ||
| return; | ||
| } | ||
|
|
||
| if (!apiParam) { | ||
| return; | ||
| } | ||
|
|
||
| const normalizedApiParam = new URL(apiParam).toString().replace(/\/+$/, ""); | ||
| window.sessionStorage.setItem(API_OVERRIDE_STORAGE_KEY, normalizedApiParam); | ||
| } catch { | ||
| // Ignore invalid URLs and storage failures. | ||
| } |
There was a problem hiding this comment.
Constrain ?api= overrides to trusted targets (and fail closed).
Any parseable URL can currently become the session API base. A crafted link can persist a malicious origin for subsequent client calls. Restrict protocol/host and clear override on invalid input.
Suggested hardening
export default function ApiOverrideSync() {
const searchParams = useSearchParams();
useEffect(() => {
const apiParam = searchParams.get("api");
+ const allowedHosts = (process.env.NEXT_PUBLIC_API_OVERRIDE_ALLOWLIST ?? "")
+ .split(",")
+ .map((host) => host.trim())
+ .filter(Boolean);
try {
if (apiParam === "clear") {
window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY);
return;
}
if (!apiParam) {
return;
}
- const normalizedApiParam = new URL(apiParam).toString().replace(/\/+$/, "");
+ const parsedApi = new URL(apiParam);
+ const isHttp = parsedApi.protocol === "http:" || parsedApi.protocol === "https:";
+ const isAllowed =
+ process.env.NODE_ENV !== "production" ||
+ allowedHosts.includes(parsedApi.host);
+
+ if (!isHttp || !isAllowed) {
+ window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY);
+ return;
+ }
+
+ const normalizedApiParam = parsedApi.toString().replace(/\/+$/, "");
window.sessionStorage.setItem(API_OVERRIDE_STORAGE_KEY, normalizedApiParam);
} catch {
- // Ignore invalid URLs and storage failures.
+ window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY);
}
}, [searchParams]);
return null;
}As per coding guidelines, "Implement built-in security practices for authentication and data handling".
📝 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.
| useEffect(() => { | |
| const apiParam = searchParams.get("api"); | |
| try { | |
| if (apiParam === "clear") { | |
| window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY); | |
| return; | |
| } | |
| if (!apiParam) { | |
| return; | |
| } | |
| const normalizedApiParam = new URL(apiParam).toString().replace(/\/+$/, ""); | |
| window.sessionStorage.setItem(API_OVERRIDE_STORAGE_KEY, normalizedApiParam); | |
| } catch { | |
| // Ignore invalid URLs and storage failures. | |
| } | |
| useEffect(() => { | |
| const apiParam = searchParams.get("api"); | |
| const allowedHosts = (process.env.NEXT_PUBLIC_API_OVERRIDE_ALLOWLIST ?? "") | |
| .split(",") | |
| .map((host) => host.trim()) | |
| .filter(Boolean); | |
| try { | |
| if (apiParam === "clear") { | |
| window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY); | |
| return; | |
| } | |
| if (!apiParam) { | |
| return; | |
| } | |
| const parsedApi = new URL(apiParam); | |
| const isHttp = parsedApi.protocol === "http:" || parsedApi.protocol === "https:"; | |
| const isAllowed = | |
| process.env.NODE_ENV !== "production" || | |
| allowedHosts.includes(parsedApi.host); | |
| if (!isHttp || !isAllowed) { | |
| window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY); | |
| return; | |
| } | |
| const normalizedApiParam = parsedApi.toString().replace(/\/+$/, ""); | |
| window.sessionStorage.setItem(API_OVERRIDE_STORAGE_KEY, normalizedApiParam); | |
| } catch { | |
| window.sessionStorage.removeItem(API_OVERRIDE_STORAGE_KEY); | |
| } | |
| }, [searchParams]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@providers/ApiOverrideSync.tsx` around lines 10 - 27, The useEffect currently
accepts any parseable URL for the ?api= override (variables:
searchParams.get("api"), normalizedApiParam, API_OVERRIDE_STORAGE_KEY), which is
a security risk; update the handler so after parsing the URL you validate its
origin against a whitelist (e.g., require protocol === "https:" and host ===
window.location.host or a configured ALLOWED_API_HOSTS list) and only persist
normalizedApiParam when it matches those constraints; on any validation failure
or exception removeItem(API_OVERRIDE_STORAGE_KEY) (fail closed) instead of
silently ignoring, and ensure the try/catch logs or triggers the same clear
behavior so a crafted or invalid value cannot remain in session storage.
Retest: Session Storage Persistence (commit
|
| # | Test Case | URL after navigation | Expected API host | Actual API host | Result |
|---|---|---|---|---|---|
| 1 | /?api=... initial load |
/?api=... |
Override | Override (/api/chats, /api/organizations, /api/artists, /api/chats/:id/artist, /api/chats/:id/messages) |
PASS |
| 2 | Client-side nav to /tasks (no ?api= in URL) |
/tasks |
Override (persisted) | Override (/api/tasks, /api/tasks/runs, /api/artists) |
PASS |
| 3 | ?api=clear clears override |
/?api=clear |
Default | Default (test-recoup-api.vercel.app for all endpoints) |
PASS |
Key Improvement Verified
The previous version lost the override on client-side navigation (clicking sidebar links stripped ?api= from URL). The new ApiOverrideSync provider writes the override to sessionStorage on mount, so getClientApiBaseUrl() reads from storage and persists across all client-side navigations. This is confirmed working in test #2 above.
Code Review Notes
ApiOverrideSyncis a clean"use client"provider that syncs?api=param → sessionStoragegetClientApiBaseUrl()now reads from sessionStorage (was reading fromwindow.location.search)?api=clearproperly removes the sessionStorage keygetPulseApiUrlwas inlined back intogetPulse.ts(simpler than separate file)API_OVERRIDE_STORAGE_KEYextracted tolib/consts.ts(shared between provider and getter)
All tests pass. Session storage persistence is working correctly.
🤖 Generated with Claude Code using Chrome DevTools MCP
|
Browser interaction retest on deployed preview (
|
2 participants
Summary
Validation
Summary by CodeRabbit