diff --git a/packages/gnss-product-management/src/gnss_product_management/factories/pipelines/resolve.py b/packages/gnss-product-management/src/gnss_product_management/factories/pipelines/resolve.py index a37fbd4..db301f6 100644 --- a/packages/gnss-product-management/src/gnss_product_management/factories/pipelines/resolve.py +++ b/packages/gnss-product-management/src/gnss_product_management/factories/pipelines/resolve.py @@ -5,8 +5,10 @@ sidecar lockfiles, and persists an aggregate lockfile. Fast path: if an aggregate lockfile already exists for -``(package, task, date, version)`` the pipeline returns immediately -without searching or downloading. +``(package, task, date, version)`` and every required entry still +validates (file present, sidecar hash matching), the pipeline returns +immediately without searching or downloading. Otherwise it falls +through to a full re-resolution. """ from __future__ import annotations @@ -23,7 +25,12 @@ from gnss_product_management.factories.remote_transport import WormHole from gnss_product_management.factories.search_planner import SearchPlanner from gnss_product_management.lockfile.manager import LockfileManager -from gnss_product_management.lockfile.operations import get_package_version +from gnss_product_management.lockfile.operations import ( + HashMismatchMode, + get_lock_product, + get_package_version, + validate_lock_product, +) from gnss_product_management.specifications.dependencies.dependencies import ( Dependency, DependencyResolution, @@ -31,7 +38,7 @@ ResolvedDependency, SearchPreference, ) -from gnss_product_management.utilities.paths import AnyPath, as_path +from gnss_product_management.utilities.paths import AnyPath logger = logging.getLogger(__name__) @@ -44,8 +51,9 @@ class ResolvePipeline: in parallel via a :class:`~concurrent.futures.ThreadPoolExecutor`. Fast path: if an aggregate lockfile already exists for the - ``(package, task, date, version)`` identity, returns immediately - without searching or downloading. + ``(package, task, date, version)`` identity and every required + entry still validates, returns immediately without searching or + downloading; otherwise re-resolves. Args: env: The product registry with built catalogs. @@ -119,15 +127,21 @@ def run( version=version, ) if existing is not None: - logger.info( - "Lockfile already exists for %s on %s — skipping resolution: %s", + resolution = self._resolution_from_lockfile(existing, spec) + if resolution.all_required_fulfilled: + logger.info( + "Lockfile already exists for %s on %s — skipping resolution: %s", + spec.name, + date.date(), + lf_path, + ) + logger.info(resolution.summary()) + return resolution, lf_path + logger.warning( + "Lockfile for %s on %s has missing or invalid entries — re-resolving", spec.name, date.date(), - lf_path, ) - resolution = self._resolution_from_lockfile(existing, spec) - logger.info(resolution.summary()) - return resolution, lf_path # --- Full resolution ------------------------------------------------- resolve_one = partial( @@ -227,6 +241,29 @@ def _resolve_one( remote_url=found.uri, ) + @staticmethod + def _lockfile_entry_is_valid(lp) -> bool: + """Check a lockfile entry's sink file: existence, then sidecar hash. + + Aggregate lockfile entries carry no hash of their own, so after + the existence check the per-file sidecar ``_lock.json`` (written + at download time) provides the hash to validate against. A sink + with no sidecar is only checked for existence. + + Args: + lp: The :class:`LockProduct` entry from the aggregate lockfile. + + Returns: + ``True`` if the sink file exists and matches its recorded hash. + """ + # Also resolves .gz sinks to their decompressed file, mutating lp.sink. + if not validate_lock_product(lp, mode=HashMismatchMode.STRICT): + return False + sidecar = get_lock_product(lp.sink) + if sidecar is None: + return True + return validate_lock_product(sidecar, mode=HashMismatchMode.STRICT) + def _resolution_from_lockfile( self, existing, @@ -235,8 +272,8 @@ def _resolution_from_lockfile( """Reconstruct a :class:`DependencyResolution` from an existing lockfile. Iterates over every dependency in the spec (not just those in - the lockfile), marking any absent or file-missing entries as - ``'missing'``. + the lockfile), marking any absent, file-missing, or + hash-mismatched entries as ``'missing'``. Args: existing: The loaded :class:`DependencyLockFile`. @@ -254,10 +291,9 @@ def _resolution_from_lockfile( ResolvedDependency(spec=dep.spec, required=dep.required, status="missing") ) continue - sink_path = as_path(lp.sink) if lp.sink else None - if sink_path is None or not sink_path.exists(): + if not lp.sink or not self._lockfile_entry_is_valid(lp): logger.warning( - "Lockfile entry for %s points to missing file %s — will re-resolve on next run", + "Lockfile entry for %s points to a missing or corrupt file %s", dep.spec, lp.sink, ) diff --git a/packages/gnss-product-management/src/gnss_product_management/factories/remote_transport.py b/packages/gnss-product-management/src/gnss_product_management/factories/remote_transport.py index 5932c20..7548eac 100644 --- a/packages/gnss-product-management/src/gnss_product_management/factories/remote_transport.py +++ b/packages/gnss-product-management/src/gnss_product_management/factories/remote_transport.py @@ -19,7 +19,7 @@ ) from gnss_product_management.specifications.products.product import PathTemplate from gnss_product_management.specifications.remote.resource import SearchTarget -from gnss_product_management.utilities.helpers import decompress_gzip +from gnss_product_management.utilities.helpers import decompress_gzip, hash_file from gnss_product_management.utilities.paths import AnyPath, as_path logger = logging.getLogger(__name__) @@ -261,25 +261,34 @@ def _update_parameters(self, search_target: SearchTarget) -> SearchTarget: return updated @staticmethod - def _validate_cached_or_evict(path: AnyPath) -> bool: - """Validate a cached file against its sidecar lockfile hash. + def _validate_cached_or_evict(path: AnyPath, checksum: str | None = None) -> bool: + """Validate a cached file against a declared or sidecar hash. - Returns ``True`` when the file can be trusted: either its SHA-256 - matches the sidecar ``_lock.json`` written at download time, or no - sidecar exists to validate against. On a hash mismatch the file - and its stale sidecar are deleted so the caller re-downloads. + When *checksum* is given (declared in the resource spec) it is + authoritative: the file's SHA-256 must match it, regardless of + what the sidecar says — this catches caches poisoned before the + sidecar was written. Otherwise the file is validated against + the sidecar ``_lock.json`` written at download time; a file with + no sidecar is trusted. On mismatch the file and its sidecar are + deleted so the caller re-downloads. Args: path: Existing cached file to validate. + checksum: Expected ``sha256:`` from the resource spec. Returns: ``True`` if the cached file is usable, ``False`` if it was evicted and must be re-downloaded. """ - lock = get_lock_product(path) - if lock is None or validate_lock_product(lock, mode=HashMismatchMode.STRICT): - return True - logger.warning("Cached file failed hash validation, evicting: %s", path) + if checksum is not None: + if hash_file(path) == checksum: + return True + logger.warning("Cached file does not match declared checksum, evicting: %s", path) + else: + lock = get_lock_product(path) + if lock is None or validate_lock_product(lock, mode=HashMismatchMode.STRICT): + return True + logger.warning("Cached file failed hash validation, evicting: %s", path) as_path(str(path) + "_lock.json").unlink(missing_ok=True) path.unlink(missing_ok=True) return False @@ -294,11 +303,12 @@ def download_one( """Synchronously download matched files for one search target. Skips the download if the destination file already exists, - is non-empty, and matches its sidecar lockfile hash (when one - exists) — a corrupt cached file is evicted and re-downloaded. - Downloaded files are verified against the remote size (when the - server reports one) and retried once on mismatch, so truncated - transfers are never cached. + is non-empty, and matches the spec-declared checksum (when the + resource spec pins one) or its sidecar lockfile hash — a corrupt + cached file is evicted and re-downloaded. Downloaded files are + verified against the remote size (when the server reports one) + and the declared checksum, retried once on mismatch, so + truncated or corrupt transfers are never cached. Args: query: The resolved search target with filename value. @@ -334,11 +344,13 @@ def download_one( return decompressed_path # Skip download if the file already exists, is non-empty, and - # passes sidecar hash validation. + # passes checksum/sidecar hash validation. A declared checksum + # describes the file as served, so it only applies to the + # non-decompressed destination path. if ( destination_path.exists() and destination_path.stat().st_size > 0 - and self._validate_cached_or_evict(destination_path) + and self._validate_cached_or_evict(destination_path, query.checksum) ): logger.debug("Skipping download, file already exists: %s", destination_path) return destination_path @@ -372,20 +384,26 @@ def download_one( return None if result is None: return None - if remote_size is None: - break + # Truncated/corrupt transfers must never be left on disk where + # a later run would treat them as satisfied dependencies. local_size = result.stat().st_size - if local_size == remote_size: + if remote_size is not None and local_size != remote_size: + logger.warning( + "Size mismatch for %s: %d bytes local vs %d remote — deleting%s", + result, + local_size, + remote_size, + ", retrying" if attempt == 0 else "", + ) + elif query.checksum is not None and hash_file(result) != query.checksum: + logger.warning( + "Checksum mismatch for %s: expected %s — deleting%s", + result, + query.checksum, + ", retrying" if attempt == 0 else "", + ) + else: break - # Truncated/corrupt transfer: never leave it on disk where a - # later run would treat it as a satisfied dependency. - logger.warning( - "Size mismatch for %s: %d bytes local vs %d remote — deleting%s", - result, - local_size, - remote_size, - ", retrying" if attempt == 0 else "", - ) result.unlink(missing_ok=True) result = None diff --git a/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource.py b/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource.py index 99a5c41..985127c 100644 --- a/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource.py +++ b/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource.py @@ -45,6 +45,11 @@ class ResourceProductSpec(BaseModel): description: Human-readable description. parameters: Parameter overrides (values pinned by this center). directory: Directory template for this product. + checksum: Expected ``sha256:`` of the served file, as + downloaded (i.e. before any decompression). Only meaningful + for entries that resolve to exactly one file — static tables + pinned to an immutable source. When set, downloads and cache + hits are validated against it. """ id: str @@ -55,6 +60,7 @@ class ResourceProductSpec(BaseModel): description: str | None = None parameters: list[Parameter] directory: PathTemplate + checksum: str | None = None class ResourceSpec(BaseModel): @@ -98,11 +104,15 @@ class SearchTarget(BaseModel): product: The product being queried. server: The server endpoint to query. directory: Directory path template for the product. + checksum: Expected ``sha256:`` of the served file, carried + from the resource spec. ``None`` when the source declares no + checksum. """ product: Product server: Server directory: PathTemplate + checksum: str | None = None def narrow(self) -> "SearchTarget": """Substitute already-known parameter values into directory/filename patterns. diff --git a/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource_catalog.py b/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource_catalog.py index 0eb4cb5..c3f859f 100644 --- a/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource_catalog.py +++ b/packages/gnss-product-management/src/gnss_product_management/specifications/remote/resource_catalog.py @@ -123,6 +123,7 @@ def build(cls, resource_spec: ResourceSpec, product_catalog) -> "ResourceCatalog product=pinned_product, server=server, directory=rp_spec.directory, + checksum=rp_spec.checksum, ).narrow() ) diff --git a/packages/gnss-product-management/test/test_download_integrity.py b/packages/gnss-product-management/test/test_download_integrity.py index cb6470d..63a0cde 100644 --- a/packages/gnss-product-management/test/test_download_integrity.py +++ b/packages/gnss-product-management/test/test_download_integrity.py @@ -23,8 +23,20 @@ get_lock_product_path, write_lock_product, ) -from gnss_product_management.specifications.products.product import PathTemplate, Product -from gnss_product_management.specifications.remote.resource import SearchTarget, Server +from gnss_product_management.specifications.products.product import ( + PathTemplate, + Product, + VariantCatalog, + VersionCatalog, +) +from gnss_product_management.specifications.remote.resource import ( + ResourceProductSpec, + ResourceSpec, + SearchTarget, + Server, +) +from gnss_product_management.specifications.remote.resource_catalog import ResourceCatalog +from gnss_product_management.utilities.helpers import hash_file # ── Constants ───────────────────────────────────────────────────── @@ -49,7 +61,7 @@ def sink_product(self, product: Product, resource_id: str, date) -> SearchTarget ) -def _make_query(remote_root: Path, filename: str) -> SearchTarget: +def _make_query(remote_root: Path, filename: str, checksum: str | None = None) -> SearchTarget: product = Product( name="ORBIT", parameters=[], @@ -59,6 +71,7 @@ def _make_query(remote_root: Path, filename: str) -> SearchTarget: product=product, server=Server(id="remote", hostname=str(remote_root)), directory=PathTemplate(pattern="products", value="products"), + checksum=checksum, ) @@ -83,8 +96,8 @@ def env(tmp_path: Path): } -def _download(env, filename: str = "TEST.SP3") -> Path | None: - query = _make_query(env["remote_root"], filename) +def _download(env, filename: str = "TEST.SP3", checksum: str | None = None) -> Path | None: + query = _make_query(env["remote_root"], filename, checksum) return env["wormhole"].download_one( query=query, local_resource_id="local_config", @@ -233,3 +246,82 @@ def test_valid_decompressed_cache_is_reused(self, gz_env, monkeypatch) -> None: ) assert _download(gz_env, filename="ATT.OBX.gz") == cached assert calls == [] + + +# ── Spec-declared checksums ─────────────────────────────────────── + + +class TestDeclaredChecksum: + def test_matching_checksum_download_succeeds(self, env) -> None: + checksum = hash_file(env["remote_dir"] / "TEST.SP3") + result = _download(env, checksum=checksum) + assert result is not None + assert result.read_bytes() == REMOTE_CONTENT + + def test_checksum_mismatch_is_deleted_not_cached(self, env) -> None: + """A remote file that doesn't match the declared checksum (upstream + drift or corruption) must fail — and leave nothing on disk.""" + wrong = "sha256:" + "0" * 64 + assert _download(env, checksum=wrong) is None + assert not (env["sink_dir"] / "TEST.SP3").exists() + + def test_poisoned_cache_with_consistent_sidecar_is_evicted(self, env) -> None: + """The case sidecar validation alone cannot catch: a truncated file + whose sidecar hash was computed from the already-truncated content. + The spec-declared checksum is the only source of truth for it.""" + cached = env["sink_dir"] / "TEST.SP3" + cached.parent.mkdir(parents=True) + cached.write_bytes(REMOTE_CONTENT[:10]) + _write_sidecar(cached) # hash matches the truncated content + + checksum = hash_file(env["remote_dir"] / "TEST.SP3") + result = _download(env, checksum=checksum) + assert result == cached + assert result.read_bytes() == REMOTE_CONTENT + + def test_checksum_flows_from_resource_spec(self) -> None: + """ResourceCatalog.build must carry a product entry's checksum + onto every SearchTarget it expands.""" + checksum = "sha256:" + "a" * 64 + spec = ResourceSpec( + id="TST", + name="Test Center", + servers=[Server(id="srv", hostname="https://example.com")], + products=[ + ResourceProductSpec( + id="tst_orbit", + server_id="srv", + product_name="ORBIT", + parameters=[], + directory=PathTemplate(pattern="products/"), + checksum=checksum, + ) + ], + ) + catalog = _StubProductCatalog( + { + "ORBIT": VersionCatalog[Product]( + versions={ + "1": VariantCatalog[Product]( + variants={ + "default": Product( + name="ORBIT", + parameters=[], + filename=PathTemplate(pattern="TEST.SP3"), + ) + } + ) + } + ) + } + ) + built = ResourceCatalog.build(spec, catalog) + assert built.queries + assert all(q.checksum == checksum for q in built.queries) + + +class _StubProductCatalog: + """Minimal stand-in for ProductCatalog in ResourceCatalog.build.""" + + def __init__(self, products): + self.products = products diff --git a/packages/gnss-product-management/test/test_lockfile.py b/packages/gnss-product-management/test/test_lockfile.py index 5370776..9198a63 100644 --- a/packages/gnss-product-management/test/test_lockfile.py +++ b/packages/gnss-product-management/test/test_lockfile.py @@ -361,3 +361,42 @@ def test_installed_version(self) -> None: v = get_package_version() # At minimum it should be a semver-ish string assert "." in v + + +# ── ResolvePipeline lockfile-entry validation ───────────────────── + + +class TestLockfileEntryValidation: + """_lockfile_entry_is_valid guards the ResolvePipeline fast path: + aggregate entries carry no hash, so the per-file sidecar written at + download time provides the hash to validate against.""" + + def test_entry_with_matching_sidecar_is_valid(self, tmp_path: Path) -> None: + from gnss_product_management.factories.pipelines.resolve import ResolvePipeline + + sink = _make_product_file(tmp_path) + write_lock_product(build_lock_product(sink=sink, url="", name="ORBIT")) + entry = LockProduct(name="ORBIT", url="", sink=str(sink)) + assert ResolvePipeline._lockfile_entry_is_valid(entry) + + def test_entry_without_sidecar_only_needs_existence(self, tmp_path: Path) -> None: + from gnss_product_management.factories.pipelines.resolve import ResolvePipeline + + sink = _make_product_file(tmp_path) + entry = LockProduct(name="ORBIT", url="", sink=str(sink)) + assert ResolvePipeline._lockfile_entry_is_valid(entry) + + def test_entry_with_missing_file_is_invalid(self, tmp_path: Path) -> None: + from gnss_product_management.factories.pipelines.resolve import ResolvePipeline + + entry = LockProduct(name="ORBIT", url="", sink=str(tmp_path / "gone.SP3")) + assert not ResolvePipeline._lockfile_entry_is_valid(entry) + + def test_entry_corrupted_after_sidecar_is_invalid(self, tmp_path: Path) -> None: + from gnss_product_management.factories.pipelines.resolve import ResolvePipeline + + sink = _make_product_file(tmp_path) + write_lock_product(build_lock_product(sink=sink, url="", name="ORBIT")) + sink.write_text("corrupted after the sidecar was written") + entry = LockProduct(name="ORBIT", url="", sink=str(sink)) + assert not ResolvePipeline._lockfile_entry_is_valid(entry) diff --git a/packages/pride-ppp/src/pride_ppp/configs/centers/pride_table_config.yaml b/packages/pride-ppp/src/pride_ppp/configs/centers/pride_table_config.yaml index 2451d41..b9001a9 100644 --- a/packages/pride-ppp/src/pride_ppp/configs/centers/pride_table_config.yaml +++ b/packages/pride-ppp/src/pride_ppp/configs/centers/pride_table_config.yaml @@ -15,9 +15,11 @@ # To re-pin: pick the new upstream commit, verify every product below # still resolves against its table/ listing # (https://api.github.com/repos/PrideLab/PRIDE-PPPAR/contents/table?ref=), -# update the hostname, and coordinate the same SHA with consumers that -# install the pdp3 binary (e.g. earthscope-sfg-workflows' clone-pride / -# install-pride tasks) so the binary and tables stay in lockstep. +# update the hostname, recompute every product checksum from the files +# at the new commit (sha256: of each file as served), and +# coordinate the same SHA with consumers that install the pdp3 binary +# (e.g. earthscope-sfg-workflows' clone-pride / install-pride tasks) so +# the binary and tables stay in lockstep. id: PRIDE name: PrideLab PRIDE-PPPAR (GitHub) @@ -47,6 +49,7 @@ products: description: Satellite name/identifier lookup table parameters: [] directory: {pattern: "table/"} + checksum: sha256:1dc164d6cc254cae58851e5f550af4dd970d384cd3d2a2c46fbf018e456a1e3e - id: pride_ocean_tide product_name: OCEAN_TIDE_FES2004 @@ -55,6 +58,7 @@ products: description: FES2004 ocean tide model coefficients parameters: [] directory: {pattern: "table/"} + checksum: sha256:5b05eb062f169fffec0f787143db7315b3a94198ee3f0bee54f1c28dca593bce - id: pride_ocean_load_green product_name: OCEAN_LOAD_GREEN @@ -63,6 +67,7 @@ products: description: Green's function coefficients for ocean tidal loading parameters: [] directory: {pattern: "table/"} + checksum: sha256:926332ba664ced0fdbec95811e45e07f1e355caa704cf215ab161cf75415e0bd - id: pride_gpt3_grid product_name: GPT3_GRID @@ -71,6 +76,7 @@ products: description: GPT3 global pressure and temperature model grid parameters: [] directory: {pattern: "table/"} + checksum: sha256:9fca4ab977bbbb195765e546fd6b67dc80d912829534975bbbd99521fc23b792 - id: pride_ocean_load product_name: OCEAN_LOAD_PROGRAM @@ -79,6 +85,7 @@ products: description: Pre-computed ocean loading parameters parameters: [] directory: {pattern: "table/"} + checksum: sha256:888f7271ad4e5ce959a7259b7a3c85e6c098fab2aa705c86ba32fb749042eb53 # ── Static table files (base products) ────────────────────────── - id: pride_leap_sec @@ -88,6 +95,7 @@ products: description: IERS leap seconds file parameters: [] directory: {pattern: "table/"} + checksum: sha256:8403d0274eaee978523674cb6a72216510400c4c4b6519f469cd0a46cc7b4f1d - id: pride_sat_params product_name: SAT_PARAMS @@ -96,6 +104,7 @@ products: description: Satellite metadata parameters table parameters: [] directory: {pattern: "table/"} + checksum: sha256:aa20d1bff19f8c2d16acff99bbc08e9a91a93df0117a574c3801c205906f54e2 # ── Antenna calibrations (ANTEX archive) ──────────────────────── - id: pride_atx_igs14 @@ -108,6 +117,7 @@ products: - {name: REFFRAME, value: igs14} - {name: GPSWEEK, value: "2247"} directory: {pattern: "table/"} + checksum: sha256:dace943a12ae87dc70c4e2a1811244bcc3054ce5f17504313309af923d023b5f - id: pride_atx_igs20 product_name: ATTATX @@ -119,6 +129,7 @@ products: - {name: REFFRAME, value: igs20} - {name: GPSWEEK, value: "2317"} directory: {pattern: "table/"} + checksum: sha256:d001d9407a1e2dd976ad2a6dd2e0a35fefd216447d3930562aec8df81e5f0edd - id: pride_atx_igsR3 product_name: ATTATX @@ -130,6 +141,7 @@ products: - {name: REFFRAME, value: igsR3} - {name: GPSWEEK, value: "2135"} directory: {pattern: "table/"} + checksum: sha256:2dd6d1e4131c012fee59ae1c351df4981b9edea4debf69371ac809aed08034de # ── Orography grid ───────────────────────────────────────────── - id: pride_orography_1x1 @@ -140,6 +152,7 @@ products: parameters: - {name: RESOLUTION, value: "1x1"} directory: {pattern: "table/"} + checksum: sha256:39715d4c9810deb6f88e30570256ec0b38348d3f465c87b7b0e67f2c1c69f0c4 - id: pride_orography product_name: OROGRAPHY @@ -148,3 +161,4 @@ products: description: Ellipsoidal terrain height grid (VMF1) parameters: [] directory: {pattern: "table/"} + checksum: sha256:ed199b341cb9b3358af62cb16148222fbefd339bbab3edfbd31adaa10685c25c