From fb0611dd5dd87ba835614092bed2f13f24e0cb07 Mon Sep 17 00:00:00 2001 From: Florian Frank Date: Sun, 20 Sep 2026 18:26:57 +0200 Subject: [PATCH] Use scan ID as written in the GUI as identifier for the report --- README.md | 15 +++ report_generator/commands/generate.py | 2 +- report_generator/sources.py | 135 ++++++++++++++++++++++++-- 3 files changed, 143 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 6671a5f..844e1ee 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,21 @@ pointing at a module that was never put on the path. report_generator --scan_id [--path ] --config config.json ``` +`--scan_id` takes either id a run is known by. A run is recorded under two: the **pipeline id**, +which the GUI shows, cipherverifier names its working directories after, and archon's `core_scan` +row is keyed by; and the **stored scan id**, minted when the artifacts are uploaded, which `scans`, +`artifacts` and the S3 keys are keyed by. `core_scanresult` is where they meet — its own id is the +stored scan, its `scan_id` the pipeline — and it is translated in both directions: + +- Reading always uses the stored scan id, because that is what the store is keyed by. +- The report always *identifies* itself by the pipeline id — the file name, the `Scan Identifier` + field and the page footers — because that is the id the GUI shows and so the only one a reader + can match against anything else. + +So both ids produce the same report, filed under the pipeline id. A run with no pipeline row falls +back to being reported under its stored id, which is then the only name it has. Passing a pipeline +id used to fail with a bare `No row was found when one was required`. + Without `--path` the report is written into `reports/`. The S3 secret comes from the environment: `PERSISTENT_STORAGE_STORAGE_CONFIG_S3_SECRET_ACCESS_KEY` (and the database password from `PERSISTENT_STORAGE_STORAGE_CONFIG_DATABASE_PASSWORD`). The name is not this tool's to choose: it is diff --git a/report_generator/commands/generate.py b/report_generator/commands/generate.py index 9efaeab..fe58500 100644 --- a/report_generator/commands/generate.py +++ b/report_generator/commands/generate.py @@ -77,7 +77,7 @@ def generate(properties: GenerateProperties) -> GeneratedReport: context.logger.info(f"Report written to {destination} ✔") - return GeneratedReport(scan_id=properties.scan_id, path=destination, size=len(content)) + return GeneratedReport(scan_id=sources.scan_id, path=destination, size=len(content)) def _destination(path: Path | None, name: str) -> Path: diff --git a/report_generator/sources.py b/report_generator/sources.py index 7dab0e9..89f2434 100644 --- a/report_generator/sources.py +++ b/report_generator/sources.py @@ -11,6 +11,8 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, cast from uuid import UUID +from sqlalchemy import create_engine, text + from cc_common.api import PipelineError, context from persistent_storage.shared import StorageConfiguration from persistent_storage.utils.db_utils import get_scan @@ -21,6 +23,7 @@ if TYPE_CHECKING: from datetime import datetime from mypy_boto3_s3 import S3Client + from sqlalchemy.sql.elements import TextClause from persistent_storage.wrapper import Artifact @@ -30,7 +33,14 @@ class ReportSources: """One scan and every stored file belonging to it. Attributes: - scan_id: The scan the report is about. + scan_id: The scan the report is about, and is named by -- the pipeline id + whenever one is recorded, because that is the id the GUI shows and so + the only one a reader of the report can match against anything. Falls + back to ``stored_scan_id`` when the run has no pipeline row. + stored_scan_id: The scan row the files were actually read from. Kept apart + from :attr:`scan_id` because the store is keyed by this one and the + report is identified by the other; collapsing them makes one of the two + wrong. run_at: When it ran, which is what the report is dated by. artifacts: The artifact rows, for the inventory the report lists. files: The stored bytes by file name. Held in memory because a report is @@ -39,6 +49,7 @@ class ReportSources: """ scan_id: UUID + stored_scan_id: UUID run_at: datetime artifacts: Sequence[Artifact] files: dict[str, bytes] @@ -98,27 +109,129 @@ def _why(configuration: StorageConfiguration, artifacts: Sequence[Artifact], err return str(error) -def collect(configuration: StorageConfiguration, scan_id: UUID) -> ReportSources: - """Read one scan and its files out of the store. +_STORAGE_SCAN_OF_PIPELINE = text( + """ + SELECT result.id + FROM core_scanresult AS result + JOIN scans AS stored ON stored.scan_id = result.id + WHERE result.scan_id = :pipeline + ORDER BY stored.run_at DESC + LIMIT 1 + """ +) + +_PIPELINE_OF_STORAGE_SCAN = text("SELECT result.scan_id FROM core_scanresult AS result WHERE result.id = :stored") + + +def _lookup(configuration: StorageConfiguration, statement: TextClause, parameter: dict[str, str]) -> UUID | None: + """Run one id translation against ``core_scanresult``, or give up. + + Every reason it can fail means the same thing to the caller -- no translation is available -- + and none of them is worth failing a report over: this deployment may have no archon tables at + all, and the ids still have to be reported under whichever name is known. + """ + engine = create_engine(configuration.database.db_url) + + try: + with engine.connect() as connection: + found = connection.execute(statement, parameter).scalar_one_or_none() + except Exception: # noqa: BLE001 + return None + finally: + engine.dispose() + + return UUID(str(found)) if found is not None else None + + +def _storage_scan_id(configuration: StorageConfiguration, pipeline_id: UUID) -> UUID | None: + """Return the stored scan holding ``pipeline_id``'s artifacts, if that is what was passed. + + A run is recorded under two different ids. The pipeline mints one -- what cipherverifier names + its working directories after, what archon's ``core_scan`` row is keyed by, and so the one a + human has to hand; uploading the artifacts afterwards mints a second, which is what ``scans`` + and ``artifacts`` are keyed by and the only one :func:`get_scan` can look up. ``core_scanresult`` + is where the two meet: its own id is the stored scan, its ``scan_id`` the pipeline that produced + it. + + Joining ``scans`` is not decoration -- it keeps a result row whose upload never landed from + being offered as an answer, and picks the newest upload when a pipeline was stored more than + once. Args: configuration: The store to read from. - scan_id: The scan to collect. + pipeline_id: The id that was passed and that ``scans`` has no row for. Returns: - The scan's rows and files. + The stored scan id, or ``None`` -- the id is not a pipeline's either, or this deployment + runs without archon's tables, which is not an error: it just means there is nothing to + translate and the original lookup failure stands. + """ + return _lookup(configuration, _STORAGE_SCAN_OF_PIPELINE, {"pipeline": str(pipeline_id)}) + + +def _pipeline_scan_id(configuration: StorageConfiguration, stored_id: UUID) -> UUID | None: + """Return the pipeline that produced ``stored_id``, which is the id the report is named by. + + The translation :func:`_storage_scan_id` performs, read the other way. Needed even though the + store was reached without it: a report identifies itself by the id the GUI shows, so passing + the stored id has to produce the same report as passing the pipeline id, not one filed under a + number nobody can look up. + + Args: + configuration: The store to read from. + stored_id: The scan the files were read from. + + Returns: + The pipeline id, or ``None`` when the run has no pipeline row and the stored id is + therefore the only name it has. + """ + return _lookup(configuration, _PIPELINE_OF_STORAGE_SCAN, {"stored": str(stored_id)}) + + +def collect(configuration: StorageConfiguration, scan_id: UUID) -> ReportSources: + """Read one scan and its files out of the store. + + Accepts either id a run is known by -- the pipeline id the GUI shows, or the stored scan id -- + and reports under the pipeline id whichever was given, so that the same run yields the same + report either way. See :func:`_storage_scan_id` and :func:`_pipeline_scan_id`. + + Args: + configuration: The store to read from. + scan_id: The scan to collect, by either of its ids. + + Returns: + The scan's rows and files, identified by the pipeline id where there is one. Raises: PipelineError: The scan is unknown, or its files cannot be fetched. """ context.logger.info(f"Collecting the artifacts of scan {scan_id}...") + reported_id = scan_id try: scan = get_scan(configuration.database, scan_id) except Exception as error: - message = f"Cannot read scan {scan_id}: {error}" + stored_id = _storage_scan_id(configuration, scan_id) - raise PipelineError(message) from error + if stored_id is None: + message = ( + f"Cannot read scan {scan_id}: {error}. No stored scan has that id, and no pipeline " + f"either -- 'persistent_storage list' shows the scans that can be reported on." + ) + + raise PipelineError(message) from error + + # A pipeline id was passed, so it is already the one to report under; only the row to read + # has to be looked up. + context.logger.info(f"{scan_id} is the pipeline; its artifacts are stored under scan {stored_id}") + scan_id = stored_id + + try: + scan = get_scan(configuration.database, scan_id) + except Exception as retried: + message = f"Cannot read scan {scan_id}: {retried}" + + raise PipelineError(message) from retried try: contents, names = fetch_artifact_files(configuration, [one.artifact_id for one in scan.artifacts]) @@ -129,8 +242,14 @@ def collect(configuration: StorageConfiguration, scan_id: UUID) -> ReportSources context.logger.info(f"Collected {len(names)} file(s) ✔") + if reported_id == scan_id: + # The stored id was passed. The report still has to name the run the way the GUI does, so + # the pipeline is looked up even though nothing needed it to reach the files. + reported_id = _pipeline_scan_id(configuration, scan_id) or scan_id + return ReportSources( - scan_id=scan_id, + scan_id=reported_id, + stored_scan_id=scan_id, run_at=scan.run_at, artifacts=scan.artifacts, files=dict(zip(names, contents, strict=True)),