Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"plugins": [
{
"name": "kbagent",
"version": "0.30.5",
"version": "0.30.6",
"source": "./plugins/kbagent",
"description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces",
"category": "development"
Expand Down
2 changes: 1 addition & 1 deletion plugins/kbagent/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "kbagent",
"version": "0.30.5",
"version": "0.30.6",
"description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces",
"author": {
"name": "Keboola",
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "keboola-agent-cli"
version = "0.30.5"
version = "0.30.6"
description = "AI-friendly CLI for managing Keboola projects"
readme = "README.md"
requires-python = ">=3.12"
Expand Down
4 changes: 4 additions & 0 deletions src/keboola_agent_cli/changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

# Ordered newest-first. Each value is a list of brief one-line descriptions.
CHANGELOG: dict[str, list[str]] = {
"0.30.6": [
"UX (sec-20 follow-up): malformed `.keboola/branch-mapping.json` now surfaces as a clean JSON error envelope (exit 5, `CONFIG_ERROR`) instead of a raw Python traceback. v0.30.5 introduced the descriptive `Invalid branch ID in branch-mapping.json` message but `load_branch_mapping()` raised it as a bare `ValueError` -- which CLI commands did not catch, so an end user with a hand-edited mapping file saw a multi-frame traceback dumped to stderr instead of a one-line error. Fixed by raising `ConfigError` from `load_branch_mapping()` directly; existing CLI `except ConfigError` handlers now produce the standard error envelope. Found during v0.30.5 e2e smoke test against the kbagent-e2e project; not a security regression but a clear UX cleanup. The descriptive content of the error is unchanged; only the wrapper class differs.",
"Tests: `test_load_branch_mapping_invalid_id_includes_path` updated to assert `ConfigError` instead of `ValueError`. `cleanup_branch_id_from_mapping()` extended to catch `ConfigError` alongside the legacy `ValueError` so its best-effort behavior is preserved. `BranchMapping.from_dict()` continues to raise `ValueError` (it's the data-parser layer); only `load_branch_mapping()` (the filesystem-aware wrapper) was promoted to `ConfigError`.",
],
"0.30.5": [
"Security (critical): `kbagent sync pull` no longer permits API-controlled `component_id` or `component_type` to escape the sync workspace via path traversal. `naming.config_path()` now passes both fields through a new `sanitize_path_segment()` that rejects `/`, `\\`, and parent-directory references (`..`) while preserving the dots, hyphens, and underscores in legitimate component IDs (`keboola.ex-db-mysql`, `kds-team.app-custom-python`). `services/sync_service.py:pull()` adds a defense-in-depth confinement check that raises ConfigError if a resolved config path is not contained in the branch directory. Issue #269 sec-01 / sec-07; threat actor: compromised stack or supply-chain attack on the project token. Pre-fix, `component_id = '../../../etc'` would write outside the project root.",
"Security (high): MCP HTTP transport subprocess no longer inherits Keboola tokens from the kbagent process environment. `mcp_transport.py:_start()` previously used `subprocess.Popen(cmd, ...)` with no `env=` argument, so when `KBAGENT_MCP_TRANSPORT=http` was set, the MCP server inherited `KBC_MASTER_TOKEN`, `KBC_MASTER_TOKEN_<ALIAS>`, `KBC_MANAGE_API_TOKEN`, and `KBC_TOKEN`. New `_build_minimal_env()` allow-lists only the env vars needed for binary discovery and locale handling (PATH, HOME, USER, LANG, LC_*, UV_CACHE_DIR, PYTHONPATH, ...) and explicitly drops every `KBC_*` token. Per-project Storage tokens still flow through HTTP request headers as before. Issue #269 sec-02 / sec-08; closes the gap left by v0.29.0's manage-token default-deny on the HTTP transport path.",
Expand Down
8 changes: 8 additions & 0 deletions src/keboola_agent_cli/commands/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -994,6 +994,14 @@ def sync_branch_status(
except FileNotFoundError as exc:
formatter.error(message=str(exc), error_code=ErrorCode.NOT_INITIALIZED)
raise typer.Exit(code=1) from None
except ConfigError as exc:
# Corrupted .keboola/branch-mapping.json -- surface as clean
# exit-5 envelope so the user sees the descriptive message
# ("Failed to parse ...: Invalid branch ID ...") instead of a
# Python traceback. Mirrors the handler other sync commands
# already have (issue #269 sec-20 follow-up).
formatter.error(message=exc.message, error_code=ErrorCode.CONFIG_ERROR)
raise typer.Exit(code=5) from None

if formatter.json_mode:
formatter.output(result)
Expand Down
21 changes: 15 additions & 6 deletions src/keboola_agent_cli/sync/branch_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from typing import Any

from ..constants import BRANCH_MAPPING_FILENAME, KEBOOLA_DIR_NAME
from ..errors import ConfigError


def _coerce_keboola_id(raw: Any) -> int | None:
Expand Down Expand Up @@ -94,9 +95,13 @@ def load_branch_mapping(project_root: Path) -> BranchMapping:

Raises:
FileNotFoundError: If the mapping file does not exist.
ValueError: If the JSON cannot be parsed or contains a malformed
ConfigError: If the JSON cannot be parsed or contains a malformed
branch ID. The descriptive message names the offending file
so the user can find and fix it (issue #269 sec-20).
so the user can find and fix it. Surfacing this as ConfigError
(rather than raw ValueError) lets CLI commands catch it via
the existing ``except ConfigError`` handler and emit a clean
JSON error envelope with exit code 5 instead of a Python
traceback (issue #269 sec-20 + smoke-test follow-up).
"""
path = project_root / KEBOOLA_DIR_NAME / BRANCH_MAPPING_FILENAME
if not path.exists():
Expand All @@ -105,9 +110,10 @@ def load_branch_mapping(project_root: Path) -> BranchMapping:
data = json.loads(path.read_text(encoding="utf-8"))
return BranchMapping.from_dict(data)
except ValueError as exc:
# _coerce_keboola_id raises ValueError on malformed IDs; wrap with
# path context so the user knows which file to fix.
raise ValueError(f"Failed to parse {path}: {exc}") from exc
# _coerce_keboola_id and json.loads both raise ValueError on
# malformed input; wrap with path context and convert to
# ConfigError so the CLI surfaces a clean error envelope.
raise ConfigError(f"Failed to parse {path}: {exc}") from exc


def save_branch_mapping(project_root: Path, mapping: BranchMapping) -> None:
Expand Down Expand Up @@ -150,7 +156,10 @@ def cleanup_branch_id_from_mapping(branch_id: int) -> dict[str, Any] | None:
return None
try:
mapping = load_branch_mapping(project_root)
except (FileNotFoundError, ValueError):
except (FileNotFoundError, ConfigError, ValueError):
# ValueError preserved for backward compat with callers that may
# still trigger it via custom paths; ConfigError is the new shape
# raised by load_branch_mapping itself.
return None

removed: list[str] = []
Expand Down
10 changes: 7 additions & 3 deletions tests/test_sync_branch_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import pytest

from keboola_agent_cli.constants import BRANCH_MAPPING_FILENAME, KEBOOLA_DIR_NAME
from keboola_agent_cli.errors import ConfigError
from keboola_agent_cli.sync.branch_mapping import (
BranchMapping,
BranchMappingEntry,
Expand Down Expand Up @@ -175,8 +176,11 @@ def test_branch_mapping_from_dict_invalid_id_descriptive_error(self) -> None:
with pytest.raises(ValueError, match="Invalid branch ID"):
BranchMapping.from_dict(data)

def test_load_branch_mapping_invalid_id_includes_path(self, tmp_path: Path) -> None:
"""``load_branch_mapping`` wraps the descriptive error with the file path."""
def test_load_branch_mapping_invalid_id_raises_config_error(self, tmp_path: Path) -> None:
"""``load_branch_mapping`` wraps the descriptive error with the file
path AND raises ConfigError so CLI commands surface a clean exit-5
envelope instead of a Python traceback (issue #269 sec-20 follow-up).
"""
keboola_dir = tmp_path / KEBOOLA_DIR_NAME
keboola_dir.mkdir()
(keboola_dir / BRANCH_MAPPING_FILENAME).write_text(
Expand All @@ -189,7 +193,7 @@ def test_load_branch_mapping_invalid_id_includes_path(self, tmp_path: Path) -> N
}
)
)
with pytest.raises(ValueError, match=r"Failed to parse .*branch-mapping\.json"):
with pytest.raises(ConfigError, match=r"Failed to parse .*branch-mapping\.json"):
load_branch_mapping(tmp_path)


Expand Down
41 changes: 41 additions & 0 deletions tests/test_sync_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1563,6 +1563,47 @@ def test_sync_branch_status_disabled_human(self, tmp_path: Path) -> None:
assert result.exit_code == 0, f"Exit code {result.exit_code}: {result.output}"
assert "not enabled" in result.output

def test_sync_branch_status_corrupted_mapping_clean_envelope(self, tmp_path: Path) -> None:
"""A corrupted .keboola/branch-mapping.json must produce a clean
JSON error envelope (exit 5, CONFIG_ERROR), not a Python traceback
(issue #269 sec-20 follow-up + #273 reviewer feedback)."""
from keboola_agent_cli.errors import ConfigError

config_dir = tmp_path / "config"
config_dir.mkdir()

store = _setup_config(config_dir, {"prod": {"token": TEST_TOKEN}})

mock_sync = _make_sync_service_mock()
# Service raises ConfigError as load_branch_mapping now does for
# malformed mappings. CLI must catch it.
mock_sync.branch_status.side_effect = ConfigError(
"Failed to parse /tmp/.keboola/branch-mapping.json: "
"Invalid branch ID in branch-mapping.json: 'not-a-number'. "
"Expected null or an integer; got str."
)

with (
patch("keboola_agent_cli.cli.ConfigStore") as MockStore,
patch("keboola_agent_cli.cli.ProjectService") as MockProjService,
patch("keboola_agent_cli.cli.SyncService") as MockSyncService,
):
MockStore.return_value = store
MockProjService.return_value = ProjectService(config_store=store)
MockSyncService.return_value = mock_sync

result = runner.invoke(
app,
["--json", "sync", "branch-status", "--directory", str(tmp_path)],
)

assert result.exit_code == 5, f"expected exit 5, got {result.exit_code}: {result.output}"
assert "Traceback" not in result.output
data = json.loads(result.output)
assert data["status"] == "error"
assert data["error"]["code"] == "CONFIG_ERROR"
assert "Invalid branch ID" in data["error"]["message"]


class TestSyncInitAdoptExistingCli:
"""Tests for `kbagent sync init --adopt-existing`."""
Expand Down
2 changes: 1 addition & 1 deletion uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading