From 85ea728d03fd9cd0fa95bd6fda7512e6b833a797 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20H=C3=A9zs=C5=91?= Date: Sun, 25 Oct 2026 14:22:32 +0100 Subject: [PATCH 1/2] harden error output, spool keys, report files and catalogue --- README.md | 10 +- cloudexit/catalogue/db.py | 2 + cloudexit/catalogue/download.py | 4 - cloudexit/catalogue/store.py | 62 +++--- cloudexit/cli/app.py | 248 +++++++++++++++++------ cloudexit/cli/prompts.py | 8 +- cloudexit/engine.py | 7 +- cloudexit/platform/client.py | 9 +- cloudexit/platform/document.py | 5 +- cloudexit/platform/run_output.py | 24 ++- cloudexit/platform/shape.py | 13 ++ cloudexit/platform/spool.py | 36 +++- cloudexit/platform/status.py | 75 ++++--- cloudexit/platform/upload.py | 3 +- cloudexit/private_files.py | 35 ++++ cloudexit/providers/azure/cli_session.py | 22 +- cloudexit/report/assets.py | 13 +- cloudexit/report/helpers.py | 9 +- cloudexit/resources.py | 41 +++- cloudexit/runconfig/loader.py | 25 ++- cloudexit/terminal.py | 28 ++- tests/catalogue/test_catalogue.py | 159 +++++++++++---- tests/cli/test_report_folder.py | 138 +++++++++++++ tests/cli/test_status.py | 87 +++++++- tests/cli/test_upload_modes.py | 61 ++++++ tests/platform/test_spool.py | 132 ++++++++++++ tests/report/test_copy_assets.py | 6 +- tests/report/test_report_flow.py | 94 ++++++++- tests/test_asset_paths.py | 140 +++++++++++++ tests/test_layering.py | 3 +- 30 files changed, 1291 insertions(+), 208 deletions(-) create mode 100644 cloudexit/platform/shape.py create mode 100644 cloudexit/private_files.py create mode 100644 tests/cli/test_report_folder.py create mode 100644 tests/test_asset_paths.py diff --git a/README.md b/README.md index 5d0f857..19b2f83 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,7 @@ cloudexit catalogue import FILE.db.gz # install a file on an air-gapped mach cloudexit aws --tfstate infra.tfstate --catalogue FILE.db.gz # use a file for one run, without installing it ``` -Set `CLOUDEXIT_CATALOGUE_URL` to download from a mirror instead of the default location. A `./datasets/data.db` from an earlier version is copied to the data directory once. +Set `CLOUDEXIT_CATALOGUE_URL` to download from a mirror instead of the default location. A `./datasets/data.db` from an earlier version is not picked up on its own; install it on purpose with `cloudexit catalogue import datasets/data.db`. A catalogue that holds a trigger or a view is refused, wherever it comes from. ## Sync document @@ -209,7 +209,7 @@ The local report is always the Basic report, computed on your machine, whether o | --- | --- | | 10 | The platform rejected the data, or it is beyond a limit the platform announces. Retrying the same data will not help. | | 11 | The API key is missing, mistyped, expired, revoked, not allowed to do this, or belongs to another region. | -| 12 | The platform is unreachable, busy or in maintenance. Running again is safe: the run keeps the same sync ID. | +| 12 | The platform is unreachable, busy or in maintenance. A spooled sync is delivered on the next run, and that run then makes a new assessment of its own: see [Retries and the spool](#retries-and-the-spool). | | 13 | Credits or a quota ran out. | | 14 | The assessment named by `--assessment-id` is missing, deleted, archived, or does not match the data. | | 15 | The platform does not accept this cloudexit version or the sync document version. | @@ -220,7 +220,7 @@ Before anything is sent the key is checked on your machine (prefix, length and c A failed upload is retried when it may pass: connection errors, timeouts, HTTP 408, 425, 429, 500, 502, 503 and 504, and any error the platform marks as retryable. The wait doubles from 1 second up to 60 seconds with random jitter, `Retry-After` is honoured, and every attempt carries the same idempotency key (the sync ID), so a repeat never stores or charges a second time. All attempts together stay within `--upload-budget` seconds (default 300, `CLOUDEXIT_UPLOAD_BUDGET`). Final failures (exit codes 10, 11, 13, 14 and 15) are not retried. -When the budget runs out on retryable failures the sync is saved to a local spool and the run exits 12 ("spooled; run `cloudexit spool flush`"). `--spool-on-error` also spools syncs the platform refused, for a later manual retry. The spool is `$XDG_STATE_HOME/cloudexit/spool//` (`~/.local/state/...`, or `CLOUDEXIT_SPOOL_DIR`): the sync document and a small file with the attempts and the last error, readable by you only, never holding the API key. The host id is a hash, so the host name is not in the path. +When the budget runs out on retryable failures the sync is saved to a local spool and the run exits 12 ("spooled; run `cloudexit spool flush`"). `--spool-on-error` also spools syncs the platform refused, for a later manual retry. The spool is `$XDG_STATE_HOME/cloudexit/spool//` (`~/.local/state/...`, or `CLOUDEXIT_SPOOL_DIR`): the sync document and a small file with the attempts and the last error, readable by you only, never holding the API key. The host id is a hash, so the host name is not in the path. Each spooled sync also records a fingerprint of the API key in use when it was spooled (a hash, never the key). ```bash cloudexit spool list # id, host id, age, attempts, last error @@ -229,7 +229,9 @@ cloudexit spool purge [--older-than 30d] [--yes] cloudexit upload raw_data/sync.json # upload sync documents (or folders of them), for example from an air-gapped machine ``` -Every run with a platform first delivers the spooled syncs of the same host (oldest first, within 60 seconds); `--no-flush` skips that, and a failing flush never blocks the new run. A spooled sync older than the platform's `max_collection_age_days` is reported as "will be rejected" and kept until you purge it. For an account key, `upload` needs `--assessment-id`, or `--name` and `--exit-strategy`, to say where the sync goes. +Every run with a platform first delivers the spooled syncs of the same host and the same API key (oldest first, within 60 seconds); `--no-flush` skips that, and a failing flush never blocks the new run. Syncs spooled with another API key are never delivered with this one: they stay in the spool, the run says so, and `cloudexit spool list` marks them (`cloudexit spool purge` removes them). A spooled sync with no recorded key is treated the same way. + +Running again after exit 12 does two things: it delivers the spooled sync under its own sync ID, which the platform stores and charges once however often it is sent, and it then makes a new assessment, which is a new sync with a new sync ID and is charged like any other. Use `--no-flush` to leave the spool alone, or `cloudexit spool flush` to deliver it without a new assessment. A spooled sync older than the platform's `max_collection_age_days` is reported as "will be rejected" and kept until you purge it. For an account key, `upload` needs `--assessment-id`, or `--name` and `--exit-strategy`, to say where the sync goes. Connections always verify the platform's certificate. For a private certificate authority use `--ca-bundle` or `CLOUDEXIT_CA_BUNDLE`; otherwise `REQUESTS_CA_BUNDLE` and `SSL_CERT_FILE` are honoured. There is no option to turn verification off. `HTTPS_PROXY` and `NO_PROXY` are honoured. Requests give up after 10 seconds to connect and 60 seconds to read (`--timeout` sets both). The platform's capabilities are cached for five minutes in `~/.cache/cloudexit/` (`$XDG_CACHE_HOME/cloudexit/`, or `%LOCALAPPDATA%\cloudexit\cache\` on Windows). diff --git a/cloudexit/catalogue/db.py b/cloudexit/catalogue/db.py index 550df27..4ce4ceb 100644 --- a/cloudexit/catalogue/db.py +++ b/cloudexit/catalogue/db.py @@ -39,6 +39,8 @@ def connect(db_path=None): db_path = db_path or active_catalogue_path() try: conn = sqlite3.connect(db_path, factory=_Connection) + # Nothing the file's own SQL defines is trusted to run + conn.execute("PRAGMA trusted_schema=OFF") return conn except sqlite3.Error as e: logger.debug(f"Error connecting to database: {e}") diff --git a/cloudexit/catalogue/download.py b/cloudexit/catalogue/download.py index b75609d..9238023 100644 --- a/cloudexit/catalogue/download.py +++ b/cloudexit/catalogue/download.py @@ -18,7 +18,6 @@ check_checksum, install_dataset, installed_catalogue, - migrate_legacy_dataset, ) logger = logging.getLogger(__name__) @@ -203,7 +202,6 @@ def ensure_catalogue( base_url: str, *, directory: Path | None = None, verify: str | bool = True ) -> EnsureResult: directory = directory or catalogue_dir() - migrated = migrate_legacy_dataset(directory) try: result = update_catalogue(base_url, directory=directory, verify=verify) except (CatalogueError, OSError) as exc: @@ -218,7 +216,6 @@ def ensure_catalogue( installed.version, installed.date, str(exc), - migrated, ) installed = installed_catalogue(directory) @@ -228,7 +225,6 @@ def ensure_catalogue( installed.version, installed.date, None, - migrated, ) diff --git a/cloudexit/catalogue/store.py b/cloudexit/catalogue/store.py index b9712eb..b259144 100644 --- a/cloudexit/catalogue/store.py +++ b/cloudexit/catalogue/store.py @@ -21,7 +21,6 @@ CATALOGUE_BASE_URL = "https://cloudexit-oss-data-eu.fsn1.your-objectstorage.com" SUPPORTED_SCHEMA = 1 REQUIRED_TABLES = ("resourcetype", "risk") -LEGACY_DATABASE = Path("datasets") / "data.db" PROVIDER_IDS = {"azure": 1, "aws": 2} GZIP_MAGIC = b"\x1f\x8b" # A dataset unpacks to a few times its download size. The limit is that ratio @@ -84,7 +83,6 @@ class EnsureResult: version: str | None date: str reason: str | None = None - migrated: bool = False def catalogue_dir( @@ -126,7 +124,10 @@ def catalogue_base_url(settings: Settings) -> str: def _open_readonly(path: Path) -> sqlite3.Connection: - return sqlite3.connect(f"{Path(path).resolve().as_uri()}?mode=ro", uri=True) + conn = sqlite3.connect(f"{Path(path).resolve().as_uri()}?mode=ro", uri=True) + # Nothing the file's own SQL defines is trusted to run + conn.execute("PRAGMA trusted_schema=OFF") + return conn def _table_names(conn: sqlite3.Connection) -> set[str]: @@ -134,6 +135,34 @@ def _table_names(conn: sqlite3.Connection) -> set[str]: return {row[0] for row in rows} +def _check_schema_objects(conn: sqlite3.Connection) -> None: + # A catalogue holds tables and indexes. A trigger or a view runs SQL of its + # own on the data a run adds to the same database, so none is accepted. + found = conn.execute( + "SELECT type, name FROM sqlite_master " + "WHERE type NOT IN ('table', 'index') ORDER BY type, name" + ).fetchall() + if found: + names = ", ".join(f"{kind} {name}" for kind, name in found[:3]) + more = f" and {len(found) - 3} more" if len(found) > 3 else "" + raise CatalogueError( + f"the catalogue holds {names}{more}, which a catalogue does not have; " + "it was not used" + ) + + +def validate_schema(path: str | Path) -> None: + # For a database that is about to be written to, or that has been + try: + conn = _open_readonly(Path(path)) + try: + _check_schema_objects(conn) + finally: + conn.close() + except sqlite3.DatabaseError as exc: + raise CatalogueError(f"not a valid catalogue database: {exc}") from exc + + def _read_meta(conn: sqlite3.Connection, tables: set[str]) -> CatalogueInfo | None: if "catalogue_meta" not in tables: return None @@ -215,6 +244,7 @@ def validate_database( raise CatalogueError( f"not a catalogue database: missing table(s) {', '.join(missing)}" ) + _check_schema_objects(conn) info = _read_meta(conn, tables) finally: conn.close() @@ -257,32 +287,6 @@ def installed_catalogue(directory: Path | None = None) -> Installed | None: return Installed(path, info, read_state(directory)) -def migrate_legacy_dataset( - directory: Path | None = None, legacy: Path | None = None -) -> bool: - directory = directory or catalogue_dir() - legacy = legacy or LEGACY_DATABASE - if installed_catalogue(directory) or not legacy.is_file(): - return False - try: - info = validate_database(legacy) - except CatalogueError: - return False - - directory.mkdir(parents=True, exist_ok=True) - shutil.copy2(legacy, directory / "data.db") - modified = datetime.fromtimestamp(legacy.stat().st_mtime, timezone.utc) - _write_state( - directory, - { - "version": info.version if info else None, - "sha256": None, - "downloaded_at": modified.isoformat(timespec="seconds"), - }, - ) - return True - - def _is_gzip(data: bytes) -> bool: return data[:2] == GZIP_MAGIC diff --git a/cloudexit/cli/app.py b/cloudexit/cli/app.py index 1c44cbf..46c2bd7 100644 --- a/cloudexit/cli/app.py +++ b/cloudexit/cli/app.py @@ -3,6 +3,7 @@ import contextlib import json import boto3 +import tempfile import time import sys import os @@ -11,7 +12,6 @@ from typing import Any from rich.console import Console from rich.logging import RichHandler -from rich.markup import escape from datetime import datetime from botocore.exceptions import NoCredentialsError, ProfileNotFound @@ -30,8 +30,12 @@ ) from cloudexit.egress.estimate import estimate_egress from cloudexit.platform.client import ApiError, check_key -from cloudexit.platform.run_output import build_run_json, exit_code_of -from cloudexit.platform.status import host_catalogue_lines, run_status +from cloudexit.platform.run_output import ( + build_run_json, + exit_code_of, + run_json_fallback, +) +from cloudexit.platform.status import StatusReport, host_catalogue_lines, run_status from cloudexit.platform.spool import ( AUTO_FLUSH_BUDGET, DEFAULT_MAX_AGE_DAYS, @@ -39,6 +43,7 @@ flush, format_age, host_id, + key_fingerprint, parse_age, ) from cloudexit.platform.upload import ( @@ -93,6 +98,7 @@ prompt_required_inputs, print_help_message, ) +from cloudexit.private_files import restrict_tree from cloudexit.terminal import ask, print_step from cloudexit.runconfig.loader import ( build_config, @@ -213,8 +219,11 @@ def _aws_provider_from_profile(profile: str) -> dict: raise ConfigError if not is_aws_profile_valid(profile): console.print( - f"[red]AWS profile '{profile}' is not configured. " - f"Use `aws configure --profile {profile}`.[/red]" + f"AWS profile '{profile}' is not configured. " + f"Use `aws configure --profile {profile}`.", + style="red", + markup=False, + emoji=False, ) raise ConfigError try: @@ -222,14 +231,20 @@ def _aws_provider_from_profile(profile: str) -> dict: credentials = session.get_credentials() if credentials is None: console.print( - f"[red]AWS profile '{profile}' has no valid credentials. " - f"Use `aws configure --profile {profile}`.[/red]" + f"AWS profile '{profile}' has no valid credentials. " + f"Use `aws configure --profile {profile}`.", + style="red", + markup=False, + emoji=False, ) raise ConfigError region = session.region_name or "us-east-1" except (NoCredentialsError, ProfileNotFound) as e: console.print( - f"[red]AWS profile error: {str(e)}. Use `aws configure` to set up a profile.[/red]" + f"AWS profile error: {str(e)}. Use `aws configure` to set up a profile.", + style="red", + markup=False, + emoji=False, ) raise ConfigError @@ -251,7 +266,9 @@ def _aws_provider_from_env() -> dict: try: validate_region(region) except ValueError as e: - console.print(f"[red]AWS_DEFAULT_REGION: {e}[/red]") + console.print( + f"AWS_DEFAULT_REGION: {e}", style="red", markup=False, emoji=False + ) raise ConfigError provider_details = { @@ -276,9 +293,19 @@ def _aws_provider_from_prompt() -> dict: validate_region(region) break except ValueError as e: - console.print(f"[red]{e} Please enter a valid AWS region.[/red]") + console.print( + f"{e} Please enter a valid AWS region.", + style="red", + markup=False, + emoji=False, + ) except Exception as e: - console.print(f"[red]Error during manual AWS configuration: {e}[/red]") + console.print( + f"Error during manual AWS configuration: {e}", + style="red", + markup=False, + emoji=False, + ) raise ConfigError return { @@ -368,7 +395,7 @@ def _require_azure_sdk() -> None: try: require_azure_sdk() except AzureSdkError as e: - console.print(f"[red]{e}[/red]") + console.print(f"{e}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) @@ -471,7 +498,7 @@ def _azure_provider_from_cli() -> dict: raise except Exception as e: logger.error(f"Error during Azure CLI processing: {e}", exc_info=True) - console.print(f"[red]An error occurred: {e}[/red]") + console.print(f"An error occurred: {e}", style="red", markup=False, emoji=False) raise ConfigError return { @@ -517,7 +544,7 @@ def _azure_provider_from_prompt() -> dict: raise except Exception as e: logger.error(f"Error during manual Azure configuration: {e}", exc_info=True) - console.print(f"[red]An error occurred: {e}[/red]") + console.print(f"An error occurred: {e}", style="red", markup=False, emoji=False) raise ConfigError return { @@ -607,16 +634,15 @@ def handle_azure(args): def create_directory(base_path="reports"): - # Generate the main directory with a timestamp + # The timestamp keeps the name readable; the random suffix and the exclusive + # creation mean two runs started in the same second never share a folder. + # The folder is for its owner only. timestamp = datetime.now().strftime("%Y%m%d%H%M%S") - directory_path = os.path.join(base_path, timestamp) - - # Create the main directory - os.makedirs(directory_path, exist_ok=True) + os.makedirs(base_path, mode=0o700, exist_ok=True) + directory_path = tempfile.mkdtemp(prefix=f"{timestamp}-", dir=base_path) - # Create the raw_data subdirectory within the main directory raw_data_path = os.path.join(directory_path, "raw_data") - os.makedirs(raw_data_path, exist_ok=True) + os.mkdir(raw_data_path, 0o700) return directory_path, raw_data_path @@ -670,6 +696,14 @@ def _report_flush(result): status="warning", logs=", ".join(sorted(set(result.failed.values()))), ) + if result.other_key: + print_step( + f"{_count(len(result.other_key), 'spooled sync')} for another API key " + "left in the spool.", + status="warning", + logs="not delivered with this key; run again with that key, " + "or `cloudexit spool purge` removes them", + ) if result.pending: print_step( f"{_count(len(result.pending), 'spooled sync')} still waiting.", @@ -845,7 +879,7 @@ def run_assessment( print_step(f"Connecting to {provider_name}...", status="ok") else: print_step(f"Connecting to {provider_name}...", status="error") - console.print(f" ↳ {logs}", style="dim") + console.print(f" ↳ {logs}", style="dim", markup=False, emoji=False) logger.error(f"Credential verification failed: {logs}") sys.exit(codes.CREDENTIALS) console.print("-------------------------------------------") @@ -1062,7 +1096,7 @@ def run_assessment( logs=str(e), ) for problem in getattr(e, "problems", [])[:10]: - console.print(f" ↳ {problem}", style="dim") + console.print(f" ↳ {problem}", style="dim", markup=False, emoji=False) sync_failed = wanted RUN["sync_json"] = str(sync_output.path) if sync_output else None @@ -1103,7 +1137,10 @@ def run_assessment( ) if upload_result.request_id: console.print( - f" ↳ request id: {upload_result.request_id}", style="dim" + f" ↳ request id: {upload_result.request_id}", + style="dim", + markup=False, + emoji=False, ) if upload_result.spooled: print_step( @@ -1181,25 +1218,57 @@ def run_assessment( else "Uploaded to the platform." ), style="cyan", + markup=False, + emoji=False, ) charge = billing_line(upload_result.billing) if charge: console.print(charge, style="cyan") if sync_output: - console.print(f"Sync document: {sync_output.path}", style="cyan") + console.print( + f"Sync document: {sync_output.path}", + style="cyan", + markup=False, + emoji=False, + ) if sync_output.emitted: - console.print(f"Emitted copy: {sync_output.emitted}", style="cyan") + console.print( + f"Emitted copy: {sync_output.emitted}", + style="cyan", + markup=False, + emoji=False, + ) html_report_path = report_status.get("reports", {}).get("HTML") if html_report_path: - console.print(f"HTML Report: {html_report_path}", style="cyan") + console.print( + f"HTML Report: {html_report_path}", + style="cyan", + markup=False, + emoji=False, + ) pdf_report_path = report_status.get("reports", {}).get("PDF") if pdf_report_path: - console.print(f"PDF Report: {pdf_report_path}", style="cyan") + console.print( + f"PDF Report: {pdf_report_path}", + style="cyan", + markup=False, + emoji=False, + ) json_report_path = report_status.get("reports", {}).get("JSON") if json_report_path: - console.print(f"JSON Report: {json_report_path}", style="cyan") + console.print( + f"JSON Report: {json_report_path}", + style="cyan", + markup=False, + emoji=False, + ) if egress_pdf_path: - console.print(f"Egress PDF: {egress_pdf_path}", style="cyan") + console.print( + f"Egress PDF: {egress_pdf_path}", + style="cyan", + markup=False, + emoji=False, + ) console.print("-------------------------------------------") try: @@ -1228,7 +1297,7 @@ def run_assessment( sys.exit(codes.UPLOAD_REJECTED) except Exception as e: - console.print(f"[red]Unexpected error: {e}[/red]") + console.print(f"Unexpected error: {e}", style="red", markup=False, emoji=False) # Persist the full traceback so a one-line error is actionable. Falls # back to the cwd when the crash happened before the report dir existed. target_dir = report_path or os.getcwd() @@ -1236,7 +1305,12 @@ def run_assessment( log_file = os.path.join(target_dir, f"error-{int(time.time())}.log") with open(log_file, "w", encoding="utf-8") as fh: fh.write(traceback.format_exc()) - console.print(f"[yellow]Full traceback written to: {log_file}[/yellow]") + console.print( + f"Full traceback written to: {log_file}", + style="yellow", + markup=False, + emoji=False, + ) except OSError: console.print( "[yellow]Could not write a traceback file; see run.log.[/yellow]" @@ -1246,6 +1320,9 @@ def run_assessment( sys.exit(codes.UNEXPECTED) finally: close_run_log_handler() + if report_path: + # Everything the run wrote, however it was written, is for its owner + restrict_tree(report_path) def _catalogue_settings(args=None): @@ -1257,7 +1334,7 @@ def _catalogue_settings(args=None): settings, ) except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) @@ -1268,7 +1345,7 @@ def prepare_catalogue(args): try: path, info = use_catalogue_file(args.catalogue) except CatalogueError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) set_active_catalogue(path) print_step( @@ -1284,15 +1361,13 @@ def prepare_catalogue(args): base_url, verify=(settings.ca_bundle.value if settings else None) or True ) except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) except CatalogueUnavailable as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) version = result.version or "unknown" - if result.migrated: - print_step("Catalogue copied from ./datasets to the data directory.", "ok") if result.status == "failed": print_step( f"Catalogue update failed, using {version} from {result.date}.", @@ -1327,7 +1402,7 @@ def handle_catalogue(args): info = import_catalogue(args.file, sha256=args.sha256) print(f"Catalogue {info.version if info else 'unknown'} installed.") except (CatalogueError, OSError) as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) @@ -1335,7 +1410,7 @@ def _settings_or_exit(args): try: return load_settings(flags_from_args(args)) except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) @@ -1354,19 +1429,21 @@ def handle_upload(args): try: check_key(settings.api_key.value) except ApiError as e: - console.print(f"[red]{escape(e.message)}[/red]") + console.print(f"{e.message}", style="red", markup=False, emoji=False) sys.exit(e.exit_code) files, problems = read_sync_files(args.paths) exit_code = codes.SUCCESS for path, message in problems: - console.print(f"[red]{escape(str(path))}: {escape(message)}[/red]") + console.print(f"{str(path)}: {message}", style="red", markup=False, emoji=False) exit_code = exit_code or codes.UPLOAD_REJECTED for path, document in files: try: validate_document(document) except SyncDocumentError as e: - console.print(f"[red]{escape(str(path))}: {escape(str(e))}[/red]") + console.print( + f"{str(path)}: {str(e)}", style="red", markup=False, emoji=False + ) exit_code = exit_code or codes.UPLOAD_REJECTED continue result = upload( @@ -1376,26 +1453,38 @@ def handle_upload(args): meta={"exit_strategy": args.exit_strategy}, spool_on_error=args.spool_on_error, ) - shown = escape(str(path)) + shown = str(path) if result.success: console.print( f"{shown}: {result.message}" + (f" ({result.url})" if result.url else ""), style="cyan", + markup=False, + emoji=False, ) charge = billing_line(result.billing) if charge: - console.print(f" {charge}", style="cyan") + console.print(f" {charge}", style="cyan", markup=False, emoji=False) for warning in result.warnings: - console.print(f" warning: {escape(warning)}", style="dim") + console.print( + f" warning: {warning}", style="dim", markup=False, emoji=False + ) continue exit_code = exit_code or result.exit_code console.print( - f"[red]{shown}: {escape(result.error_code or result.reason or '')}: " - f"{escape(result.message)}[/red]" + f"{shown}: {result.error_code or result.reason or ''}: " + f"{result.message}", + style="red", + markup=False, + emoji=False, ) if result.request_id: - console.print(f" request id: {result.request_id}", style="dim") + console.print( + f" request id: {result.request_id}", + style="dim", + markup=False, + emoji=False, + ) if result.spooled: console.print(" spooled; run `cloudexit spool flush`", style="dim") if exit_code: @@ -1410,7 +1499,7 @@ def handle_spool(args): try: only = host_id(flag_host) if flag_host else None except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) if command == "list": @@ -1420,6 +1509,7 @@ def handle_spool(args): return now = time.time() print(f"{'SYNC ID':<38} {'HOST':<17} {'AGE':<8} {'ATTEMPTS':<9} LAST ERROR") + key_id = key_fingerprint(settings.api_key.value) for entry in entries: print( f"{entry.sync_id:<38} {entry.host_id:<17} " @@ -1431,6 +1521,7 @@ def handle_spool(args): if now - entry.created > DEFAULT_MAX_AGE_DAYS * 86400 else "" ) + + ("" if entry.belongs_to(key_id) else " not for the key in use") ) return @@ -1440,10 +1531,10 @@ def handle_spool(args): check_key(settings.api_key.value) result = flush(settings, spool, host=flag_host) except ApiError as e: - console.print(f"[red]{escape(e.message)}[/red]") + console.print(f"{e.message}", style="red", markup=False, emoji=False) sys.exit(e.exit_code) except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) if result.empty: print("Nothing to deliver for this host.") @@ -1456,7 +1547,7 @@ def handle_spool(args): try: older_than = parse_age(args.older_than) if args.older_than else None except ValueError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) now = time.time() due = [ @@ -1485,7 +1576,13 @@ def handle_spool(args): def handle_status(args): settings = _settings_or_exit(args) - report = run_status(settings) + try: + report = run_status(settings) + except Exception as e: + # Whatever the platform sent, the answer is still a report + logger.debug("The status check failed", exc_info=True) + report = StatusReport() + report.problem(codes.UNEXPECTED, f"the status check failed: {type(e).__name__}") if args.output == "json": print(json.dumps(report.as_json(), indent=2)) else: @@ -1503,7 +1600,7 @@ def handle_config(args): try: settings = load_settings(flags_from_args(args)) except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) for line in describe(settings): print(line) @@ -1543,7 +1640,7 @@ def _dispatch(args): try: args.settings = load_settings(flags_from_args(args)) except SettingsError as e: - console.print(f"[red]{escape(str(e))}[/red]") + console.print(f"{str(e)}", style="red", markup=False, emoji=False) sys.exit(codes.CONFIG) # Make sure a catalogue is available before any work starts @@ -1577,15 +1674,27 @@ def _absolute(path): def _emit_run_json(stream, exit_code): # Absolute, so a script can use them from any directory - summary = build_run_json( - exit_code=exit_code, - report_dir=_absolute(RUN.get("report_dir")), - sync_json=_absolute(RUN.get("sync_json")), - upload=RUN.get("upload"), - skipped_reason=RUN.get("skipped_reason"), - catalogue_version=RUN.get("catalogue_version"), - ) - stream.write(json.dumps(summary, indent=2) + "\n") + try: + summary = build_run_json( + exit_code=exit_code, + report_dir=_absolute(RUN.get("report_dir")), + sync_json=_absolute(RUN.get("sync_json")), + upload=RUN.get("upload"), + skipped_reason=RUN.get("skipped_reason"), + catalogue_version=RUN.get("catalogue_version"), + ) + text = json.dumps(summary, indent=2, allow_nan=False) + except Exception as e: + # One object on stdout whatever happened, so a script can always read it + logger.debug("The run summary could not be built", exc_info=True) + summary = run_json_fallback( + exit_code=exit_code, + report_dir=_absolute(RUN.get("report_dir")), + sync_json=_absolute(RUN.get("sync_json")), + error=f"the run summary could not be built: {type(e).__name__}", + ) + text = json.dumps(summary, indent=2) + stream.write(text + "\n") stream.flush() @@ -1603,7 +1712,12 @@ def _dispatch_json(args): exit_code = exit_code_of(e.code) raise except Exception as e: - console.print(f"[red]Unexpected error: {escape(str(e))}[/red]") + console.print( + f"Unexpected error: {str(e)}", + style="red", + markup=False, + emoji=False, + ) sys.exit(codes.UNEXPECTED) finally: _emit_run_json(stdout, exit_code) @@ -1645,5 +1759,5 @@ def run(): main() except Exception as e: # logger.error(f"An unexpected error occurred: {e}", exc_info=True) - console.print(f"[red]Unexpected error: {e}[/red]") + console.print(f"Unexpected error: {e}", style="red", markup=False, emoji=False) sys.exit(1) diff --git a/cloudexit/cli/prompts.py b/cloudexit/cli/prompts.py index 26c11e7..0aa04c1 100644 --- a/cloudexit/cli/prompts.py +++ b/cloudexit/cli/prompts.py @@ -20,7 +20,9 @@ def prompt_required_inputs() -> tuple[int, int]: break except ValueError as e: logger.debug(f"Invalid exit strategy input: {e}") - console.print(f"[red]{e} Please enter 1 or 3.[/red]") + console.print( + f"{e} Please enter 1 or 3.", style="red", markup=False, emoji=False + ) while True: try: @@ -33,7 +35,9 @@ def prompt_required_inputs() -> tuple[int, int]: break except ValueError as e: logger.debug(f"Invalid assessment type input: {e}") - console.print(f"[red]{e} Please enter 1 or 2.[/red]") + console.print( + f"{e} Please enter 1 or 2.", style="red", markup=False, emoji=False + ) return exit_strategy, assessment_type diff --git a/cloudexit/engine.py b/cloudexit/engine.py index 15a4390..0871ece 100644 --- a/cloudexit/engine.py +++ b/cloudexit/engine.py @@ -21,6 +21,7 @@ from cloudexit.providers.azure.inventory import build_azure_resource_inventory from cloudexit.providers.azure.sdk import azure_import from cloudexit.catalogue.db import connect, load_data +from cloudexit.catalogue.store import CatalogueError from cloudexit.tfstate import build_tfstate_resource_inventory from cloudexit.report.context import ( EGRESS_DOCUMENT_CODE, @@ -268,7 +269,11 @@ def create_resource_inventory( raw_data_path: str, ) -> dict[str, Any]: # Copy assets and datasets folders data - copy_assets(report_path, cloud_service_provider) + try: + copy_assets(report_path, cloud_service_provider) + except CatalogueError as e: + logger.error(f"The catalogue cannot be used: {str(e)}") + return {"success": False, "logs": str(e)} try: diff --git a/cloudexit/platform/client.py b/cloudexit/platform/client.py index 1cb21d1..81f64be 100644 --- a/cloudexit/platform/client.py +++ b/cloudexit/platform/client.py @@ -29,6 +29,7 @@ ) from cloudexit.version import __version__ +from cloudexit.platform.shape import as_dict, as_list from cloudexit.platform.document import ( MAX_BODY_BYTES, MAX_CATALOGUE_TYPE_IDS, @@ -341,7 +342,7 @@ def _one_line(text: Any) -> str: def client_policy(meta: Mapping[str, Any]) -> tuple[str, str]: # What the platform says about this version of cloudexit: accepted, # deprecated or unsupported, and why. - policy = (meta.get("clients") or {}).get(CLIENT_NAME) + policy = as_dict(meta.get("clients")).get(CLIENT_NAME) current = _version(__version__) if not isinstance(policy, dict) or not current: return "accepted", f"cloudexit {__version__} is accepted" @@ -349,7 +350,7 @@ def client_policy(meta: Mapping[str, Any]) -> tuple[str, str]: f"; upgrade to {policy['recommended']}" if policy.get("recommended") else "" ) minimum = _version(policy.get("min_supported")) - blocked = [_version(v) for v in policy.get("blocked") or []] + blocked = [_version(v) for v in as_list(policy.get("blocked"))] if current in blocked or (minimum and current < minimum): return ( "unsupported", @@ -769,7 +770,7 @@ def whoami(self) -> dict[str, Any]: def _check_meta( self, meta: Mapping[str, Any], document: Mapping[str, Any] ) -> list[dict[str, str | None]]: - versions = (meta.get("contract") or {}).get("schema_versions") + versions = as_dict(meta.get("contract")).get("schema_versions") schema_version = document.get("schema_version") if isinstance(versions, list) and schema_version not in versions: raise ApiError( @@ -889,7 +890,7 @@ def _push( "utf-8" ) problems = limit_problems( - outgoing, meta.get("limits") or {}, size=len(body), now=self._clock() + outgoing, as_dict(meta.get("limits")), size=len(body), now=self._clock() ) if problems: raise ApiError( diff --git a/cloudexit/platform/document.py b/cloudexit/platform/document.py index a57a34c..cb9fa50 100644 --- a/cloudexit/platform/document.py +++ b/cloudexit/platform/document.py @@ -23,6 +23,7 @@ catalogue_dir, catalogue_info, ) +from cloudexit.private_files import write_private from cloudexit.version import __version__ from cloudexit.catalogue.db import load_data @@ -770,13 +771,13 @@ def write_sync_document( digest = hashlib.sha256(data).hexdigest() path = Path(raw_data_path) / "sync.json" - path.write_bytes(data) + write_private(path, data) emitted = None if emit_dir: emitted = Path(emit_dir) / f"{document['sync']['id']}.json" emitted.parent.mkdir(parents=True, exist_ok=True) - emitted.write_bytes(data) + write_private(emitted, data) summary = summary_line(document, len(data), digest) logger.info(summary) diff --git a/cloudexit/platform/run_output.py b/cloudexit/platform/run_output.py index 6b0caae..72a1097 100644 --- a/cloudexit/platform/run_output.py +++ b/cloudexit/platform/run_output.py @@ -5,6 +5,7 @@ from cloudexit import codes from cloudexit.catalogue.store import compare_catalogue_versions +from cloudexit.platform.shape import as_dict, as_text from cloudexit.platform.upload import UploadResult @@ -23,10 +24,10 @@ def catalogue_outdated( # used; None when nothing tells. if result is None or not result.success: return None - codes_seen = {item.get("code") for item in result.warning_items} + codes_seen = {as_dict(item).get("code") for item in result.warning_items} if "collector_catalogue_outdated" in codes_seen: return True - host = ((result.catalogue or {}).get("host") or {}).get("version") + host = as_text(as_dict(as_dict(result.catalogue).get("host")).get("version")) standing = compare_catalogue_versions(local_version, host) if standing is None: return None @@ -81,6 +82,25 @@ def build_run_json( } +def run_json_fallback( + *, + exit_code: int, + report_dir: str | None, + sync_json: str | None, + error: str, +) -> dict[str, Any]: + # The same keys as the full summary, with what is known and the reason + return { + "exit_code": int(exit_code), + "report_dir": report_dir, + "sync_json": sync_json, + "upload": None, + "results": None, + "catalogue": {"version": None, "outdated": None}, + "error": error, + } + + def exit_code_of(value: Any) -> int: if value is None: return codes.SUCCESS diff --git a/cloudexit/platform/shape.py b/cloudexit/platform/shape.py new file mode 100644 index 0000000..26d218c --- /dev/null +++ b/cloudexit/platform/shape.py @@ -0,0 +1,13 @@ +from typing import Any + + +def as_dict(value: Any) -> dict[str, Any]: + return value if isinstance(value, dict) else {} + + +def as_list(value: Any) -> list[Any]: + return value if isinstance(value, list) else [] + + +def as_text(value: Any) -> str | None: + return value if isinstance(value, str) and value else None diff --git a/cloudexit/platform/spool.py b/cloudexit/platform/spool.py index e582090..ebdc385 100644 --- a/cloudexit/platform/spool.py +++ b/cloudexit/platform/spool.py @@ -44,6 +44,15 @@ def host_id(host: str) -> str: return hashlib.sha256(f"{parts.scheme}://{netloc}".encode("utf-8")).hexdigest()[:16] +def key_fingerprint(key: str | None) -> str | None: + # Which API key spooled a sync, without keeping the key: a hash, never the key + if not key: + return None + return hashlib.sha256(b"cloudexit spool key\0" + key.encode("utf-8")).hexdigest()[ + :16 + ] + + def state_dir( environ: Mapping[str, str] | None = None, *, platform: str = sys.platform ) -> Path: @@ -90,6 +99,15 @@ class SpoolEntry: def created(self) -> float: return float(self.meta.get("created") or 0) + @property + def key_id(self) -> str | None: + value = self.meta.get("key_id") + return value if isinstance(value, str) and value else None + + def belongs_to(self, key_id: str | None) -> bool: + # An entry without a recorded key cannot be matched to any key + return key_id is not None and self.key_id == key_id + @dataclass class FlushResult: @@ -98,10 +116,18 @@ class FlushResult: rejected: list[str] = field(default_factory=list) failed: dict[str, str] = field(default_factory=dict) stopped: str | None = None + # Spooled with another API key: left where they are + other_key: list[str] = field(default_factory=list) @property def empty(self) -> bool: - return not (self.uploaded or self.pending or self.rejected or self.failed) + return not ( + self.uploaded + or self.pending + or self.rejected + or self.failed + or self.other_key + ) class Spool: @@ -140,6 +166,7 @@ def add( name: str | None, exit_strategy: int | None, assessment_id: str | None, + key_id: str | None = None, ) -> SpoolEntry: sync_id = ((document.get("sync") or {}).get("id")) or "" if not isinstance(sync_id, str) or not SYNC_ID.match(sync_id): @@ -164,6 +191,7 @@ def add( "name": name, "exit_strategy": exit_strategy, "assessment_id": assessment_id, + "key_id": key_id, } # The document only: never the key, never a response self._write( @@ -283,7 +311,11 @@ def flush( # for the whole flush; without it every entry gets the upload budget. host = host or settings.host.value result = FlushResult() - entries = spool.entries(host_id(host)) + key_id = key_fingerprint(settings.api_key.value) + found = spool.entries(host_id(host)) + # A sync goes out only with the key it was spooled with + entries = [entry for entry in found if entry.belongs_to(key_id)] + result.other_key = [entry.sync_id for entry in found if entry not in entries] if not entries: return result diff --git a/cloudexit/platform/status.py b/cloudexit/platform/status.py index 1fbacf7..0360e74 100644 --- a/cloudexit/platform/status.py +++ b/cloudexit/platform/status.py @@ -27,8 +27,15 @@ client_policy, resolve_ca_bundle, ) -from cloudexit.platform.spool import DEFAULT_MAX_AGE_DAYS, Spool, format_age, host_id +from cloudexit.platform.spool import ( + DEFAULT_MAX_AGE_DAYS, + Spool, + format_age, + host_id, + key_fingerprint, +) from cloudexit.platform.document import SCHEMA_VERSION +from cloudexit.platform.shape import as_dict, as_list, as_text STATUS_BUDGET = 15.0 KEY_EXPIRY_WARNING_DAYS = 14 @@ -316,18 +323,30 @@ def host_catalogue_lines( make_client = make_client or ApiClient.from_settings try: client = make_client(settings, budget=status_budget(settings)) - host = ((client.meta(fresh=True).get("catalogue") or {}).get("version")) or None + host = as_text(as_dict(client.meta(fresh=True).get("catalogue")).get("version")) except ApiError as error: return [f"platform: unavailable ({error.message})"] return [f"platform: {catalogue_standing(local, host)}"] +def _finite(value: Any) -> Any: + # JSON output has no NaN or infinity + if isinstance(value, bool) or not isinstance(value, (int, float)): + return None + return value if value == value and abs(value) != float("inf") else None + + def _expiry_lines(key: Mapping[str, Any], now: float, report: StatusReport) -> str: expires = key.get("expires_at") - if not isinstance(expires, (int, float)): + if isinstance(expires, bool) or not isinstance(expires, (int, float)): + return "unknown" + try: + when = datetime.fromtimestamp(expires, tz=timezone.utc).strftime("%Y-%m-%d") + days = (expires - now) / 86400 + int(days) + except (OverflowError, OSError, ValueError): + # A time no calendar holds, or not a number at all return "unknown" - when = datetime.fromtimestamp(expires, tz=timezone.utc).strftime("%Y-%m-%d") - days = (expires - now) / 86400 if days < KEY_EXPIRY_WARNING_DAYS: report.warnings.append( f"the API key expires in {max(0, int(days))} days ({when}); " @@ -422,12 +441,13 @@ def finish_spool(days: int) -> None: report.problem(error.exit_code, f"{error.code}: {error.message}") return report - host_info = meta.get("host") or {} - contract = meta.get("contract") or {} - catalogue = meta.get("catalogue") or {} - limits = meta.get("limits") or {} + host_info = as_dict(meta.get("host")) + contract = as_dict(meta.get("contract")) + catalogue = as_dict(meta.get("catalogue")) + limits = as_dict(meta.get("limits")) + catalogue_version = as_text(catalogue.get("version")) state, message = client_policy(meta) - versions = contract.get("schema_versions") or [] + versions = as_list(contract.get("schema_versions")) report.add( "Platform (GET /meta)", [ @@ -440,7 +460,7 @@ def finish_spool(days: int) -> None: ), f"schema: {', '.join(map(str, versions)) or 'unknown'}" + (f" (current {contract['current']})" if contract.get("current") else ""), - f"catalogue: {catalogue.get('version') or 'unknown'}", + f"catalogue: {catalogue_version or 'unknown'}", "limits: " + ", ".join(f"{k}={limits[k]}" for k in LIMITS_SHOWN if k in limits), f"client: {message}", @@ -451,7 +471,7 @@ def finish_spool(days: int) -> None: "region": host_info.get("region"), "app_version": host_info.get("app_version"), "schema_versions": versions, - "catalogue_version": catalogue.get("version"), + "catalogue_version": catalogue_version, "limits": {k: limits[k] for k in LIMITS_SHOWN if k in limits}, "client_policy": {"status": state, "message": message}, } @@ -474,8 +494,9 @@ def finish_spool(days: int) -> None: report.problem(error.exit_code, f"{error.code}: {error.message}") finish_spool(days) return report - key_info = whoami.get("key") or {} - tenant = whoami.get("tenant") or {} + key_info = as_dict(whoami.get("key")) + tenant = as_dict(whoami.get("tenant")) + scopes = [str(scope) for scope in as_list(key_info.get("scopes"))] expiry = _expiry_lines(key_info, now(), report) report.add( "Key (GET /whoami)", @@ -484,19 +505,19 @@ def finish_spool(days: int) -> None: f"key: {key_info.get('display')} \"{key_info.get('name')}\", " f"{key_info.get('kind')}", f"assessment: {key_info.get('assessment_id') or 'any (not bound)'}", - f"scopes: {', '.join(key_info.get('scopes') or []) or 'none'}", + f"scopes: {', '.join(scopes) or 'none'}", f"expires: {expiry}", ], ) - for item in whoami.get("warnings") or []: + for item in as_list(whoami.get("warnings")): if isinstance(item, dict) and item.get("message"): report.warnings.append(str(item["message"])) report.data["key"] = { "organisation": tenant.get("name"), "kind": key_info.get("kind"), "assessment_id": key_info.get("assessment_id"), - "scopes": key_info.get("scopes") or [], - "expires_at": key_info.get("expires_at"), + "scopes": scopes, + "expires_at": _finite(key_info.get("expires_at")), } finish_spool(days) @@ -504,13 +525,13 @@ def finish_spool(days: int) -> None: "Catalogue", [ f"installed: {local or 'none'}", - f"platform: {catalogue_standing(local, catalogue.get('version'))}", + f"platform: {catalogue_standing(local, catalogue_version)}", ], ) report.data["catalogue"] = { "local": local, - "platform": catalogue.get("version"), - "standing": compare_catalogue_versions(local, catalogue.get("version")), + "platform": catalogue_version, + "standing": compare_catalogue_versions(local, catalogue_version), } return report @@ -519,15 +540,19 @@ def _spool_lines( settings: Any, spool: Spool | None, days: int, now: float, report: StatusReport ) -> list[str]: spool = spool or Spool.from_settings(settings) - mine = spool.entries(host_id(settings.host.value)) - others = len(spool.entries()) - len(mine) + on_host = spool.entries(host_id(settings.host.value)) + key_id = key_fingerprint(settings.api_key.value) + mine = [e for e in on_host if e.belongs_to(key_id)] + other_keys = len(on_host) - len(mine) + others = len(spool.entries()) - len(on_host) stale = [e for e in mine if now - e.created > days * 86400] report.data["spool"] = { "pending": len(mine), "too_old": len(stale), "other_hosts": others, + "other_keys": other_keys, } - if not mine and not others: + if not on_host and not others: return ["empty"] lines = [] if mine: @@ -538,6 +563,8 @@ def _spool_lines( ) if stale: lines.append(f"{len(stale)} will be rejected: older than {days} days") + if other_keys: + lines.append(f"{other_keys} for another API key (not delivered with this one)") if others: lines.append(f"{others} for other hosts") return lines diff --git a/cloudexit/platform/upload.py b/cloudexit/platform/upload.py index c63ee36..fde9dd2 100644 --- a/cloudexit/platform/upload.py +++ b/cloudexit/platform/upload.py @@ -11,7 +11,7 @@ from cloudexit.settings import Settings, SettingsError, load_settings from cloudexit.platform.client import ApiClient, ApiError -from cloudexit.platform.spool import Spool +from cloudexit.platform.spool import Spool, key_fingerprint logger = logging.getLogger(__name__) @@ -119,6 +119,7 @@ def _spool( name=name, exit_strategy=meta.get("exit_strategy"), assessment_id=settings.assessment_id.value, + key_id=key_fingerprint(settings.api_key.value), ) return True except (OSError, ValueError, SettingsError) as exc: diff --git a/cloudexit/private_files.py b/cloudexit/private_files.py new file mode 100644 index 0000000..8e0b306 --- /dev/null +++ b/cloudexit/private_files.py @@ -0,0 +1,35 @@ +import os +import stat +import sys +from pathlib import Path + + +def write_private(path: str | Path, data: bytes) -> None: + # Readable by the owner only; a stricter umask is never loosened + descriptor = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(descriptor, "wb") as handle: + handle.write(data) + + +def restrict_tree(root: str | Path) -> None: + # Remove the group and other permissions of a folder and everything in it; + # whatever the owner has stays as it is + if sys.platform == "win32": + return + root = Path(root) + if not root.is_dir(): + return + for directory, folders, files in os.walk(root): + for name in [*folders, *files]: + _restrict(Path(directory) / name) + _restrict(root) + + +def _restrict(path: Path) -> None: + try: + if path.is_symlink(): + return + mode = stat.S_IMODE(path.stat().st_mode) + path.chmod(mode & 0o700) + except OSError: + pass diff --git a/cloudexit/providers/azure/cli_session.py b/cloudexit/providers/azure/cli_session.py index 2c03c54..d555f8e 100644 --- a/cloudexit/providers/azure/cli_session.py +++ b/cloudexit/providers/azure/cli_session.py @@ -51,7 +51,11 @@ def select_subscription(subscriptions: list[Any]) -> Any: # logger.info("Listing available subscriptions for selection.") console.print("Available Subscriptions:") for idx, sub in enumerate(subscriptions, start=1): - console.print(f"{idx}. {sub.display_name} ({sub.subscription_id})") + console.print( + f"{idx}. {sub.display_name} ({sub.subscription_id})", + markup=False, + emoji=False, + ) while True: try: selection = int(ask("Select a subscription by number: ").strip()) @@ -62,14 +66,19 @@ def select_subscription(subscriptions: list[Any]) -> Any: return selected_subscription except ValueError as e: logger.debug(f"Invalid subscription selection: {e}") - console.print(f"[red]{e} Please select a valid number.[/red]") + console.print( + f"{e} Please select a valid number.", + style="red", + markup=False, + emoji=False, + ) def select_resource_group(resource_groups: list[Any]) -> str: # logger.info("Listing available resource groups for selection.") console.print("Available Resource Groups:") for idx, rg in enumerate(resource_groups, start=1): - console.print(f"{idx}. {rg.name}") + console.print(f"{idx}. {rg.name}", markup=False, emoji=False) while True: try: selection = int(ask("Select a resource group by number: ").strip()) @@ -80,4 +89,9 @@ def select_resource_group(resource_groups: list[Any]) -> str: return selected_resource_group except ValueError as e: logger.debug(f"Invalid resource group selection: {e}") - console.print(f"[red]{e} Please select a valid number.[/red]") + console.print( + f"{e} Please select a valid number.", + style="red", + markup=False, + emoji=False, + ) diff --git a/cloudexit/report/assets.py b/cloudexit/report/assets.py index 784d0dd..e08be54 100644 --- a/cloudexit/report/assets.py +++ b/cloudexit/report/assets.py @@ -3,7 +3,11 @@ import shutil import logging -from cloudexit.catalogue.store import active_catalogue_path +from cloudexit.catalogue.store import ( + CatalogueError, + active_catalogue_path, + validate_schema, +) from cloudexit.resources import asset_path @@ -71,3 +75,10 @@ def copy_assets(report_path: str, cloud_service_provider: int) -> None: # Only copy if the destination doesn't already exist if not os.path.exists(db_dest_path): shutil.copyfile(db_src_path, db_dest_path) + # The run writes its results into this copy, so what it holds is checked + # here too: a copy with a trigger or a view is not kept + try: + validate_schema(db_dest_path) + except CatalogueError: + os.unlink(db_dest_path) + raise diff --git a/cloudexit/report/helpers.py b/cloudexit/report/helpers.py index 3536541..a7221ea 100644 --- a/cloudexit/report/helpers.py +++ b/cloudexit/report/helpers.py @@ -1,4 +1,6 @@ -import os +from pathlib import Path + +from cloudexit.resources import resolve_inside, safe_relative CURRENCY_SYMBOLS = {"USD": "$", "EUR": "€", "GBP": "£"} @@ -84,8 +86,9 @@ def initials(name): def report_icon(report_path, icon_rel): # The page only points at files in its own folder: an icon the report # folder does not hold is left out, and the page draws a placeholder - if icon_rel and os.path.isfile(os.path.join(report_path, REPORT_ASSETS, icon_rel)): - return f"{REPORT_ASSETS}/{icon_rel}" + found = resolve_inside(Path(report_path) / REPORT_ASSETS, icon_rel) + if found is not None and found.is_file(): + return f"{REPORT_ASSETS}/{safe_relative(icon_rel).as_posix()}" return None diff --git a/cloudexit/resources.py b/cloudexit/resources.py index 1398800..9dcdff1 100644 --- a/cloudexit/resources.py +++ b/cloudexit/resources.py @@ -12,14 +12,45 @@ def asset_path(*parts: str) -> Path: return root.joinpath("assets", *parts) +def safe_relative(relative: object) -> PurePosixPath | None: + # A path a catalogue names for an asset, as a relative path inside the + # assets folder, or None. One leading slash is how the catalogue writes its + # icons ("/icons/aws/..."): it means the assets folder, not the file system. + text = str(relative or "") + if text.startswith("/"): + text = text[1:] + if not text or text.startswith(("/", "\\")): + return None + # No backslash (UNC and Windows separators), no colon (drive letters and + # stream names) and no NUL; nothing that could leave the folder + if any(char in text for char in ("\\", ":", "\x00")): + return None + parts = PurePosixPath(text).parts + if not parts or ".." in parts: + return None + return PurePosixPath(*parts) + + +def resolve_inside(base: str | Path, relative: object) -> Path | None: + # base/relative when it stays inside base, whatever links lie on the way; + # nothing outside base is looked at + safe = safe_relative(relative) + if safe is None: + return None + root = Path(base) + candidate = root.joinpath(*safe.parts) + try: + inside = candidate.resolve().is_relative_to(root.resolve()) + except (OSError, RuntimeError, ValueError): + return None + return candidate if inside else None + + def find_asset(report_path: str | Path, relative: str) -> Path | None: # The report folder carries its own copy of the assets; the installed # package is the fallback. - parts = PurePosixPath(str(relative).lstrip("/")).parts - if not parts or ".." in parts: - return None for base in (Path(report_path) / "assets", asset_path()): - candidate = base.joinpath(*parts) - if candidate.is_file(): + candidate = resolve_inside(base, relative) + if candidate is not None and candidate.is_file(): return candidate return None diff --git a/cloudexit/runconfig/loader.py b/cloudexit/runconfig/loader.py index aab6ec1..887b704 100644 --- a/cloudexit/runconfig/loader.py +++ b/cloudexit/runconfig/loader.py @@ -22,7 +22,9 @@ def load_config(file_path: str) -> dict[str, Any] | None: return config except Exception as e: logger.error(f"Error loading config file: {e}", exc_info=True) - console.print(f"[red]Error loading config file: {e}[/red]") + console.print( + f"Error loading config file: {e}", style="red", markup=False, emoji=False + ) return None @@ -53,8 +55,11 @@ def require_env(var: str, description: str) -> str: value = os.environ.get(var, "").strip() if not value: console.print( - f"[red]--non-interactive requires {description}. " - f"Set the {var} environment variable.[/red]" + f"--non-interactive requires {description}. " + f"Set the {var} environment variable.", + style="red", + markup=False, + emoji=False, ) sys.exit(codes.CONFIG) return value @@ -66,9 +71,19 @@ def require_env_int(var: str, description: str, valid: set) -> int: try: value = int(raw) except ValueError: - console.print(f"[red]{var} must be an integer, got: {raw!r}[/red]") + console.print( + f"{var} must be an integer, got: {raw!r}", + style="red", + markup=False, + emoji=False, + ) sys.exit(codes.CONFIG) if value not in valid: - console.print(f"[red]{var} must be one of {sorted(valid)}, got: {value}[/red]") + console.print( + f"{var} must be one of {sorted(valid)}, got: {value}", + style="red", + markup=False, + emoji=False, + ) sys.exit(codes.CONFIG) return value diff --git a/cloudexit/terminal.py b/cloudexit/terminal.py index 51f263d..60cf2f3 100644 --- a/cloudexit/terminal.py +++ b/cloudexit/terminal.py @@ -2,6 +2,7 @@ import getpass import sys from rich.console import Console +from rich.markup import escape from rich.style import Style from time import sleep @@ -47,18 +48,33 @@ def print_step( # Handle the pending status with a spinner if status == "pending": with console.status( - f"{description:<50} [yellow]{status_map['pending']}[/yellow]", + f"{escape(description):<50} [yellow]{status_map['pending']}[/yellow]", spinner="dots", ): sleep(2) print_step(description, status="ok") elif status == "ok": - console.print(f"{description:<50} {status_map['ok']}", style=ok_style) + console.print( + f"{description:<50} {status_map['ok']}", + style=ok_style, + markup=False, + emoji=False, + ) elif status == "warning": - console.print(f"{description:<50} {status_map['warning']}", style=warning_style) + console.print( + f"{description:<50} {status_map['warning']}", + style=warning_style, + markup=False, + emoji=False, + ) if logs: - console.print(f" ↳ {logs}", style="dim") + console.print(f" ↳ {logs}", style="dim", markup=False, emoji=False) elif status == "error": - console.print(f"{description:<50} {status_map['error']}", style=error_style) + console.print( + f"{description:<50} {status_map['error']}", + style=error_style, + markup=False, + emoji=False, + ) if logs: - console.print(f" ↳ {logs}", style="dim") + console.print(f" ↳ {logs}", style="dim", markup=False, emoji=False) diff --git a/tests/catalogue/test_catalogue.py b/tests/catalogue/test_catalogue.py index 6b1c841..d7dadef 100644 --- a/tests/catalogue/test_catalogue.py +++ b/tests/catalogue/test_catalogue.py @@ -15,6 +15,9 @@ from cloudexit.cli import app from cloudexit.cli.parser import parse_arguments from cloudexit.catalogue import db as catalogue_db +from cloudexit.catalogue.db import connect +from cloudexit.engine import create_resource_inventory +from cloudexit.report.assets import copy_assets from tests.support.isolation import isolated_environ from tests.support.catalogue import ( build_catalogue, @@ -38,10 +41,10 @@ catalogue_info, import_catalogue, installed_catalogue, - migrate_legacy_dataset, read_state, set_active_catalogue, use_catalogue_file, + _open_readonly, ) from cloudexit.catalogue.download import ( ensure_catalogue, @@ -63,12 +66,6 @@ def setUp(self): self.tmp = Path(tmp.name) self.dir = self.tmp / "catalogue" self.addCleanup(set_active_catalogue, None) - patcher = patch( - "cloudexit.catalogue.store.LEGACY_DATABASE", - self.tmp / "no-legacy" / "data.db", - ) - patcher.start() - self.addCleanup(patcher.stop) def install(self, version=V1, **kwargs): gz = catalogue_gz(version=version, **kwargs) @@ -408,51 +405,145 @@ def test_an_invalid_file_is_an_error(self): use_catalogue_file(path) -class MigrationTests(CatalogueTestCase): +class LegacyDatasetTests(CatalogueTestCase): def legacy(self, **kwargs): path = self.tmp / "datasets" / "data.db" path.parent.mkdir() build_catalogue(path, **kwargs) return path - def test_copies_the_legacy_dataset_once(self): + def run_in_tmp(self, action): + previous = os.getcwd() + os.chdir(self.tmp) + try: + return action() + finally: + os.chdir(previous) + + def test_a_datasets_folder_is_never_installed_on_its_own(self): + self.legacy(version=V2) + + def ensure(): + with fake_http(requests.ConnectionError("offline")): + return ensure_catalogue(BASE, directory=self.dir) + + with self.assertRaises(CatalogueUnavailable): + self.run_in_tmp(ensure) + + self.assertFalse((self.dir / "data.db").exists()) + self.assertIsNone(installed_catalogue(self.dir)) + + def test_it_is_installed_when_it_is_imported_on_purpose(self): legacy = self.legacy(version=V2) - self.assertTrue(migrate_legacy_dataset(self.dir, legacy)) + info = import_catalogue(legacy, directory=self.dir) + self.assertEqual(info.version, V2) self.assertEqual(self.version(), V2) - self.assertEqual(read_state(self.dir)["version"], V2) - self.assertEqual((self.dir / "data.db").read_bytes(), legacy.read_bytes()) - self.assertFalse(migrate_legacy_dataset(self.dir, legacy)) - def test_does_not_overwrite_an_installed_dataset(self): - self.install(V3) - legacy = self.legacy(version=V1) - self.assertFalse(migrate_legacy_dataset(self.dir, legacy)) - self.assertEqual(self.version(), V3) +class HardeningTests(CatalogueTestCase): + def catalogue_with(self, sql, name="hostile.db"): + path = self.tmp / name + build_catalogue(path) + conn = sqlite3.connect(path) + conn.executescript(sql) + conn.commit() + conn.close() + return path - def test_an_old_or_invalid_legacy_file(self): - self.assertTrue(migrate_legacy_dataset(self.dir, self.legacy(meta=False))) - self.assertIsNone(self.version()) + TRIGGER = """ + CREATE TABLE resource_inventory (resource_type INTEGER, count INTEGER); + CREATE TRIGGER rewrite AFTER INSERT ON resource_inventory + BEGIN UPDATE resource_inventory SET count = 999999; END; + """ + VIEW = "CREATE VIEW everything AS SELECT * FROM resourcetype;" - other = self.tmp / "other" - bad = self.tmp / "bad.db" - bad.write_bytes(b"nope") - self.assertFalse(migrate_legacy_dataset(other, bad)) - self.assertFalse(migrate_legacy_dataset(other, self.tmp / "missing.db")) + def test_a_catalogue_with_a_trigger_or_a_view_is_refused_at_install(self): + for what, sql in (("trigger", self.TRIGGER), ("view", self.VIEW)): + with self.subTest(what): + path = self.catalogue_with(sql, f"{what}.db") + + with self.assertRaises(CatalogueError) as raised: + import_catalogue(path, directory=self.dir) + + self.assertIn(what, str(raised.exception)) + self.assertIn("it was not used", str(raised.exception)) + self.assertFalse((self.dir / "data.db").exists()) + + def test_a_gzipped_catalogue_with_a_trigger_is_refused_too(self): + path = self.catalogue_with(self.TRIGGER) + packed = self.tmp / "hostile.db.gz" + packed.write_bytes(gzip.compress(path.read_bytes())) + + with self.assertRaises(CatalogueError): + import_catalogue(packed, directory=self.dir) + with self.assertRaises(CatalogueError): + use_catalogue_file(packed) + self.assertIsNone(installed_catalogue(self.dir)) + + def test_a_catalogue_file_given_on_the_command_line_is_refused(self): + path = self.catalogue_with(self.VIEW) + + with self.assertRaises(CatalogueError): + use_catalogue_file(path) + + def test_extra_tables_and_indexes_are_still_accepted(self): + path = self.catalogue_with( + "CREATE TABLE extra (a TEXT, b TEXT);" + "CREATE INDEX extra_a ON extra (a);" + "CREATE INDEX resourcetype_code ON resourcetype (code);" + ) + + info = import_catalogue(path, directory=self.dir) + + self.assertEqual(info.version, "2026-11-02") + + def test_a_trigger_that_got_past_the_install_is_stopped_before_the_copy_is_used( + self, + ): + path = self.catalogue_with(self.TRIGGER) + set_active_catalogue(path) + report = self.tmp / "report" + (report / "data").mkdir(parents=True) + + with patch("cloudexit.report.assets.shutil.copytree"): + with self.assertRaises(CatalogueError): + copy_assets(str(report), 2) + + self.assertFalse((report / "data" / "assessment.db").exists()) + + def test_the_resource_inventory_stage_fails_without_running_the_triggers(self): + path = self.catalogue_with(self.TRIGGER) + set_active_catalogue(path) + report = self.tmp / "report" + raw = report / "raw_data" + raw.mkdir(parents=True) - def test_ensure_reports_the_migration(self): - legacy = self.legacy(version=V2) - _, entry = self.release(V2) with ( - patch("cloudexit.catalogue.store.LEGACY_DATABASE", legacy), - fake_http(self.manifest_reply(entry)), + patch("cloudexit.report.assets.shutil.copytree"), + patch("cloudexit.engine.build_aws_resource_inventory") as collected, ): - result = ensure_catalogue(BASE, directory=self.dir) + result = create_resource_inventory(2, {}, str(report), str(raw)) - self.assertTrue(result.migrated) - self.assertEqual(result.status, "current") + self.assertFalse(result["success"]) + self.assertIn("trigger", result["logs"]) + collected.assert_not_called() + self.assertFalse((report / "data" / "assessment.db").exists()) + + def test_the_connections_do_not_trust_the_schema(self): + path = self.tmp / "plain.db" + build_catalogue(path) + + with connect(path) as conn: + self.assertEqual(conn.execute("PRAGMA trusted_schema").fetchone()[0], 0) + read_only = _open_readonly(path) + try: + self.assertEqual( + read_only.execute("PRAGMA trusted_schema").fetchone()[0], 0 + ) + finally: + read_only.close() class EnsureCatalogueTests(CatalogueTestCase): diff --git a/tests/cli/test_report_folder.py b/tests/cli/test_report_folder.py new file mode 100644 index 0000000..9efadd8 --- /dev/null +++ b/tests/cli/test_report_folder.py @@ -0,0 +1,138 @@ +import os +import stat +import sys +import tempfile +import unittest +from datetime import datetime +from pathlib import Path +from unittest.mock import patch + +from cloudexit.cli import app +from cloudexit.private_files import restrict_tree, write_private + +MOMENT = datetime(2026, 11, 12, 9, 30, 0) +POSIX_ONLY = unittest.skipIf(sys.platform == "win32", "POSIX file modes") + + +def frozen(): + class FrozenDatetime(datetime): + @classmethod + def now(cls, tz=None): + return MOMENT + + return FrozenDatetime + + +def mode(path): + return stat.S_IMODE(os.stat(path).st_mode) + + +class FolderTestCase(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.base = Path(tmp.name) / "reports" + + +class UniqueFolderTests(FolderTestCase): + def test_two_runs_in_the_same_second_get_different_folders(self): + with patch("cloudexit.cli.app.datetime", frozen()): + first, first_raw = app.create_directory(str(self.base)) + second, second_raw = app.create_directory(str(self.base)) + + self.assertNotEqual(first, second) + self.assertNotEqual(first_raw, second_raw) + for folder, raw in ((first, first_raw), (second, second_raw)): + self.assertTrue(Path(raw).is_dir()) + self.assertEqual(Path(raw).parent, Path(folder)) + self.assertTrue(Path(folder).name.startswith("20261112093000-")) + self.assertEqual(len(list(self.base.iterdir())), 2) + + def test_a_folder_that_exists_is_never_reused(self): + seen = set() + with patch("cloudexit.cli.app.datetime", frozen()): + for _ in range(25): + folder, _raw = app.create_directory(str(self.base)) + self.assertNotIn(folder, seen) + seen.add(folder) + self.assertEqual(len(seen), 25) + + def test_what_an_earlier_run_left_behind_is_untouched(self): + old = self.base / "20261112093000-abcdefgh" + (old / "raw_data").mkdir(parents=True) + (old / "raw_data" / "sync.json").write_text("earlier") + + with patch("cloudexit.cli.app.datetime", frozen()): + folder, _raw = app.create_directory(str(self.base)) + + self.assertNotEqual(Path(folder), old) + self.assertEqual((old / "raw_data" / "sync.json").read_text(), "earlier") + self.assertEqual(list(Path(folder).glob("**/*.json")), []) + + +@POSIX_ONLY +class PrivateFolderTests(FolderTestCase): + def setUp(self): + super().setUp() + previous = os.umask(0) + self.addCleanup(os.umask, previous) + + def test_the_folder_and_its_raw_data_are_for_the_owner_only(self): + folder, raw = app.create_directory(str(self.base)) + + self.assertEqual(mode(folder), 0o700) + self.assertEqual(mode(raw), 0o700) + self.assertEqual(mode(self.base), 0o700) + + def test_a_private_file_is_not_readable_by_others_and_keeps_a_stricter_umask(self): + path = self.base.parent / "a.json" + write_private(path, b"{}") + self.assertEqual(mode(path), 0o600) + + os.umask(0o277) + stricter = self.base.parent / "b.json" + write_private(stricter, b"{}") + self.assertEqual(mode(stricter), 0o400) + + def test_restricting_a_tree_removes_group_and_other_and_never_loosens(self): + root = self.base / "run" + (root / "assets").mkdir(parents=True) + files = { + root / "index.html": 0o644, + root / "assets" / "logo.svg": 0o664, + root / "report.pdf": 0o600, + root / "read-only.json": 0o400, + } + for path, wanted in files.items(): + path.write_text("x") + path.chmod(wanted) + os.chmod(root, 0o755) + os.chmod(root / "assets", 0o775) + + restrict_tree(root) + + self.assertEqual(mode(root), 0o700) + self.assertEqual(mode(root / "assets"), 0o700) + self.assertEqual(mode(root / "index.html"), 0o600) + self.assertEqual(mode(root / "assets" / "logo.svg"), 0o600) + self.assertEqual(mode(root / "report.pdf"), 0o600) + self.assertEqual(mode(root / "read-only.json"), 0o400) + + def test_a_link_is_left_alone(self): + root = self.base / "run" + root.mkdir(parents=True) + outside = self.base.parent / "outside.txt" + outside.write_text("x") + outside.chmod(0o644) + (root / "link").symlink_to(outside) + + restrict_tree(root) + + self.assertEqual(mode(outside), 0o644) + + def test_a_missing_folder_is_not_an_error(self): + restrict_tree(self.base / "nothing") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/cli/test_status.py b/tests/cli/test_status.py index 403a10d..dcf0e12 100644 --- a/tests/cli/test_status.py +++ b/tests/cli/test_status.py @@ -12,7 +12,7 @@ from cloudexit.cli import app from cloudexit.platform.client import ApiClient -from cloudexit.platform.spool import Spool +from cloudexit.platform.spool import Spool, key_fingerprint from cloudexit.platform.status import ( TlsResult, catalogue_standing, @@ -444,6 +444,7 @@ def test_the_spool_is_summarised(self): name=None, exit_strategy=None, assessment_id=None, + key_id=key_fingerprint(KEY), ) with fake_http(meta_reply(), whoami_reply()): @@ -558,6 +559,90 @@ def test_an_unreachable_platform_is_reported_not_hidden(self): self.assertIn("unavailable", lines[0]) +class ShapeTests(StatusTestCase): + def test_a_platform_answer_of_the_wrong_shape_never_crashes_status(self): + for field in ("host", "contract", "catalogue", "limits", "clients"): + for value in ("escapecloud", ["x"], 7, None, True): + with self.subTest(field=field, value=value): + report = self.status( + configured(), + [meta_reply(**{field: value}), whoami_reply()], + ) + + self.assertEqual(report.exit_code, codes.SUCCESS) + self.assertIn("Platform (GET /meta)", self.text(report)) + json.dumps(report.as_json(), allow_nan=False) + + def test_a_whoami_of_the_wrong_shape_never_crashes_status(self): + for field, value in ( + ("key", "esc_sk"), + ("tenant", ["Org"]), + ("warnings", "careful"), + ("key", {"scopes": "syncs:write", "expires_at": "soon"}), + ("key", {"scopes": 5, "expires_at": True}), + ): + with self.subTest(field=field, value=value): + report = self.status( + configured(), + [meta_reply(), whoami_reply(**{field: value})], + ) + + self.assertEqual(report.exit_code, codes.SUCCESS) + self.assertIn("Key (GET /whoami)", self.text(report)) + json.dumps(report.as_json(), allow_nan=False) + + def test_an_expiry_no_calendar_holds_is_unknown(self): + for expires in (1e20, -1e20, float("inf"), float("nan"), 2**80): + with self.subTest(expires=expires): + report = self.status( + configured(), + [meta_reply(), whoami_reply(key={"expires_at": expires})], + ) + + self.assertEqual(report.exit_code, codes.SUCCESS) + self.assertIn("expires: unknown", self.text(report)) + data = json.loads(json.dumps(report.as_json(), allow_nan=False)) + if expires != expires or abs(expires) == float("inf"): + self.assertIsNone(data["key"]["expires_at"]) + + def test_a_catalogue_of_the_wrong_shape_is_unknown_on_the_catalogue_command(self): + for catalogue in ("2026-11-02", ["2026-11-02"], {"version": 20261102}): + with self.subTest(catalogue=catalogue): + with ( + patch( + "cloudexit.platform.status.ApiClient.from_settings", + side_effect=self.make_client, + ), + fake_http(meta_reply(catalogue=catalogue)), + ): + lines = host_catalogue_lines(configured(), "2026-11-01") + + self.assertEqual(lines, ["platform: the platform reports none"]) + + def test_the_status_command_answers_even_when_the_check_itself_fails(self): + for output in ("text", "json"): + with self.subTest(output=output): + out = io.StringIO() + with ( + patch("sys.argv", ["cloudexit", "status", "--output", output]), + patch("cloudexit.cli.app.load_settings", return_value=configured()), + patch( + "cloudexit.cli.app.run_status", + side_effect=AttributeError("boom"), + ), + contextlib.redirect_stdout(out), + ): + with self.assertRaises(SystemExit) as raised: + app.main() + + self.assertEqual(raised.exception.code, codes.UNEXPECTED) + self.assertNotIn("boom", out.getvalue()) + if output == "json": + data = json.loads(out.getvalue()) + self.assertEqual(data["exit_code"], codes.UNEXPECTED) + self.assertIn("AttributeError", data["problems"][0]["message"]) + + class CommandTests(StatusTestCase): def run_command(self, argv, replies=None, settings=None): out = io.StringIO() diff --git a/tests/cli/test_upload_modes.py b/tests/cli/test_upload_modes.py index 7cd86f2..aa3d4f7 100644 --- a/tests/cli/test_upload_modes.py +++ b/tests/cli/test_upload_modes.py @@ -457,6 +457,34 @@ def host(version): self.assertIsNone(catalogue_outdated("2026-11-02", self.result())) self.assertIsNone(catalogue_outdated("2026-11-02", None)) + def test_a_platform_catalogue_of_the_wrong_shape_is_no_answer(self): + for catalogue in ( + {"host": "2026-11-02"}, + {"host": ["2026-11-02"]}, + {"host": {"version": 20261102}}, + {"host": {"version": ["x"]}}, + {"host": None}, + {}, + ): + with self.subTest(catalogue=catalogue): + result = self.result(catalogue=catalogue) + self.assertIsNone(catalogue_outdated("2026-11-02", result)) + warned = self.result(warning_items=("not a dict", OUTDATED)) + self.assertIs(catalogue_outdated("2026-11-02", warned), True) + + def test_a_summary_with_a_platform_answer_of_the_wrong_shape_is_complete(self): + summary = build_run_json( + exit_code=0, + report_dir=None, + sync_json=None, + upload=self.result(catalogue={"host": "escapecloud"}), + skipped_reason=None, + catalogue_version="2026-11-02", + ) + + self.assertEqual(summary["catalogue"]["outdated"], None) + self.assertEqual(summary["upload"]["status"], "uploaded") + def test_a_missing_upload_gives_a_complete_summary(self): summary = build_run_json( exit_code=2, @@ -506,6 +534,39 @@ def dispatch(args): self.assertIn("a step", err) self.assertNotIn("Preliminary Stage", out) + def test_a_platform_catalogue_of_the_wrong_shape_still_gives_one_summary(self): + def dispatch(args): + app.RUN["upload"] = UploadResult( + True, True, sync_id="s", catalogue={"host": "escapecloud"} + ) + app.RUN["catalogue_version"] = "2026-11-02" + + code, out, _ = self.run_json(dispatch) + + data = json.loads(out) + self.assertEqual(code, 0) + self.assertEqual(data["exit_code"], 0) + self.assertIsNone(data["catalogue"]["outdated"]) + self.assertNotIn("error", data) + + def test_a_summary_that_cannot_be_built_is_still_one_object_with_an_error(self): + def dispatch(args): + sys.exit(codes.UPLOAD_REJECTED) + + with patch( + "cloudexit.cli.app.build_run_json", side_effect=AttributeError("boom") + ): + code, out, _ = self.run_json(dispatch) + + data = json.loads(out) + self.assertEqual(code, codes.UPLOAD_REJECTED) + self.assertEqual(data["exit_code"], codes.UPLOAD_REJECTED) + self.assertIn("AttributeError", data["error"]) + self.assertNotIn("boom", out) + self.assertEqual(out.count("\n"), len(out.splitlines())) + for key in ("report_dir", "sync_json", "upload", "results", "catalogue"): + self.assertIn(key, data) + def test_a_clean_run_reports_exit_code_zero(self): code, out, _ = self.run_json(lambda args: None) self.assertEqual(code, 0) diff --git a/tests/platform/test_spool.py b/tests/platform/test_spool.py index 3935a34..968a0cb 100644 --- a/tests/platform/test_spool.py +++ b/tests/platform/test_spool.py @@ -21,6 +21,7 @@ flush, format_age, host_id, + key_fingerprint, parse_age, spool_root, state_dir, @@ -28,6 +29,7 @@ from cloudexit.platform.upload import UploadResult, read_sync_files, upload from tests.support.isolation import isolated_environ from tests.support.api import ( + EU_KEY, HOST, KEY, created_reply, @@ -112,6 +114,7 @@ def add(self, sync_id=OLD, *, created=None, age_days=0, error=None, **meta): name=meta.pop("name", "Spooled"), exit_strategy=meta.pop("exit_strategy", 1), assessment_id=meta.pop("assessment_id", None), + key_id=meta.pop("key_id", key_fingerprint(KEY)), ) self.now = NOW return entry @@ -245,6 +248,7 @@ def test_the_meta_holds_the_attempts_and_what_a_later_upload_needs(self): "name": "Prod", "exit_strategy": 3, "assessment_id": "a", + "key_id": key_fingerprint(KEY), }, ) @@ -536,6 +540,104 @@ def test_nothing_spooled_means_no_request(self): self.assertTrue(result.empty) self.assertEqual(fake.requests, []) + def flush_with(self, key, replies=()): + settings = self.settings( + api_key=Setting(key, "test"), upload_budget=Setting(5.0, "test") + ) + with fake_http(*replies) as fake: + result = flush(settings, self.spool, **self.options()) + return result, fake + + def test_a_sync_is_not_delivered_with_another_key_and_stays(self): + self.add(OLD) + + result, fake = self.flush_with(EU_KEY) + + self.assertEqual(fake.requests, []) + self.assertEqual(result.uploaded, []) + self.assertEqual(result.other_key, [OLD]) + self.assertFalse(result.empty) + self.assertEqual([e.sync_id for e in self.spool.entries()], [OLD]) + + def test_a_sync_is_delivered_once_with_the_key_that_spooled_it(self): + self.add(OLD) + self.flush_with(EU_KEY) + + result, fake = self.flush_with( + KEY, [meta_reply(), whoami_reply(), created_reply()] + ) + again, second = self.flush_with(KEY) + + self.assertEqual(result.uploaded, [OLD]) + self.assertEqual(result.other_key, []) + self.assertEqual(self.spool.entries(), []) + self.assertEqual(second.requests, []) + self.assertTrue(again.empty) + + def test_only_the_entries_of_the_key_in_use_are_delivered(self): + self.add(OLD, created=NOW - 300, key_id=key_fingerprint(EU_KEY)) + self.add(NEW, created=NOW - 10) + + result, fake = self.flush_with( + KEY, [meta_reply(), whoami_reply(), created_reply()] + ) + + self.assertEqual(self.keys(fake), [NEW]) + self.assertEqual(result.uploaded, [NEW]) + self.assertEqual(result.other_key, [OLD]) + self.assertEqual([e.sync_id for e in self.spool.entries()], [OLD]) + + def test_an_entry_without_a_recorded_key_is_never_delivered(self): + entry = self.add(OLD) + meta = json.loads(entry.meta_path.read_text(encoding="utf-8")) + del meta["key_id"] + entry.meta_path.write_text(json.dumps(meta), encoding="utf-8") + lost = self.add(NEW) + lost.meta_path.unlink() + + result, fake = self.flush_with(KEY) + + self.assertEqual(fake.requests, []) + self.assertEqual(sorted(result.other_key), sorted([OLD, NEW])) + self.assertEqual(len(self.spool.entries()), 2) + + def test_the_spool_never_holds_a_key(self): + self.add(OLD) + self.add(NEW, key_id=key_fingerprint(EU_KEY)) + + text = "".join( + path.read_text(encoding="utf-8") + for path in (self.tmp / "spool").rglob("*") + if path.is_file() + ) + + self.assertTrue(text) + for key in (KEY, EU_KEY): + self.assertNotIn(key, text) + self.assertNotIn(key.split("_sk_")[-1], text) + self.assertEqual(len(key_fingerprint(KEY)), 16) + self.assertNotEqual(key_fingerprint(KEY), key_fingerprint(EU_KEY)) + self.assertIsNone(key_fingerprint(None)) + self.assertIsNone(key_fingerprint("")) + + def test_a_failed_upload_spools_with_the_fingerprint_of_its_key(self): + settings = self.settings(upload_budget=Setting(0.0, "test")) + document = self.document(OLD) + + with fake_http(meta_reply(), whoami_reply(), unavailable()): + result = upload( + document, + settings, + name="Prod", + meta={"exit_strategy": 1}, + spool=self.spool, + **self.options(), + ) + + self.assertTrue(result.spooled) + entries = self.spool.entries(host_id(HOST)) + self.assertEqual([e.key_id for e in entries], [key_fingerprint(KEY)]) + def test_entries_of_another_host_are_left_alone(self): self.spool.add( "https://other.example.com", @@ -826,6 +928,15 @@ def test_what_was_delivered_and_what_waits_is_said(self): self.assertIn("1 spooled sync still waiting.", text) self.assertIn("maintenance", text) + def test_what_belongs_to_another_key_is_said_and_the_run_goes_on(self): + self.add(OLD, key_id=key_fingerprint(EU_KEY)) + order, steps = self.run_run(flush_result=FlushResult(other_key=[OLD])) + + text = " | ".join(str(a[0]) + " " + str(k.get("logs", "")) for a, k in steps) + self.assertIn("1 spooled sync for another API key left in the spool.", text) + self.assertIn("not delivered with this key", text) + self.assertIn("upload", [name for name, _ in order]) + def test_a_failing_flush_never_blocks_the_new_run(self): self.add(OLD) for error in ( @@ -1144,6 +1255,27 @@ def test_list_shows_id_host_age_attempts_and_last_error(self): lines[1].split(), [OLD, host_id(HOST), "2d", "4", "maintenance"] ) + def test_list_marks_what_belongs_to_another_key(self): + self.add(OLD) + self.add(NEW, key_id=key_fingerprint(EU_KEY)) + with patch("time.time", return_value=NOW): + _, out, _ = self.run_command(app.handle_spool, self.spool_args("list")) + + lines = {line.split()[0]: line for line in out.strip().splitlines()[1:]} + self.assertNotIn("not for the key in use", lines[OLD]) + self.assertIn("not for the key in use", lines[NEW]) + + def test_flush_leaves_the_entries_of_another_key_and_says_so(self): + self.add(OLD, key_id=key_fingerprint(EU_KEY)) + code, out, fake = self.run_command( + app.handle_spool, self.spool_args("flush"), [] + ) + + self.assertEqual(code, 0) + self.assertIn("1 spooled sync for another API key left in the spool.", out) + self.assertEqual([r for r in fake.requests if r.method == "POST"], []) + self.assertEqual(len(self.spool.entries()), 1) + def test_list_marks_what_the_platform_will_reject(self): self.add(OLD, created=NOW - 31 * DAY) with patch("time.time", return_value=NOW): diff --git a/tests/report/test_copy_assets.py b/tests/report/test_copy_assets.py index c7911e9..830e21e 100644 --- a/tests/report/test_copy_assets.py +++ b/tests/report/test_copy_assets.py @@ -1,5 +1,6 @@ import os import shutil +import sqlite3 import tempfile import unittest from contextlib import contextmanager @@ -21,7 +22,10 @@ def staged(cloud_service_provider): # on shutil itself: copytree calls copyfile internally, so patching it # globally would break the asset copy this module is testing. fake_shutil = mock.MagicMock(wraps=shutil) - fake_shutil.copyfile = mock.MagicMock() + # What the real copy leaves behind is a database; an empty one will do + fake_shutil.copyfile = mock.MagicMock( + side_effect=lambda source, target: sqlite3.connect(target).close() + ) with tempfile.TemporaryDirectory() as report_dir: with mock.patch("cloudexit.report.assets.shutil", fake_shutil): diff --git a/tests/report/test_report_flow.py b/tests/report/test_report_flow.py index 6277a8c..3e799a6 100644 --- a/tests/report/test_report_flow.py +++ b/tests/report/test_report_flow.py @@ -1,8 +1,10 @@ import copy import inspect +import io import json import os import sqlite3 +import sys import tempfile import unittest from dataclasses import replace @@ -11,6 +13,7 @@ from unittest.mock import patch import requests +from rich.console import Console from cloudexit.cli import app from cloudexit.engine import generate_report, perform_risk_assessment @@ -31,6 +34,7 @@ KEY, URL, corrupt, + created_reply, error_body, meta_reply, minimal_document, @@ -419,9 +423,11 @@ def run_assessment( replies=None, egress_ok=None, assessment_type=2, + real_terminal=False, **kwargs, ): self.steps = [] + self.output = io.StringIO() raw_data = os.path.join(report, "raw_data") console_patch = patch("cloudexit.cli.app.console.print") patches = [ @@ -443,12 +449,22 @@ def run_assessment( return_value={"success": True, "logs": ""}, ), patch("cloudexit.cli.app.add_run_log_handler"), - patch( - "cloudexit.cli.app.print_step", - side_effect=lambda *a, **k: self.steps.append((a, k)), - ), - console_patch, ] + if real_terminal: + # The real printing, into a buffer: what the user would read + shown = Console(file=self.output, width=200, color_system=None) + patches += [ + patch("cloudexit.cli.app.console", shown), + patch("cloudexit.terminal.console", shown), + ] + else: + patches += [ + patch( + "cloudexit.cli.app.print_step", + side_effect=lambda *a, **k: self.steps.append((a, k)), + ), + console_patch, + ] if egress_ok is not None: patches += [ patch( @@ -559,6 +575,74 @@ def test_connected_runs_keep_the_requested_depth_in_the_document(self): messages = [args[0] for args, _ in self.steps] self.assertEqual([m for m in messages if m.startswith("Forcing Basic")], []) + def test_platform_text_with_brackets_is_shown_literally_and_the_run_goes_on(self): + message = "rejected [/bold] [red]x[/red] :warning: [1, 2]" + report = self.report() + replies = [ + meta_reply(), + whoami_reply(), + reply(422, error_body("validation_failed", message)), + ] + + with self.assertRaises(SystemExit) as ctx: + self.run_assessment( + report, settings=CONFIGURED, replies=replies, real_terminal=True + ) + + self.assertEqual(ctx.exception.code, codes.UPLOAD_REJECTED) + self.assertIn(message, self.output.getvalue()) + self.assertNotIn("Unexpected error", self.output.getvalue()) + self.assert_local_report(report) + + def test_platform_capabilities_of_the_wrong_shape_do_not_stop_an_upload(self): + report = self.report() + replies = [ + meta_reply(contract="v2", clients=["x"], limits="none", host=7), + whoami_reply(), + created_reply(), + ] + + self.run_assessment(report, settings=CONFIGURED, replies=replies) + + self.assert_local_report(report) + record = json.loads(self.raw(report, "upload.json").read_text()) + self.assertEqual(record["status"], "uploaded") + + @unittest.skipIf(sys.platform == "win32", "POSIX file modes") + def test_a_finished_run_leaves_nothing_readable_by_others(self): + for replies, settings in ((None, None), (success_replies(), CONFIGURED)): + with self.subTest(uploaded=settings is not None): + report = self.report() + for path in Path(report).rglob("*"): + path.chmod(0o777 if path.is_dir() else 0o666) + + self.run_assessment(report, settings=settings, replies=replies) + + self.assert_local_report(report) + loose = [ + str(path.relative_to(report)) + for path in [Path(report), *Path(report).rglob("*")] + if not path.is_symlink() and path.stat().st_mode & 0o077 + ] + self.assertEqual(loose, []) + self.assertEqual( + self.raw(report, "sync.json").stat().st_mode & 0o777, 0o600 + ) + + def test_an_unexpected_error_with_brackets_is_shown_literally(self): + report = self.report() + with ( + patch( + "cloudexit.cli.app.prepare_sync_document", + side_effect=RuntimeError("bad [/bold] text"), + ), + self.assertRaises(SystemExit) as ctx, + ): + self.run_assessment(report, real_terminal=True) + + self.assertEqual(ctx.exception.code, codes.UNEXPECTED) + self.assertIn("Unexpected error: bad [/bold] text", self.output.getvalue()) + def test_a_failed_upload_still_writes_the_report_and_exits_with_an_upload_code( self, ): diff --git a/tests/test_asset_paths.py b/tests/test_asset_paths.py new file mode 100644 index 0000000..2cbc33a --- /dev/null +++ b/tests/test_asset_paths.py @@ -0,0 +1,140 @@ +import os +import sys +import tempfile +import unittest +from pathlib import Path, PurePosixPath +from unittest.mock import patch + +from cloudexit.report.helpers import report_icon +from cloudexit.resources import find_asset, resolve_inside, safe_relative + +REFUSED = ( + "", + "/", + "..", + "../x.png", + "icons/../../x.png", + "/../x.png", + "//host/share/x.png", + "///x.png", + "\\\\host\\share\\x.png", + "\\\\host\\share\\x.png".replace("\\", "/"), + "C:\\x.png", + "C:/x.png", + "c:x.png", + "icons\\aws\\EC2.png", + "/icons\\aws.png", + "x.png:stream", + "x\x00.png", +) + + +class AssetPathTestCase(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = Path(tmp.name) + self.report = self.tmp / "report" + self.assets = self.report / "assets" + (self.assets / "icons" / "aws").mkdir(parents=True) + (self.assets / "icons" / "aws" / "EC2.png").write_bytes(b"png") + self.outside = self.tmp / "outside.png" + self.outside.write_bytes(b"secret") + + +class SafeRelativeTests(unittest.TestCase): + def test_a_normal_relative_path_is_kept(self): + self.assertEqual( + safe_relative("icons/aws/EC2.png"), PurePosixPath("icons/aws/EC2.png") + ) + + def test_the_catalogue_form_with_one_leading_slash_means_the_assets_folder(self): + self.assertEqual( + safe_relative("/icons/aws/EC2.png"), PurePosixPath("icons/aws/EC2.png") + ) + + def test_everything_that_could_leave_the_folder_is_refused(self): + for relative in REFUSED: + with self.subTest(relative=relative): + self.assertIsNone(safe_relative(relative)) + self.assertIsNone(safe_relative(None)) + + +class FindAssetTests(AssetPathTestCase): + def test_a_normal_icon_still_resolves(self): + found = find_asset(self.report, "/icons/aws/EC2.png") + + self.assertEqual(found, self.assets / "icons" / "aws" / "EC2.png") + + def test_refused_paths_find_nothing_and_touch_no_file(self): + with ( + patch.object(Path, "is_file", side_effect=AssertionError("looked")), + patch.object(Path, "resolve", side_effect=AssertionError("looked")), + patch.object(os.path, "isfile", side_effect=AssertionError("looked")), + patch.object(os, "stat", side_effect=AssertionError("looked")), + ): + for relative in REFUSED: + with self.subTest(relative=relative): + self.assertIsNone(find_asset(self.report, relative)) + + def test_an_absolute_path_is_read_inside_the_assets_only(self): + looked = [] + real = Path.is_file + + def spy(path): + looked.append(path) + return real(path) + + with patch.object(Path, "is_file", spy): + found = find_asset(self.report, str(self.outside)) + + self.assertIsNone(found) + for path in looked: + self.assertNotEqual(path, self.outside) + + @unittest.skipIf(sys.platform == "win32", "needs symbolic links") + def test_a_link_out_of_the_assets_is_not_followed(self): + (self.assets / "icons" / "link.png").symlink_to(self.outside) + + self.assertIsNone(find_asset(self.report, "icons/link.png")) + self.assertIsNone(resolve_inside(self.assets, "icons/link.png")) + + +class ReportIconTests(AssetPathTestCase): + def test_a_normal_icon_points_into_the_folder(self): + self.assertEqual( + report_icon(self.report, "icons/aws/EC2.png"), "assets/icons/aws/EC2.png" + ) + self.assertEqual( + report_icon(self.report, "/icons/aws/EC2.png"), "assets/icons/aws/EC2.png" + ) + + def test_a_missing_icon_is_none(self): + self.assertIsNone(report_icon(self.report, "icons/aws/missing.png")) + self.assertIsNone(report_icon(self.report, None)) + self.assertIsNone(report_icon(self.report, "")) + + def test_refused_paths_give_the_placeholder_and_touch_no_file(self): + with ( + patch.object(Path, "is_file", side_effect=AssertionError("looked")), + patch.object(Path, "resolve", side_effect=AssertionError("looked")), + patch.object(os.path, "isfile", side_effect=AssertionError("looked")), + patch.object(os, "stat", side_effect=AssertionError("looked")), + ): + for relative in REFUSED: + with self.subTest(relative=relative): + self.assertIsNone(report_icon(self.report, relative)) + + def test_a_file_outside_the_folder_is_never_pointed_at(self): + for relative in ( + str(self.outside), + "../outside.png", + "..\\outside.png", + f"/{self.outside}", + ): + with self.subTest(relative=relative): + self.assertIsNone(report_icon(self.report, relative)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_layering.py b/tests/test_layering.py index dd428d4..4ec827c 100644 --- a/tests/test_layering.py +++ b/tests/test_layering.py @@ -12,6 +12,7 @@ "codes": set(), "version": set(), "resources": set(), + "private_files": set(), "terminal": {"codes"}, "settings": set(), "catalogue": {"settings"}, @@ -20,7 +21,7 @@ "providers": {"catalogue", "egress", "terminal"}, # egress.estimate reaches the providers inside a function, on purpose "egress": {"catalogue", "providers"}, - "platform": {"catalogue", "settings", "version", "codes"}, + "platform": {"catalogue", "settings", "version", "codes", "private_files"}, "report": {"catalogue", "egress", "providers", "resources", "version", "codes"}, "engine": {"catalogue", "providers", "report", "tfstate", "terminal", "codes"}, } From a9fd198c8319ebf40e16e4efbeecc4177e3824a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20H=C3=A9zs=C5=91?= Date: Sun, 25 Oct 2026 14:35:46 +0100 Subject: [PATCH 2/2] write the report logo path with forward slashes --- cloudexit/report/cover.py | 4 ++-- tests/test_asset_paths.py | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/cloudexit/report/cover.py b/cloudexit/report/cover.py index 6e9c0a6..74ab6d4 100644 --- a/cloudexit/report/cover.py +++ b/cloudexit/report/cover.py @@ -1,5 +1,4 @@ import logging -import os from dataclasses import dataclass from xml.sax.saxutils import escape @@ -29,7 +28,8 @@ TITLE_TOP = 8.1 * cm FOOTER_Y = 1.5 * cm ROW_SPACING = 14 -LOGO_FILE = os.path.join("img", "logo", "report_logo.png") +# Written with forward slashes on every system: asset paths are not file system paths +LOGO_FILE = "img/logo/report_logo.png" TITLE_STYLE = ParagraphStyle( "CoverTitle", diff --git a/tests/test_asset_paths.py b/tests/test_asset_paths.py index 2cbc33a..1d17b13 100644 --- a/tests/test_asset_paths.py +++ b/tests/test_asset_paths.py @@ -5,6 +5,7 @@ from pathlib import Path, PurePosixPath from unittest.mock import patch +from cloudexit.report.cover import LOGO_FILE, logo_reader from cloudexit.report.helpers import report_icon from cloudexit.resources import find_asset, resolve_inside, safe_relative @@ -61,6 +62,12 @@ def test_everything_that_could_leave_the_folder_is_refused(self): class FindAssetTests(AssetPathTestCase): + def test_the_report_logo_is_found_by_its_own_name_on_every_system(self): + self.assertNotIn("\\", LOGO_FILE) + self.assertEqual(safe_relative(LOGO_FILE), PurePosixPath(LOGO_FILE)) + self.assertIsNotNone(find_asset(self.report, LOGO_FILE)) + self.assertIsNotNone(logo_reader(self.report)) + def test_a_normal_icon_still_resolves(self): found = find_asset(self.report, "/icons/aws/EC2.png")