diff --git a/.env.example b/.env.example index 8139060..85b0ddf 100644 --- a/.env.example +++ b/.env.example @@ -212,7 +212,10 @@ RESOLVER_BASE_URL=https://id.odal-node.io # # Public base URL under which this deployment *serves* its continuity snapshots. # Declared to the registry as each passport's independently-hosted back-up, as -# `/.json`. +# `//public.json` — the same key the snapshot store writes, so +# whatever serves this base must expose the bucket's own layout rather than a +# flattened one. This line previously documented `/.json`, which +# addressed nothing. # # Deliberately separate from SNAPSHOT_S3_BUCKET: writing snapshots to object # storage does not make them reachable. Until an operator states that they are diff --git a/CHANGELOG.md b/CHANGELOG.md index ad44323..59b4397 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -208,6 +208,36 @@ under the pre-1.0 conventions in [VERSIONING.md](docs/governance/VERSIONING.md): ### Fixed +- **The registry back-up URL pointed one path segment away from the snapshot it + named.** The snapshot store writes `{dppId}/public.json`; `publish` declared + the back-up at `{dppId}.json`, and `.env.example` documented that second shape + — so the code and its documentation agreed with each other and disagreed with + the only thing that writes an object. Any operator who set + `SNAPSHOT_PUBLIC_BASE_URL` published a link that fetches nothing. + + Nothing could catch it. `RegistrationRequest::validate` checks that the URL is + HTTPS and stops, because `https://host/dpp/{id}.json` is well formed and simply + addresses nothing; reachability is the registry's check, so the first party to + discover it would have been the registry, on a live registration. No test + covered the relationship either — the registry suite only ever exercised + `backup_url: None`, which is the default. + + The path now comes from `snapshot_json_key` in `dpp-types`, which is what the + store writes and what the declaration reads. One definition, visible to both + crates, is what stops them drifting apart again; a test asserts the declared + URL ends with the key the store writes, and fails with both halves named when + it does not. A trailing slash on the configured base no longer produces a + double slash, which is a different path to most static servers. + + Latent rather than live: `SNAPSHOT_PUBLIC_BASE_URL` is unset by default and + commented out in `.env.example`, so no back-up link was being declared at all. + The defect surfaced the moment an operator did what the documentation said. + + `.env.example` now states that whatever serves the base must expose the + bucket's own layout rather than a flattened one. The rendered HTML sibling is + deliberately not what the back-up points at: that link is consumed by machines, + and the JSON view carries the signatures a verifier needs. + - **Fourteen model groups rendered as API sections with an empty Operations heading.** `Errors`, `Passport`, … `Regulatory Catalog` were declared as tags and 109 schemas pointed at them with `x-tags`. That is a Redoc grouping diff --git a/crates/dpp-node/src/infra/s3_snapshot.rs b/crates/dpp-node/src/infra/s3_snapshot.rs index e02a612..f54da64 100644 --- a/crates/dpp-node/src/infra/s3_snapshot.rs +++ b/crates/dpp-node/src/infra/s3_snapshot.rs @@ -27,7 +27,7 @@ use aws_sdk_s3::{ primitives::ByteStream, }; use dpp_domain::error::DppError; -use dpp_types::snapshot::{SnapshotMeta, SnapshotStore}; +use dpp_types::snapshot::{SnapshotMeta, SnapshotStore, snapshot_html_key, snapshot_json_key}; pub struct S3SnapshotConfig { pub endpoint: Option, @@ -95,12 +95,15 @@ impl S3SnapshotStore { } } + /// Delegates to `dpp-types` rather than formatting here. `publish` builds + /// the registry back-up URL from the same definition, and the two used to + /// disagree by a path segment. fn key(dpp_id: &str) -> String { - format!("{dpp_id}/public.json") + snapshot_json_key(dpp_id) } fn html_key(dpp_id: &str) -> String { - format!("{dpp_id}/public.html") + snapshot_html_key(dpp_id) } /// Apply the staleness headers every snapshot object carries. diff --git a/crates/dpp-types/src/snapshot.rs b/crates/dpp-types/src/snapshot.rs index 5cb7dd0..5a64afa 100644 --- a/crates/dpp-types/src/snapshot.rs +++ b/crates/dpp-types/src/snapshot.rs @@ -39,6 +39,40 @@ use async_trait::async_trait; use serde::{Deserialize, Serialize}; +/// The object key a passport's machine-readable snapshot is written at, +/// relative to the bucket root and to whatever base URL serves it. +/// +/// # Why this is not private to the store +/// +/// Two places have to agree on it and they live in different crates: the store +/// writes the object, and `publish` declares a back-up URL to the EU registry +/// that has to point at the object the store wrote. They did not agree — the +/// store wrote `{id}/public.json` while the declaration said `{id}.json`, so an +/// operator who configured the feature published a link one path segment away +/// from the file, and `.env.example` documented the wrong one of the two. +/// +/// Nothing could catch that. The registry payload's own validation checks the +/// scheme and stops, because `https://host/dpp/{id}.json` is a perfectly +/// well-formed URL that happens to address nothing; reachability is the +/// registry's check, so the first party to notice would have been the registry, +/// on a live registration. +/// +/// One definition, used by both, is what stops them drifting apart again. +#[must_use] +pub fn snapshot_json_key(dpp_id: &str) -> String { + format!("{dpp_id}/public.json") +} + +/// The rendered HTML sibling of [`snapshot_json_key`]. +/// +/// Written for a human who reaches the static tier directly. Deliberately *not* +/// what the registry back-up URL points at: that link is consumed by machines, +/// and the JSON view is the one carrying the signatures a verifier needs. +#[must_use] +pub fn snapshot_html_key(dpp_id: &str) -> String { + format!("{dpp_id}/public.html") +} + use dpp_domain::{DppError, passport::PassportId}; /// When a snapshot was taken and how long it vouches for itself, carried diff --git a/crates/dpp-vault/src/domain/service/publish.rs b/crates/dpp-vault/src/domain/service/publish.rs index 3e1d922..a79f9f6 100644 --- a/crates/dpp-vault/src/domain/service/publish.rs +++ b/crates/dpp-vault/src/domain/service/publish.rs @@ -12,7 +12,10 @@ use dpp_domain::{ product_group::ProductGroupData, status::PassportStatus, }; -use dpp_types::{STANDALONE_OPERATOR_ID, audit::PassportAuditEntry, auth::AuthContext}; +use dpp_types::{ + STANDALONE_OPERATOR_ID, audit::PassportAuditEntry, auth::AuthContext, + snapshot::snapshot_json_key, +}; use super::{PassportService, retention_years_for}; use super::{catalog, schema_registry}; @@ -362,10 +365,11 @@ impl PassportService { // Declare the back-up only where this deployment actually // publishes one. The snapshot tier writing to object storage is // not enough — the registry has to be able to fetch it. + // let backup_url = self .snapshot_public_base_url .as_ref() - .map(|base| format!("{base}/{}.json", passport.id)); + .map(|base| snapshot_backup_url(base, &passport.id.to_string())); let mut reg_req = RegistrationRequest::from_published_passport( &passport, RegisteringOperator { @@ -520,6 +524,26 @@ fn validate_schema_for_publish(product_group_data: &ProductGroupData) -> Result< .map_err(DppError::from) } +/// The independently-hosted back-up URL declared to the EU registry. +/// +/// The path is [`snapshot_json_key`], which is the same definition the snapshot +/// store writes its object at. That is the whole point of this function +/// existing: the two were formatted independently, and disagreed — the store +/// wrote `{id}/public.json` while this declared `{id}.json`, so any operator who +/// set `SNAPSHOT_PUBLIC_BASE_URL` published a link one path segment away from +/// the file. Nothing could catch it, because +/// `RegistrationRequest::validate` checks the scheme and stops, and +/// `https://host/dpp/{id}.json` is a well-formed HTTPS URL that addresses +/// nothing. The registry would have been the first to find out, on a live +/// registration. +fn snapshot_backup_url(base: &str, dpp_id: &str) -> String { + format!( + "{}/{}", + base.trim_end_matches('/'), + snapshot_json_key(dpp_id) + ) +} + /// Build the carrier (QR / Data Matrix) URL a passport should encode, on the /// node's configured resolver base. /// @@ -599,7 +623,9 @@ mod rejection_reasons { #[cfg(test)] mod tests { - use super::{build_carrier_url, validate_schema_for_publish}; + use super::{ + build_carrier_url, snapshot_backup_url, snapshot_json_key, validate_schema_for_publish, + }; use chrono::Utc; use dpp_domain::{ error::DppError, @@ -705,4 +731,39 @@ mod tests { let err = validate_schema_for_publish(&sd).unwrap_err(); assert!(matches!(err, DppError::Validation(_))); } + + /// The declared URL must end with the key the store writes. This is the + /// assertion the defect needed and did not have: both sides formatted the + /// path independently, so they could — and did — disagree while every test + /// and every validator stayed green. + #[test] + fn the_declared_backup_url_ends_with_the_key_the_store_writes() { + let id = "01a06300-571f-7993-a59a-3ca9bb80db56"; + let url = snapshot_backup_url("https://backup.example.com/dpp", id); + assert!( + url.ends_with(&snapshot_json_key(id)), + "the registry back-up URL must address the object the snapshot store \ + writes; declared `{url}`, store writes `{}`", + snapshot_json_key(id) + ); + assert_eq!( + url, + format!("https://backup.example.com/dpp/{id}/public.json") + ); + } + + /// A base with a trailing slash is the same base. Left unhandled it produces + /// a double slash, which is a different path to most static servers. + #[test] + fn a_trailing_slash_on_the_base_does_not_double_up() { + let id = "01a06300-571f-7993-a59a-3ca9bb80db56"; + assert_eq!( + snapshot_backup_url("https://backup.example.com/dpp/", id), + snapshot_backup_url("https://backup.example.com/dpp", id), + ); + assert!( + !snapshot_backup_url("https://backup.example.com/dpp/", id).contains("//dpp"), + "no empty path segment" + ); + } }