From bffc8b3d1f3247440097f249000e2a420490d55f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 25 Aug 2026 21:12:43 -0700 Subject: [PATCH] fix(chat): conceal a missing deployment the way an unreachable one is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The internal chat error policy rewrites a cross-tenant authorization failure to `Chat not found or access denied`, and says in its own doc that a missing deployment and an unreachable one must stay indistinguishable. The domain answers an absent deployment with its own wording, though, and the policy passed that through — so the editor received `Chat deployment not found` for a deployment that is not there and `Chat not found or access denied` for one it may not reach. Two 404s a caller can tell apart is the existence oracle the concealment exists to close, and because the legacy `code` is derived from the message, it leaked on both fields. Every `not_found` reachable through this policy means the same thing, so the projection now renders all of them as the concealed message. The two existing 404 tests asserted only the status, which is how the bodies drifted apart unnoticed; the new one compares the two responses. --- apps/sim/app/api/chat/error-policy.ts | 22 +++++++++++-- .../app/api/chat/manage/[id]/route.test.ts | 32 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/apps/sim/app/api/chat/error-policy.ts b/apps/sim/app/api/chat/error-policy.ts index c1ae0655ee3..e053a89626e 100644 --- a/apps/sim/app/api/chat/error-policy.ts +++ b/apps/sim/app/api/chat/error-policy.ts @@ -37,9 +37,27 @@ export function createInternalChatDeploymentErrorPolicy(fallback: string): Inter } const classified = asOrchestrationError(error) if (!classified) return null + /** + * A not-found is concealed wherever it came from, not only when the + * concealment policy rewrites an authorization failure into one. + * + * The domain answers an absent deployment with its own wording, so the + * editor received `Chat deployment not found` for a deployment that is + * not there and `Chat not found or access denied` for one it may not + * reach — two 404s a caller can tell apart, which is the existence + * oracle this policy exists to close. The `code` is derived from the + * message, so leaving the message alone leaked it twice over. + * + * Every `not_found` reachable here means the same thing: the chat + * deployment is not available to this caller. None of them carries a + * distinction worth preserving at the cost of the one they must not + * make. + */ + const message = + classified.code === 'not_found' ? CHAT_NOT_FOUND_MESSAGE : classified.message return internalErrorResponse(statusForOrchestrationError(classified.code), { - error: classified.message, - code: legacyCode(classified.message), + error: message, + code: legacyCode(message), }) }, unhandled() { diff --git a/apps/sim/app/api/chat/manage/[id]/route.test.ts b/apps/sim/app/api/chat/manage/[id]/route.test.ts index 3236bb444e6..51d0d94b0f0 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.test.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.test.ts @@ -242,6 +242,38 @@ describe('internal chat deployment routes', () => { expect(response.status).toBe(404) }) + /** + * Both tests above assert only the status, which is what let the two 404s + * drift apart: the domain answered an absent deployment with its own + * wording while the concealment policy rewrote an unreachable one, so the + * body — and the `code` derived from it — told a caller which of the two it + * had hit. Comparing the responses is the assertion that keeps them one + * answer. + */ + it('answers a missing and an unreachable deployment identically', async () => { + mocks.getChatDeploymentWithWorkspace.mockResolvedValue(null) + const missing = await GET( + new NextRequest(`http://localhost:3000/api/chat/manage/${CHAT_ID}`), + params + ) + const missingBody = await missing.json() + + mocks.getChatDeploymentWithWorkspace.mockResolvedValue({ + chat: chatRow(), + workspaceId: WORKSPACE_ID, + }) + mocks.resolvePermission.mockResolvedValue(null) + const unreachable = await GET( + new NextRequest(`http://localhost:3000/api/chat/manage/${CHAT_ID}`), + params + ) + const unreachableBody = await unreachable.json() + + expect(missing.status).toBe(unreachable.status) + expect(missingBody).toEqual(unreachableBody) + expect(missingBody.error).toBe('Chat not found or access denied') + }) + it('refuses a workspace member below admin the gate configuration', async () => { mocks.resolvePermission.mockResolvedValue('read')