fix(credentials): conceal inaccessible credential reads (#6730)

This commit is contained in:
Theodore Li
2026-08-15 12:46:59 -04:00
committed by GitHub
parent 337a53f12c
commit c269e883bf
5 changed files with 95 additions and 3 deletions
@@ -0,0 +1,58 @@
/**
* @vitest-environment node
*/
import { authMockFns, createMockRequest } from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { CredentialAccessRequiredError } from '@/lib/credentials/application/authorized-credential-use-case'
const mocks = vi.hoisted(() => ({
read: vi.fn(),
update: vi.fn(),
remove: vi.fn(),
}))
vi.mock('@/lib/credentials/application/credential-crud', () => ({
CredentialProviderOperationError: class CredentialProviderOperationError extends Error {},
getWorkspaceCredentialUseCase: {
operation: { id: 'credentials.read' },
execute: mocks.read,
},
updateWorkspaceCredentialUseCase: {
operation: { id: 'credentials.update' },
execute: mocks.update,
},
}))
vi.mock('@/lib/credentials/application/service-account', () => ({
deleteCredentialUseCase: {
operation: { id: 'credentials.delete' },
execute: mocks.remove,
},
}))
import { GET } from '@/app/api/credentials/[id]/route'
const CREDENTIAL_ID = 'credential-1'
const routeContext = { params: Promise.resolve({ id: CREDENTIAL_ID }) }
describe('GET /api/credentials/[id]', () => {
beforeEach(() => {
vi.clearAllMocks()
authMockFns.mockGetSession.mockResolvedValue({
user: { id: 'writer-1' },
session: { id: 'session-1' },
})
})
it('preserves the generic denial for a workspace writer without credential access', async () => {
mocks.read.mockRejectedValue(new CredentialAccessRequiredError())
const response = await GET(
createMockRequest('GET', undefined, {}, `http://localhost/api/credentials/${CREDENTIAL_ID}`),
routeContext
)
expect(response.status).toBe(403)
expect(await response.json()).toEqual({ error: 'Forbidden' })
})
})
+2 -1
View File
@@ -10,6 +10,7 @@ import {
} from '@/lib/api/server/routes'
import {
credentialValidationParseOptions,
internalCredentialDetailErrorPolicy,
internalCredentialErrorPolicy,
} from '@/lib/credentials/api/route-policies'
import {
@@ -27,7 +28,7 @@ export const GET = defineInternalJsonRoute({
auth: internalSessionAuth,
operation: credentialOperations.read,
rateLimit,
errorPolicy: internalCredentialErrorPolicy,
errorPolicy: internalCredentialDetailErrorPolicy,
parseOptions: credentialValidationParseOptions,
mapInput: ({ params }) => ({ credentialId: params.id }),
useCase: getWorkspaceCredentialUseCase,
@@ -7,6 +7,7 @@ import { getValidationErrorMessage, validationErrorResponse } from '@/lib/api/se
import { NoWorkspaceAccessError } from '@/lib/core/application'
import { ForbiddenOperationError } from '@/lib/core/application/forbidden'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { CredentialAccessRequiredError } from '@/lib/credentials/application/authorized-credential-use-case'
import { CredentialProviderOperationError } from '@/lib/credentials/application/credential-crud'
export const credentialValidationParseOptions = {
@@ -25,6 +26,14 @@ export const internalCredentialErrorPolicy = extendInternalErrorPolicy(
}
)
export const internalCredentialDetailErrorPolicy = extendInternalErrorPolicy(
internalCredentialErrorPolicy,
(error) =>
error instanceof CredentialAccessRequiredError
? internalErrorResponse(403, { error: 'Forbidden' })
: null
)
export const internalCredentialMemberListErrorPolicy = extendInternalErrorPolicy(
internalCredentialErrorPolicy,
(error) => {
@@ -3,7 +3,10 @@
*/
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { defineWorkspaceOperation } from '@/lib/core/application'
import { defineAuthorizedCredentialUseCase } from '@/lib/credentials/application/authorized-credential-use-case'
import {
CredentialAccessRequiredError,
defineAuthorizedCredentialUseCase,
} from '@/lib/credentials/application/authorized-credential-use-case'
import { defineCredentialOperation } from '@/lib/credentials/application/operations'
const mocks = vi.hoisted(() => ({
@@ -85,6 +88,20 @@ describe('defineAuthorizedCredentialUseCase', () => {
expect(mocks.execute).toHaveBeenCalledOnce()
})
it('denies member-level reads without credential membership', async () => {
mocks.getActor.mockResolvedValue({
credential,
member: null,
hasWorkspaceAccess: true,
isAdmin: false,
})
await expect(
createUseCase(memberOperation).execute({ principal, input: undefined })
).rejects.toBeInstanceOf(CredentialAccessRequiredError)
expect(mocks.execute).not.toHaveBeenCalled()
})
it('requires credential admin independently of workspace read access', async () => {
await expect(
createUseCase(adminOperation).execute({ principal, input: undefined })
@@ -17,6 +17,13 @@ export interface CredentialAuthorizationContext extends WorkspaceAuthorizationCo
credentialAccess?: CredentialActorContext
}
export class CredentialAccessRequiredError extends OrchestrationError {
constructor() {
super('forbidden', 'Credential access required')
this.name = 'CredentialAccessRequiredError'
}
}
export function requireCredentialAccess(
context: CredentialAuthorizationContext
): CredentialActorContext {
@@ -61,7 +68,7 @@ export function defineAuthorizedCredentialUseCase<
switch (definition.operation.minimumCredentialRole) {
case 'member':
if (!actor.member && !actor.isAdmin) {
throw new OrchestrationError('forbidden', 'Credential access required')
throw new CredentialAccessRequiredError()
}
return
case 'admin':