Skip to content

Commit 558c77c

Browse files
waleedlatif1waleedlatif1
committed
fix(knowledge): document connector auth in the v2 contract and reject $NAME secrets
1 parent 2527a79 commit 558c77c

10 files changed

Lines changed: 99 additions & 21 deletions

File tree

‎apps/docs/content/docs/cli/knowledge.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -657,8 +657,8 @@ Create Knowledge Connector (OAuth login or personal API key required)
657657
| Option | Required | Description |
658658
| --- | --- | --- |
659659
| `--connector-type <value>` | Yes | Registered connector type. |
660-
| `--credential-id <value>` | No | OAuth credential identifier for connectors that require OAuth. |
661-
| `--api-key <value>` | No | Write-only API key for connectors that use API-key authentication. |
660+
| `--credential-id <value>` | No | OAuth credential identifier for connector types whose `auth.mode` is `oauth` (see connector types); omit it for `apiKey` connectors. |
661+
| `--api-key <value>` | No | Write-only API key for connector types whose `auth.mode` is `apiKey` (see connector types), or a personal access token for an OAuth connector that also accepts one, such as GitHub. Send it instead of `credentialId`. Pass a raw key, or a secret reference written as the whole value `&#123;&#123;SECRET_NAME&#125;&#125;`, which the server resolves; `$SECRET_NAME` is not a reference. |
662662
| `--source-config <json\|@file>` | Yes | Connector-specific source selection and filtering configuration. (JSON, or @path / @- to read a file or stdin). |
663663
| `--sync-interval-minutes <value>` | No | Scheduled synchronization interval in minutes; zero disables scheduling. |
664664

‎apps/docs/content/docs/cli/reference.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2162,8 +2162,8 @@ sim knowledge connectors create <knowledgeBaseId> [options]
21622162
| Option | Required | Description |
21632163
| --- | --- | --- |
21642164
| `--connector-type <value>` | Yes | Registered connector type. |
2165-
| `--credential-id <value>` | No | OAuth credential identifier for connectors that require OAuth. |
2166-
| `--api-key <value>` | No | Write-only API key for connectors that use API-key authentication. |
2165+
| `--credential-id <value>` | No | OAuth credential identifier for connector types whose `auth.mode` is `oauth` (see connector types); omit it for `apiKey` connectors. |
2166+
| `--api-key <value>` | No | Write-only API key for connector types whose `auth.mode` is `apiKey` (see connector types), or a personal access token for an OAuth connector that also accepts one, such as GitHub. Send it instead of `credentialId`. Pass a raw key, or a secret reference written as the whole value `&#123;&#123;SECRET_NAME&#125;&#125;`, which the server resolves; `$SECRET_NAME` is not a reference. |
21672167
| `--source-config <json\|@file>` | Yes | Connector-specific source selection and filtering configuration. (JSON, or @path / @- to read a file or stdin). |
21682168
| `--sync-interval-minutes <value>` | No | Scheduled synchronization interval in minutes; zero disables scheduling. |
21692169

‎apps/docs/openapi-v2-knowledge.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5449,13 +5449,13 @@
54495449
"description": "Registered connector type."
54505450
},
54515451
"credentialId": {
5452-
"description": "OAuth credential identifier for connectors that require OAuth.",
5452+
"description": "OAuth credential identifier for connector types whose `auth.mode` is `oauth` (see connector types); omit it for `apiKey` connectors.",
54535453
"type": "string",
54545454
"minLength": 1,
54555455
"maxLength": 255
54565456
},
54575457
"apiKey": {
5458-
"description": "Write-only API key for connectors that use API-key authentication.",
5458+
"description": "Write-only API key for connector types whose `auth.mode` is `apiKey` (see connector types), or a personal access token for an OAuth connector that also accepts one, such as GitHub. Send it instead of `credentialId`. Pass a raw key, or a secret reference written as the whole value `{{SECRET_NAME}}`, which the server resolves; `$SECRET_NAME` is not a reference.",
54595459
"type": "string",
54605460
"minLength": 1,
54615461
"maxLength": 10000

‎apps/docs/openapi-v2-resources.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15414,7 +15414,7 @@
1541415414
"mode": {
1541515415
"type": "string",
1541615416
"const": "oauth",
15417-
"description": "Authenticates with an OAuth credential."
15417+
"description": "Authenticates with an OAuth credential; pass `credentialId` when creating the connector."
1541815418
},
1541915419
"provider": {
1542015420
"type": "string",
@@ -15437,7 +15437,7 @@
1543715437
"mode": {
1543815438
"type": "string",
1543915439
"const": "apiKey",
15440-
"description": "Authenticates with a stored API key."
15440+
"description": "Authenticates with a stored API key; pass `apiKey`, and no `credentialId`, when creating the connector."
1544115441
},
1544215442
"label": {
1544315443
"description": "Label shown above the key field.",
@@ -15456,7 +15456,7 @@
1545615456
"additionalProperties": false
1545715457
}
1545815458
],
15459-
"description": "How the connector authenticates against its source."
15459+
"description": "How the connector authenticates against its source: `oauth` connectors take `credentialId` (GitHub also accepts a personal access token as `apiKey`), `apiKey` connectors take `apiKey`."
1546015460
},
1546115461
"configFields": {
1546215462
"type": "array",
@@ -15634,7 +15634,7 @@
1563415634
"mode": {
1563515635
"type": "string",
1563615636
"enum": ["oauth", "apiKey"],
15637-
"description": "How the connector authenticates against its source."
15637+
"description": "How the connector authenticates against its source: `oauth` connectors take `credentialId` (GitHub also accepts a personal access token as `apiKey`), `apiKey` connectors take `apiKey`."
1563815638
}
1563915639
},
1564015640
"required": ["mode"],

‎apps/sim/lib/api/contracts/v2/catalog.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -671,15 +671,23 @@ export const v2ConnectorTypeSchema = z
671671
auth: z
672672
.discriminatedUnion('mode', [
673673
z.object({
674-
mode: z.literal('oauth').describe('Authenticates with an OAuth credential.'),
674+
mode: z
675+
.literal('oauth')
676+
.describe(
677+
'Authenticates with an OAuth credential; pass `credentialId` when creating the connector.'
678+
),
675679
provider: z.string().describe('OAuth service the credential must authenticate.'),
676680
requiredScopes: z
677681
.array(z.string())
678682
.optional()
679683
.describe('Scopes the credential must carry.'),
680684
}),
681685
z.object({
682-
mode: z.literal('apiKey').describe('Authenticates with a stored API key.'),
686+
mode: z
687+
.literal('apiKey')
688+
.describe(
689+
'Authenticates with a stored API key; pass `apiKey`, and no `credentialId`, when creating the connector.'
690+
),
683691
label: z.string().optional().describe('Label shown above the key field.'),
684692
placeholder: z.string().optional().describe('Placeholder shown in the key field.'),
685693
optional: z
@@ -689,7 +697,9 @@ export const v2ConnectorTypeSchema = z
689697
),
690698
}),
691699
])
692-
.describe('How the connector authenticates against its source.'),
700+
.describe(
701+
'How the connector authenticates against its source: `oauth` connectors take `credentialId` (GitHub also accepts a personal access token as `apiKey`), `apiKey` connectors take `apiKey`.'
702+
),
693703
configFields: z
694704
.array(v2ConnectorConfigFieldSchema)
695705
.describe('Fields that make up the connector’s `sourceConfig`.'),
@@ -731,7 +741,9 @@ export const v2ConnectorTypeSummarySchema = z
731741
.object({
732742
mode: z
733743
.enum(['oauth', 'apiKey'])
734-
.describe('How the connector authenticates against its source.'),
744+
.describe(
745+
'How the connector authenticates against its source: `oauth` connectors take `credentialId` (GitHub also accepts a personal access token as `apiKey`), `apiKey` connectors take `apiKey`.'
746+
),
735747
})
736748
.describe(
737749
'Authentication mode only. `detail=full` adds the OAuth provider and scopes, or the API-key field labels.'

‎apps/sim/lib/api/contracts/v2/knowledge.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1730,13 +1730,17 @@ export const v2CreateKnowledgeConnectorBodySchema = z
17301730
.min(1)
17311731
.max(255)
17321732
.optional()
1733-
.describe('OAuth credential identifier for connectors that require OAuth.'),
1733+
.describe(
1734+
'OAuth credential identifier for connector types whose `auth.mode` is `oauth` (see connector types); omit it for `apiKey` connectors.'
1735+
),
17341736
apiKey: z
17351737
.string()
17361738
.min(1)
17371739
.max(10_000)
17381740
.optional()
1739-
.describe('Write-only API key for connectors that use API-key authentication.'),
1741+
.describe(
1742+
'Write-only API key for connector types whose `auth.mode` is `apiKey` (see connector types), or a personal access token for an OAuth connector that also accepts one, such as GitHub. Send it instead of `credentialId`. Pass a raw key, or a secret reference written as the whole value `{{SECRET_NAME}}`, which the server resolves; `$SECRET_NAME` is not a reference.'
1743+
),
17401744
sourceConfig: z
17411745
.record(z.string(), z.unknown().describe('Connector-specific source configuration value.'))
17421746
.describe('Connector-specific source selection and filtering configuration.'),

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,37 @@ describe('knowledge connector application use cases', () => {
294294
)
295295
})
296296

297+
it('rejects a $NAME spelling of an existing secret instead of storing it as the key', async () => {
298+
mocks.resolveEnvironment.mockResolvedValue({ GITLAB_PAT: { value: 'resolved-pat' } })
299+
await expect(
300+
createKnowledgeConnector.execute({
301+
principal: patPrincipal,
302+
input: { ...patInput, apiKey: ' $GITLAB_PAT ' },
303+
})
304+
).rejects.toMatchObject({
305+
code: 'validation',
306+
message:
307+
'Secret references use {{GITLAB_PAT}}, not $GITLAB_PAT. Pass apiKey as "{{GITLAB_PAT}}" to use the secret.',
308+
})
309+
expect(mocks.resolveEnvironment).toHaveBeenCalledWith('writer', 'workspace-b', ['GITLAB_PAT'])
310+
expect(mocks.createConnector).not.toHaveBeenCalled()
311+
})
312+
313+
it('passes a $-prefixed literal through when no secret has that name', async () => {
314+
mocks.resolveEnvironment.mockResolvedValue({})
315+
mocks.createConnector.mockResolvedValueOnce({
316+
success: true,
317+
connector: { id: 'new-connector', connectorType: 'sftp', accessMode: 'workspace' },
318+
})
319+
await createKnowledgeConnector.execute({
320+
principal: patPrincipal,
321+
input: { ...patInput, apiKey: '$Summer2024' },
322+
})
323+
expect(mocks.createConnector).toHaveBeenCalledWith(
324+
expect.objectContaining({ apiKey: '$Summer2024' })
325+
)
326+
})
327+
297328
it('refuses workspace-wide or unreviewed source ingestion into the canonical search index', async () => {
298329
mocks.resolveKnowledgeBase.mockResolvedValue({
299330
...crossWorkspaceContext,

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -785,14 +785,43 @@ async function summarizeConnectorMembers(
785785
return { active: row?.active ?? 0, suspended: row?.suspended ?? 0, stale: row?.stale ?? 0 }
786786
}
787787

788+
/** Whole-value `$NAME`, the shell-style spelling of a secret reference that is never resolved. */
789+
const SHELL_STYLE_SECRET_PATTERN = /^\$([A-Za-z_][A-Za-z0-9_]*)$/
790+
791+
/**
792+
* Rejects an API key spelled `$NAME` when the caller has a secret named `NAME`, so the literal
793+
* reference is not sent to the provider and stored as the key. A `$`-prefixed value that names no
794+
* secret passes through, since password-style keys (SFTP, ServiceNow) can legitimately look alike.
795+
*/
796+
async function rejectShellStyleSecretReference(
797+
apiKey: string,
798+
principal: Principal,
799+
workspaceId: string | undefined
800+
): Promise<void> {
801+
const name = apiKey.trim().match(SHELL_STYLE_SECRET_PATTERN)?.[1]
802+
if (!name) return
803+
const userId = resolvePrincipalSubjectUserId(principal)
804+
if (!userId) return
805+
const variables = await resolveEffectiveEnvironmentVariables(userId, workspaceId, [name])
806+
if (!Object.hasOwn(variables, name)) return
807+
throw new OrchestrationError(
808+
'validation',
809+
`Secret references use {{${name}}}, not $${name}. Pass apiKey as "{{${name}}}" to use the secret.`
810+
)
811+
}
812+
788813
/** Resolves a secret reference at setup time; the connector stores an encrypted token snapshot. */
789814
async function resolveConnectorApiKey(
790815
apiKey: string | undefined,
791816
principal: Principal,
792817
workspaceId: string | undefined
793818
): Promise<string | undefined> {
794-
const name = apiKey?.trim().match(/^\{\{\s*([A-Za-z_][A-Za-z0-9_]*)\s*\}\}$/)?.[1]
795-
if (!name) return apiKey
819+
if (apiKey === undefined) return undefined
820+
const name = apiKey.trim().match(/^\{\{\s*([A-Za-z_][A-Za-z0-9_]*)\s*\}\}$/)?.[1]
821+
if (!name) {
822+
await rejectShellStyleSecretReference(apiKey, principal, workspaceId)
823+
return apiKey
824+
}
796825
const userId = resolvePrincipalSubjectUserId(principal)
797826
if (!userId) {
798827
throw new OrchestrationError('forbidden', 'Secret references require a user identity')

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ export async function performCreateKnowledgeConnector(
357357
return fail(
358358
(configValidation.error &&
359359
redactKnownSensitiveValues(configValidation.error, [accessToken])) ||
360-
`The ${connectorType} connector rejected sourceConfig without a reason — re-check its required fields in knowledgebases/connectors/${connectorType}.json before retrying; the same config will fail again.`,
360+
`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.`,
361361
'validation'
362362
)
363363
}

‎packages/sim-cli/src/generated/v2-api.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14802,11 +14802,13 @@ export const V2_OPERATIONS = {
1480214802
connectorType: { kind: 'string', required: true, describe: 'Registered connector type.' },
1480314803
credentialId: {
1480414804
kind: 'string',
14805-
describe: 'OAuth credential identifier for connectors that require OAuth.',
14805+
describe:
14806+
'OAuth credential identifier for connector types whose `auth.mode` is `oauth` (see connector types); omit it for `apiKey` connectors.',
1480614807
},
1480714808
apiKey: {
1480814809
kind: 'string',
14809-
describe: 'Write-only API key for connectors that use API-key authentication.',
14810+
describe:
14811+
'Write-only API key for connector types whose `auth.mode` is `apiKey` (see connector types), or a personal access token for an OAuth connector that also accepts one, such as GitHub. Send it instead of `credentialId`. Pass a raw key, or a secret reference written as the whole value `{{SECRET_NAME}}`, which the server resolves; `$SECRET_NAME` is not a reference.',
1481014812
},
1481114813
sourceConfig: {
1481214814
kind: 'object',

0 commit comments

Comments
 (0)