feat(snowflake): snowflake integration #1993
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile OverviewGreptile SummaryThis PR adds a comprehensive Snowflake integration with OAuth authentication and multiple database operation tools. The implementation includes proper PKCE flow for OAuth security and covers common database operations (query, insert, update, delete, and various list operations). Key Changes:
Critical Issues:
Recommendations:
Confidence Score: 1/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Frontend
participant AuthAPI as Auth Routes
participant Snowflake
participant Database
participant ToolAPI as Tool Execution
Note over User,Database: OAuth Flow
User->>Frontend: Initiate connection
Frontend->>AuthAPI: Start OAuth
AuthAPI->>AuthAPI: Generate PKCE
AuthAPI->>Snowflake: Authorization request
Snowflake->>User: Consent prompt
User->>Snowflake: Grant permission
Snowflake->>AuthAPI: Return authorization
AuthAPI->>Snowflake: Exchange for credentials
Snowflake->>AuthAPI: Provide credentials
AuthAPI->>Database: Store credentials
AuthAPI->>Frontend: Complete flow
Note over User,ToolAPI: Query Execution
User->>Frontend: Execute SQL operation
Frontend->>ToolAPI: Operation request
ToolAPI->>ToolAPI: Build SQL
ToolAPI->>Database: Get credentials
ToolAPI->>Snowflake: Execute query
Snowflake->>ToolAPI: Query results
ToolAPI->>Frontend: Return data
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
14
to
29
| function buildDeleteSQL( | ||
| database: string, | ||
| schema: string, | ||
| table: string, | ||
| whereClause?: string | ||
| ): string { | ||
| const fullTableName = `${database}.${schema}.${table}` | ||
|
|
||
| let sql = `DELETE FROM ${fullTableName}` | ||
|
|
||
| // Add WHERE clause if provided | ||
| if (whereClause && whereClause.trim()) { | ||
| sql += ` WHERE ${whereClause}` | ||
| } | ||
|
|
||
| return sql |
Contributor
There was a problem hiding this comment.
logic: SQL injection vulnerability: database, schema, table, and whereClause are concatenated directly into SQL without any validation or identifier quoting. Malicious input like table = "users; DROP TABLE users--" could execute arbitrary SQL.
Suggested change
| function buildDeleteSQL( | |
| database: string, | |
| schema: string, | |
| table: string, | |
| whereClause?: string | |
| ): string { | |
| const fullTableName = `${database}.${schema}.${table}` | |
| let sql = `DELETE FROM ${fullTableName}` | |
| // Add WHERE clause if provided | |
| if (whereClause && whereClause.trim()) { | |
| sql += ` WHERE ${whereClause}` | |
| } | |
| return sql | |
| function buildDeleteSQL( | |
| database: string, | |
| schema: string, | |
| table: string, | |
| whereClause?: string | |
| ): string { | |
| // Snowflake identifier quoting requires double quotes for SQL identifiers | |
| const quotedDatabase = `"${database.replace(/"/g, '""')}"` | |
| const quotedSchema = `"${schema.replace(/"/g, '""')}"` | |
| const quotedTable = `"${table.replace(/"/g, '""')}"` | |
| const fullTableName = `${quotedDatabase}.${quotedSchema}.${quotedTable}` | |
| let sql = `DELETE FROM ${fullTableName}` | |
| // Add WHERE clause if provided | |
| // Note: whereClause still needs proper parameterization - consider using Snowflake bind variables | |
| if (whereClause && whereClause.trim()) { | |
| sql += ` WHERE ${whereClause}` | |
| } | |
| return sql | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/tools/snowflake/delete_rows.ts
Line: 14:29
Comment:
**logic:** SQL injection vulnerability: `database`, `schema`, `table`, and `whereClause` are concatenated directly into SQL without any validation or identifier quoting. Malicious input like `table = "users; DROP TABLE users--"` could execute arbitrary SQL.
```suggestion
function buildDeleteSQL(
database: string,
schema: string,
table: string,
whereClause?: string
): string {
// Snowflake identifier quoting requires double quotes for SQL identifiers
const quotedDatabase = `"${database.replace(/"/g, '""')}"`
const quotedSchema = `"${schema.replace(/"/g, '""')}"`
const quotedTable = `"${table.replace(/"/g, '""')}"`
const fullTableName = `${quotedDatabase}.${quotedSchema}.${quotedTable}`
let sql = `DELETE FROM ${fullTableName}`
// Add WHERE clause if provided
// Note: whereClause still needs proper parameterization - consider using Snowflake bind variables
if (whereClause && whereClause.trim()) {
sql += ` WHERE ${whereClause}`
}
return sql
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
14
to
45
| function buildInsertSQL( | ||
| database: string, | ||
| schema: string, | ||
| table: string, | ||
| columns: string[], | ||
| values: any[][] | ||
| ): string { | ||
| const fullTableName = `${database}.${schema}.${table}` | ||
| const columnList = columns.join(', ') | ||
|
|
||
| // Build values clause for multiple rows | ||
| const valuesClause = values | ||
| .map((rowValues) => { | ||
| const formattedValues = rowValues.map((val) => { | ||
| if (val === null || val === undefined) { | ||
| return 'NULL' | ||
| } | ||
| if (typeof val === 'string') { | ||
| // Escape single quotes by doubling them | ||
| return `'${val.replace(/'/g, "''")}'` | ||
| } | ||
| if (typeof val === 'boolean') { | ||
| return val ? 'TRUE' : 'FALSE' | ||
| } | ||
| return String(val) | ||
| }) | ||
| return `(${formattedValues.join(', ')})` | ||
| }) | ||
| .join(', ') | ||
|
|
||
| return `INSERT INTO ${fullTableName} (${columnList}) VALUES ${valuesClause}` | ||
| } |
Contributor
There was a problem hiding this comment.
logic: SQL injection vulnerability: database, schema, table, and columns array are concatenated directly into SQL without identifier quoting. Malicious input in column names like "col1, (SELECT password FROM users)--" could expose data.
Suggested change
| function buildInsertSQL( | |
| database: string, | |
| schema: string, | |
| table: string, | |
| columns: string[], | |
| values: any[][] | |
| ): string { | |
| const fullTableName = `${database}.${schema}.${table}` | |
| const columnList = columns.join(', ') | |
| // Build values clause for multiple rows | |
| const valuesClause = values | |
| .map((rowValues) => { | |
| const formattedValues = rowValues.map((val) => { | |
| if (val === null || val === undefined) { | |
| return 'NULL' | |
| } | |
| if (typeof val === 'string') { | |
| // Escape single quotes by doubling them | |
| return `'${val.replace(/'/g, "''")}'` | |
| } | |
| if (typeof val === 'boolean') { | |
| return val ? 'TRUE' : 'FALSE' | |
| } | |
| return String(val) | |
| }) | |
| return `(${formattedValues.join(', ')})` | |
| }) | |
| .join(', ') | |
| return `INSERT INTO ${fullTableName} (${columnList}) VALUES ${valuesClause}` | |
| } | |
| function buildInsertSQL( | |
| database: string, | |
| schema: string, | |
| table: string, | |
| columns: string[], | |
| values: any[][] | |
| ): string { | |
| // Quote SQL identifiers to prevent injection | |
| const quotedDatabase = `"${database.replace(/"/g, '""')}"` | |
| const quotedSchema = `"${schema.replace(/"/g, '""')}"` | |
| const quotedTable = `"${table.replace(/"/g, '""')}"` | |
| const fullTableName = `${quotedDatabase}.${quotedSchema}.${quotedTable}` | |
| // Quote column names as SQL identifiers | |
| const columnList = columns.map(col => `"${col.replace(/"/g, '""')}"`).join(', ') | |
| // Build values clause for multiple rows | |
| const valuesClause = values | |
| .map((rowValues) => { | |
| const formattedValues = rowValues.map((val) => { | |
| if (val === null || val === undefined) { | |
| return 'NULL' | |
| } | |
| if (typeof val === 'string') { | |
| // Escape single quotes by doubling them | |
| return `'${val.replace(/'/g, "''")}'` | |
| } | |
| if (typeof val === 'boolean') { | |
| return val ? 'TRUE' : 'FALSE' | |
| } | |
| return String(val) | |
| }) | |
| return `(${formattedValues.join(', ')})` | |
| }) | |
| .join(', ') | |
| return `INSERT INTO ${fullTableName} (${columnList}) VALUES ${valuesClause}` | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/tools/snowflake/insert_rows.ts
Line: 14:45
Comment:
**logic:** SQL injection vulnerability: `database`, `schema`, `table`, and `columns` array are concatenated directly into SQL without identifier quoting. Malicious input in column names like `"col1, (SELECT password FROM users)--"` could expose data.
```suggestion
function buildInsertSQL(
database: string,
schema: string,
table: string,
columns: string[],
values: any[][]
): string {
// Quote SQL identifiers to prevent injection
const quotedDatabase = `"${database.replace(/"/g, '""')}"`
const quotedSchema = `"${schema.replace(/"/g, '""')}"`
const quotedTable = `"${table.replace(/"/g, '""')}"`
const fullTableName = `${quotedDatabase}.${quotedSchema}.${quotedTable}`
// Quote column names as SQL identifiers
const columnList = columns.map(col => `"${col.replace(/"/g, '""')}"`).join(', ')
// Build values clause for multiple rows
const valuesClause = values
.map((rowValues) => {
const formattedValues = rowValues.map((val) => {
if (val === null || val === undefined) {
return 'NULL'
}
if (typeof val === 'string') {
// Escape single quotes by doubling them
return `'${val.replace(/'/g, "''")}'`
}
if (typeof val === 'boolean') {
return val ? 'TRUE' : 'FALSE'
}
return String(val)
})
return `(${formattedValues.join(', ')})`
})
.join(', ')
return `INSERT INTO ${fullTableName} (${columnList}) VALUES ${valuesClause}`
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
14
to
51
| function buildUpdateSQL( | ||
| database: string, | ||
| schema: string, | ||
| table: string, | ||
| updates: Record<string, any>, | ||
| whereClause?: string | ||
| ): string { | ||
| const fullTableName = `${database}.${schema}.${table}` | ||
|
|
||
| // Build SET clause | ||
| const setClause = Object.entries(updates) | ||
| .map(([column, value]) => { | ||
| let formattedValue: string | ||
|
|
||
| if (value === null || value === undefined) { | ||
| formattedValue = 'NULL' | ||
| } else if (typeof value === 'string') { | ||
| // Escape single quotes by doubling them | ||
| formattedValue = `'${value.replace(/'/g, "''")}'` | ||
| } else if (typeof value === 'boolean') { | ||
| formattedValue = value ? 'TRUE' : 'FALSE' | ||
| } else { | ||
| formattedValue = String(value) | ||
| } | ||
|
|
||
| return `${column} = ${formattedValue}` | ||
| }) | ||
| .join(', ') | ||
|
|
||
| let sql = `UPDATE ${fullTableName} SET ${setClause}` | ||
|
|
||
| // Add WHERE clause if provided | ||
| if (whereClause && whereClause.trim()) { | ||
| sql += ` WHERE ${whereClause}` | ||
| } | ||
|
|
||
| return sql | ||
| } |
Contributor
There was a problem hiding this comment.
logic: SQL injection vulnerability: database, schema, table, and column names in updates object are concatenated directly into SQL without identifier quoting. Malicious column names could inject arbitrary SQL.
Suggested change
| function buildUpdateSQL( | |
| database: string, | |
| schema: string, | |
| table: string, | |
| updates: Record<string, any>, | |
| whereClause?: string | |
| ): string { | |
| const fullTableName = `${database}.${schema}.${table}` | |
| // Build SET clause | |
| const setClause = Object.entries(updates) | |
| .map(([column, value]) => { | |
| let formattedValue: string | |
| if (value === null || value === undefined) { | |
| formattedValue = 'NULL' | |
| } else if (typeof value === 'string') { | |
| // Escape single quotes by doubling them | |
| formattedValue = `'${value.replace(/'/g, "''")}'` | |
| } else if (typeof value === 'boolean') { | |
| formattedValue = value ? 'TRUE' : 'FALSE' | |
| } else { | |
| formattedValue = String(value) | |
| } | |
| return `${column} = ${formattedValue}` | |
| }) | |
| .join(', ') | |
| let sql = `UPDATE ${fullTableName} SET ${setClause}` | |
| // Add WHERE clause if provided | |
| if (whereClause && whereClause.trim()) { | |
| sql += ` WHERE ${whereClause}` | |
| } | |
| return sql | |
| } | |
| function buildUpdateSQL( | |
| database: string, | |
| schema: string, | |
| table: string, | |
| updates: Record<string, any>, | |
| whereClause?: string | |
| ): string { | |
| // Quote SQL identifiers to prevent injection | |
| const quotedDatabase = `"${database.replace(/"/g, '""')}"` | |
| const quotedSchema = `"${schema.replace(/"/g, '""')}"` | |
| const quotedTable = `"${table.replace(/"/g, '""')}"` | |
| const fullTableName = `${quotedDatabase}.${quotedSchema}.${quotedTable}` | |
| // Build SET clause with quoted column identifiers | |
| const setClause = Object.entries(updates) | |
| .map(([column, value]) => { | |
| // Quote column name as SQL identifier | |
| const quotedColumn = `"${column.replace(/"/g, '""')}"` | |
| let formattedValue: string | |
| if (value === null || value === undefined) { | |
| formattedValue = 'NULL' | |
| } else if (typeof value === 'string') { | |
| // Escape single quotes by doubling them | |
| formattedValue = `'${value.replace(/'/g, "''")}'` | |
| } else if (typeof value === 'boolean') { | |
| formattedValue = value ? 'TRUE' : 'FALSE' | |
| } else { | |
| formattedValue = String(value) | |
| } | |
| return `${quotedColumn} = ${formattedValue}` | |
| }) | |
| .join(', ') | |
| let sql = `UPDATE ${fullTableName} SET ${setClause}` | |
| // Add WHERE clause if provided | |
| // Note: whereClause still needs proper parameterization | |
| if (whereClause && whereClause.trim()) { | |
| sql += ` WHERE ${whereClause}` | |
| } | |
| return sql | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/tools/snowflake/update_rows.ts
Line: 14:51
Comment:
**logic:** SQL injection vulnerability: `database`, `schema`, `table`, and column names in `updates` object are concatenated directly into SQL without identifier quoting. Malicious column names could inject arbitrary SQL.
```suggestion
function buildUpdateSQL(
database: string,
schema: string,
table: string,
updates: Record<string, any>,
whereClause?: string
): string {
// Quote SQL identifiers to prevent injection
const quotedDatabase = `"${database.replace(/"/g, '""')}"`
const quotedSchema = `"${schema.replace(/"/g, '""')}"`
const quotedTable = `"${table.replace(/"/g, '""')}"`
const fullTableName = `${quotedDatabase}.${quotedSchema}.${quotedTable}`
// Build SET clause with quoted column identifiers
const setClause = Object.entries(updates)
.map(([column, value]) => {
// Quote column name as SQL identifier
const quotedColumn = `"${column.replace(/"/g, '""')}"`
let formattedValue: string
if (value === null || value === undefined) {
formattedValue = 'NULL'
} else if (typeof value === 'string') {
// Escape single quotes by doubling them
formattedValue = `'${value.replace(/'/g, "''")}'`
} else if (typeof value === 'boolean') {
formattedValue = value ? 'TRUE' : 'FALSE'
} else {
formattedValue = String(value)
}
return `${quotedColumn} = ${formattedValue}`
})
.join(', ')
let sql = `UPDATE ${fullTableName} SET ${setClause}`
// Add WHERE clause if provided
// Note: whereClause still needs proper parameterization
if (whereClause && whereClause.trim()) {
sql += ` WHERE ${whereClause}`
}
return sql
}
```
How can I resolve this? If you propose a fix, please make it concise.
aadamgough
force-pushed
the
feat/snowflake
branch
from
a128838 to
c0d357d
Compare
AVtheking
reviewed
Nov 19, 2025
| ) | ||
| } | ||
|
|
||
| const clientId = env.SNOWFLAKE_CLIENT_ID |
There was a problem hiding this comment.
isn't snowflake require clientId and client secret of the user account ?
aadamgough
force-pushed
the
feat/snowflake
branch
from
1f8c88f to
ee04fdc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Snowflake integration. Still testing some auth stuff for creating and deleting tables.
Fixes #(issue)
Type of Change
Testing
Tested manually
Checklist