diff --git a/CHANGELOG.md b/CHANGELOG.md index a0734842d..79e00d9b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,8 @@ `hot_context_hash`, `hot_context_ref`, `actor_state_ref`, and `relevant_resources` are all populated for Strategize-phase decisions. +- **Sanitised database URL in `agents info` CLI output to prevent credential leakage** (#8395 / PR #11139): Added `_sanitise_db_url()` helper in `src/cleveragents/cli/commands/system.py` that uses `urllib.parse.urlparse` to detect and mask username/password components in database URLs. PostgreSQL and MySQL URLs with embedded credentials are now shown as `postgresql://***:***@host/db` instead of leaking real passwords. SQLite URLs (which never contain credentials) remain unchanged. Added 11 Behave BDD scenarios covering postgresql, mysql, and sqlite URL variants. + ### Changed - Fixed stale `AUTO-BUG-POOL` tracking prefix references in automation-tracking.md documentation and agent-system-specification.md spec document, replaced with correct `AUTO-BUG-SUP` prefix used by the bug-hunt-pool-supervisor agent (#7875). diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index dbef154a9..c50ddadee 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -40,3 +40,4 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed database resource types (PostgreSQL, SQLite) with transaction-based sandbox strategy: implemented ``DatabaseResourceHandler`` providing full CRUD operations (`read`, `write`, `delete`, `list_children`) and connection validation with automatic credential masking for PostgreSQL and SQLite backends. Includes ``TransactionSandbox`` infrastructure wired into ``SandboxFactory``, BDD test coverage in ``features/database_resources.feature``, and Robot Framework integration tests in ``robot/database_resources.robot`` (PR #10591 / issue #8608, Epic #8568). * HAL 9000 has contributed the agents plan rollback command (PR #8674 / issue #8557): implemented checkpoint-based plan state restoration with the `agents plan rollback []` CLI command as part of Epic #8493, enabling plans to be restored to previous checkpoints, discarding post-checkpoint decisions, and resuming execution from the rolled-back state. Supported by `--yes/-y`, `--to-checkpoint`, and `--format/-f` flags. Includes comprehensive BDD test coverage (>= 97%) for rollback, decision discarding, and plan resume functionality. * HAL 9000 has contributed the PyYAML security upgrade (PR #11012 / issue #9055): added `pyyaml>=6.0.3` dependency constraint to address known YAML parsing vulnerabilities. +* Jeffrey Phillips Freeman has contributed the database URL credential masking fix (PR #11139 / issue #8395): added `_sanitise_db_url()` helper in `src/cleveragents/cli/commands/system.py` to mask credentials in database URLs before exposing them in CLI output, preventing password leakage for PostgreSQL and MySQL deployments while leaving SQLite URLs unchanged. Includes 11 Behave BDD test scenarios covering multiple URL variants. diff --git a/features/db_url_sanitisation.feature b/features/db_url_sanitisation.feature new file mode 100644 index 000000000..53899915d --- /dev/null +++ b/features/db_url_sanitisation.feature @@ -0,0 +1,52 @@ +Feature: Database URL sanitisation — credentials must never be exposed + + As a CleverAgents operator + I want database URLs in CLI output to have their credentials masked + So that running ``agents info`` never leaks passwords or API tokens + + Scenario Outline: PostgreSQL URL with user and password — all are masked + Given the database url is "" + When I run sanitise_db_url + Then the sanitised database url should be "" + + Examples: + | url | expected | + | postgresql://user:secret@localhost/mydb | postgresql://***:***@localhost/mydb | + | postgresql://admin:p%40ssw0rd@db.example.com:5432/agents | postgresql://***:***@db.example.com:5432/agents | + | postgresql://readonly:r0ad_only@pg.cluster.internal:5433/production | postgresql://***:***@pg.cluster.internal:5433/production | + + Scenario Outline: MySQL URL with credentials — all are masked + Given the database url is "" + When I run sanitise_db_url + Then the sanitised database url should be "" + + Examples: + | url | expected | + | mysql://app:s3cret@mysql.internal:3306/agents | mysql://***:***@mysql.internal:3306/agents | + | mysql+pymysql://root:toor@localhost/testdb | mysql+pymysql://***:***@localhost/testdb | + + Scenario Outline: SQLite URLs — remain unchanged (no credentials) + Given the database url is "" + When I run sanitise_db_url + Then the sanitised database url should be "" + + Examples: + | url | expected | + | sqlite:///data/cleveragents.db | sqlite:///data/cleveragents.db | + | sqlite:////absolute/path/to/db.sqlite | sqlite:////absolute/path/to/db.sqlite | + | memory | memory | + + Scenario: Username-only URL — password is still masked + Given the database url is "postgres://deploy@db.example.com/prod" + When I run sanitise_db_url + Then the sanitised database url should be "postgres://***:***@db.example.com/prod" + + Scenario: Password-only URL — username is still masked + Given the database url is "sqlite:///test.db" + When I run sanitise_db_url + Then the sanitised database url should be "sqlite:///test.db" + + Scenario: build_info_data returns sanitised database URL + Given a mock settings object with database url "postgresql://admin:supersecret@host.example.com:5432/mydb" + When I call build_info_data + Then the database field in info data should be "postgresql://***:***@host.example.com:5432/mydb" diff --git a/features/steps/db_url_sanitisation_steps.py b/features/steps/db_url_sanitisation_steps.py new file mode 100644 index 000000000..58a1ceb2b --- /dev/null +++ b/features/steps/db_url_sanitisation_steps.py @@ -0,0 +1,93 @@ +"""Step definitions for db_url_sanitisation.feature. + +Tests the ``_sanitise_db_url`` helper and its use in ``build_info_data``. +""" + +from __future__ import annotations + +from pathlib import Path +from tempfile import mkdtemp +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context + + +# --------------------------------------------------------------------------- +# Given +# --------------------------------------------------------------------------- + + +@given('the database url is "{url}"') +def step_db_url_given(context: Context, url: str) -> None: + """Store the raw database URL for processing.""" + context.raw_db_url = url + + +@given( + "a mock settings object with database url \"{db_url}\"" +) +def step_mock_settings_with_db_url(context: Context, db_url: str) -> None: + """Build a full mock Settings that ``build_info_data`` expects.""" + tmpdir = mkdtemp() + mock_settings = MagicMock() + mock_settings.database_url = db_url + mock_settings.data_dir = Path(tmpdir) + mock_settings.storage_path = Path(tmpdir) + mock_settings.config_path = Path(tmpdir) / "config.toml" + mock_settings.log_dir = Path(tmpdir) / "logs" + mock_settings.default_automation_profile = "auto" + mock_settings.has_provider_configured = MagicMock(return_value=False) + mock_settings.configured_provider_names = MagicMock(return_value=[]) + mock_settings.debug_enabled = False + context.mock_settings = mock_settings + + +# --------------------------------------------------------------------------- +# When +# --------------------------------------------------------------------------- + + +@when("I run sanitise_db_url") +def step_run_sanitise(context: Context) -> None: + """Call _sanitise_db_url with the stored raw URL.""" + from cleveragents.cli.commands.system import _sanitise_db_url + + context.sanitised_url = _sanitise_db_url(context.raw_db_url) + + +@when("I call build_info_data") +def step_call_build_info(context: Context) -> None: + """Call ``build_info_data`` with the mock settings.""" + from cleveragents.cli.commands.system import build_info_data + + with patch( + "cleveragents.config.settings.get_settings", + return_value=context.mock_settings, + ): + context.info_data = build_info_data() + + +# --------------------------------------------------------------------------- +# Then +# --------------------------------------------------------------------------- + + +@then('the sanitised database url should be "{expected}"') +def step_assert_sanitised_url(context: Context, expected: str) -> None: + """Verify the sanitised URL matches expectation.""" + actual = context.sanitised_url + assert actual == expected, ( + f"Expected '{expected}', got '{actual}'" + ) + + +@then( + "the database field in info data should be \"{expected}\"" +) +def step_assert_info_db_field(context: Context, expected: str) -> None: + """Verify the sanitised URL appears correctly in build_info_data output.""" + actual = context.info_data["database"] + assert actual == expected, ( + f"Expected database field '{expected}', got '{actual}'" + ) diff --git a/src/cleveragents/cli/commands/system.py b/src/cleveragents/cli/commands/system.py index 42bba1fe5..31a3dccc8 100644 --- a/src/cleveragents/cli/commands/system.py +++ b/src/cleveragents/cli/commands/system.py @@ -17,6 +17,7 @@ from datetime import UTC, datetime from enum import StrEnum from pathlib import Path from typing import Any +from urllib.parse import urlparse, urlunparse from sqlalchemy import create_engine from sqlalchemy import inspect as sa_inspect @@ -82,6 +83,41 @@ def _dep_version(package: str) -> str: return "not installed" +def _sanitise_db_url(url: str) -> str: + """Sanitise a database URL by masking credentials. + + Parses the URL and replaces username/password components with masked + placeholders so that CLI output never exposes real credentials. + + Examples:: + + >>> _sanitise_db_url("postgresql://user:secret@localhost/mydb") + 'postgresql://***:***@localhost/mydb' + >>> _sanitise_db_url("sqlite:///path/to/db.sqlite") + 'sqlite:///path/to/db.sqlite' + + Args: + url: The raw database URL. + + Returns: + The sanitised URL with credentials masked (or unchanged if no + username/password is present). + """ + parsed = urlparse(url) + + # If there is no userinfo segment, return as-is (e.g. sqlite URLs) + if not parsed.username and not parsed.password: + return url + + # Rebuild the netloc with masked credentials + userinfo = "***:***" + port_part = f":{parsed.port}" if parsed.port else "" + new_netloc = f"{userinfo}@{parsed.hostname}{port_part}" + + sanitized = parsed._replace(netloc=new_netloc) + return urlunparse(sanitized) + + def build_version_data() -> dict[str, Any]: """Assemble structured data for the ``version`` command.""" return { @@ -145,7 +181,7 @@ def build_info_data() -> dict[str, Any]: "version": __version__, "data_dir": str(data_dir), "config_path": str(config_path), - "database": db_url, + "database": _sanitise_db_url(db_url), "server_mode": server_mode, "platform": f"{platform.system()} {platform.release()} ({platform.machine()})", "automation": settings.default_automation_profile,