Closed
Conversation
dosubot
Bot
added
the
size:XXL
Contributor
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| ErrorCode.FEATURE_NOT_SUPPORTED, | ||
| "Only MCPJam hosted models are supported in hosted mode", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Guest model restriction missing on web chat-v2 endpoint
High Severity
The web chat-v2 guest path validates that the model is an isMCPJamProvidedModel but never checks isGuestAllowedModel. A guest user with a valid guest token could bypass client-side model filtering and request premium models (e.g., anthropic/claude-sonnet-4.5). The non-hosted mcp/chat-v2.ts route correctly enforces isGuestAllowedModel with a 403 for unauthorized models, but this hosted web route does not.
| setAuthHeaders({ Authorization: `Bearer ${guestToken}` }); | ||
| } else { | ||
| setAuthHeaders(undefined); | ||
| } |
There was a problem hiding this comment.
Redundant nested HOSTED_MODE check creates dead code
Low Severity
The inner if (HOSTED_MODE) on line 536 is always true because the outer condition on line 533 already requires HOSTED_MODE to be truthy. This makes the else branch (line 544, setAuthHeaders(undefined)) unreachable dead code.
Contributor
WalkthroughThis pull request introduces comprehensive guest authentication support alongside hosted workspace enhancements. Key additions include a new 📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.ts (1)
134-155:⚠️ Potential issue | 🟡 MinorGuard against fake timer leakage on test failure.
Should the test throw before reaching line 154,
vi.useRealTimers()never executes—leaving subsequent tests in a polluted timer state. Atry/finallywrapper ensures restoration regardless of outcome.Additionally, the magic number
30_001would benefit from a brief comment linking it to the actual cache TTL (presumably 30 seconds).🛡️ Proposed fix for timer cleanup
it("re-evaluates guest token after cache expiry", async () => { vi.useFakeTimers(); + try { + setHostedApiContext({ + workspaceId: null, + isAuthenticated: false, + serverIdsByName: {}, + }); - setHostedApiContext({ - workspaceId: null, - isAuthenticated: false, - serverIdsByName: {}, - }); + vi.mocked(getGuestBearerToken).mockResolvedValueOnce("guest-1"); + vi.mocked(getGuestBearerToken).mockResolvedValueOnce("guest-2"); - vi.mocked(getGuestBearerToken).mockResolvedValueOnce("guest-1"); - vi.mocked(getGuestBearerToken).mockResolvedValueOnce("guest-2"); + const result1 = await getHostedAuthorizationHeader(); + expect(result1).toBe("Bearer guest-1"); - const result1 = await getHostedAuthorizationHeader(); - expect(result1).toBe("Bearer guest-1"); + // Cache TTL is 30 seconds; advance just past expiry + vi.advanceTimersByTime(30_001); - vi.advanceTimersByTime(30_001); - - const result2 = await getHostedAuthorizationHeader(); - expect(result2).toBe("Bearer guest-2"); - - vi.useRealTimers(); + const result2 = await getHostedAuthorizationHeader(); + expect(result2).toBe("Bearer guest-2"); + } finally { + vi.useRealTimers(); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.ts` around lines 134 - 155, Wrap the test body that uses vi.useFakeTimers() in a try/finally so vi.useRealTimers() is always called (protecting against test failures); locate the test that calls getHostedAuthorizationHeader and mocks getGuestBearerToken, call vi.useRealTimers() in the finally block and keep the assertions in the try block. Also replace or annotate the magic number 30_001 used with vi.advanceTimersByTime (or reference the cache TTL constant if available) by adding a brief comment linking it to the guest token cache TTL (e.g., "30_001 /* > HOSTED_GUEST_TOKEN_CACHE_TTL (30s) */") or by using the TTL constant instead to make the intent explicit.
🧹 Nitpick comments (8)
mcpjam-inspector/server/middleware/bearer-auth.ts (1)
33-35: Consider narrowing the exception scope.The catch block assumes all exceptions indicate an uninitialized service, yet network failures during JWKS retrieval or other transient errors would also land here—silently promoting such requests to the WorkOS path.
For security observability, distinguishing "service not ready" from "validation failed due to external error" may prove valuable. A brief log entry (using the logger utility) would aid incident triage without altering control flow.
🔍 Optional: Add diagnostic logging
- } catch { - // Guest token service not initialized — treat as non-guest token + } catch (err) { + // Guest token service not initialized or validation error — treat as non-guest token + // Uncomment for debugging transient JWKS failures: + // logger.debug("Guest token validation skipped", { error: err }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/middleware/bearer-auth.ts` around lines 33 - 35, The empty catch in bearer-auth.ts that treats any exception as "service not initialized" should be narrowed: catch specific errors that indicate the guest token service is uninitialized (e.g., check for a known ServiceNotReady/NotInitialized error or the absence of the guest token service instance) and re-throw or handle other exceptions; additionally log unexpected errors using the existing logger utility (e.g., logger.warn or logger.error) inside the catch so network/JWKS failures are recorded while preserving the current control flow. Target the catch in the guest token validation area of the middleware (the try/catch around the guest token service lookup/validation) and update handling for transient/validation errors vs "service not ready" cases.mcpjam-inspector/client/src/lib/apis/mcp-skills-api.ts (1)
27-29: Don’t collapse “unavailable in hosted mode” into “no skills.”Returning
[]here makes hosted mode indistinguishable from a genuinely empty skill set, so callers can render the wrong empty state while the rest of this module still exposes skill operations. Prefer surfacing an explicit unsupported-state signal instead of silently pretending the collection is empty.Possible adjustment
export async function listSkills(): Promise<SkillListItem[]> { if (HOSTED_MODE) { - return []; + throw new Error("Skills are unavailable in hosted mode"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/apis/mcp-skills-api.ts` around lines 27 - 29, The HOSTED_MODE guard currently returns an empty array (return []) which hides the fact that skills are unsupported in hosted mode; change that branch to surface an explicit unsupported-state (e.g. throw a clear error like UnsupportedInHostedModeError or return a distinct sentinel such as null/undefined or a Result/Error object) instead of returning [] so callers can detect and render the correct UI; update the HOSTED_MODE check in mcp-skills-api.ts (the branch using HOSTED_MODE) to throw or return the explicit signal and ensure calling code handles that case.mcpjam-inspector/client/src/lib/apis/web/context.ts (1)
266-316: Potential duplicategetGuestBearerTokeninvocations.When
shouldPreferGuestBearer()returnstruebutgetGuestBearerToken()yieldsnull(lines 277-286), andhasHostedGuestAccess()remainstrue(line 302), the function callsgetGuestBearerToken()again (line 307). If the token fetch is expensive or has side effects, consider caching the first result:♻️ Suggested consolidation
export async function getHostedAuthorizationHeader(): Promise<string | null> { if (!HOSTED_MODE) return null; const now = Date.now(); if (cachedBearerToken && cachedBearerToken.expiresAt > now) { return `Bearer ${cachedBearerToken.token}`; } + // Attempt guest token first for guest-capable surfaces + if (shouldPreferGuestBearer()) { + const guestToken = await getGuestBearerToken(); + if (guestToken) { + cachedBearerToken = { + token: guestToken, + expiresAt: now + TOKEN_CACHE_TTL_MS, + }; + return `Bearer ${guestToken}`; + } + // Guest token unavailable; no WorkOS fallback for guest surfaces + return null; + } - if (shouldPreferGuestBearer()) { - const guestToken = await getGuestBearerToken(); - ... - } - // Try WorkOS (logged-in user) ... - - if (!hasHostedGuestAccess()) { - return null; - } - - // Fall back to guest token... + return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/apis/web/context.ts` around lines 266 - 316, getHostedAuthorizationHeader currently may call getGuestBearerToken twice when shouldPreferGuestBearer() is true but the first call returns null and hasHostedGuestAccess() later allows a second call; to fix, call getGuestBearerToken once and store its result in a local variable (e.g., guestToken) the first time you need it (used in the shouldPreferGuestBearer branch and the later fallback), then reuse that variable instead of invoking getGuestBearerToken() again; update references inside getHostedAuthorizationHeader to set cachedBearerToken and return using that local guestToken when present.mcpjam-inspector/client/src/lib/__tests__/hosted-workspace.test.ts (1)
5-17: Consider adding anundefinedinput test case.The function signature accepts
string | null | undefined, yet onlynullis tested. Adding a case forundefinedwould complete coverage:it("returns null when workspace id is undefined", () => { expect(resolveHostedWorkspaceId(true, undefined)).toBeNull(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/__tests__/hosted-workspace.test.ts` around lines 5 - 17, Add a test that covers the undefined input path for resolveHostedWorkspaceId: update the hosted-workspace.test.ts test suite to include a case like it("returns null when workspace id is undefined", () => { expect(resolveHostedWorkspaceId(true, undefined)).toBeNull(); }); so the function signature string | null | undefined is fully exercised; locate the describe("resolveHostedWorkspaceId", ...) block and add this new it() alongside the existing null test.mcpjam-inspector/server/routes/web/__tests__/guest-jwks.test.ts (1)
52-61: Solid test coverage for the JWKS endpoint.The assertions validate essential JWKS properties. For completeness, consider asserting
kty: "RSA"and presence of the public key components (n,e) to ensure the key material is actually exportable:expect.objectContaining({ kid: "guest-1", kty: "RSA", alg: "RS256", use: "sig", n: expect.any(String), e: expect.any(String), }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/web/__tests__/guest-jwks.test.ts` around lines 52 - 61, Update the JWKS test assertions in the guest-jwks.test.ts where you inspect the parsed response body (the `body` variable from `await response.json()`) to also assert key type and key material: in the existing expect on `keys` that uses `expect.objectContaining({ kid: "guest-1", alg: "RS256", use: "sig" })`, add `kty: "RSA"` and `n: expect.any(String)` and `e: expect.any(String)` so the test ensures the public key is exportable and has the RSA modulus/exponent.mcpjam-inspector/server/routes/web/__tests__/guest-session.test.ts (1)
135-170: Add the remote-session failure-path test.This new mode also returns 503 when
fetchRemoteGuestSession()yieldsnull, but the suite only exercises the happy path. That is the branch most likely to matter during an upstream outage, and right now it can regress silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/web/__tests__/guest-session.test.ts` around lines 135 - 170, Add a test for the remote-guest-session failure path: set process.env.MCPJAM_USE_LOCAL_GUEST_SIGNING = "false" and process.env.MCPJAM_GUEST_SESSION_URL to the remote URL, mock global.fetch (the underlying fetchRemoteGuestSession call) to return a non-success/null result so fetchRemoteGuestSession yields null, call app.request("/guest-session", { method: "POST" }) and assert the response status is 503 and that global.fetch was invoked with the remote URL and expected options; include the same request headers check as the happy-path test to ensure parity.mcpjam-inspector/shared/types.ts (1)
138-149: Avoid a second hard-coded guest allowlist.This client-side policy now has to stay aligned with the hosted backend by hand. Once they drift, guests will either see models the server rejects or lose access to models the server already allows. Prefer deriving this from a shared contract or server-delivered config instead of duplicating IDs here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/shared/types.ts` around lines 138 - 149, The file currently hard-codes a second guest allowlist (GUEST_ALLOWED_MODEL_IDS) and the helper isGuestAllowedModel, which drifts from the backend; remove the hard-coded array and change isGuestAllowedModel to read the allowed model IDs from a single authoritative source (import a shared contract/module if available or fetch the list from the server at startup and cache it), falling back to an empty list if the contract/config is unavailable; ensure the symbol names GUEST_ALLOWED_MODEL_IDS and isGuestAllowedModel are replaced or adapted so callers still work (e.g., export a resolved list or an async getter) and add a clear comment that the list is authoritative from the backend rather than duplicated.mcpjam-inspector/client/src/lib/__tests__/session-token.hosted-retry.test.ts (1)
167-193: Tighten this persisted-token assertion.Right now this still passes on any generic 401 retry. Assert that the second call carries
Authorization: "Bearer fresh-token"so the test proves the retry used the refreshed persisted guest token.Possible tightening
expect(response.status).toBe(200); expect(resetTokenCache).toHaveBeenCalledTimes(1); expect(forceRefreshGuestSession).toHaveBeenCalledTimes(1); expect(global.fetch).toHaveBeenCalledTimes(2); + expect(global.fetch).toHaveBeenNthCalledWith( + 2, + "/api/web/chat-v2", + expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: "Bearer fresh-token", + }), + }), + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/__tests__/session-token.hosted-retry.test.ts` around lines 167 - 193, The test currently only asserts two fetch calls after a 401; tighten it by verifying the second fetch was invoked with the refreshed guest token in its headers: after calling authFetch in the "retries when the failing request used the persisted guest token" test, assert that global.fetch was called twice and that the second call (inspect the arguments of the second invocation of global.fetch) includes an Authorization header equal to "Bearer fresh-token" (while keeping existing expectations for resetTokenCache and forceRefreshGuestSession); reference the test helpers getHostedAuthorizationHeader, forceRefreshGuestSession, peekStoredGuestToken and the authFetch invocation to locate where to add this assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/hooks/use-chat-session.ts`:
- Around line 246-258: The current guest-mode logic uses truthiness of
hostedWorkspaceId (in directGuestMode/sharedGuestMode and guestMode) which
conflates "still resolving" with "resolved-empty"; change this to explicitly
distinguish unresolved (undefined) vs resolved-empty (null or a sentinel) and
reuse a single predicate (e.g., isHostedWorkspaceResolved and
isHostedWorkspaceEmpty) across directGuestMode, sharedGuestMode, the guest-token
fallback, and hostedContextNotReady checks; update the checks that reference
hostedWorkspaceId and hostedShareToken and the functions/variables
useChatSession (or the hook-level variables directGuestMode, sharedGuestMode,
guestMode, hostedContextNotReady) so unresolved (undefined) keeps the page
blocked while only resolved-empty (null/sentinel) allows the guest fallback.
In `@mcpjam-inspector/client/src/lib/__tests__/hosted-web-context.test.ts`:
- Around line 74-92: The test should also assert that the auth accessor is not
used when hasSession is true but isAuthenticated is false: mock or spy on
getAccessToken (the function used by your auth accessor) before calling
setHostedApiContext/buildHostedServerRequest and verify getAccessToken was not
called; keep the existing expectation on buildHostedServerRequest("myServer") to
ensure the returned shape remains the same, but add the negative call assertion
on getAccessToken to prove we do not re-enter the authenticated flow.
In `@mcpjam-inspector/client/src/lib/guest-session.ts`:
- Line 18: The in-flight older getOrCreateGuestSession promise can overwrite
storage after forceRefreshGuestSession clears the reference; to fix, add a
request version/check so only the winner writes to storage: introduce a local
incrementing token or requestId tied to forceRefreshInFlight (and stored in the
closure) and have getOrCreateGuestSession and its eventual
writeToStorage(session) verify the current token matches before writing; update
forceRefreshGuestSession() to bump the token when forcing refresh and ensure
inFlightRequest is only cleared/updated after validating the token so stale
promises cannot persist a stale session.
In `@mcpjam-inspector/server/routes/mcp/__tests__/chat-v2.test.ts`:
- Around line 87-98: Add two Vitest test cases in chat-v2.test.ts that exercise
the guest gate: one where isGuestAllowedModel returns false and you assert the
route responds with 403, and another where getProductionGuestAuthHeader is made
to reject (or throw) and you assert the route responds with 503; modify the
existing vi.mock for "@/shared/types" and "../../../utils/guest-auth.js" within
those tests (via vi.mockImplementationOnce or per-test mocking) so other tests
remain unchanged, and call the same route handler used by current tests to
verify the access-policy behavior for isGuestAllowedModel and
getProductionGuestAuthHeader.
In `@mcpjam-inspector/server/routes/mcp/chat-v2.ts`:
- Around line 143-151: The current branch only handles a falsy return from
getProductionGuestAuthHeader() but doesn't catch exceptions thrown by that
helper, which causes the outer catch to convert guest-auth failures into a 500;
wrap the await getProductionGuestAuthHeader() call in a try/catch inside this
route handler, assign authHeader from the try body, and on any caught error
return the same c.json({...}, 503) response (same message used for the falsy
case) so both thrown and falsy guest-auth failures map to the advertised 503
retryable response.
In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts`:
- Around line 58-63: The test suite needs to run with an isolated guest key
directory so initGuestTokenSecret() doesn't fall back to the real ~/.mcpjam; in
beforeEach set process.env.GUEST_JWT_KEY_DIR to a newly created temp directory
(use fs.mkdtempSync or a tmpdir helper) and in afterEach restore the original
env var (saved alongside
originalConvexHttpUrl/originalNodeEnv/originalHostedGuestJwksUrl/originalLocalSigning)
and delete the temp directory (fs.rmSync(tempDir, { recursive: true, force: true
})). Ensure the same pattern is applied for the tests covering lines ~100-138 so
no real key material is read or left behind.
In `@mcpjam-inspector/server/routes/web/chat-v2.ts`:
- Around line 53-65: The handler is unsafe because rawBody is being double-cast
to ChatV2Request (via rawBody as unknown as ChatV2Request) so malformed
temperature, systemPrompt, headers, etc. can reach prepareChatV2() /
convertToModelMessages() and cause 500s; fix this by defining and using a strict
runtime schema (e.g., Zod/AJV) for the full guest payload (fields: messages,
model, systemPrompt, temperature, requireToolApproval, serverUrl, serverHeaders,
oauthAccessToken), parse/validate rawBody with that schema at the start of the
route, return a 4xx on validation failure, and then use the validated result
(not rawBody) when calling prepareChatV2(), convertToModelMessages(), and
elsewhere (refer to rawBody, ChatV2Request, prepareChatV2,
convertToModelMessages, messages, model, systemPrompt, temperature).
In `@mcpjam-inspector/server/services/guest-token.ts`:
- Around line 232-296: When hostedGuestPublicKeysCache is present but the
requested kid is not found, perform a bounded refresh: call the same JWKS fetch
logic (fetch(getHostedGuestJwksUrl(), ...), parsing into keysByKid/fallbackKey)
even if now - hostedGuestPublicKeysCache.fetchedAt < HOSTED_GUEST_JWKS_CACHE_MS;
if the refresh succeeds, replace hostedGuestPublicKeysCache and return the newly
found key or new fallbackKey; if the refresh fails, do NOT overwrite
hostedGuestPublicKeysCache and continue returning the existing
hostedGuestPublicKeysCache.fallbackKey (so stale cache is used on refresh
failure), and ensure errors during parsing skip malformed keys as before and
preserve the last-good cache on any network/parse error.
- Around line 206-209: The expiration check currently uses "if (nowSeconds >
payload.exp)" which allows tokens to be accepted for the exact second of
payload.exp; change the comparison to "nowSeconds >= payload.exp" so tokens are
rejected at the exact exp boundary (update the check that references nowSeconds
and payload.exp in the guest token verification logic).
In `@mcpjam-inspector/server/utils/guest-session-source.ts`:
- Around line 20-31: Validate and normalize the MCPJAM_GUEST_SESSION_URL inside
getRemoteGuestSessionUrl and harden fetchRemoteGuestSession: parse the
configured URL and reject it unless its protocol is "https:" or it is an
explicit loopback/dev host (localhost, 127.0.0.1, ::1) to prevent accidental
downgrade to http; if invalid, throw and fall back to
DEFAULT_REMOTE_GUEST_SESSION_URL or fail fast. In fetchRemoteGuestSession, use
an AbortController with a short configurable timeout (e.g., 2–5s) to abort slow
requests and pass the controller.signal to fetch, and ensure any thrown errors
from parsing/timeout are caught and logged/handled consistently. Reference:
getRemoteGuestSessionUrl and fetchRemoteGuestSession.
---
Outside diff comments:
In
`@mcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.ts`:
- Around line 134-155: Wrap the test body that uses vi.useFakeTimers() in a
try/finally so vi.useRealTimers() is always called (protecting against test
failures); locate the test that calls getHostedAuthorizationHeader and mocks
getGuestBearerToken, call vi.useRealTimers() in the finally block and keep the
assertions in the try block. Also replace or annotate the magic number 30_001
used with vi.advanceTimersByTime (or reference the cache TTL constant if
available) by adding a brief comment linking it to the guest token cache TTL
(e.g., "30_001 /* > HOSTED_GUEST_TOKEN_CACHE_TTL (30s) */") or by using the TTL
constant instead to make the intent explicit.
---
Nitpick comments:
In `@mcpjam-inspector/client/src/lib/__tests__/hosted-workspace.test.ts`:
- Around line 5-17: Add a test that covers the undefined input path for
resolveHostedWorkspaceId: update the hosted-workspace.test.ts test suite to
include a case like it("returns null when workspace id is undefined", () => {
expect(resolveHostedWorkspaceId(true, undefined)).toBeNull(); }); so the
function signature string | null | undefined is fully exercised; locate the
describe("resolveHostedWorkspaceId", ...) block and add this new it() alongside
the existing null test.
In
`@mcpjam-inspector/client/src/lib/__tests__/session-token.hosted-retry.test.ts`:
- Around line 167-193: The test currently only asserts two fetch calls after a
401; tighten it by verifying the second fetch was invoked with the refreshed
guest token in its headers: after calling authFetch in the "retries when the
failing request used the persisted guest token" test, assert that global.fetch
was called twice and that the second call (inspect the arguments of the second
invocation of global.fetch) includes an Authorization header equal to "Bearer
fresh-token" (while keeping existing expectations for resetTokenCache and
forceRefreshGuestSession); reference the test helpers
getHostedAuthorizationHeader, forceRefreshGuestSession, peekStoredGuestToken and
the authFetch invocation to locate where to add this assertion.
In `@mcpjam-inspector/client/src/lib/apis/mcp-skills-api.ts`:
- Around line 27-29: The HOSTED_MODE guard currently returns an empty array
(return []) which hides the fact that skills are unsupported in hosted mode;
change that branch to surface an explicit unsupported-state (e.g. throw a clear
error like UnsupportedInHostedModeError or return a distinct sentinel such as
null/undefined or a Result/Error object) instead of returning [] so callers can
detect and render the correct UI; update the HOSTED_MODE check in
mcp-skills-api.ts (the branch using HOSTED_MODE) to throw or return the explicit
signal and ensure calling code handles that case.
In `@mcpjam-inspector/client/src/lib/apis/web/context.ts`:
- Around line 266-316: getHostedAuthorizationHeader currently may call
getGuestBearerToken twice when shouldPreferGuestBearer() is true but the first
call returns null and hasHostedGuestAccess() later allows a second call; to fix,
call getGuestBearerToken once and store its result in a local variable (e.g.,
guestToken) the first time you need it (used in the shouldPreferGuestBearer
branch and the later fallback), then reuse that variable instead of invoking
getGuestBearerToken() again; update references inside
getHostedAuthorizationHeader to set cachedBearerToken and return using that
local guestToken when present.
In `@mcpjam-inspector/server/middleware/bearer-auth.ts`:
- Around line 33-35: The empty catch in bearer-auth.ts that treats any exception
as "service not initialized" should be narrowed: catch specific errors that
indicate the guest token service is uninitialized (e.g., check for a known
ServiceNotReady/NotInitialized error or the absence of the guest token service
instance) and re-throw or handle other exceptions; additionally log unexpected
errors using the existing logger utility (e.g., logger.warn or logger.error)
inside the catch so network/JWKS failures are recorded while preserving the
current control flow. Target the catch in the guest token validation area of the
middleware (the try/catch around the guest token service lookup/validation) and
update handling for transient/validation errors vs "service not ready" cases.
In `@mcpjam-inspector/server/routes/web/__tests__/guest-jwks.test.ts`:
- Around line 52-61: Update the JWKS test assertions in the guest-jwks.test.ts
where you inspect the parsed response body (the `body` variable from `await
response.json()`) to also assert key type and key material: in the existing
expect on `keys` that uses `expect.objectContaining({ kid: "guest-1", alg:
"RS256", use: "sig" })`, add `kty: "RSA"` and `n: expect.any(String)` and `e:
expect.any(String)` so the test ensures the public key is exportable and has the
RSA modulus/exponent.
In `@mcpjam-inspector/server/routes/web/__tests__/guest-session.test.ts`:
- Around line 135-170: Add a test for the remote-guest-session failure path: set
process.env.MCPJAM_USE_LOCAL_GUEST_SIGNING = "false" and
process.env.MCPJAM_GUEST_SESSION_URL to the remote URL, mock global.fetch (the
underlying fetchRemoteGuestSession call) to return a non-success/null result so
fetchRemoteGuestSession yields null, call app.request("/guest-session", {
method: "POST" }) and assert the response status is 503 and that global.fetch
was invoked with the remote URL and expected options; include the same request
headers check as the happy-path test to ensure parity.
In `@mcpjam-inspector/shared/types.ts`:
- Around line 138-149: The file currently hard-codes a second guest allowlist
(GUEST_ALLOWED_MODEL_IDS) and the helper isGuestAllowedModel, which drifts from
the backend; remove the hard-coded array and change isGuestAllowedModel to read
the allowed model IDs from a single authoritative source (import a shared
contract/module if available or fetch the list from the server at startup and
cache it), falling back to an empty list if the contract/config is unavailable;
ensure the symbol names GUEST_ALLOWED_MODEL_IDS and isGuestAllowedModel are
replaced or adapted so callers still work (e.g., export a resolved list or an
async getter) and add a clear comment that the list is authoritative from the
backend rather than duplicated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3fbcf2e-9aee-4b40-9421-925720af84f5
📒 Files selected for processing (36)
mcpjam-inspector/client/src/App.tsxmcpjam-inspector/client/src/components/ChatTabV2.tsxmcpjam-inspector/client/src/components/ui-playground/PlaygroundMain.tsxmcpjam-inspector/client/src/hooks/__tests__/use-chat-session.hosted.test.tsxmcpjam-inspector/client/src/hooks/hosted/use-hosted-api-context.tsmcpjam-inspector/client/src/hooks/use-chat-session.tsmcpjam-inspector/client/src/lib/__tests__/hosted-web-context.test.tsmcpjam-inspector/client/src/lib/__tests__/hosted-workspace.test.tsmcpjam-inspector/client/src/lib/__tests__/session-token.hosted-retry.test.tsmcpjam-inspector/client/src/lib/apis/mcp-skills-api.tsmcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.tsmcpjam-inspector/client/src/lib/apis/web/context.tsmcpjam-inspector/client/src/lib/guest-session.tsmcpjam-inspector/client/src/lib/hosted-workspace.tsmcpjam-inspector/client/src/lib/session-token.tsmcpjam-inspector/client/src/stores/traffic-log-store.tsmcpjam-inspector/server/app.tsmcpjam-inspector/server/index.tsmcpjam-inspector/server/middleware/bearer-auth.tsmcpjam-inspector/server/routes/mcp/__tests__/chat-v2.test.tsmcpjam-inspector/server/routes/mcp/chat-v2.tsmcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.tsmcpjam-inspector/server/routes/web/__tests__/guest-jwks.test.tsmcpjam-inspector/server/routes/web/__tests__/guest-session.test.tsmcpjam-inspector/server/routes/web/auth.tsmcpjam-inspector/server/routes/web/chat-v2.tsmcpjam-inspector/server/routes/web/guest-session.tsmcpjam-inspector/server/routes/web/index.tsmcpjam-inspector/server/services/__tests__/guest-token.test.tsmcpjam-inspector/server/services/guest-token.tsmcpjam-inspector/server/utils/__tests__/guest-auth.test.tsmcpjam-inspector/server/utils/__tests__/mcpjam-stream-handler.test.tsmcpjam-inspector/server/utils/convex-guest-auth-sync.tsmcpjam-inspector/server/utils/guest-auth.tsmcpjam-inspector/server/utils/guest-session-source.tsmcpjam-inspector/shared/types.ts
Comment on lines
+246
to
+258
| const directGuestMode = | ||
| HOSTED_MODE && | ||
| !isAuthenticated && | ||
| !isAuthLoading && | ||
| !hostedWorkspaceId && | ||
| !hostedShareToken; | ||
| const sharedGuestMode = | ||
| HOSTED_MODE && | ||
| !isAuthenticated && | ||
| !isAuthLoading && | ||
| !!hostedWorkspaceId && | ||
| !!hostedShareToken; | ||
| const guestMode = directGuestMode || sharedGuestMode; |
Contributor
There was a problem hiding this comment.
Don't use !hostedWorkspaceId as the direct-guest signal.
Here that value also means “workspace still resolving.” In that window we can bypass hostedContextNotReady, fall back to a guest token, and send a workspace-less hosted request on pages that should become workspace-scoped. Please distinguish “resolved with no workspace” from “not resolved yet” — for example null vs undefined, or an explicit guest-surface flag — and reuse that single predicate in the guest fallback and blocking logic.
Possible direction
+ const hostedWorkspaceResolved = hostedWorkspaceId !== undefined;
const directGuestMode =
HOSTED_MODE &&
!isAuthenticated &&
!isAuthLoading &&
- !hostedWorkspaceId &&
+ hostedWorkspaceResolved &&
+ hostedWorkspaceId === null &&
!hostedShareToken;Apply the same resolved/sentinel check in the guest-token fallback and hostedContextNotReady branches so undefined keeps the page blocked while workspace resolution is still in flight.
Also applies to: 360-363, 529-535, 751-756
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/hooks/use-chat-session.ts` around lines 246 -
258, The current guest-mode logic uses truthiness of hostedWorkspaceId (in
directGuestMode/sharedGuestMode and guestMode) which conflates "still resolving"
with "resolved-empty"; change this to explicitly distinguish unresolved
(undefined) vs resolved-empty (null or a sentinel) and reuse a single predicate
(e.g., isHostedWorkspaceResolved and isHostedWorkspaceEmpty) across
directGuestMode, sharedGuestMode, the guest-token fallback, and
hostedContextNotReady checks; update the checks that reference hostedWorkspaceId
and hostedShareToken and the functions/variables useChatSession (or the
hook-level variables directGuestMode, sharedGuestMode, guestMode,
hostedContextNotReady) so unresolved (undefined) keeps the page blocked while
only resolved-empty (null/sentinel) allows the guest fallback.
Comment on lines
+74
to
+92
| it("keeps using direct guest requests when AuthKit still reports a session", () => { | ||
| setHostedApiContext({ | ||
| workspaceId: null, | ||
| hasSession: true, | ||
| isAuthenticated: false, | ||
| serverIdsByName: {}, | ||
| serverConfigs: { | ||
| myServer: { | ||
| url: "https://example.com/mcp", | ||
| requestInit: { headers: { "X-Api-Key": "key123" } }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(buildHostedServerRequest("myServer")).toEqual({ | ||
| serverUrl: "https://example.com/mcp", | ||
| serverHeaders: { "X-Api-Key": "key123" }, | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Assert the auth accessor stays untouched in this guest-session branch.
Right now this only proves the returned shape. A mocked getAccessToken plus a negative call assertion would pin down the more important regression: hasSession must not re-enter the authenticated flow here.
🔍 Tighten the regression test
it("keeps using direct guest requests when AuthKit still reports a session", () => {
+ const getAccessToken = vi.fn(async () => "session-access-token");
+
setHostedApiContext({
workspaceId: null,
hasSession: true,
isAuthenticated: false,
+ getAccessToken,
serverIdsByName: {},
serverConfigs: {
myServer: {
url: "https://example.com/mcp",
requestInit: { headers: { "X-Api-Key": "key123" } },
@@
expect(buildHostedServerRequest("myServer")).toEqual({
serverUrl: "https://example.com/mcp",
serverHeaders: { "X-Api-Key": "key123" },
});
+ expect(getAccessToken).not.toHaveBeenCalled();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/lib/__tests__/hosted-web-context.test.ts` around
lines 74 - 92, The test should also assert that the auth accessor is not used
when hasSession is true but isAuthenticated is false: mock or spy on
getAccessToken (the function used by your auth accessor) before calling
setHostedApiContext/buildHostedServerRequest and verify getAccessToken was not
called; keep the existing expectation on buildHostedServerRequest("myServer") to
ensure the returned shape remains the same, but add the negative call assertion
on getAccessToken to prove we do not re-enter the authenticated flow.
Comment on lines
87
to
98
| vi.mock("@/shared/types", () => ({ | ||
| isGPT5Model: vi.fn().mockReturnValue(false), | ||
| isMCPJamProvidedModel: vi.fn().mockReturnValue(false), | ||
| isGuestAllowedModel: vi.fn().mockReturnValue(true), | ||
| })); | ||
|
|
||
| // Mock guest-auth to avoid needing real JWT keys in tests | ||
| vi.mock("../../../utils/guest-auth.js", () => ({ | ||
| getProductionGuestAuthHeader: vi | ||
| .fn() | ||
| .mockResolvedValue("Bearer mock-guest-token"), | ||
| })); |
Contributor
There was a problem hiding this comment.
Cover the new guest gate, not just mock past it.
These mocks force guest eligibility and guest-header acquisition to succeed for every MCPJam request, so the new 403 branch for disallowed models and the 503 branch for guest-auth acquisition failures remain unexercised here. Please add route assertions for both paths so the access policy itself is tested.
Based on learnings: All changes should include tests using Vitest.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/server/routes/mcp/__tests__/chat-v2.test.ts` around lines 87
- 98, Add two Vitest test cases in chat-v2.test.ts that exercise the guest gate:
one where isGuestAllowedModel returns false and you assert the route responds
with 403, and another where getProductionGuestAuthHeader is made to reject (or
throw) and you assert the route responds with 503; modify the existing vi.mock
for "@/shared/types" and "../../../utils/guest-auth.js" within those tests (via
vi.mockImplementationOnce or per-test mocking) so other tests remain unchanged,
and call the same route handler used by current tests to verify the
access-policy behavior for isGuestAllowedModel and getProductionGuestAuthHeader.
Comment on lines
+53
to
+65
| const body = rawBody as unknown as ChatV2Request & { | ||
| serverUrl?: string; | ||
| serverHeaders?: Record<string, string>; | ||
| oauthAccessToken?: string; | ||
| }; | ||
|
|
||
| const { | ||
| messages, | ||
| model, | ||
| systemPrompt, | ||
| temperature, | ||
| requireToolApproval, | ||
| } = body; |
Contributor
There was a problem hiding this comment.
Stop double-casting the guest payload.
This branch trusts rawBody via as ChatV2Request and only spot-checks messages plus model. Because the endpoint is public, malformed temperature, systemPrompt, or header fields can still leak into prepareChatV2() / convertToModelMessages() and turn bad input into 500s instead of clean 4xx responses. Parse the full guest body with a dedicated schema and use the parsed value downstream.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/server/routes/web/chat-v2.ts` around lines 53 - 65, The
handler is unsafe because rawBody is being double-cast to ChatV2Request (via
rawBody as unknown as ChatV2Request) so malformed temperature, systemPrompt,
headers, etc. can reach prepareChatV2() / convertToModelMessages() and cause
500s; fix this by defining and using a strict runtime schema (e.g., Zod/AJV) for
the full guest payload (fields: messages, model, systemPrompt, temperature,
requireToolApproval, serverUrl, serverHeaders, oauthAccessToken), parse/validate
rawBody with that schema at the start of the route, return a 4xx on validation
failure, and then use the validated result (not rawBody) when calling
prepareChatV2(), convertToModelMessages(), and elsewhere (refer to rawBody,
ChatV2Request, prepareChatV2, convertToModelMessages, messages, model,
systemPrompt, temperature).
Comment on lines
+20
to
+31
| export function getRemoteGuestSessionUrl(): string { | ||
| return ( | ||
| process.env.MCPJAM_GUEST_SESSION_URL || DEFAULT_REMOTE_GUEST_SESSION_URL | ||
| ); | ||
| } | ||
|
|
||
| export async function fetchRemoteGuestSession(): Promise<RemoteGuestSession | null> { | ||
| try { | ||
| const response = await fetch(getRemoteGuestSessionUrl(), { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Harden this remote guest-session call.
MCPJAM_GUEST_SESSION_URL is accepted verbatim and the outbound request has no deadline. That means a misconfigured http:// endpoint can leak bearer tokens, and a slow or unreachable upstream can leave guest-session requests hanging on the server. Please require HTTPS outside explicit loopback/dev URLs and fail fast with a timeout.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/server/utils/guest-session-source.ts` around lines 20 - 31,
Validate and normalize the MCPJAM_GUEST_SESSION_URL inside
getRemoteGuestSessionUrl and harden fetchRemoteGuestSession: parse the
configured URL and reject it unless its protocol is "https:" or it is an
explicit loopback/dev host (localhost, 127.0.0.1, ::1) to prevent accidental
downgrade to http; if invalid, throw and fall back to
DEFAULT_REMOTE_GUEST_SESSION_URL or fail fast. In fetchRemoteGuestSession, use
an AbortController with a short configurable timeout (e.g., 2–5s) to abort slow
requests and pass the controller.signal to fetch, and ensure any thrown errors
from parsing/timeout are caught and logged/handled consistently. Reference:
getRemoteGuestSessionUrl and fetchRemoteGuestSession.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
mcpjam-inspector/server/routes/web/chat-v2.ts (1)
53-66:⚠️ Potential issue | 🟠 MajorValidate the full guest payload before type-casting.
Lines 53-57 trust
rawBodyvia cast; onlymessages/modelare validated afterward. Invalidtemperature,requireToolApproval, or related fields can still reachprepareChatV2(Line 159) and fail as 5xx instead of clean 4xx.Suggested direction
- const body = rawBody as unknown as ChatV2Request & { - serverUrl?: string; - serverHeaders?: Record<string, string>; - oauthAccessToken?: string; - }; + // Parse with a strict guest schema and use parsed values only. + const body = parseWithSchema(guestChatSchema, rawBody);Also applies to: 159-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/web/chat-v2.ts` around lines 53 - 66, The handler currently casts rawBody to ChatV2Request and then destructures fields, but only validates messages/model later; you must perform full runtime validation of the guest payload (rawBody) before casting and before calling prepareChatV2: validate types and ranges for messages, model, systemPrompt (string), temperature (number within allowed range), requireToolApproval (boolean), and optional serverUrl/serverHeaders/oauthAccessToken shapes; if any field is missing or invalid return a 400 with a clear error. Update the code around the rawBody casting (where ChatV2Request is assigned) to use a defensive guard/validator and add the same checks right before prepareChatV2 (referencing prepareChatV2, rawBody, ChatV2Request, temperature, requireToolApproval, serverUrl, serverHeaders, oauthAccessToken) so malformed requests never reach business logic and produce 4xx responses.mcpjam-inspector/server/services/guest-token.ts (1)
246-275:⚠️ Potential issue | 🟠 MajorDo not replace last-good JWKS cache with an empty keyset.
When parsing succeeds but yields no usable keys, Lines 270-274 still overwrite
hostedGuestPublicKeysCache. A transient 200-with-bad-body can evict valid keys and break hosted guest token validation.Suggested patch
- hostedGuestPublicKeysCache = { - fetchedAt: now, - keysByKid, - fallbackKey, - }; + if (keysByKid.size === 0 && !fallbackKey) { + logger.warn("[guest-auth] Hosted JWKS contained no usable keys; keeping last good cache"); + return resolveKeyFromCache(kid); + } + + hostedGuestPublicKeysCache = { + fetchedAt: now, + keysByKid, + fallbackKey, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/services/guest-token.ts` around lines 246 - 275, The code currently overwrites hostedGuestPublicKeysCache even when parsing yields no usable keys; change the logic after building keysByKid/fallbackKey so you only assign hostedGuestPublicKeysCache when you actually found a usable key (e.g., keysByKid.size > 0 or fallbackKey !== null). Locate the block that constructs keysByKid/fallbackKey (uses createPublicKey and iterates over keys) and wrap the hostedGuestPublicKeysCache = { fetchedAt: now, keysByKid, fallbackKey } assignment in a conditional that prevents replacing the existing cache when no valid keys were produced.mcpjam-inspector/client/src/lib/guest-session.ts (1)
71-74:⚠️ Potential issue | 🟠 MajorInvalidate login clears the same way you invalidate 401 refreshes.
sessionGenerationonly changes inforceRefreshGuestSession(). IfclearGuestSession()runs during login while an oldergetOrCreateGuestSession()is still in flight, the Line 72 guard still passes and that older response can write the guest token back into storage after the clear. That preserves guest identity across auth transitions. Please route both paths through the same invalidation helper so every logical clear bumps the generation and drops the in-flight state.Possible fix
+function invalidateGuestSession(): void { + sessionGeneration++; + localStorage.removeItem(STORAGE_KEY); + inFlightRequest = null; + forceRefreshInFlight = null; +} + export function clearGuestSession(): void { - localStorage.removeItem(STORAGE_KEY); + invalidateGuestSession(); } ... - clearGuestSession(); - sessionGeneration++; - inFlightRequest = null; + invalidateGuestSession();Also applies to: 129-131
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/guest-session.ts` around lines 71 - 74, The guard using sessionGeneration in getOrCreateGuestSession/writeToStorage allows a race where clearGuestSession can be bypassed; update clearGuestSession to call the same invalidation helper used by forceRefreshGuestSession (the routine that bumps sessionGeneration) and modify any direct clears to route through that helper so both clearGuestSession() and forceRefreshGuestSession() increment sessionGeneration before wiping storage; ensure getOrCreateGuestSession() still checks sessionGeneration === generation before writeToStorage(session) so any in-flight responses are dropped after the shared invalidation.mcpjam-inspector/client/src/hooks/use-chat-session.ts (1)
246-258:⚠️ Potential issue | 🟠 MajorDon’t treat “workspace still resolving” as direct guest mode.
!hostedWorkspaceIdstill conflatesundefinedandnull. In the unresolved window, an unauthenticated hosted page is classified asdirectGuestMode, which then spills into model gating, the guest-token fallback,buildHostedBody(), andhostedContextNotReady. The result is a workspace-less guest request path on pages that should stay blocked until the hosted workspace finishes resolving.Possible fix
+ const hostedWorkspaceResolved = hostedWorkspaceId !== undefined; + const hasHostedWorkspace = + hostedWorkspaceResolved && hostedWorkspaceId !== null; const directGuestMode = HOSTED_MODE && !isAuthenticated && !isAuthLoading && - !hostedWorkspaceId && + hostedWorkspaceResolved && + !hasHostedWorkspace && !hostedShareToken; const sharedGuestMode = HOSTED_MODE && !isAuthenticated && !isAuthLoading && - !!hostedWorkspaceId && + hasHostedWorkspace && !!hostedShareToken; const guestMode = directGuestMode || sharedGuestMode; ... - if (!hostedWorkspaceId) { + if (!hasHostedWorkspace) { return {}; } ... - (!hostedWorkspaceId || !!hostedShareToken) + hostedWorkspaceResolved && + (!hasHostedWorkspace || !!hostedShareToken) ... - (!hostedWorkspaceId || + (!hasHostedWorkspace || (selectedServers.length > 0 && hostedSelectedServerIds.length !== selectedServers.length));Also applies to: 273-283, 360-363, 529-535, 747-752
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/hooks/use-chat-session.ts` around lines 246 - 258, The code treats an unresolved hostedWorkspaceId (undefined) the same as an explicitly absent workspace (null), causing pages still resolving to be classified as direct guest; update the checks so only explicit null means "no workspace" and presence checks accept non-null values: replace occurrences of !hostedWorkspaceId with hostedWorkspaceId === null and replace !!hostedWorkspaceId with hostedWorkspaceId != null (do the same for hostedShareToken), and apply the same change to the other occurrences (lines referenced around directGuestMode/sharedGuestMode and the other listed ranges) so unresolved/undefined stays blocked until resolution.
🧹 Nitpick comments (3)
mcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.ts (1)
12-18: Unused import:buildGuestServerRequestThis import is never exercised within the test file.
🧹 Remove unused import
import { getHostedAuthorizationHeader, setHostedApiContext, isGuestMode, buildHostedServerRequest, - buildGuestServerRequest, } from "../context";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.ts` around lines 12 - 18, The test file imports buildGuestServerRequest but never uses it; remove buildGuestServerRequest from the import list in the test to eliminate the unused-import. Locate the import statement that currently lists getHostedAuthorizationHeader, setHostedApiContext, isGuestMode, buildHostedServerRequest, buildGuestServerRequest from "../context" and delete only the buildGuestServerRequest identifier (and any trailing comma if needed) so the remaining symbols are unchanged.mcpjam-inspector/server/routes/mcp/chat-v2.ts (1)
143-147: Log suppressed guest-auth acquisition errors.Line 145 swallows exceptions from
getProductionGuestAuthHeader(). In this auth path, a warning log will materially improve 503 triage.Suggested patch
- try { - authHeader = (await getProductionGuestAuthHeader()) ?? undefined; - } catch { - authHeader = undefined; - } + try { + authHeader = (await getProductionGuestAuthHeader()) ?? undefined; + } catch (error) { + logger.warn("[mcp/chat-v2] failed to acquire guest auth header", error); + authHeader = undefined; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/mcp/chat-v2.ts` around lines 143 - 147, The try/catch around getProductionGuestAuthHeader() is swallowing errors; update the catch to capture the thrown error (e.g., catch (err)) and emit a warning including that error when setting authHeader to undefined so failed guest-auth acquisitions are visible in logs—use the module's logger (or console.warn) to log a clear message that includes err and context (e.g., "failed to acquire production guest auth header").mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts (1)
54-54: Align this route test with the shared server helper set.Line 54 uses a web-local helper import. Please switch to the shared server route helpers under
server/routes/mcp/__tests__/helpers/to keep route-test setup patterns consistent.As per coding guidelines:
mcpjam-inspector/server/routes/**/__tests__/*.test.ts: Use server test helpers fromserver/routes/mcp/__tests__/helpers/such ascreateTestApp(),createMockMcpClientManager(),postJson(),expectError()in server route tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts` at line 54, The test currently imports web-local helpers (createWebTestApp, expectJson, postJson) from "./helpers/test-app.js"; update the import to use the shared server route test helpers instead (replace with createTestApp, createMockMcpClientManager, postJson, expectError as needed) and adapt any calls to those functions in this file to the shared API (e.g., swap createWebTestApp(...) → createTestApp(...), adjust mock client setup via createMockMcpClientManager() and replace expectJson() assertions with expectError()/postJson() from the shared helpers) so the test matches the common patterns used by server/routes/mcp/__tests__/helpers/.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@mcpjam-inspector/client/src/hooks/use-chat-session.ts`:
- Around line 246-258: The code treats an unresolved hostedWorkspaceId
(undefined) the same as an explicitly absent workspace (null), causing pages
still resolving to be classified as direct guest; update the checks so only
explicit null means "no workspace" and presence checks accept non-null values:
replace occurrences of !hostedWorkspaceId with hostedWorkspaceId === null and
replace !!hostedWorkspaceId with hostedWorkspaceId != null (do the same for
hostedShareToken), and apply the same change to the other occurrences (lines
referenced around directGuestMode/sharedGuestMode and the other listed ranges)
so unresolved/undefined stays blocked until resolution.
In `@mcpjam-inspector/client/src/lib/guest-session.ts`:
- Around line 71-74: The guard using sessionGeneration in
getOrCreateGuestSession/writeToStorage allows a race where clearGuestSession can
be bypassed; update clearGuestSession to call the same invalidation helper used
by forceRefreshGuestSession (the routine that bumps sessionGeneration) and
modify any direct clears to route through that helper so both
clearGuestSession() and forceRefreshGuestSession() increment sessionGeneration
before wiping storage; ensure getOrCreateGuestSession() still checks
sessionGeneration === generation before writeToStorage(session) so any in-flight
responses are dropped after the shared invalidation.
In `@mcpjam-inspector/server/routes/web/chat-v2.ts`:
- Around line 53-66: The handler currently casts rawBody to ChatV2Request and
then destructures fields, but only validates messages/model later; you must
perform full runtime validation of the guest payload (rawBody) before casting
and before calling prepareChatV2: validate types and ranges for messages, model,
systemPrompt (string), temperature (number within allowed range),
requireToolApproval (boolean), and optional
serverUrl/serverHeaders/oauthAccessToken shapes; if any field is missing or
invalid return a 400 with a clear error. Update the code around the rawBody
casting (where ChatV2Request is assigned) to use a defensive guard/validator and
add the same checks right before prepareChatV2 (referencing prepareChatV2,
rawBody, ChatV2Request, temperature, requireToolApproval, serverUrl,
serverHeaders, oauthAccessToken) so malformed requests never reach business
logic and produce 4xx responses.
In `@mcpjam-inspector/server/services/guest-token.ts`:
- Around line 246-275: The code currently overwrites hostedGuestPublicKeysCache
even when parsing yields no usable keys; change the logic after building
keysByKid/fallbackKey so you only assign hostedGuestPublicKeysCache when you
actually found a usable key (e.g., keysByKid.size > 0 or fallbackKey !== null).
Locate the block that constructs keysByKid/fallbackKey (uses createPublicKey and
iterates over keys) and wrap the hostedGuestPublicKeysCache = { fetchedAt: now,
keysByKid, fallbackKey } assignment in a conditional that prevents replacing the
existing cache when no valid keys were produced.
---
Nitpick comments:
In
`@mcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.ts`:
- Around line 12-18: The test file imports buildGuestServerRequest but never
uses it; remove buildGuestServerRequest from the import list in the test to
eliminate the unused-import. Locate the import statement that currently lists
getHostedAuthorizationHeader, setHostedApiContext, isGuestMode,
buildHostedServerRequest, buildGuestServerRequest from "../context" and delete
only the buildGuestServerRequest identifier (and any trailing comma if needed)
so the remaining symbols are unchanged.
In `@mcpjam-inspector/server/routes/mcp/chat-v2.ts`:
- Around line 143-147: The try/catch around getProductionGuestAuthHeader() is
swallowing errors; update the catch to capture the thrown error (e.g., catch
(err)) and emit a warning including that error when setting authHeader to
undefined so failed guest-auth acquisitions are visible in logs—use the module's
logger (or console.warn) to log a clear message that includes err and context
(e.g., "failed to acquire production guest auth header").
In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts`:
- Line 54: The test currently imports web-local helpers (createWebTestApp,
expectJson, postJson) from "./helpers/test-app.js"; update the import to use the
shared server route test helpers instead (replace with createTestApp,
createMockMcpClientManager, postJson, expectError as needed) and adapt any calls
to those functions in this file to the shared API (e.g., swap
createWebTestApp(...) → createTestApp(...), adjust mock client setup via
createMockMcpClientManager() and replace expectJson() assertions with
expectError()/postJson() from the shared helpers) so the test matches the common
patterns used by server/routes/mcp/__tests__/helpers/.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 70d34595-6761-47de-b610-0f2a20081388
📒 Files selected for processing (8)
mcpjam-inspector/client/src/hooks/use-chat-session.tsmcpjam-inspector/client/src/lib/apis/web/__tests__/context.guest-fallback.test.tsmcpjam-inspector/client/src/lib/guest-session.tsmcpjam-inspector/server/routes/mcp/chat-v2.tsmcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.tsmcpjam-inspector/server/routes/web/__tests__/guest-jwks.test.tsmcpjam-inspector/server/routes/web/chat-v2.tsmcpjam-inspector/server/services/guest-token.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- mcpjam-inspector/server/routes/web/tests/guest-jwks.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
mcpjam-inspector/client/src/lib/apis/web/context.ts (1)
37-58: Consider validating token expiration before returning.This function retrieves
access_tokenfrom localStorage but ignores theexpires_infield present in the stored structure (seeuse-server-state.ts:896-905). A stale token may be returned and used, only to fail at request time.🔧 Possible expiration check
function readStoredGuestOAuthAccessToken( serverName: string, ): string | undefined { if (typeof window === "undefined") return undefined; try { const raw = localStorage.getItem(`mcp-tokens-${serverName}`); if (!raw) return undefined; - const parsed = JSON.parse(raw) as { access_token?: unknown }; + const parsed = JSON.parse(raw) as { + access_token?: unknown; + expires_in?: number; + created_at?: number; + }; + + // Skip expired tokens if timestamp metadata is available + if (parsed.created_at && parsed.expires_in) { + const expiresAt = parsed.created_at + parsed.expires_in * 1000; + if (Date.now() >= expiresAt) return undefined; + } + if ( typeof parsed.access_token === "string" && parsed.access_token.trim().length > 0 ) { return parsed.access_token; } } catch { // Ignore malformed localStorage data and fall back to in-memory context. } return undefined; }Note: This requires the write side (
use-server-state.ts) to also persistcreated_at. Alternatively, treat localStorage tokens as best-effort and rely on 401 retry to refresh.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/lib/apis/web/context.ts` around lines 37 - 58, readStoredGuestOAuthAccessToken currently returns any stored access_token without checking expiry; update it to validate the token's lifetime (use the stored expires_in and created_at fields written by use-server-state.ts) by parsing parsed.expires_in and parsed.created_at (or deriving created_at if your write-side uses a different name), computing whether Date.now() (or current epoch seconds) is past created_at + expires_in, and if expired return undefined; keep existing try/catch behavior and fall back to undefined so callers can rely on in-memory context or 401 refresh logic.mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts (1)
203-214: Please cover the one-server direct-guest branch.Line 205 and Line 213 hardcode
selectedServersto[], so this suite never exercises the new path that lets an unauthenticated guest attach one validated MCP server. Add one case with a single selected server and assert it reachesprepareChatV2/handleMCPJamFreeChatModelonly after guest auth succeeds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts` around lines 203 - 214, Add a test that exercises the one-server direct-guest branch by simulating a successful guest auth and passing a single validated MCP server in selectedServers; update the assertions for prepareChatV2Mock and handleMCPJamFreeChatModelMock to expect selectedServers: [{ id: ..., url: ... }] (or the same server object used in the test) instead of [], and assert those mocks are invoked only after the guest auth flow completes (e.g., ensure guest auth mock was called and resolved before checking prepareChatV2Mock/handleMCPJamFreeChatModelMock). Target the existing mocks prepareChatV2Mock and handleMCPJamFreeChatModelMock and reuse the test setup for the guest auth call to guarantee the one-server path is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/lib/apis/web/context.ts`:
- Around line 76-94: hasHostedGuestAccess currently ignores the
hostedApiContext.hasSession flag, allowing a race where hasSession===true but
isAuthenticated===false leads to guest bearer preference; update
hasHostedGuestAccess to return false when hostedApiContext.hasSession is true
(i.e., treat pending WorkOS sessions as authenticated for the purpose of
preferring non-guest tokens) and keep shouldPreferGuestBearer delegating to
hasHostedGuestAccess so the single source of truth is updated.
In `@mcpjam-inspector/server/app.ts`:
- Line 59: syncGuestAuthConfigToConvex() is constructing a JWKS URL using the
wrong path (/guest/jwks) which doesn't match the registered endpoint
(/api/web/guest-jwks); update the JWKS URL construction in
convex-guest-auth-sync.ts (the code that currently uses '/guest/jwks') to point
to '/api/web/guest-jwks' (or derive the path from the same mounting logic used
in routes/web/index.ts and app.ts) so Convex fetches the correct endpoint for
guest JWT verification.
In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts`:
- Around line 218-229: The test "accepts a hosted guest token in development
when local signing is disabled" installs a fetch mock but never asserts it was
used; update the test (the it(...) block that calls signHostedGuestToken and
stubs global.fetch) to assert that global.fetch was called exactly once and with
process.env.MCPJAM_GUEST_JWKS_URL (or the literal
"https://app.mcpjam.com/api/web/guest-jwks") so the hosted-JWKS round trip is
verified; place this assertion after the request/handler invocation (where the
stream handler stub runs) and before test teardown to ensure the guest validator
actually fetched the JWKS.
---
Nitpick comments:
In `@mcpjam-inspector/client/src/lib/apis/web/context.ts`:
- Around line 37-58: readStoredGuestOAuthAccessToken currently returns any
stored access_token without checking expiry; update it to validate the token's
lifetime (use the stored expires_in and created_at fields written by
use-server-state.ts) by parsing parsed.expires_in and parsed.created_at (or
deriving created_at if your write-side uses a different name), computing whether
Date.now() (or current epoch seconds) is past created_at + expires_in, and if
expired return undefined; keep existing try/catch behavior and fall back to
undefined so callers can rely on in-memory context or 401 refresh logic.
In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts`:
- Around line 203-214: Add a test that exercises the one-server direct-guest
branch by simulating a successful guest auth and passing a single validated MCP
server in selectedServers; update the assertions for prepareChatV2Mock and
handleMCPJamFreeChatModelMock to expect selectedServers: [{ id: ..., url: ... }]
(or the same server object used in the test) instead of [], and assert those
mocks are invoked only after the guest auth flow completes (e.g., ensure guest
auth mock was called and resolved before checking
prepareChatV2Mock/handleMCPJamFreeChatModelMock). Target the existing mocks
prepareChatV2Mock and handleMCPJamFreeChatModelMock and reuse the test setup for
the guest auth call to guarantee the one-server path is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d29b53bd-eb4c-40fd-9a0b-4e0e613f14af
📒 Files selected for processing (14)
mcpjam-inspector/client/src/App.tsxmcpjam-inspector/client/src/components/ChatTabV2.tsxmcpjam-inspector/client/src/hooks/hosted/use-hosted-api-context.tsmcpjam-inspector/client/src/hooks/use-chat-session.tsmcpjam-inspector/client/src/lib/__tests__/hosted-web-context.test.tsmcpjam-inspector/client/src/lib/apis/web/context.tsmcpjam-inspector/client/src/main.tsxmcpjam-inspector/server/app.tsmcpjam-inspector/server/index.tsmcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.tsmcpjam-inspector/server/routes/web/auth.tsmcpjam-inspector/server/routes/web/chat-v2.tsmcpjam-inspector/server/routes/web/index.tsmcpjam-inspector/server/utils/__tests__/mcpjam-stream-handler.test.ts
✅ Files skipped from review due to trivial changes (1)
- mcpjam-inspector/client/src/main.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- mcpjam-inspector/client/src/components/ChatTabV2.tsx
- mcpjam-inspector/client/src/App.tsx
- mcpjam-inspector/client/src/lib/tests/hosted-web-context.test.ts
- mcpjam-inspector/server/utils/tests/mcpjam-stream-handler.test.ts
- mcpjam-inspector/server/routes/web/index.ts
Comment on lines
+76
to
+94
| /** | ||
| * Hosted guest access comes in 2 shapes: | ||
| * - direct guest: no workspace, direct serverUrl requests | ||
| * - shared guest: workspace-scoped share token, Convex-backed requests | ||
| */ | ||
| function hasHostedGuestAccess(): boolean { | ||
| if (!HOSTED_MODE) return false; | ||
| if (hostedApiContext.isAuthenticated) return false; | ||
| return !hostedApiContext.workspaceId || !!hostedApiContext.shareToken; | ||
| } | ||
|
|
||
| /** | ||
| * Prefer the guest bearer for both direct guests and shared guests. | ||
| * Shared guests still use Convex-backed requests; they only differ in how the | ||
| * bearer is obtained. | ||
| */ | ||
| function shouldPreferGuestBearer(): boolean { | ||
| return hasHostedGuestAccess(); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other usages of hasSession to confirm it's truly unused in decision logic
rg -n 'hasSession' --type=ts -C2Repository: MCPJam/inspector
Length of output: 5460
🏁 Script executed:
#!/bin/bash
# Read the context.ts file to see all usages of hasSession and hostedApiContext
wc -l mcpjam-inspector/client/src/lib/apis/web/context.tsRepository: MCPJam/inspector
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Read the context.ts file to understand the full scope
cat -n mcpjam-inspector/client/src/lib/apis/web/context.tsRepository: MCPJam/inspector
Length of output: 12315
🏁 Script executed:
#!/bin/bash
# Search for any conditional checks using hostedApiContext.hasSession
rg -n 'hostedApiContext\.hasSession' --type=ts -C3Repository: MCPJam/inspector
Length of output: 42
hasSession is stored but never consulted—race window remains open.
The newly added hasSession flag (Line 16) is documented as "True when a WorkOS session exists, even if token hasn't resolved yet." However, neither hasHostedGuestAccess() nor shouldPreferGuestBearer() consult it:
function hasHostedGuestAccess(): boolean {
if (hostedApiContext.isAuthenticated) return false; // only checks isAuthenticated
...
}During the race window where hasSession === true but isAuthenticated === false, the system incorrectly prefers guest bearer tokens over the pending WorkOS session, allowing unauthorized token bypass.
🛠️ Suggested fix
function hasHostedGuestAccess(): boolean {
if (!HOSTED_MODE) return false;
- if (hostedApiContext.isAuthenticated) return false;
+ if (hostedApiContext.isAuthenticated || hostedApiContext.hasSession) return false;
return !hostedApiContext.workspaceId || !!hostedApiContext.shareToken;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/lib/apis/web/context.ts` around lines 76 - 94,
hasHostedGuestAccess currently ignores the hostedApiContext.hasSession flag,
allowing a race where hasSession===true but isAuthenticated===false leads to
guest bearer preference; update hasHostedGuestAccess to return false when
hostedApiContext.hasSession is true (i.e., treat pending WorkOS sessions as
authenticated for the purpose of preferring non-guest tokens) and keep
shouldPreferGuestBearer delegating to hasHostedGuestAccess so the single source
of truth is updated.
|
|
||
| // Initialize RS256 key pair for guest JWTs | ||
| initGuestTokenSecret(); | ||
| syncGuestAuthConfigToConvex(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the JWKS path mismatch between sync config and actual endpoint
echo "=== Path in convex-guest-auth-sync.ts ==="
rg -n "guestJwksUrl" mcpjam-inspector/server/utils/convex-guest-auth-sync.ts
echo ""
echo "=== Actual JWKS endpoint registration ==="
rg -n "guest-jwks" mcpjam-inspector/server/routes/web/index.tsRepository: MCPJam/inspector
Length of output: 279
🏁 Script executed:
sed -n '30,40p' mcpjam-inspector/server/utils/convex-guest-auth-sync.tsRepository: MCPJam/inspector
Length of output: 410
🏁 Script executed:
sed -n '1,100p' mcpjam-inspector/server/app.ts | grep -A 5 -B 5 "web"Repository: MCPJam/inspector
Length of output: 565
🏁 Script executed:
rg -n "webRoutes" mcpjam-inspector/server/app.tsRepository: MCPJam/inspector
Length of output: 149
JWKS URL path mismatch will break guest token verification.
The syncGuestAuthConfigToConvex() function constructs a JWKS URL with path /guest/jwks (line 34 of convex-guest-auth-sync.ts), but the actual endpoint is /api/web/guest-jwks (registered at line 43 of routes/web/index.ts and mounted at /api/web in app.ts line 184). Convex will fetch from the incorrect path, causing guest JWT verification to fail.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/server/app.ts` at line 59, syncGuestAuthConfigToConvex() is
constructing a JWKS URL using the wrong path (/guest/jwks) which doesn't match
the registered endpoint (/api/web/guest-jwks); update the JWKS URL construction
in convex-guest-auth-sync.ts (the code that currently uses '/guest/jwks') to
point to '/api/web/guest-jwks' (or derive the path from the same mounting logic
used in routes/web/index.ts and app.ts) so Convex fetches the correct endpoint
for guest JWT verification.
Comment on lines
+218
to
+229
| it("accepts a hosted guest token in development when local signing is disabled", async () => { | ||
| process.env.NODE_ENV = "development"; | ||
| process.env.MCPJAM_USE_LOCAL_GUEST_SIGNING = "false"; | ||
| process.env.MCPJAM_GUEST_JWKS_URL = | ||
| "https://app.mcpjam.com/api/web/guest-jwks"; | ||
| const { token, jwks } = signHostedGuestToken(); | ||
| global.fetch = vi.fn().mockResolvedValue( | ||
| new Response(JSON.stringify(jwks), { | ||
| status: 200, | ||
| headers: { "Content-Type": "application/json" }, | ||
| }), | ||
| ) as typeof fetch; |
Contributor
There was a problem hiding this comment.
Assert the hosted-JWKS round trip.
Line 224 installs a fetch mock, but Lines 247-253 never prove the guest validator actually used it. A broader auth bypass would still leave this test green because the stream handler is already stubbed to return 200. Please assert a single fetch to MCPJAM_GUEST_JWKS_URL.
🔍 Suggested tightening
- global.fetch = vi.fn().mockResolvedValue(
+ const fetchMock = vi.fn().mockResolvedValue(
new Response(JSON.stringify(jwks), {
status: 200,
headers: { "Content-Type": "application/json" },
}),
- ) as typeof fetch;
+ );
+ global.fetch = fetchMock as typeof fetch;
@@
expect(response.status).toBe(200);
expect(await response.text()).toBe("ok");
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ expect(String(fetchMock.mock.calls[0]?.[0])).toBe(
+ "https://app.mcpjam.com/api/web/guest-jwks",
+ );
expect(handleMCPJamFreeChatModelMock).toHaveBeenCalledWith(
expect.objectContaining({
authHeader: `Bearer ${token}`,
}),
);Also applies to: 247-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/server/routes/web/__tests__/chat-v2.guest.test.ts` around
lines 218 - 229, The test "accepts a hosted guest token in development when
local signing is disabled" installs a fetch mock but never asserts it was used;
update the test (the it(...) block that calls signHostedGuestToken and stubs
global.fetch) to assert that global.fetch was called exactly once and with
process.env.MCPJAM_GUEST_JWKS_URL (or the literal
"https://app.mcpjam.com/api/web/guest-jwks") so the hosted-JWKS round trip is
verified; place this assertion after the request/handler invocation (where the
stream handler stub runs) and before test teardown to ensure the guest validator
actually fetched the JWKS.
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
2 participants
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.
Note
High Risk
Touches authentication/authorization paths for both hosted and non-hosted chat, including guest JWT issuance/validation, bearer selection, and 401 retry behavior. Misclassification or token/JWKS misconfiguration could break access or unintentionally broaden guest capabilities.
Overview
Hardens guest access across hosted + local runtimes. Adds explicit direct guest and shared-chat guest modes, with stricter workspace ID resolution (clearing stale workspace IDs when signed out) and model gating so unauthenticated users only see/submit guest-allowed MCPJam models.
Hosted web auth plumbing is updated for reliability.
useHostedApiContextnow carrieshasSession, hosted context prefers guest bearer tokens on guest-capable surfaces, and guest server requests prefer persisted OAuth access tokens fromlocalStorage.authFetch401 retry logic can now refresh when the failing request used a persisted guest token.Server-side guest JWT + JWKS flow is reworked. JWKS is moved under
GET /api/web/guest-jwks(avoiding SPA static interception) and guest token validation is upgraded to support hosted JWKS verification when local signing is disabled. Guest session issuance can be proxied to the hosted endpoint via config, local dev guest signing keys are persisted to disk, and a dev-only sync pushes guest key/JWKS settings to Convex.Web
POST /api/web/chat-v2gains a dedicated direct-guest path. Guest requests (noworkspaceId) are validated via guest bearer middleware, optionally connect to a validated single MCP server, and stream hosted-model responses without Convex authorization; authenticated/Convex-backed requests continue on the existing path. Non-hostedPOST /api/mcp/chat-v2now injects a production guest bearer when callers omit auth for guest-allowed models, otherwise returns 403.Written by Cursor Bugbot for commit c46cdd0. This will update automatically on new commits. Configure here.