feat(extensions): add databricks skill pack scaffold#56592
Conversation
Greptile SummaryThis PR adds a new Key findings:
Confidence Score: 4/5Not safe to merge as-is — the package scope violates the documented One P1 finding: the
|
| Filename | Overview |
|---|---|
| extensions/databricks/package.json | Package scope uses @kansodata/databricks instead of the required @openclaw/databricks; version 0.0.1 deviates from the repo-wide date-based convention. |
| .github/labeler.yml | Adds the extensions: databricks labeler rule; correct glob pattern, but the entry is placed after extensions: deepseek when it should appear before it alphabetically. |
| extensions/databricks/index.ts | Scaffold entry point following the same shape as extensions/open-prose/index.ts; no functional code, only metadata registration. |
| extensions/databricks/openclaw.plugin.json | Plugin manifest correctly mirrors the open-prose pattern; id, name, description, skills, and configSchema all look correct. |
| extensions/databricks/runtime-api.ts | Re-export barrel for the plugin SDK; identical pattern to other extensions, no issues. |
| extensions/databricks/skills/databricks/SKILL.md | Well-structured skill file with clear scope, input prompts, output format, and three templated workflows (SQL, jobs, governance). No issues. |
| extensions/databricks/README.md | Install and enable instructions look correct; uses the @kansodata/databricks install spec which would need updating if the package is renamed. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/databricks/package.json
Line: 2
Comment:
**Package scope violates the repo naming convention**
`CLAUDE.md` / `AGENTS.md` explicitly requires that all repo-owned workspace plugins use the `@openclaw/<id>` scope (or an approved suffix form like `-provider`, `-plugin`, etc.). Every other extension in this repo follows that convention (e.g. `@openclaw/open-prose`, `@openclaw/deepseek-provider`). The `@kansodata/` scope here is a third-party vendor scope.
The guideline also states:
> "Exceptions must be explicit and covered by the repo invariant test."
This PR includes neither an updated invariant test nor an explicit in-repo exception for `@kansodata/databricks`. If the `check-additional` CI architecture gate enforces the naming invariant, this PR will fail.
Either rename the package to `@openclaw/databricks` (the expected form for first-party workspace plugins) or add a documented, test-covered exception before landing.
```suggestion
"name": "@openclaw/databricks",
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/databricks/package.json
Line: 3
Comment:
**Version does not follow the repo date-based convention**
All existing extension packages use a date-based version string (e.g. `"2026.3.27"`). `0.0.1` is inconsistent with this pattern and may cause confusion or break any tooling that expects the canonical versioning format.
```suggestion
"version": "2026.3.28",
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: .github/labeler.yml
Line: 244-248
Comment:
**Labeler entry placed out of alphabetical order**
The new `"extensions: databricks"` entry is inserted after `"extensions: deepseek"`, but `databricks` (d-a-t-a…) sorts before `deepseek` (d-e-e-p…) alphabetically. The surrounding entries follow alphabetical ordering. Move this block before the `"extensions: deepseek"` entry to stay consistent.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(extensions): add databricks skill p..." | Re-trigger Greptile
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f100aec792
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,12 @@ | |||
| { | |||
| "name": "@kansodata/databricks", | |||
There was a problem hiding this comment.
This package name violates the bundled plugin naming invariant: src/plugins/bundled-plugin-naming.test.ts only allows @openclaw/<id> (plus approved suffixes), so setting @kansodata/databricks will make that guardrail fail when the test suite runs and breaks CI for this commit. If a non-@openclaw namespace is intentional, the invariant/docs need to be updated in the same change; otherwise this should be renamed to an allowed canonical form.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,12 @@ | |||
| { | |||
| "name": "@kansodata/databricks", | |||
There was a problem hiding this comment.
CLAUDE.md / AGENTS.md explicitly requires that all repo-owned workspace plugins use the @openclaw/<id> scope (or an approved suffix form like -provider, -plugin, etc.). Every other extension in this repo follows that convention (e.g. @openclaw/open-prose, @openclaw/deepseek-provider). The @kansodata/ scope here is a third-party vendor scope.
The guideline also states:
"Exceptions must be explicit and covered by the repo invariant test."
This PR includes neither an updated invariant test nor an explicit in-repo exception for @kansodata/databricks. If the check-additional CI architecture gate enforces the naming invariant, this PR will fail.
Either rename the package to @openclaw/databricks (the expected form for first-party workspace plugins) or add a documented, test-covered exception before landing.
| "name": "@kansodata/databricks", | |
| "name": "@openclaw/databricks", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/databricks/package.json
Line: 2
Comment:
**Package scope violates the repo naming convention**
`CLAUDE.md` / `AGENTS.md` explicitly requires that all repo-owned workspace plugins use the `@openclaw/<id>` scope (or an approved suffix form like `-provider`, `-plugin`, etc.). Every other extension in this repo follows that convention (e.g. `@openclaw/open-prose`, `@openclaw/deepseek-provider`). The `@kansodata/` scope here is a third-party vendor scope.
The guideline also states:
> "Exceptions must be explicit and covered by the repo invariant test."
This PR includes neither an updated invariant test nor an explicit in-repo exception for `@kansodata/databricks`. If the `check-additional` CI architecture gate enforces the naming invariant, this PR will fail.
Either rename the package to `@openclaw/databricks` (the expected form for first-party workspace plugins) or add a documented, test-covered exception before landing.
```suggestion
"name": "@openclaw/databricks",
```
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,12 @@ | |||
| { | |||
| "name": "@kansodata/databricks", | |||
| "version": "0.0.1", | |||
There was a problem hiding this comment.
All existing extension packages use a date-based version string (e.g. "2026.3.27"). 0.0.1 is inconsistent with this pattern and may cause confusion or break any tooling that expects the canonical versioning format.
| "version": "0.0.1", | |
| "version": "2026.3.28", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/databricks/package.json
Line: 3
Comment:
**Version does not follow the repo date-based convention**
All existing extension packages use a date-based version string (e.g. `"2026.3.27"`). `0.0.1` is inconsistent with this pattern and may cause confusion or break any tooling that expects the canonical versioning format.
```suggestion
"version": "2026.3.28",
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| "extensions: databricks": | ||
| - changed-files: | ||
| - any-glob-to-any-file: | ||
| - "extensions/databricks/**" | ||
| "extensions: anthropic": |
There was a problem hiding this comment.
The new "extensions: databricks" entry is inserted after "extensions: deepseek", but databricks (d-a-t-a…) sorts before deepseek (d-e-e-p…) alphabetically. The surrounding entries follow alphabetical ordering. Move this block before the "extensions: deepseek" entry to stay consistent.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/labeler.yml
Line: 244-248
Comment:
**Labeler entry placed out of alphabetical order**
The new `"extensions: databricks"` entry is inserted after `"extensions: deepseek"`, but `databricks` (d-a-t-a…) sorts before `deepseek` (d-e-e-p…) alphabetically. The surrounding entries follow alphabetical ordering. Move this block before the `"extensions: deepseek"` entry to stay consistent.
How can I resolve this? If you propose a fix, please make it concise.
Kansodata
changed the title
|
Gracias por el feedback. Ya quedó abordado en el último commit de este PR:
Validación local ejecutada:
|
d9dce15 to
a33f5f1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a33f5f136e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ## Install | ||
|
|
||
| ```bash | ||
| openclaw plugins install @openclaw/databricks |
There was a problem hiding this comment.
The README currently tells users to run openclaw plugins install @openclaw/databricks, but this package is not publishable as committed (extensions/databricks/package.json sets private: true and does not opt into release publishing), so npm-based installs will fail for normal users outside this repo. This creates a broken setup path in the new plugin docs; either publish metadata needs to be added (see collectPublishablePluginPackages in scripts/lib/plugin-npm-release.ts) or the README should be changed to the bundled-enable flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 913feb9881
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| throw error; | ||
| } | ||
| return payload; |
There was a problem hiding this comment.
The client returns immediately on any HTTP 2xx response, but Databricks statement submission can return a non-terminal status.state (for example PENDING/RUNNING) with only a statement_id. In that case this tool exits without result rows and, because there is no follow-up GET /api/2.0/sql/statements/{statement_id} path in this plugin, users cannot retrieve completion data for slower queries. This makes the runtime unreliable for many real queries that do not finish within the initial submit response window.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66ddb16591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| assertAllowlistTarget({ | ||
| allowedCatalogs: config.allowedCatalogs, | ||
| allowedSchemas: config.allowedSchemas, | ||
| catalog, | ||
| schema, |
There was a problem hiding this comment.
The allowlist gate here validates only the optional catalog/schema parameters, but those values are caller-controlled and never checked against the actual SQL statement. If allowedCatalogs/allowedSchemas are configured, a request can still pass validation by supplying an allowlisted hint while querying a fully-qualified table in a different catalog/schema, which bypasses the intended fail-closed data-scope control.
Useful? React with 👍 / 👎.
| const status = getStatementStatus(initialPayload); | ||
| if (!isPendingStatus(status)) { | ||
| return initialPayload; | ||
| } |
There was a problem hiding this comment.
resolveFinalStatementPayload treats every non-pending state as success, so a Databricks response with status.state of FAILED or CANCELED is returned as a normal tool result instead of an error. This causes failed SQL executions to look successful to callers and skips normal failure handling paths (for example retries/fallback logic that depends on tool errors).
Useful? React with 👍 / 👎.
|
Thanks for the feedback. This has been addressed in the latest commit of this PR:
Local validation executed:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d30526ed7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sanitized = stripSqlCommentsAndLiterals(rawSql); | ||
| const references = new Set<string>(); | ||
| const targets: ResolvedSqlTarget[] = []; | ||
| const pattern = /\b(?:from|join)\s+((?:[A-Za-z_][\w$]*)(?:\s*\.\s*[A-Za-z_][\w$]*){1,2})\b/giu; |
There was a problem hiding this comment.
resolveSqlTargets only extracts relations that immediately follow FROM or JOIN, so a query like SELECT ... FROM main.public.ok, secret.finance.payroll yields targets for only the first table. Because assertAllowlistTarget accepts any non-empty target list and validates only extracted targets, this lets disallowed catalog/schema references slip through when allowlists are enabled but at least one allowlisted table is also present. This is a fail-open path in the data-scope control and should be handled by parsing comma-separated FROM entries (or rejecting unsupported FROM forms when allowlists are active).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41ca95b425
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!parsed.target) { | ||
| continue; |
There was a problem hiding this comment.
The resolver currently drops unresolved FROM/JOIN identifiers (if (!parsed.target) continue), which means allowlist enforcement can be bypassed whenever at least one allowlisted table is also present. For example, SELECT ... FROM main.public.orders o JOIN payroll p ... yields only main.public.orders, so assertAllowlistTarget passes even though payroll is an ambiguous/default-schema table outside explicit scope. This is a fail-open data-scope issue; unresolved relation targets need to be treated as unsafe when allowlists are active.
Useful? React with 👍 / 👎.
41ca95b to
71b0c0a
Compare
🔒 Aisle Security AnalysisWe found 4 potential security issue(s) in this PR:
1. 🟠 Allowlist bypass via comma-separated FROM sources not resolved by SQL target extractor
Description
This misses additional table references expressed via comma-separated SELECT *
FROM allowed.schema_table a, disallowed.secret_schema.secret_table s
WHERE a.id = s.idIn this case:
This creates a policy bypass for configured allowlists. RecommendationTreat any table source in the Minimum fix (to cover the bypass): after resolving the first For example, extend the loop to also parse // Pseudocode sketch
if (token.value === "FROM") {
let cursor = index + 1;
while (cursor < tokens.length) {
// parse one source (qualified identifier or subquery)
// ...
// if next token is ',', advance and parse another source
if (tokens[cursor]?.kind === "symbol" && tokens[cursor].value === ",") {
cursor += 1;
continue;
}
break;
}
}Also add an explicit regression test such as: expect(resolveSqlTargets("select * from sales.orders o, private.secrets s")).toEqual([
{ schema: "sales", table: "orders", raw: "sales.orders" },
{ schema: "private", table: "secrets", raw: "private.secrets" },
]);If comprehensive SQL parsing is out of scope, an alternative is to fail closed when a comma appears in a 2. 🟠 SSRF and Databricks token exfiltration via configurable `host`
DescriptionThe Databricks plugin constructs outbound HTTPS requests using a configurable
If an attacker can influence the plugin configuration or environment (e.g., in a multi-tenant deployment, compromised config channel, or untrusted runtime overrides) while a privileged token is resolved from a secret store/env, they can set Vulnerable code (host normalization + request construction): // config.ts
return url.toString().replace(/\/+$/u, "");
// client.ts
const endpoint = `${this.config.host}/api/2.0/sql/statements`;
headers: { Authorization: `Bearer ${this.config.token}` }RecommendationConstrain and sanitize the Minimum hardening:
Example: function normalizeHost(rawHost: string): string {
const prefixed = /^https?:\/\//i.test(rawHost) ? rawHost : `https://${rawHost}`;
let url: URL;
try { url = new URL(prefixed); } catch { throw new DatabricksConfigError("Invalid databricks host URL."); }
if (url.protocol !== "https:") throw new DatabricksConfigError("Databricks host must use HTTPS.");
if (url.username || url.password) throw new DatabricksConfigError("Databricks host must not include credentials.");
if (url.pathname && url.pathname !== "/") throw new DatabricksConfigError("Databricks host must not include a path.");
// Optional but recommended: allowlist known Databricks host suffixes.
const hostname = url.hostname.toLowerCase();
const allowedSuffixes = [".databricks.com"]; // extend for your deployment
if (!allowedSuffixes.some((suffix) => hostname === suffix.slice(1) || hostname.endsWith(suffix))) {
throw new DatabricksConfigError("Databricks host must be a databricks.com domain.");
}
url.pathname = "";
url.search = "";
url.hash = "";
return url.toString().replace(/\/+$/u, "");
}If your product must support arbitrary/custom Databricks domains, add an operator-controlled 3. 🟡 Unbounded Databricks SQL query execution and inline result return can cause resource exhaustion
DescriptionThe Impact:
Vulnerable code paths:
RecommendationEnforce a result/cost budget at the tool boundary. Options (prefer combining more than one):
const MAX_ROWS = 1000;
const safeSql = assertReadOnlySqlStatement(sql);
if (!/\bLIMIT\b/i.test(stripSqlCommentsAndLiterals(safeSql))) {
throw new DatabricksPolicyError(`Query must include LIMIT <= ${MAX_ROWS}.`);
}
4. 🟡 Databricks SQL statements may continue running after client timeout/polling timeout (no cancellation)
Description
Impact:
Why this is grounded in code:
RecommendationOn timeout / polling budget exhaustion, attempt to cancel the running statement in Databricks. Example approach:
async cancelStatement(statementId: string) {
await this.fetchImpl(`${this.config.host}/api/2.0/sql/statements/${encodeURIComponent(statementId)}/cancel`, {
method: "POST",
headers: { Authorization: `Bearer ${this.config.token}` },
});
}
// On timeout/polling timeout:
await this.cancelStatement(statementId);
throw new DatabricksError({ code: "STATEMENT_TIMEOUT", ... });Also consider server-side safeguards: warehouse query timeouts, statement limits, and per-tool quotas. Analyzed PR: #56592 at commit Last updated on: 2026-03-31T17:02:52Z |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71b0c0a808
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return new DatabricksError({ | ||
| code: "UNKNOWN_ERROR", | ||
| message: error.message || fallbackMessage, | ||
| retryable: false, | ||
| }); |
There was a problem hiding this comment.
When fetch throws a transport-level error (for example DNS/connect reset/temporary socket failures), this branch maps it to UNKNOWN_ERROR with retryable: false, so both executeStatement and polling fail immediately instead of honoring configured retries. In Node/Undici these transient failures are commonly surfaced as non-AbortError exceptions, so this causes avoidable query failures during brief network instability.
Useful? React with 👍 / 👎.
71b0c0a to
1c5e119
Compare
openclaw-barnacle
Bot
added
channel: googlechat
1c5e119 to
6a162b2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a162b26dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "REVOKE", | ||
| "CALL", | ||
| "COPY", | ||
| "REPLACE", |
There was a problem hiding this comment.
assertNoMutatingKeyword applies this list to all SQL text, so including REPLACE causes legitimate read-only statements such as SELECT REPLACE(col, 'a', 'b') FROM ... to be rejected with a policy violation. In this plugin’s only supported mode (SELECT/WITH ... SELECT), that creates false positives for common analytics queries and unnecessarily blocks valid usage without improving write-safety.
Useful? React with 👍 / 👎.
# Conflicts: # src/generated/bundled-plugin-entries.generated.ts # src/plugins/bundled-plugin-metadata.generated.ts
# Conflicts: # src/plugins/bundled-plugin-metadata.generated.ts
# Conflicts: # src/plugins/bundled-plugin-metadata.generated.ts
2eb7603 to
f29baff
Compare
openclaw-barnacle
Bot
removed
the
channel: googlechat
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f29baff42e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (normalized.code === "TIMEOUT") { | ||
| throw new DatabricksError({ | ||
| code: "STATEMENT_TIMEOUT", | ||
| message: normalized.message, | ||
| retryable: normalized.retryable, | ||
| }); |
There was a problem hiding this comment.
This timeout branch throws immediately, so retryCount is never applied when the initial SQL submission aborts. As a result, transient timeout conditions (for example brief network stalls where a second attempt would succeed) always fail the tool call on the first timeout instead of using the configured retry budget, which makes production query execution less reliable under intermittent connectivity.
Useful? React with 👍 / 👎.
|
Thanks for this contribution. We’re moving new plugins like this to ClawHub instead of
|
2 participants
Summary
@kansodata/databricksunderextensions/databricksskills/databricks/SKILL.mdopenclaw.plugin.jsonand extension entrypointWhy
@kansodata/databricks)Validation