Skip to content

Commit f9cebf4

Browse files
committed
fix(snowflake): close unload-query breakouts, drop parked secrets, correct docs
- assertBalancedQuery now skips // line comments, $$ dollar quoting and rejects ambiguous nested block comments; each hid a paren that let an injected OVERWRITE = TRUE escape the derived table - always emit OVERWRITE so an injected duplicate is rejected by Snowflake rather than silently replacing staged files - _removed_ migration targets now drop the stored value instead of parking it under a dead key, where export scrubbing (which walks the block config) would never clear it - 403 falls back to the shared invalid-credentials message, which names the network policy and SQL API causes Snowflake does not distinguish in the body - correct the network-policy-by-user-type claim: only SERVICE_AGENT is exempt - correct MAX_FILE_SIZE and errorOnly tool descriptions to match the fixed code
1 parent f19e949 commit f9cebf4

15 files changed

Lines changed: 249 additions & 78 deletions

File tree

apps/docs/content/docs/en/integrations/snowflake-service-account.mdx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@ Sim stores the token alongside your account host as one credential. Once it is a
1818
- A network policy covering the user, or an authentication policy that waives the requirement (see below).
1919

2020
<Callout type="warn">
21-
By default Snowflake requires a **human** user to be subject to a **network policy** before a programmatic access token can be generated or used. If your account has no network policy, either create one (allowing Sim's egress) or set `NETWORK_POLICY_EVALUATION = ENFORCED_NOT_REQUIRED` on an authentication policy applied to the user. Without one of these, token generation fails and a generated token is rejected at run time. Service users (`TYPE = SERVICE`) are exempt from the requirement.
21+
Snowflake's **network policy** requirement varies by user type, and getting it wrong is the most common reason a token is rejected:
22+
23+
- `TYPE = PERSON` — you can generate a token without a network policy, but the user **must** be covered by one to authenticate with it.
24+
- `TYPE = SERVICE` and `TYPE = LEGACY_SERVICE` — a network policy is required to generate **and** to use a token.
25+
- `TYPE = SERVICE_AGENT` — exempt; generate and use freely.
26+
27+
If your account has no network policy, either create one (allowing Sim's egress) or set `NETWORK_POLICY_EVALUATION = ENFORCED_NOT_REQUIRED` on an authentication policy applied to the user.
2228
</Callout>
2329

2430
## Creating the Token
@@ -48,9 +54,9 @@ ALTER USER my_service_user ADD PROGRAMMATIC ACCESS TOKEN sim_workflows
4854
DAYS_TO_EXPIRY = 90;
4955
```
5056

51-
`DAYS_TO_EXPIRY` defaults to 15 days and cannot exceed 365. **A token can never be non-expiring**, and the value cannot be changed after creation — to extend it, generate a new token and swap the credential in Sim. Plan the rotation when you create it.
57+
`DAYS_TO_EXPIRY` defaults to 15 days and cannot exceed 365 — an authentication policy can lower that ceiling further via `PROGRAMMATIC_ACCESS_TOKEN_MAX_EXPIRY_IN_DAYS`. **A token can never be non-expiring**, and the value cannot be changed after creation — to extend it, generate a new token and swap the credential in Sim. Plan the rotation when you create it.
5258

53-
Service users (`TYPE = SERVICE` or `SERVICE_AGENT`) **must** set `ROLE_RESTRICTION`. For person users it is optional but recommended: a restricted token can only ever act as that one role.
59+
Service users (`TYPE = SERVICE`, `LEGACY_SERVICE`, or `SERVICE_AGENT`) **must** set `ROLE_RESTRICTION`, unless an authentication policy exempts them. For person users it is optional but recommended: a restricted token can only ever act as that one role.
5460

5561
<Callout type="info">
5662
If an authentication policy applies to the user, `'PROGRAMMATIC_ACCESS_TOKEN'` must appear in its `AUTHENTICATION_METHODS` list, otherwise the token is refused.
@@ -98,7 +104,7 @@ A token's expiry is fixed at creation. To rotate, generate a new token on the sa
98104
{ question: "Why a programmatic access token instead of a password?", answer: "The token is scoped to one user, can be restricted to a single role, expires on a schedule you choose, and can be revoked on its own without changing anyone's password or breaking other integrations." },
99105
{ question: "Does the token expire?", answer: "Yes. DAYS_TO_EXPIRY defaults to 15 days and can be set up to 365 at creation. It cannot be changed afterwards, so pick the value you want up front and plan a rotation." },
100106
{ question: "I lost the token — can I see it again?", answer: "No. Snowflake shows the secret only at creation. Generate a new token and update the credential in Sim." },
101-
{ question: "Why does adding the credential fail with an authentication error?", answer: "The three common causes are a token that has expired or been revoked, a human user with no network policy (Snowflake requires one for token use unless an authentication policy waives it), and an authentication policy that omits PROGRAMMATIC_ACCESS_TOKEN from its AUTHENTICATION_METHODS. A wrong account host is reported separately — Snowflake resolves any *.snowflakecomputing.com name, so Sim identifies a mistyped host by the 404 it answers with." },
107+
{ question: "Why does adding the credential fail with an authentication error?", answer: "The three common causes are a token that has expired or been revoked, a user with no network policy (required to authenticate for every type except SERVICE_AGENT, unless an authentication policy waives it), and an authentication policy that omits PROGRAMMATIC_ACCESS_TOKEN from its AUTHENTICATION_METHODS. A wrong account host is reported separately — Snowflake resolves any *.snowflakecomputing.com name, so Sim identifies a mistyped host by the 404 it answers with." },
102108
{ question: "Why is a picker empty?", answer: "The pickers run SHOW statements as the token's user under its default role — the block's execution role is not applied to them. If the objects you expect are visible only to another role, grant the default role usage on them, restrict the token to the role that has access, or type the name in advanced mode." },
103109
{ question: "Does listing objects cost credits?", answer: "No. Every picker and the credential check run metadata-only statements, which Snowflake serves without a running warehouse." },
104110
{ question: "Can one credential reach two Snowflake accounts?", answer: "No. A token is bound to the user in one account, and the credential stores that account's host. Add one credential per account." },

apps/docs/content/docs/en/integrations/snowflake.mdx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,10 +389,10 @@ Export a table or query result to files in a Snowflake stage with COPY INTO.
389389
| `table` | string | No | Source table to unload; provide either this or a statement, not both |
390390
| `statement` | string | No | Source SELECT statement to unload; provide either this or a table, not both |
391391
| `fileFormat` | string | No | Named file format applied to the unloaded files |
392-
| `header` | boolean | No | Whether to write column headings into the unloaded files |
392+
| `header` | boolean | No | Whether to write column headings into the unloaded files; supported for CSV and Parquet only |
393393
| `overwrite` | boolean | No | Whether to replace existing files with matching names in the stage |
394394
| `singleFile` | boolean | No | Whether to write one file instead of splitting the output across files |
395-
| `maxFileSizeBytes` | number | No | Upper size limit per unloaded file in bytes; Snowflake defaults to 16000000 and allows up to 5000000000 |
395+
| `maxFileSizeBytes` | number | No | Upper size limit per unloaded file in bytes; Snowflake defaults to 16777216 \(16 MB\) and allows up to 5368709120 \(5 GB\) |
396396

397397
#### Output
398398

@@ -721,7 +721,7 @@ Resize a Snowflake warehouse or change its auto-suspend and auto-resume settings
721721
| `statementTimeoutSeconds` | number | No | Statement timeout in seconds; 0 uses Snowflake maximum of 604800 seconds |
722722
| `warehouseName` | string | Yes | Warehouse name |
723723
| `warehouseSize` | string | No | New warehouse size, one of $\{SNOWFLAKE_WAREHOUSE_SIZES.join\(', '\)\} |
724-
| `autoSuspendSeconds` | number | No | Seconds of inactivity before the warehouse suspends; 0 disables automatic suspension |
724+
| `autoSuspendSeconds` | number | No | Seconds of inactivity before the warehouse suspends. Snowflake polls every 30 seconds, so values under 30 or not a multiple of 30 may not behave as expected. 0 means the warehouse never suspends and keeps consuming credits |
725725
| `autoResume` | boolean | No | Whether the warehouse resumes automatically when a statement is submitted |
726726

727727
#### Output
@@ -1151,7 +1151,7 @@ List queries that completed in the last seven days, optionally filtered.
11511151
| `warehouseName` | string | No | Only queries run on this warehouse; cannot be combined with userName |
11521152
| `startTime` | string | No | ISO-8601 start of the query completion window, within the last seven days |
11531153
| `endTime` | string | No | ISO-8601 end of the query completion window, within the last seven days |
1154-
| `errorOnly` | boolean | No | Whether to return only queries whose execution status is FAIL |
1154+
| `errorOnly` | boolean | No | Whether to return only queries that failed \(execution status FAILED_WITH_ERROR or FAILED_WITH_INCIDENT\). Snowflake applies the limit before this filter, so it selects the failures among the most recent queries rather than the most recent failures |
11551155
| `limit` | number | No | Maximum query rows, from 1 to 10000 |
11561156

11571157
#### Output

apps/sim/app/api/tools/snowflake/objects/route.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
128128
return NextResponse.json({ error: getErrorMessage(error, 'Invalid request') }, { status: 400 })
129129
}
130130

131+
let upstreamStatus = 0
131132
try {
132133
const response = await fetch(`${baseUrl}/api/v2/statements`, {
133134
method: 'POST',
@@ -153,6 +154,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
153154
)
154155
}
155156

157+
// A 4xx names something wrong with the request (missing object, privilege
158+
// gap); only a 5xx or a malformed body is a gateway failure.
159+
upstreamStatus = response.status
156160
const result = await readSnowflakeResult(response)
157161
// A metadata-only statement completes synchronously; a 202 means Snowflake
158162
// deferred it, and returning an empty list would read as "no objects".
@@ -179,7 +183,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
179183
logger.error('Failed to list Snowflake objects', { kind, error })
180184
return NextResponse.json(
181185
{ error: getErrorMessage(error, 'Failed to list Snowflake objects') },
182-
{ status: 502 }
186+
{ status: upstreamStatus >= 400 && upstreamStatus < 500 ? 400 : 502 }
183187
)
184188
}
185189
})

apps/sim/hooks/selectors/providers/snowflake/selectors.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ import type { SnowflakeSelectorKind } from '@/tools/snowflake/selector-kinds'
1212
type SnowflakeSelectorKey = Extract<SelectorKey, `snowflake.${string}`>
1313

1414
/** What each picker needs in scope before it can list anything. */
15-
type SnowflakeSelectorScope = 'account' | 'database' | 'schema'
15+
type SnowflakeSelectorScopeLevel = 'account' | 'database' | 'schema'
1616

1717
interface SnowflakeSelectorSpec {
1818
kind: SnowflakeSelectorKind
19-
scope: SnowflakeSelectorScope
19+
scope: SnowflakeSelectorScopeLevel
2020
}
2121

2222
const SNOWFLAKE_SELECTOR_SPECS: Record<SnowflakeSelectorKey, SnowflakeSelectorSpec> = {

apps/sim/lib/credentials/token-service-accounts/descriptors.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,9 @@ export const TOKEN_SERVICE_ACCOUNT_DESCRIPTORS: Record<
413413
],
414414
docsUrl: 'https://docs.sim.ai/integrations/snowflake-service-account',
415415
helpText:
416-
'A programmatic access token acts as the Snowflake user that owns it, or as the single role it was restricted to. It always expires — 15 days by default, 365 at most — and human users must be covered by a network policy unless an authentication policy waives it.',
416+
'A programmatic access token acts as the Snowflake user that owns it, or as the single role it was restricted to. It always expires — 15 days by default, 365 at most — and every user type except SERVICE_AGENT must be covered by a network policy to authenticate with it, unless an authentication policy waives that.',
417417
invalidCredentialsHelp:
418-
'Snowflake rejected this token. Check that it belongs to a user on this exact account host, has not expired, and that a network policy allows Sim to reach the account.',
418+
'Snowflake rejected this token. Check that it belongs to a user on this exact account host, that it has not expired or been revoked, that a network policy allows Sim to reach the account, and that the SQL API is enabled for the account.',
419419
},
420420
}
421421

apps/sim/lib/credentials/token-service-accounts/validators/snowflake.test.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@ function jsonResponse(status: number, body: unknown): Response {
1616

1717
const fields = { apiToken: 'pat-secret', domain: 'MyOrg-MyAccount.snowflakecomputing.com' }
1818

19-
async function expectCode(promise: Promise<unknown>, code: string) {
19+
async function expectCode(promise: Promise<unknown>, code: string, status?: number) {
2020
await expect(promise).rejects.toBeInstanceOf(TokenServiceAccountValidationError)
2121
await promise.catch((error: TokenServiceAccountValidationError) => {
2222
expect(error.code).toBe(code)
23+
if (status !== undefined) expect(error.status).toBe(status)
2324
})
2425
}
2526

@@ -30,7 +31,9 @@ describe('validateSnowflakeServiceAccount', () => {
3031

3132
afterEach(() => {
3233
vi.unstubAllGlobals()
33-
vi.clearAllMocks()
34+
// resetAllMocks, not clearAllMocks: the latter leaves queued
35+
// mockResolvedValueOnce values behind to leak into the next test.
36+
vi.resetAllMocks()
3437
})
3538

3639
it('verifies through the SQL API with the PAT headers the tools use', async () => {
@@ -72,20 +75,30 @@ describe('validateSnowflakeServiceAccount', () => {
7275
await expectCode(validateSnowflakeServiceAccount(fields), 'site_not_found')
7376
})
7477

75-
it('maps 401 to a rejected token and 403 to the SQL API being disabled', async () => {
78+
/**
79+
* Snowflake answers 403 both for a disabled SQL API and for a network-policy
80+
* rejection, so both must reach the provider's invalid-credentials help,
81+
* which names every cause — not a "provider is down" message.
82+
*/
83+
it('maps 401 and 403 to a rejected credential', async () => {
7684
mockFetch.mockResolvedValueOnce(jsonResponse(401, { message: 'invalid token' }))
77-
await expectCode(validateSnowflakeServiceAccount(fields), 'invalid_credentials')
85+
await expectCode(validateSnowflakeServiceAccount(fields), 'invalid_credentials', 401)
7886

79-
mockFetch.mockResolvedValueOnce(jsonResponse(403, { message: 'SQL API not enabled' }))
80-
await expectCode(validateSnowflakeServiceAccount(fields), 'provider_unavailable')
87+
mockFetch.mockResolvedValueOnce(
88+
jsonResponse(403, { message: 'not allowed to access Snowflake' })
89+
)
90+
await expectCode(validateSnowflakeServiceAccount(fields), 'invalid_credentials', 403)
8191
})
8292

83-
it('treats a deferred statement and a metadata-less success as provider problems', async () => {
93+
it('treats a deferred statement and a metadata-less success as distinct provider problems', async () => {
94+
// The status is what separates these two: without the 202 branch the
95+
// deferred response would fall through to the metadata-less path and throw
96+
// 502, so asserting only the code cannot tell them apart.
8497
mockFetch.mockResolvedValueOnce(jsonResponse(202, { statementHandle: 'abc' }))
85-
await expectCode(validateSnowflakeServiceAccount(fields), 'provider_unavailable')
98+
await expectCode(validateSnowflakeServiceAccount(fields), 'provider_unavailable', 202)
8699

87100
mockFetch.mockResolvedValueOnce(jsonResponse(200, { data: [] }))
88-
await expectCode(validateSnowflakeServiceAccount(fields), 'provider_unavailable')
101+
await expectCode(validateSnowflakeServiceAccount(fields), 'provider_unavailable', 502)
89102
})
90103

91104
it('falls back to the account when the token reports no user', async () => {

apps/sim/lib/credentials/token-service-accounts/validators/snowflake.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -90,16 +90,11 @@ export async function validateSnowflakeServiceAccount(
9090
body: await readProviderErrorSnippet(res),
9191
})
9292
}
93-
// Snowflake documents 403 as "the SQL API is not enabled for this account",
94-
// which the shared 401/403 branch would misreport as a rejected token.
95-
if (res.status === 403) {
96-
throw new TokenServiceAccountValidationError('provider_unavailable', 403, {
97-
step: 'identity_statement',
98-
domain,
99-
reason: 'SQL API not enabled for this account',
100-
body: await readProviderErrorSnippet(res),
101-
})
102-
}
93+
// 403 is left to the shared 401/403 branch on purpose. Snowflake returns it
94+
// both when the SQL API is disabled and when a network policy rejects the
95+
// caller's IP, and the response wording for neither is documented — so rather
96+
// than guess from the body, both surface the provider's `invalidCredentialsHelp`,
97+
// which names every cause.
10398
await throwForProviderResponse(res, 'identity_statement', { domain })
10499

105100
// 202 means the statement is still executing. A context-only SELECT that

apps/sim/lib/workflows/migrations/subblock-migrations.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,56 @@ describe('migrateSubblockIds', () => {
6161
expect(blocks.b1.subBlocks.metrics).toBeUndefined()
6262
})
6363

64+
describe('snowflake block', () => {
65+
it('renames the object fields onto their pickers', () => {
66+
const input: Record<string, BlockState> = {
67+
b1: makeBlock({
68+
type: 'snowflake',
69+
subBlocks: {
70+
operation: { id: 'operation', type: 'dropdown', value: 'insert_rows' },
71+
database: { id: 'database', type: 'short-input', value: 'ANALYTICS' },
72+
schema: { id: 'schema', type: 'short-input', value: 'PUBLIC' },
73+
table: { id: 'table', type: 'short-input', value: 'EVENTS' },
74+
},
75+
}),
76+
}
77+
78+
const { blocks, migrated } = migrateSubblockIds(input)
79+
80+
expect(migrated).toBe(true)
81+
expect(blocks.b1.subBlocks.databaseSelector?.value).toBe('ANALYTICS')
82+
expect(blocks.b1.subBlocks.schemaSelector?.value).toBe('PUBLIC')
83+
expect(blocks.b1.subBlocks.tableSelector?.value).toBe('EVENTS')
84+
expect(blocks.b1.subBlocks.database).toBeUndefined()
85+
})
86+
87+
/**
88+
* Secret scrubbing for exports walks the block config, so a value parked
89+
* under a key the config no longer declares would never be cleared. A
90+
* `_removed_` target must drop the value, not carry it forward.
91+
*/
92+
it('discards the retired host and programmatic access token', () => {
93+
const input: Record<string, BlockState> = {
94+
b1: makeBlock({
95+
type: 'snowflake',
96+
subBlocks: {
97+
host: { id: 'host', type: 'short-input', value: 'acme.snowflakecomputing.com' },
98+
apiKey: { id: 'apiKey', type: 'short-input', value: 'super-secret-pat' },
99+
},
100+
}),
101+
}
102+
103+
const { blocks, migrated } = migrateSubblockIds(input)
104+
105+
expect(migrated).toBe(true)
106+
expect(blocks.b1.subBlocks.apiKey).toBeUndefined()
107+
expect(blocks.b1.subBlocks.host).toBeUndefined()
108+
expect(blocks.b1.subBlocks._removed_apiKey).toBeUndefined()
109+
expect(blocks.b1.subBlocks._removed_host).toBeUndefined()
110+
expect(JSON.stringify(blocks.b1)).not.toContain('super-secret-pat')
111+
})
112+
})
113+
64114
describe('knowledge block', () => {
65115
it('should rename knowledgeBaseId to knowledgeBaseSelector', () => {
66116
const input: Record<string, BlockState> = {

apps/sim/lib/workflows/migrations/subblock-migrations.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ import type { BlockState } from '@/stores/workflows/workflow/types'
1313

1414
const logger = createLogger('SubblockMigrations')
1515

16+
/**
17+
* Marks a migration target as "this field is gone", rather than a rename. The
18+
* old value is discarded instead of being carried into workflow state.
19+
*/
20+
const REMOVED_SUBBLOCK_ID_PREFIX = '_removed_'
21+
1622
/**
1723
* Maps old subblock IDs to their current equivalents per block type.
1824
*
@@ -21,6 +27,10 @@ const logger = createLogger('SubblockMigrations')
2127
* serializer silently drops the value, breaking execution.
2228
*
2329
* Format: { blockType: { oldSubblockId: newSubblockId } }
30+
*
31+
* A target prefixed with `_removed_` means the field was deleted outright; the
32+
* stored value is dropped. Use it for fields with no replacement — never map a
33+
* secret onto a live subblock.
2434
*/
2535
export const SUBBLOCK_ID_MIGRATIONS: Record<string, Record<string, string>> = {
2636
instagram: {
@@ -151,6 +161,16 @@ function migrateBlockSubblockIds(
151161
for (const [oldId, newId] of Object.entries(renames)) {
152162
if (!(oldId in result)) continue
153163

164+
// A `_removed_` target means the field no longer exists in the block. Drop
165+
// the value rather than parking it under a dead key: nothing ever reads
166+
// these keys, and secret scrubbing walks the block config, so a parked
167+
// `password: true` value would never be cleared and would ride along in
168+
// workflow exports and templates.
169+
if (newId.startsWith(REMOVED_SUBBLOCK_ID_PREFIX)) {
170+
delete result[oldId]
171+
continue
172+
}
173+
154174
if (newId in result) {
155175
delete result[oldId]
156176
continue

apps/sim/tools/generated/tool-metadata.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)