Conversation
There was a problem hiding this comment.
This is a new plugin with several documentation issues already flagged inline, plus author.type is set to "labs" rather than "community" as required by guidelines — worth a human look to confirm that's intentional.
Extended reasoning...
Overview\nThis PR introduces a brand-new Snowflake plugin, adding 8 files: metadata.json, ui.json, configValidation.json, a SQL query data stream (with post-processing script), an SVG icon, README, and a setup guide. It is a self-contained plugin directory with no changes to existing files.\n\n### Security risks\nThe plugin uses OAuth 2.0 with auth code grant, which is the appropriate authentication pattern. The OAuth client secret is handled via a password field and the oauth2ClientSecretLocationDuringAuth: header setting. No hardcoded secrets or credentials are present. The SQL query data stream passes user-supplied SQL directly to Snowflake's SQL API — this is inherent to the plugin's purpose and consistent with other SQL-based plugins in the repo.\n\n### Level of scrutiny\nNew plugin PRs warrant human review regardless of bug count. The inline bug comments cover several documentation nits (typo, copy-paste errors, heading convention violations), but there is one additional concern not covered by inline comments: metadata.json sets author.type to "labs" and author.name to "SquaredUp Labs". The repo guidelines state author.type should always be "community". This may be intentional for a SquaredUp-internal plugin, but a human should confirm.\n\n### Other factors\nNo CODEOWNERS entry is added for this plugin directory, which the guidelines encourage for new plugins. The multiple documentation issues (4 nits) suggest the docs could use a polish pass before merging.
|
|
||
| ``` | ||
| CREATE SECURITY INTEGRATION oauth_squaredup | ||
| TYPE = oauth | ||
| OAUTH_CLIENT = custom | ||
| OAUTH_CLIENT_TYPE = 'CONFIDENTIAL' | ||
| OAUTH_REDIRECT_URI = 'https://app.squaredup.com/settings/pluginsoauth2' | ||
| COMMENT = 'Used by SquaredUp to connect to this Snowflake account' | ||
| ``` | ||
|
|
||
| If your SquaredUp account is in the EU region (default): | ||
|
|
||
| ``` | ||
| CREATE SECURITY INTEGRATION oauth_squaredup | ||
| TYPE = oauth |
There was a problem hiding this comment.
🟡 Both the US and EU region OAuth sections in docs/setup.md are labeled (default), but only one region can be the default — the EU section's (default) label is a copy-paste error. Remove (default) from the EU region heading so users aren't confused about which region applies to them.
Extended reasoning...
What the bug is and how it manifests
In plugins/Snowflake/v1/docs/setup.md, two consecutive OAuth integration code blocks guide users to create a Snowflake security integration with the correct redirect URI for their SquaredUp region. Both section headings currently read (default): line 15 says "If your SquaredUp account is in the US region (default):" and line 26 says "If your SquaredUp account is in the EU region (default):". By definition, only a single region can be the default.
The specific code path that triggers it
This is purely a documentation issue in the newly added setup.md file. The EU section heading was authored by copy-pasting the US section heading and updating the region name and redirect URI, but the (default) label was accidentally left in place.
Why existing checks don't prevent it
There is no automated lint or validation step for documentation prose in this repository, so the copy-paste error passed undetected. Both code blocks differ only in the OAUTH_REDIRECT_URI value (https://app.squaredup.com/... vs https://eu.app.squaredup.com/...), which means the actual functional content is correct — only the label is wrong.
Impact
A new EU user reading the setup guide sees both sections labeled (default) and is uncertain which applies to them. They may follow the US instructions and configure the wrong redirect URI, causing OAuth authorization to fail entirely. Conversely, a US user skimming the page could be misled into thinking the EU section is equally authoritative as the default path.
How to fix it
Remove the word (default) from line 26 so it reads: "If your SquaredUp account is in the EU region:"
Step-by-step proof
- Open
plugins/Snowflake/v1/docs/setup.md. - Line 15: "If your SquaredUp account is in the US region (default):" — correct, US is the default.
- Line 26: "If your SquaredUp account is in the EU region (default):" — incorrect, EU cannot also be the default.
- A user in the EU reads both labels and reasonably concludes either section applies, potentially selecting the US section and setting
OAUTH_REDIRECT_URI = 'https://app.squaredup.com/settings/pluginsoauth2', which will cause OAuth login to fail for their EU account because SquaredUp will reject the redirect.
|
|
||
| Enter your Snowflake account identifier. | ||
|
|
||
| This can be found in the Snowflake portal under 'Your Username' > Account > Account Identifier. |
There was a problem hiding this comment.
🟡 The line 'For example: https://<your-opensearch-host>:9200' in the Snowflake account identifier section of docs/setup.md is a copy-paste error from an OpenSearch plugin. It should be removed, as the correct format is already documented on the line above: <org_name>-<account_name>, e.g. ABCDEFG-XYZ12345.
Extended reasoning...
What the bug is: Line 71 of plugins/Snowflake/v1/docs/setup.md contains the text For example: https://:9200`` inside the "Snowflake account identifier" configuration section. This is an OpenSearch/Elasticsearch URL referencing port 9200 (the default OpenSearch port) and has absolutely no relevance to Snowflake.
The specific code path that triggers it: Any user reading the setup documentation to configure the Snowflake plugin will encounter this misleading example when trying to determine what value to enter for the "Snowflake account identifier" field.
Why existing code doesn't prevent it: This is a documentation error — there is no automated check to catch copy-paste mistakes in markdown files. The surrounding text on the previous line already correctly describes the format (<org_name>-<account_name>, e.g. ABCDEFG-XYZ12345), making the erroneous "For example" line all the more confusing since it contradicts the correct description immediately above it.
Impact: A user reading this documentation would see two conflicting pieces of information: one saying the format is <org_name>-<account_name> and another saying it looks like https://<your-opensearch-host>:9200. This would confuse users and may cause them to enter an incorrect value for the account identifier field, resulting in failed authentication.
How to fix it: Simply remove line 71 (For example: https://:9200``) entirely. The correct format and example (ABCDEFG-XYZ12345) is already present on the preceding line, so no replacement is needed.
Step-by-step proof:
- Open
plugins/Snowflake/v1/docs/setup.mdand navigate to the "Snowflake account identifier" section (around line 63). - Line 70 correctly states:
The account identifier It is in the format <org_name>-<account_name>, e.g. ABCDEFG-XYZ12345 - Line 71 then states:
For example:https://:9200`` — this references an OpenSearch host URL with port 9200. - Compare with any OpenSearch plugin docs; this example belongs there, not in a Snowflake account identifier section.
- A user following this doc would be confused about whether to enter
ABCDEFG-XYZ12345orhttps://myhost:9200as their account identifier.
| @@ -0,0 +1,3 @@ | |||
| ## Overview | |||
There was a problem hiding this comment.
🟡 The README.md starts with ## Overview, which violates two repo guidelines: headings should start at level 1 (not level 2), and headings should not use 'Overview'. Replace it with a level-1 heading like # Before you start to match the pattern used in setup.md.
Extended reasoning...
The README.md file opens with ## Overview, which simultaneously breaks two explicit repository guidelines.
Guideline 1 — Start headings from level 1: The repository documentation guidelines state that docs should typically start headings from level 1 (#), not level 2 (##). Using ## as the first heading implies there should be a parent # heading above it, which does not exist. The companion file setup.md demonstrates the correct pattern: it begins with # Before you start (a level-1 heading).
Guideline 2 — Avoid Overview as a heading: The guidelines explicitly call out Overview as a heading to avoid, along with headings that simply repeat the plugin name. Overview is generic and adds no informational value compared to a descriptive heading like # Before you start or # Prerequisites.
Code path: The violation is at line 1 of plugins/Snowflake/v1/README.md. The entire file is only three lines: the ## Overview heading, a blank line, and a single descriptive sentence.
Why existing code does not prevent it: There is no automated linting or CI check enforcing heading level or heading text conventions — this is a style guideline that requires human review.
Impact: The README.md is the file shown within SquaredUp under the Need help section, making it user-facing. A level-2 heading as the first heading can render oddly in some Markdown viewers and signals a structural inconsistency compared to other plugin READMEs in the repo.
Step-by-step proof:
- Open plugins/Snowflake/v1/README.md
- Line 1 is ## Overview — a level-2 heading (two # symbols)
- Repo guideline says start at level 1 (#) — violation confirmed
- Repo guideline says avoid Overview as a heading text — violation confirmed
- Compare to plugins/Snowflake/v1/docs/setup.md line 1: # Before you start — this is the correct pattern
Fix: Change ## Overview to # Before you start (or another descriptive level-1 heading that is not Overview).
| ## Creating an OAuth integration in Snowflake | ||
|
|
||
| The Snowflake data source authenticates using OAuth. | ||
|
|
There was a problem hiding this comment.
🟡 Line 6 of plugins/Snowflake/v1/docs/setup.md contains a typo: "bby" should be "by" in the sentence "register SquaredUp with your Snowflake account bby creating a custom integration." This is a minor documentation error that should be corrected before merging.
Extended reasoning...
The typo 'bby' (a double-b error) appears on line 6 of docs/setup.md in the newly added Snowflake plugin documentation. The affected sentence reads: 'Before configuring the data source you will need to register SquaredUp with your Snowflake account bby creating a custom integration.'
The correct word should be 'by', making the sentence: 'Before configuring the data source you will need to register SquaredUp with your Snowflake account by creating a custom integration.'
This is a straightforward typographical error introduced in this PR. The file docs/setup.md is a new file added as part of the Snowflake plugin, so the typo was introduced in this change.
While the error does not affect the functionality of the plugin in any way (it is purely documentation), it presents a poor first impression to users following the setup guide. The sentence appears in the introductory section explaining OAuth setup, which users will read first when configuring the plugin.
The fix is trivial: replace 'bby' with 'by' on line 6 of plugins/Snowflake/v1/docs/setup.md.
Step-by-step proof: Reading the diff for docs/setup.md, line 6 (the 6th non-blank added line) reads: 'Before configuring the data source you will need to register SquaredUp with your Snowflake account bby creating a custom integration.' The word 'bby' is clearly 'by' with an extra 'b' character.
🧩 Plugin PR Summary📦 Modified Plugins
📋 Results
🔍 Validation Details✅
|
🔌 Plugin overview
🖼️ Plugin screenshots
Plugin configuration
Default dashboards
None
🧪 Testing
Tested against a Snowflake Standard on AWS.
Supports just a single
SQL Querydata stream that connects to the Snowflake SQL API. No other Snowflake REST APIs supported.📚 Checklist