mirror of
https://github.com/simstudioai/sim.git
synced 2026-08-30 17:05:18 +08:00
fix(chat): conceal a missing deployment the way an unreachable one is (#7100)
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.
This commit is contained in:
@@ -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() {
|
||||
|
||||
@@ -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')
|
||||
|
||||
|
||||
Reference in New Issue
Block a user