diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 9ebded7e..1b5a43a2 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -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" diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index 2df7c46f..5d976416 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -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", diff --git a/pyproject.toml b/pyproject.toml index 399d7a81..5aed53cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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" diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index e07c114f..403e1055 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -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_`, `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.", diff --git a/src/keboola_agent_cli/commands/sync.py b/src/keboola_agent_cli/commands/sync.py index 3433e5a3..fb92a64c 100644 --- a/src/keboola_agent_cli/commands/sync.py +++ b/src/keboola_agent_cli/commands/sync.py @@ -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) diff --git a/src/keboola_agent_cli/sync/branch_mapping.py b/src/keboola_agent_cli/sync/branch_mapping.py index acbbb389..1b4a8f28 100644 --- a/src/keboola_agent_cli/sync/branch_mapping.py +++ b/src/keboola_agent_cli/sync/branch_mapping.py @@ -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: @@ -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(): @@ -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: @@ -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] = [] diff --git a/tests/test_sync_branch_mapping.py b/tests/test_sync_branch_mapping.py index 011ab19e..a062aa34 100644 --- a/tests/test_sync_branch_mapping.py +++ b/tests/test_sync_branch_mapping.py @@ -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, @@ -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( @@ -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) diff --git a/tests/test_sync_cli.py b/tests/test_sync_cli.py index 1190ca2f..3ade3529 100644 --- a/tests/test_sync_cli.py +++ b/tests/test_sync_cli.py @@ -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`.""" diff --git a/uv.lock b/uv.lock index 2ccb8ed7..f1101073 100644 --- a/uv.lock +++ b/uv.lock @@ -439,7 +439,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.30.5" +version = "0.30.6" source = { editable = "." } dependencies = [ { name = "httpx" },