feat: MCP parity commands + fail-closed tool firewall (0.73.0, epic #390 + #478 phase 0) - #508
Conversation
Unknown tool names now classify as 'destructive' (strictest category) instead of falling through to 'read'. Real catalog tools run_job, run_sync_action, modify_*, deploy_* move from read to write -- they were passing --deny-writes and fanning out to every configured project. classify_mcp_tool in permissions.py is now the single source of truth; mcp_service dispatch derives multi_project from it (unknown tools stay single-project).
…ntries for parity commands (#478) The tool group callback checks only the coarse 'tool.call' operation and the service-level check reads only the persisted policy, so a session --deny-destructive still allowed 'tool call delete_bucket'. The command now checks the session engine against 'tool:<name>' (fail-closed classifier). Registers operations for the incoming #390 parity commands (docs.query, config.examples, component.sync-action, semantic-layer.schema, transformation.*, flow.examples).
Ports the docs_query MCP tool: AiServiceClient.docs_question (POST /docs/question), DocsService, thin docs command group. Live-verified against the AI service. Adds the 0.73.0 changelog entry.
…type (#394) Ports get_semantic_schema: MetastoreClient.get_schema (GET /api/v1/schema/{type}, no data-envelope unwrap), service fan-out over requested types with fail-fast validation, thin command with --type CSV / --all. Schemas are server-fetched so they always match the deployed metastore.
Ports run_sync_action (dedicated sync-actions.{stack} host, camelCase
body, shallow root+row merge identical to the MCP tool) and
get_config_examples (surfaces rootConfigurationExamples /
rowConfigurationExamples the CLI already fetched but discarded).
…ON Schema (#394) Live metastore returns only a {versions:[...]} listing from the bare /api/v1/schema/{type} endpoint -- no schema body. The upstream MCP tool ships that listing as-is (upstream gap, not mirrored): the service now resolves isDefault (fallback: first entry) and fetches /{version}, returning {type, schema, schema_version}. Live-verified: real $schema/ properties document, version 1.0.0.
…ation authoring (#396) Ports create_sql_transformation + update_sql_transformation: 9-op block/code edit engine (index walk, no jsonpath; batch-start id semantics; unknown ids list valid ones), synthetic b{i}/b{i}.c{j} ids, dialect from project default_backend (no workspace provisioning), statement split via the existing sync/sql_split.py state machine. Deliberate improvement over MCP: parameters non-blocks keys preserved on edit. Live-verified full lifecycle on project 5946 (create -> show -> str_replace edit -> verify -> sync-push delete). Docs: CLAUDE.md, AGENT_CONTEXT, commands-reference, new transformation-workflow.md, gotchas 0.73.0 section (incl. fail-closed firewall notes).
…397) Ports get_flow_examples (vendored JSONL examples for keboola.flow + legacy keboola.orchestrator, informational-only warning for the latter). flow schema --full without --project now serves a bundled schema snapshot (source=bundled) instead of exit 2; the conditional-flow snapshot is the LIVE keboola.flow configurationSchema from the public component index (upstream deleted their drifted copy -- AJDA-2810). Also fixes real drift in the offline YAML template (retryOn object form, delay/ maxRetries) now pinned by validation tests.
…s docs, expert prompt 0.73.0 block, SKILL triggers)
…er, SKILL description under 1024 chars
…back reality (Devin finding 1)
…TRIBUTING 1:1 convention)
POST /documentation/query, GET /configs/examples/{component_id},
POST /components/{id}/actions/{action}, GET /semantic-layer/schema,
POST/GET/PATCH /transformations, GET /flows/examples. DocsService +
TransformationService registered in the ServiceRegistry. Router is
/documentation (NOT /docs): BearerAuthMiddleware exempts the /docs
Swagger namespace, a /docs router would ship unauthenticated -- locked
by test_docs_query_requires_bearer_auth.
padak
left a comment
There was a problem hiding this comment.
Review of #508 — feat: MCP parity commands + fail-closed tool firewall (0.73.0, epic #390 + #478 phase 0)
Generated by
kbagent-pr-reviewersubagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed viamake check, not duplicated here.
Summary
This PR bundles two coordinated 0.73.0 changes: (1) a fail-closed MCP tool firewall (unknown tool names now classify destructive instead of falling through to read, closing a real gap where run_job/run_sync_action/modify_*/deploy_* previously bypassed --deny-writes and fanned out to every configured project), and (2) six new native CLI commands that port the remaining keboola-mcp-server tools (docs query, config examples, semantic-layer schema, component sync-action, transformation create|show|edit, flow examples). Both changes are executed to a very high standard: the firewall fix has dedicated regression tests proving the pre-0.73.0 gap and its closure, every new command has service/CLI/router/E2E coverage, all BLOCKING documentation surfaces (AGENT_CONTEXT, CLAUDE.md, commands-reference.md, gotchas.md, OPERATION_REGISTRY, server routers) are correctly updated with proper (since v0.73.0) tags, and a /docs-vs-/documentation auth-bypass trap was proactively avoided and locked with a dedicated test. make check is green (4597 passed, 0 failed). Verdict: COMMENT — no blocking issues found; two small silent-drift/content gaps worth fixing before or shortly after merge.
Verdict
- Verdict: COMMENT
- Blocking findings: 0
- Non-blocking findings: 2
- Nits: 1
Blocking findings
(none)
Non-blocking findings
[NB-1] plugins/kbagent/skills/kbagent/SKILL.md:366-394 — new transformation-workflow.md not added to the "Workflow references" table
The PR adds plugins/kbagent/skills/kbagent/references/transformation-workflow.md (70 lines, well-written) and correctly links to it from commands-reference.md ("See transformation-workflow.md for the show-before-edit recipe"), and correctly adds trigger keywords to SKILL.md's description: frontmatter. However, SKILL.md's own "Workflow references" table (the bottom index every other topic workflow file is listed in, e.g. sync-workflow, semantic-layer-workflow, dev-portal-workflow) has no row for transformation-workflow. Per CONTRIBUTING.md > "Documentation changes (mandatory!)": "add a workflow row to the bottom table if you created a new references/<topic>-workflow.md" — this is explicitly a "NO" CI-coverage row in the Plugin synchronization map. Not command-discovery-breaking (the commands are fully documented in AGENT_CONTEXT and commands-reference.md already), but an agent skimming SKILL.md's workflow index for "how do I edit a SQL transformation safely" won't find the dedicated guide.
Fix: add | SQL transformations (create / show / edit, show-before-edit rule) | [transformation-workflow](references/transformation-workflow.md) | to the table.
[NB-2] src/keboola_agent_cli/commands/component.py:285 — docstring wording breaks the auto-generated SKILL.md decision-table row
component_sync_action's docstring is """Run a synchronous component action (e.g. testConnection).""". scripts/generate_skill.py::_first_sentence() (pre-existing, not touched by this PR) truncates a help string at the first . (period + space) — and "e.g." itself contains one. The generated table row at plugins/kbagent/skills/kbagent/SKILL.md:100 reads verbatim: | Run a synchronous component action (e.g. | kbagent component sync-action ... | — a garbled, unclosed-parenthesis sentence. make skill-gen/CI's freshness check both pass (the committed file matches the generator's output byte-for-byte), so this ships silently: freshness ≠ correctness. This is the only docstring in commands/*.py that leads with an "e.g." abbreviation before its first real period, so no other row is affected.
Fix: reword the docstring's first sentence to avoid a mid-sentence "e.g. " token, e.g. """Run a synchronous component action such as testConnection.""".
Nits
[NIT-1]plugins/kbagent/agents/keboola-expert.md:84(Tool Selection Matrix) — the newdocscommand group (read-only,docs.query) has no matrix row (onlytransformationgot the two expected rows). Per the CONTRIBUTING.md severity note this is explicitly non-blocking (the matrix rule targets write/destructive groups;AGENT_CONTEXTalready documentsdocs queryfully), but a one-line row would help an agent choosedocs queryoverkai askat a glance instead of relying on the gotchas.md bullet.
(none further)
Verification log
gh auth status→ authenticated aspadak✓gh pr checkout 508→ switched tofeat/mcp-fail-closed-and-parity-390;git rev-parse HEAD=6d2f71dc65a03bef6f92d055528b61927544019e, matchesgh pr view 508 --json headRefOidexactly ✓gh pr view 508 --json title,body,files,additions,deletions,...→ 58 files, +7966/-77, state OPEN, conventionalfeat:prefix ✓- Read
CONTRIBUTING.md(Checklist, Plugin synchronization map, Releasing sections),CLAUDE.md(post-checkout, reflects the PR's own commands already, confirming the docs-sync diff applied cleanly),plugins/kbagent/agents/keboola-expert.md§1/§3 ✓ - Layer-violation greps (typer/click in services, httpx in commands, formatter/typer in clients) → all empty ✓ no violations
git diff main...HEAD -- src/keboola_agent_cli/permissions.py→ verifiedclassify_mcp_toolorder (destructive > read > write > fail-closed default destructive); cross-checked all 8 new commands againstOPERATION_REGISTRY— all present (config.examples,component.sync-action,docs.query,transformation.create/show/edit,semantic-layer.schema,flow.examples) ✓tests/test_permissions.py+tests/test_tool_call_permissions.py→ dedicated regression tests provingrun_job/modify_*/deploy_*were previously misclassifiedreadand now classifywrite; unknown tools fail-closed todestructive; session-only--deny-destructiveblockstool call delete_bucketand unknown tools ✓- Verified
/documentation(not/docs) router prefix againstserver/auth.py:60(path.startswith("/docs")bypasses bearer auth) — confirmed the security rationale is real, andtest_docs_query_requires_bearer_authintests/test_server_router_calls.py:1140locks it ✓ - Behavior verification: read
src/keboola_agent_cli/resources/flow/conditional-flow-schema.json'sretryConfigurationdefinition and confirmed the new offline YAML template'sstrategyParams: {maxRetries, delay}/retryOn: [{type, value}]shape matches the live schema exactly, while the OLD template'sdelaySeconds/retryOn: ["error"]did not — the PR's "fixes real drift" claim is verified true, not just asserted ✓ - Cross-checked every "NO"-coverage row in the Plugin synchronization map:
commands/context.py(AGENT_CONTEXT) ✓ complete with(since 0.73.0)tags for all 6 commands,CLAUDE.md✓,commands-reference.md✓,gotchas.md✓ (new section properly tagged(since v0.73.0)),keboola-expert.md§1/§3 ✓ (2 matrix rows + VERSION GATE gotcha),server/routers/*1:1 ✓ (6/6 routes present, route-ordering comment updated for/examples) — only gap found is[NB-1]above make check→4597 passed, 8 skipped, 132 deselected, 16 warnings in 99.09s, exit 0 (lint + format + typecheck + skill-check + version-check + command-sync-check + changelog-check + error-codes + full test suite) ✓- Convention-compliance greps (magic numbers, raw
error_code="..."strings, bareexcept:,print(), token leakage, newtuple[...]returns) → all clean; the only rawerror_code="..."hits and the only newtuple[...]returns are confined totests/*.pyfixture helpers, matching 21 pre-existing occurrences of the identical pattern repo-wide ✓ git log main..HEAD --format='%B' | grep -iE 'co-authored-by|generated with|claude code'→ no matches ✓ clean commit trailers- Version consistency:
pyproject.toml=0.73.0,plugin.json=0.73.0,marketplace.jsonplugin entry =0.73.0,changelog.pyhas a matching"0.73.0"entry with per-bulletSecurity:/New:-equivalent prefixes ✓ - Did not reproduce live (no credentials in this environment for a real Keboola project); all behavior claims were instead verified by static cross-reference against the bundled JSON Schema and the test suite's assertions, which is stronger than a single live smoke-test for the specific claims in this PR (schema drift fix, shallow-merge semantics, positional-id renumbering)
Open questions for the author
(none)
NB-2: reword component sync-action docstring first sentence ('e.g.
testConnection' -> 'such as testConnection') so generate_skill.py's
first-sentence split no longer truncates the SKILL.md decision-table
row mid-parenthesis; regenerated SKILL.md.
NB-1: add the new transformation-workflow.md row to SKILL.md's
Workflow references table.
NIT-1: add a docs-query row to keboola-expert.md tool selection matrix.
|
Addressed the automated review findings in d82433b (docs-only, no behavior change):
|
…rom-snapshot (0.75.0, #512) (#516) Closes #512. Full table-snapshot lifecycle as first-class storage commands: snapshot-create (write), snapshots (read), snapshot-detail (read), table-from-snapshot (write, the core ask), snapshot-delete (destructive), plus 1:1 kbagent serve REST routes. Restore goes through the classic tables-async endpoint (tables-definition does not accept snapshots) -- hence dedicated commands, not a create-table flag. Verified live on us-east4.gcp project 5946: --name is REQUIRED (API rejects empty; PHP client docblock is stale), no overwrite semantics (restore under a new name, verify, then swap-tables), restored table matches source exactly. New modules: services/snapshot_service.py, commands/_storage_snapshots.py (storage.py/storage_service.py are past their size ceilings). 30 unit tests + 5 router-parity tests + E2E step 11.2. Ride-along E2E fixes: #508 parity step could never pass (expected-error parsed by exit-0-asserting helper) and add/delete-column verification now polls (read-after-DDL staleness, measured live). Full TestFullE2E passes end-to-end with these fixes. All plugin sync-map surfaces updated (context.py, CLAUDE.md, keboola-expert.md, SKILL.md + triggers, commands-reference.md, gotchas.md, new snapshot-workflow.md).
Summary
Two coordinated changes from the #478 plan (0.73.0):
Phase 0 — fail-closed MCP firewall (part of #478; phases 2–3 tracked there):
destructive(strictest) instead of falling through toread. Real catalog toolsrun_job,run_sync_action,modify_*,deploy_*move from read to write — they previously passed--deny-writesand fanned out to every configured project in parallel.permissions.classify_mcp_tool): only known-read tools fan out; unknowns stay single-project.tool callnow enforces the SESSION firewall per tool name — a session-only--deny-destructiveblockstool call delete_bucket(previously only the persisted policy was checked at tool granularity).Phase 1 — CLI parity, epic #390. Closes #392, closes #393, closes #394, closes #395, closes #396, closes #397:
docs query "QUESTION"docs_query(AI ServicePOST /docs/question)config examples --component-id ID [--row]get_config_examples(data was already fetched, then discarded)semantic-layer schema (--type ... | --all)get_semantic_schema— improved: resolves the default schema version to a real JSON Schema (upstream ships the bare{versions}listing)component sync-action ACTIONrun_sync_action(dedicatedsync-actions.{stack}host; shallow root+row merge, MCP parity)transformation create/show/editcreate/update_sql_transformation— 9-op block/code engine, syntheticb{i}/b{i}.c{j}ids, dialect from projectdefault_backend(no workspace provisioning); improved: non-blocksparameterskeys preserved on editflow examples+flow schema --fullbundled fallbackget_flow_examples; conditional-flow schema bundled from the LIVE public component index (upstream deleted their drifted copy) + fixes real drift in the offline YAML templateVerification
make checkgreen: 4575 passed, ruff/format/ty/skill/version/changelog/command-sync gates all pass (246 commands documented).e2e-snowflake): docs query (answer + 8 sources), config examples (10 root examples), semantic-layer schema (real$schemadocument, version 1.0.0), sync-action round-trip (structured error from the realsync-actions.us-east4.gcphost), full transformation lifecycle (create → showb0/b0.c0→str_replaceedit → verify → delete), flow examples + bundled schema (offline). Firewall live checks:--deny-writesblocksrun_job,--deny-destructiveblocksdelete_bucketAND unknown names, reads pass.Docs
CLAUDE.md command list,
AGENT_CONTEXT,commands-reference.md, newtransformation-workflow.md,gotchas.md0.73.0 section,keboola-expert.md(matrix rows + 0.73.0 VERSION GATE block, 57.3/62 KB budget), SKILL.md triggers. Changelog + version 0.73.0 (make version-syncapplied).Serve routes (CONTRIBUTING 1:1 command→route convention)
/documentation/querydocs query/configs/examples/{component_id}config examples/components/{component_id}/actions/{action}component sync-action/semantic-layer/schema(repeated?type=; omitted = all)semantic-layer schema/transformations/{project}[/{config_id}]transformation create/show/edit/flows/examples(offline)flow examplesNote: the docs router lives at
/documentation, NOT/docs—BearerAuthMiddlewareexempts the/docsSwagger namespace, so a/docsrouter would ship unauthenticated. Locked bytest_docs_query_requires_bearer_auth; no future router may use a/docsprefix.