From 44bfee44d3bb310be8e2d6e4b56648e4be006caf Mon Sep 17 00:00:00 2001 From: LKSNDRTMLKV Date: Thu, 24 Sep 2026 13:38:22 +0200 Subject: [PATCH] fix(vault)!: require RESOLVER_BASE_URL --- .env.example | 2 +- CHANGELOG.md | 8 +++++ CLAUDE.md | 2 +- crates/dpp-common/src/config.rs | 18 +++++----- crates/dpp-node/src/main.rs | 4 +-- crates/dpp-node/tests/seal_outbox.rs | 5 +++ crates/dpp-node/tests/smoke.rs | 1 + crates/dpp-vault/src/config.rs | 36 ++++++++++++++++++- crates/dpp-vault/src/domain/service/mod.rs | 19 ++++------ crates/dpp-vault/src/main.rs | 1 + crates/dpp-vault/tests/continuity_snapshot.rs | 1 + crates/dpp-vault/tests/evidence_dossier.rs | 1 + crates/dpp-vault/tests/helpers/mod.rs | 1 + 13 files changed, 73 insertions(+), 26 deletions(-) diff --git a/.env.example b/.env.example index 82e2de61..94094efe 100644 --- a/.env.example +++ b/.env.example @@ -122,7 +122,7 @@ VAULT_BASE_URL=http://localhost:8001/vault # points at the node's vault sub-pat CACHE_TTL_SECS=30 # worst-case recall-propagation window; raise only with that tradeoff in mind RATE_LIMIT_RPM=120 # per-IP request limit -# REQUIRED, by both the node and the resolver — neither has a default. +# REQUIRED, by the node, the resolver and a standalone vault — none has a default. # # The public origin printed onto the product: the host a scanned QR code # resolves against, baked into every passport's carrier URL at publish time. diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d512616..dd13d184 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,14 @@ under the pre-1.0 conventions in [VERSIONING.md](docs/governance/VERSIONING.md): another component lives — which is exactly what neither binary can know, and a wrong guess is signed into labels that cannot be recalled. + **A standalone vault requires it too.** *(Breaking for `dpp-vault` run as its + own binary, which no image or compose file ships.)* It never read the + variable: it built its passport service through a constructor that defaulted + to the same dead host, so every carrier it signed pointed there whatever the + environment said. It now reads the value through the same reader, and the + constructor takes it as an argument with no default, so no caller can build a + service that signs carriers without being told where they resolve. + - **The redaction moved to `dpp-domain`, and two things it does differently are visible on the wire.** *(Breaking for **newly published** passports only. Every public and audience route serves the payload decoded out of the stored diff --git a/CLAUDE.md b/CLAUDE.md index e02c6867..bb4e6d91 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -322,7 +322,7 @@ Anything not covered by a recipe is plain cargo — e.g. `cargo run -p dpp-node` to run the node, and `cargo run -p dpp-cli -- bootstrap` to seed operator config and the first API key. -**Environment**: Copy `.env.example` to `.env` before running. Required vars: `DATABASE_URL`, `KEY_STORE_PATH`, `KEY_STORE_PASSPHRASE`, `DID_WEB_BASE_URL`, `RESOLVER_BASE_URL` (the resolver requires it too — it has no default in either binary). +**Environment**: Copy `.env.example` to `.env` before running. Required vars: `DATABASE_URL`, `KEY_STORE_PATH`, `KEY_STORE_PASSPHRASE`, `DID_WEB_BASE_URL`, `RESOLVER_BASE_URL` (the resolver and a standalone vault require it too — no binary has a default). ## Architecture diff --git a/crates/dpp-common/src/config.rs b/crates/dpp-common/src/config.rs index d335f8ef..7198094e 100644 --- a/crates/dpp-common/src/config.rs +++ b/crates/dpp-common/src/config.rs @@ -42,15 +42,15 @@ pub const RESOLVER_BASE_URL: &str = "RESOLVER_BASE_URL"; /// Read [`RESOLVER_BASE_URL`], the one value the node and the resolver must /// agree on. /// -/// **Required, with no default, in both binaries.** The node writes it into -/// every passport's carrier URL at publish, inside the signature, so a wrong -/// value cannot be corrected afterwards. The resolver builds its GS1 Digital -/// Link redirects and canonical links from it. Both used to fall back to a -/// hosted address that does not resolve, and the resolver's copy was never -/// handed the operator's value by the compose file — so every scanned carrier -/// redirected to a dead host while the node's own configuration looked right. -/// A default here is a guess about where another component lives, which is -/// exactly what neither binary can know. +/// **Required, with no default, in every binary that reads it.** The node — or +/// a standalone vault — writes it into every passport's carrier URL at publish, +/// inside the signature, so a wrong value cannot be corrected afterwards. The +/// resolver builds its GS1 Digital Link redirects and canonical links from it. +/// All three used to fall back to a hosted address that does not resolve, and +/// the resolver's copy was never handed the operator's value by the compose +/// file — so every scanned carrier redirected to a dead host while the node's +/// own configuration looked right. A default here is a guess about where +/// another component lives, which is exactly what no binary can know. /// /// # Errors /// diff --git a/crates/dpp-node/src/main.rs b/crates/dpp-node/src/main.rs index 2b45f55b..57eebbd1 100644 --- a/crates/dpp-node/src/main.rs +++ b/crates/dpp-node/src/main.rs @@ -256,6 +256,7 @@ async fn main() -> anyhow::Result<()> { registry_sync, archive, operator, + cfg.resolver_base_url.clone(), ) .with_registry_reader(db.operator_repo.clone()) .with_registry_outbox(db.registry_outbox.clone()) @@ -264,8 +265,7 @@ async fn main() -> anyhow::Result<()> { .with_transfer_store(db.transfer_store.clone()) .with_transfer_outbox(db.transfer_outbox.clone()) .with_evidence_store(db.evidence_store.clone()) - .with_webhooks(db.webhook_outbox.clone()) - .with_resolver_base_url(cfg.resolver_base_url.clone()); + .with_webhooks(db.webhook_outbox.clone()); if let Some(base) = cfg.snapshot_public_base_url.clone() { passport_service = passport_service.with_snapshot_public_base_url(base); } diff --git a/crates/dpp-node/tests/seal_outbox.rs b/crates/dpp-node/tests/seal_outbox.rs index 1b644124..018b1d19 100644 --- a/crates/dpp-node/tests/seal_outbox.rs +++ b/crates/dpp-node/tests/seal_outbox.rs @@ -316,6 +316,7 @@ async fn publish_then_drain_seals_the_passport_end_to_end() { legal_name: "Test Operator GmbH".to_owned(), country: "MK".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_seal_outbox(seal_outbox.clone()); @@ -483,6 +484,7 @@ async fn a_republish_needs_and_gets_its_own_seal() { legal_name: "Test Operator GmbH".to_owned(), country: "MK".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_seal_outbox(seal_outbox.clone()); @@ -661,6 +663,7 @@ async fn a_locally_sealed_passport_reports_that_no_provider_issued_it() { legal_name: "Test Operator GmbH".to_owned(), country: "MK".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_seal_outbox(seal_outbox.clone()) // Wired exactly as the composition root wires it, and unconditionally for @@ -1021,6 +1024,7 @@ async fn a_seal_corrupted_at_rest_is_found_and_repaired() { legal_name: "Test Operator GmbH".to_owned(), country: "MK".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_seal_outbox(seal_outbox.clone()) .with_seal_inspector(Arc::new(dpp_seal::CadesInspector::new())); @@ -1525,6 +1529,7 @@ async fn a_seal_made_under_an_invalid_certificate_is_found_and_not_called_broken legal_name: "Test Operator GmbH".to_owned(), country: "MK".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_seal_outbox(seal_outbox.clone()) .with_seal_inspector(Arc::new(dpp_seal::CadesInspector::new())); diff --git a/crates/dpp-node/tests/smoke.rs b/crates/dpp-node/tests/smoke.rs index 04153127..802da7d4 100644 --- a/crates/dpp-node/tests/smoke.rs +++ b/crates/dpp-node/tests/smoke.rs @@ -185,6 +185,7 @@ async fn start_node_with_ruleset( legal_name: "Test Operator GmbH".to_owned(), country: "DE".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_transfer_store(Arc::new(PgTransferRepo::new(dal.clone()))) .with_evidence_store(Arc::new(PgEvidenceDossierRepo::new(dal.clone()))) diff --git a/crates/dpp-vault/src/config.rs b/crates/dpp-vault/src/config.rs index 8fece8e2..3110cf11 100644 --- a/crates/dpp-vault/src/config.rs +++ b/crates/dpp-vault/src/config.rs @@ -30,6 +30,13 @@ pub struct Config { /// mint the first API key via the CLI before any key exists). pub admin_username: Option, pub admin_password: Option, + + /// Base URL the public resolver serves on, stamped into each passport's + /// carrier (QR) URL at publish. Required, with no default — read through + /// [`dpp_common::config::resolver_base_url`], the reader the node and the + /// resolver use, so a standalone vault cannot sign a carrier under a + /// different rule than the node does. + pub resolver_base_url: String, } impl std::fmt::Debug for Config { @@ -46,6 +53,7 @@ impl std::fmt::Debug for Config { "admin_password", &self.admin_password.as_ref().map(|_| REDACTED), ) + .field("resolver_base_url", &self.resolver_base_url) .finish() } } @@ -53,7 +61,7 @@ impl std::fmt::Debug for Config { impl Config { /// Load configuration from environment variables. /// - /// **Required**: `DATABASE_URL`, `IDENTITY_SERVICE_URL`. + /// **Required**: `DATABASE_URL`, `IDENTITY_SERVICE_URL`, `RESOLVER_BASE_URL`. /// **Optional**: `PORT` (default 8001), `LOG_LEVEL` (default `"info"`), /// `CORS_ALLOWED_ORIGINS` (default empty), `ADMIN_USERNAME`, `ADMIN_PASSWORD`. /// @@ -83,6 +91,7 @@ impl Config { admin_password: std::env::var("ADMIN_PASSWORD") .ok() .filter(|s| !s.is_empty()), + resolver_base_url: dpp_common::config::resolver_base_url()?, }) } } @@ -105,6 +114,7 @@ mod tests { "postgres://odal_app:test@localhost:5432/odal", ); std::env::set_var("IDENTITY_SERVICE_URL", "http://identity:8002"); + std::env::set_var("RESOLVER_BASE_URL", "https://resolver.example.com/"); } } @@ -113,6 +123,7 @@ mod tests { for v in &[ "DATABASE_URL", "IDENTITY_SERVICE_URL", + "RESOLVER_BASE_URL", "PORT", "LOG_LEVEL", "CORS_ALLOWED_ORIGINS", @@ -138,12 +149,34 @@ mod tests { "postgres://odal_app:test@localhost:5432/odal" ); assert_eq!(cfg.identity_service_url, "http://identity:8002"); + // Through the shared reader, so the trailing `/` is gone exactly as it + // is for the node and the resolver. + assert_eq!(cfg.resolver_base_url, "https://resolver.example.com"); assert_eq!(cfg.port, 8001); assert_eq!(cfg.log_level, "info"); assert!(cfg.cors_allowed_origins.is_empty()); clear_all(); } + /// The standalone vault signs carriers exactly as the node does, so it must + /// refuse to start without the resolver's origin rather than fall back to + /// one. It used to: `PassportService::new` defaulted to a hosted address + /// that does not resolve, and this binary never overrode it. + #[test] + fn a_standalone_vault_refuses_to_start_without_a_resolver_base_url() { + let _g = ENV_LOCK.lock().unwrap(); + set_required(); + unsafe { + std::env::remove_var("RESOLVER_BASE_URL"); + } + let err = Config::from_env().expect_err("no resolver origin, no vault"); + assert!( + format!("{err:#}").contains("RESOLVER_BASE_URL"), + "the refusal must name the variable to set: {err:#}" + ); + clear_all(); + } + #[test] fn port_and_log_level_are_overridable() { let _g = ENV_LOCK.lock().unwrap(); @@ -195,6 +228,7 @@ mod tests { cors_allowed_origins: Vec::new(), admin_username: Some("odal-admin".into()), admin_password: Some("admin-pass-must-not-leak".into()), + resolver_base_url: "https://resolver.example.com".into(), }; let rendered = format!("{cfg:?}"); assert!( diff --git a/crates/dpp-vault/src/domain/service/mod.rs b/crates/dpp-vault/src/domain/service/mod.rs index 71680cd2..ab09ef7e 100644 --- a/crates/dpp-vault/src/domain/service/mod.rs +++ b/crates/dpp-vault/src/domain/service/mod.rs @@ -168,9 +168,11 @@ pub struct PassportService { /// URL the registry cannot fetch is worse than none. pub snapshot_public_base_url: Option, /// Base URL the resolver serves on, used to build each passport's carrier - /// (QR) URL at publish. Defaults to `https://id.odal-node.io`; set per - /// deployment (a self-hoster's own domain) via [`Self::with_resolver_base_url`] - /// so printed labels carry the operator's domain, not a hardcoded host. + /// (QR) URL at publish. A constructor argument with no default: it is + /// signed into every carrier, so a fallback here would be a guess about + /// where the resolver lives — the same guess `RESOLVER_BASE_URL` stopped + /// making in the binaries. A binary reads it through + /// `dpp_common::config::resolver_base_url`. pub resolver_base_url: String, } @@ -186,6 +188,7 @@ impl PassportService { registry_sync: Arc, archive: Arc, operator: OperatorIdentity, + resolver_base_url: String, ) -> Self { Self { repo, @@ -208,7 +211,7 @@ impl PassportService { seal_outbox: None, seal_inspector: None, snapshot_public_base_url: None, - resolver_base_url: "https://id.odal-node.io".to_owned(), + resolver_base_url, } } @@ -321,14 +324,6 @@ impl PassportService { self } - /// Set the resolver base URL used to build passport carrier (QR) URLs at - /// publish. Defaults to `https://id.odal-node.io` when not set. - #[must_use] - pub fn with_resolver_base_url(mut self, base: String) -> Self { - self.resolver_base_url = base; - self - } - /// Emit an event after a successful commit. Failures are logged, never /// propagated — the DB write is the source of truth. async fn emit(&self, event_type: &str, data: serde_json::Value) { diff --git a/crates/dpp-vault/src/main.rs b/crates/dpp-vault/src/main.rs index 30c7e27f..0f1f1530 100644 --- a/crates/dpp-vault/src/main.rs +++ b/crates/dpp-vault/src/main.rs @@ -126,6 +126,7 @@ async fn main() -> anyhow::Result<()> { registry_sync, Arc::new(GhostArchive), domain::service::OperatorIdentity::default(), + cfg.resolver_base_url.clone(), ) .with_registry_reader(operator_repo.clone()) .with_evidence_store(evidence_repo) diff --git a/crates/dpp-vault/tests/continuity_snapshot.rs b/crates/dpp-vault/tests/continuity_snapshot.rs index 275cb762..c36a9185 100644 --- a/crates/dpp-vault/tests/continuity_snapshot.rs +++ b/crates/dpp-vault/tests/continuity_snapshot.rs @@ -211,6 +211,7 @@ async fn build_service() -> ( legal_name: "Test Operator GmbH".to_owned(), country: "DE".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_snapshot_outbox(Arc::new(snapshots.clone())); (service, snapshots, identity, public_key, key_dir) diff --git a/crates/dpp-vault/tests/evidence_dossier.rs b/crates/dpp-vault/tests/evidence_dossier.rs index 0e531234..6e7bf26b 100644 --- a/crates/dpp-vault/tests/evidence_dossier.rs +++ b/crates/dpp-vault/tests/evidence_dossier.rs @@ -203,6 +203,7 @@ async fn build_service() -> (PassportService, Arc, String) legal_name: "Test Operator GmbH".to_owned(), country: "DE".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_transfer_store(Arc::new(InMemoryTransferStore::default())) .with_evidence_store(evidence_store.clone()); diff --git a/crates/dpp-vault/tests/helpers/mod.rs b/crates/dpp-vault/tests/helpers/mod.rs index 4018cd04..55a5e260 100644 --- a/crates/dpp-vault/tests/helpers/mod.rs +++ b/crates/dpp-vault/tests/helpers/mod.rs @@ -317,6 +317,7 @@ async fn start_vault_with_identity( legal_name: "Test Operator GmbH".to_owned(), country: "DE".to_owned(), }, + "https://resolver.example.com".to_owned(), ) .with_registry_reader(operator_repo.clone()) // Mirror production here too: the node wires both registry outboxes, so