Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
# `<base>/<dppId>.json`.
# `<base>/<dppId>/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 `<base>/<dppId>.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
Expand Down
30 changes: 30 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions crates/dpp-node/src/infra/s3_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
Expand Down Expand Up @@ -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.
Expand Down
34 changes: 34 additions & 0 deletions crates/dpp-types/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 64 additions & 3 deletions crates/dpp-vault/src/domain/service/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"
);
}
}
Loading