Skip to content

Commit d72284c

Browse files
committed
fix(knowledge): redact saved AWS session secrets and share the credential service check
A saved AWS credential resolves to an empty bearer with its secret key and session token in awsSession, so redacting accessToken alone left them in provider validation text. Both validation sites now redact every secret the resolved token carries (connectorTokenSecrets). Setup and access updates now share requireConnectorCredentialService, so a key connector's saved-credential setup refuses a missing service config instead of skipping the provider check.
1 parent 358150e commit d72284c

7 files changed

Lines changed: 160 additions & 23 deletions

File tree

‎apps/sim/lib/knowledge/application/connector-access.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
mirrorsSourceAcls,
3636
supportsConnectorAccessMode,
3737
} from '@/lib/knowledge/connectors/access-modes'
38+
import { requireConnectorCredentialService } from '@/lib/knowledge/connectors/credential-service'
3839
import { validateKnowledgeConnectorMembersBinding } from '@/lib/knowledge/connectors/member-access'
3940
import {
4041
provisionKnowledgeConnectorMembersBinding,
@@ -50,7 +51,6 @@ import {
5051
import { getKnowledgeConnector } from '@/lib/knowledge/orchestration/connectors'
5152
import type { KnowledgeOperationSource } from '@/lib/knowledge/orchestration/shared'
5253
import { requireOrganizationSearchApproval } from '@/lib/knowledge/search/integration-policy'
53-
import { getServiceConfigByProviderId, getServiceConfigByServiceId } from '@/lib/oauth'
5454
import { getConnectorCredentialService } from '@/connectors/auth'
5555
import { getConnectorMeta } from '@/connectors/registry'
5656
import type { ConnectorMeta } from '@/connectors/types'
@@ -435,15 +435,7 @@ async function requireUsableCredential(input: {
435435
)
436436
}
437437
await requireConnectorSavedCredentialSetup(auth, input.credentialId)
438-
const service =
439-
getServiceConfigByServiceId(credentialService) ??
440-
getServiceConfigByProviderId(credentialService)
441-
if (!service) {
442-
throw new OrchestrationError(
443-
'validation',
444-
`${input.connectorMeta.name} has no OAuth service to validate the credential against`
445-
)
446-
}
438+
const service = requireConnectorCredentialService(input.connectorMeta)
447439
const token = await resolveConnectorCredentialAccessToken({
448440
principal: input.principal,
449441
credentialId: input.credentialId,

‎apps/sim/lib/knowledge/application/connectors.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,15 @@ vi.mock('@/connectors/registry.server', () => ({
9696
auth: { mode: 'oauth', requiredScopes: ['read:confluence-content.all'] },
9797
validateConfig: hoisted.validateConnectorConfig,
9898
},
99+
s3: {
100+
name: 'Amazon S3',
101+
auth: {
102+
mode: 'apiKey',
103+
label: 'Secret Access Key',
104+
savedCredential: { provider: 'aws', label: 'Saved AWS credential' },
105+
},
106+
validateConfig: hoisted.validateConnectorConfig,
107+
},
99108
google_drive: {
100109
name: 'Google Drive',
101110
auth: {
@@ -1090,6 +1099,52 @@ describe('organization connector credential authorization', () => {
10901099
}
10911100
)
10921101

1102+
it.each(['result', 'exception'] as const)(
1103+
'redacts a saved AWS session from settings validation %s',
1104+
async (failure) => {
1105+
const awsSession = {
1106+
accessKeyId: 'ASIAEXAMPLE',
1107+
secretAccessKey: 'session/secret',
1108+
sessionToken: 'session-token',
1109+
}
1110+
mocks.authorizeOrganizationCredentialUse.mockResolvedValueOnce({
1111+
credential: { ...credential, providerId: 'aws' },
1112+
userId: principal.userId,
1113+
})
1114+
internalAwsCredentialsMockFns.mockResolveAuthorizedAwsCredential.mockResolvedValueOnce(
1115+
awsSession
1116+
)
1117+
const message = `Denied ${awsSession.secretAccessKey} ${awsSession.sessionToken}`
1118+
if (failure === 'exception') {
1119+
mocks.validateConnectorConfig.mockRejectedValueOnce(
1120+
new OrchestrationError('validation', message)
1121+
)
1122+
} else {
1123+
mocks.validateConnectorConfig.mockResolvedValueOnce({ valid: false, error: message })
1124+
}
1125+
const result = validateConnectorSourceConfig({
1126+
principal,
1127+
requestId: 'request',
1128+
organizationId: 'org',
1129+
actingUserId: principal.userId,
1130+
sourceConfig: { region: 'us-east-1', bucket: 'docs' },
1131+
connector: {
1132+
id: 'connector-s3',
1133+
connectorType: 's3',
1134+
credentialId: credential.id,
1135+
encryptedApiKey: null,
1136+
accessMode: 'workspace',
1137+
} as Parameters<typeof validateConnectorSourceConfig>[0]['connector'],
1138+
})
1139+
const redacted = 'Denied [REDACTED] [REDACTED]'
1140+
if (failure === 'exception') {
1141+
await expect(result).rejects.toMatchObject({ code: 'validation', message: redacted })
1142+
} else {
1143+
await expect(result).resolves.toEqual({ errorCode: 'validation', message: redacted })
1144+
}
1145+
}
1146+
)
1147+
10931148
it('does not mint a token after the credential creator leaves the organization', async () => {
10941149
mocks.resolveTokenIdentity.mockResolvedValueOnce(null)
10951150
await expect(resolveConnectorCredentialAccessToken(input)).resolves.toBeNull()

‎apps/sim/lib/knowledge/application/connectors.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,11 @@ import {
5959
import {
6060
type ConnectorAccessToken,
6161
ConnectorAwsCredentialError,
62+
connectorTokenSecrets,
6263
resolveConnectorAccessToken,
6364
syncContextForToken,
6465
} from '@/lib/knowledge/connectors/access-token'
66+
import { requireConnectorCredentialService } from '@/lib/knowledge/connectors/credential-service'
6567
import { requiresConnectorIndexing } from '@/lib/knowledge/connectors/indexing-policy'
6668
import {
6769
provisionKnowledgeConnectorMembersBinding,
@@ -115,11 +117,7 @@ import type {
115117
KnowledgeOperationSource,
116118
KnowledgeOrchestrationResult,
117119
} from '@/lib/knowledge/orchestration/shared'
118-
import {
119-
credentialProviderMatchesService,
120-
getServiceConfigByServiceId,
121-
type ServiceProviderIdentity,
122-
} from '@/lib/oauth'
120+
import { credentialProviderMatchesService, type ServiceProviderIdentity } from '@/lib/oauth'
123121
import { ServiceAccountTokenError } from '@/lib/oauth/credential-service'
124122
import { CAPABILITY_RULES, refuseCapability } from '@/lib/permission-groups/capabilities'
125123
import { resolvePermissionGroupConfig } from '@/lib/permission-groups/config-scope.server'
@@ -501,19 +499,21 @@ export async function validateConnectorSourceConfig(input: {
501499
)
502500
}
503501
}
502+
const secrets = connectorTokenSecrets(resolved)
504503
const validation = await connectorConfig
505504
.validateConfig(resolved.accessToken, input.sourceConfig, validationContext)
506505
.catch((error: unknown) => {
507506
const sanitized = toError(error)
508-
sanitized.message = redactKnownSensitiveValues(sanitized.message, [resolved.accessToken])
507+
sanitized.message = redactKnownSensitiveValues(sanitized.message, secrets)
509508
throw sanitized
510509
})
511510
return validation.valid
512511
? null
513512
: {
514-
message: redactKnownSensitiveValues(validation.error || 'Invalid source configuration', [
515-
resolved.accessToken,
516-
]),
513+
message: redactKnownSensitiveValues(
514+
validation.error || 'Invalid source configuration',
515+
secrets
516+
),
517517
errorCode: 'validation',
518518
}
519519
}
@@ -757,8 +757,8 @@ async function executeCreateKnowledgeConnector({
757757
await requireConnectorSavedCredentialSetup(connectorMeta.auth, input.credentialId)
758758
/** A key connector's saved credential must be of its declared service, as on an access update. */
759759
const savedCredentialService =
760-
connectorMeta.auth.mode === 'apiKey' && connectorMeta.auth.savedCredential
761-
? (getServiceConfigByServiceId(connectorMeta.auth.savedCredential.provider) ?? undefined)
760+
connectorMeta.auth.mode === 'apiKey' && connectorMeta.auth.savedCredential && input.credentialId
761+
? requireConnectorCredentialService(connectorMeta)
762762
: undefined
763763
if (
764764
context.knowledgeBase.isSearchIndex &&

‎apps/sim/lib/knowledge/connectors/access-token.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ export interface ConnectorAccessToken {
5656
expiresAt?: Date
5757
}
5858

59+
/**
60+
* Every secret a resolved token carries, for redacting provider text that may
61+
* echo one back. A saved AWS credential's secrets sit in `awsSession`, not in
62+
* the empty bearer, so callers redact this list rather than `accessToken` alone.
63+
*/
64+
export function connectorTokenSecrets(token: ConnectorAccessToken): string[] {
65+
return [
66+
token.accessToken,
67+
...(token.awsSession
68+
? [token.awsSession.secretAccessKey, token.awsSession.sessionToken ?? '']
69+
: []),
70+
]
71+
}
72+
5973
/** The connector-facing service that a saved AWS credential authenticates. */
6074
const AWS_CREDENTIAL_SERVICE = 'aws'
6175

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { OrchestrationError } from '@/lib/core/orchestration/types'
2+
import {
3+
getServiceConfigByProviderId,
4+
getServiceConfigByServiceId,
5+
type ServiceProviderIdentity,
6+
} from '@/lib/oauth'
7+
import { getConnectorCredentialService } from '@/connectors/auth'
8+
import type { ConnectorMeta } from '@/connectors/types'
9+
10+
/**
11+
* The OAuth service a connector's credential must belong to. Setting up a key
12+
* connector on its saved credential and every access update resolve it here, so
13+
* a connector whose declared service has no config is refused instead of
14+
* skipping the provider check.
15+
*/
16+
export function requireConnectorCredentialService(
17+
connectorMeta: Pick<ConnectorMeta, 'name' | 'auth'>
18+
): ServiceProviderIdentity {
19+
const credentialService = getConnectorCredentialService(connectorMeta.auth)
20+
const service = credentialService
21+
? (getServiceConfigByServiceId(credentialService) ??
22+
getServiceConfigByProviderId(credentialService))
23+
: null
24+
if (!service) {
25+
throw new OrchestrationError(
26+
'validation',
27+
`${connectorMeta.name} has no OAuth service to validate the credential against`
28+
)
29+
}
30+
return service
31+
}

‎apps/sim/lib/knowledge/orchestration/connectors.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,14 @@ const {
3434

3535
mockEncryptApiKey,
3636
mockValidateGitHub,
37+
mockValidateS3,
3738
mockEnqueueConnectorDeletion,
3839
mockEnqueueConnectorDetachment,
3940
} = vi.hoisted(() => ({
4041
mockDispatchSync: vi.fn(),
4142
mockEncryptApiKey: vi.fn(),
4243
mockValidateGitHub: vi.fn(),
44+
mockValidateS3: vi.fn(),
4345
mockEnqueueConnectorDeletion: vi.fn(),
4446
mockEnqueueConnectorDetachment: vi.fn(),
4547
}))
@@ -76,6 +78,16 @@ vi.mock('@/connectors/registry.server', () => ({
7678
configFields: [],
7779
validateConfig: mockValidateGitHub,
7880
},
81+
s3: {
82+
name: 'Amazon S3',
83+
auth: {
84+
mode: 'apiKey',
85+
label: 'Secret Access Key',
86+
savedCredential: { provider: 'aws', label: 'Saved AWS credential' },
87+
},
88+
configFields: [],
89+
validateConfig: mockValidateS3,
90+
},
7991
notion: {
8092
auth: { mode: 'apiKey', optional: true },
8193
configFields: [],
@@ -188,6 +200,35 @@ describe('performCreateKnowledgeConnector', () => {
188200
}
189201
)
190202

203+
it.each(['result', 'exception'])(
204+
'redacts a saved AWS session from provider validation %s errors',
205+
async (failure) => {
206+
const awsSession = {
207+
accessKeyId: 'ASIAEXAMPLE',
208+
secretAccessKey: 'session/secret',
209+
sessionToken: 'session-token',
210+
}
211+
const message = `Denied ${awsSession.secretAccessKey} ${awsSession.sessionToken}`
212+
if (failure === 'result')
213+
mockValidateS3.mockResolvedValueOnce({ valid: false, error: message })
214+
else mockValidateS3.mockRejectedValueOnce(new OrchestrationError('validation', message))
215+
const request = performCreateKnowledgeConnector({
216+
...createParams,
217+
connectorType: 's3',
218+
credentialId: 'aws-credential',
219+
resolveAccessToken: vi.fn().mockResolvedValue({ accessToken: '', awsSession }),
220+
})
221+
const expected = { errorCode: 'validation', error: 'Denied [REDACTED] [REDACTED]' }
222+
if (failure === 'result') await expect(request).resolves.toMatchObject(expected)
223+
else
224+
await expect(request).rejects.toMatchObject({
225+
code: expected.errorCode,
226+
message: expected.error,
227+
})
228+
expect(dbChainMockFns.insert).not.toHaveBeenCalled()
229+
}
230+
)
231+
191232
it('validates and encrypts a GitHub PAT without resolving an OAuth account or returning the secret', async () => {
192233
dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'kb-1' }])
193234
dbChainMockFns.returning.mockResolvedValueOnce([

‎apps/sim/lib/knowledge/orchestration/connectors.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
} from '@/lib/knowledge/connectors/access-modes'
4444
import {
4545
type ConnectorAccessToken,
46+
connectorTokenSecrets,
4647
syncContextForToken,
4748
} from '@/lib/knowledge/connectors/access-token'
4849
import { enqueueConnectorDeletion } from '@/lib/knowledge/connectors/deletion'
@@ -300,6 +301,7 @@ export async function performCreateKnowledgeConnector(
300301
let resolvedCredentialId: string | null = null
301302
let resolvedEncryptedApiKey: string | null = null
302303
let accessToken: string | null = null
304+
let tokenSecrets: string[] = []
303305
let tokenContext: Record<string, unknown> = {}
304306
const apiKeyConfig = getConnectorApiKeyConfig(connectorConfig.auth)
305307
if (apiKey && (!apiKeyConfig || accessMode === 'members')) {
@@ -338,6 +340,7 @@ export async function performCreateKnowledgeConnector(
338340
return fail('API key is required', 'validation')
339341
}
340342
accessToken = apiKey ?? ''
343+
tokenSecrets = [accessToken]
341344
} else if (!membersBinding || credentialId) {
342345
if (!credentialId) {
343346
return fail('Credential is required', 'validation')
@@ -352,6 +355,7 @@ export async function performCreateKnowledgeConnector(
352355
return fail('Credential has no access token. Please reconnect your account.', 'validation')
353356
}
354357
accessToken = token.accessToken
358+
tokenSecrets = connectorTokenSecrets(token)
355359
tokenContext = syncContextForToken(token)
356360
resolvedCredentialId = credentialId
357361
}
@@ -367,13 +371,13 @@ export async function performCreateKnowledgeConnector(
367371
.validateConfig(accessToken, sourceConfig, validationContext)
368372
.catch((error: unknown) => {
369373
const sanitized = toError(error)
370-
sanitized.message = redactKnownSensitiveValues(sanitized.message, [accessToken])
374+
sanitized.message = redactKnownSensitiveValues(sanitized.message, tokenSecrets)
371375
throw sanitized
372376
})
373377
if (!configValidation.valid) {
374378
return fail(
375379
(configValidation.error &&
376-
redactKnownSensitiveValues(configValidation.error, [accessToken])) ||
380+
redactKnownSensitiveValues(configValidation.error, tokenSecrets)) ||
377381
`The ${connectorType} connector rejected sourceConfig without a reason — re-check its required fields with \`sim connector-types list --detail full\` (GET /api/v2/connector-types?detail=full) before retrying; the same config will fail again.`,
378382
'validation'
379383
)

0 commit comments

Comments
 (0)