From 69345328bb6c528ef3e24d6d5c26ca4d3694c16e Mon Sep 17 00:00:00 2001 From: Erik Jaaniso Date: Tue, 20 Oct 2026 11:32:59 +0300 Subject: [PATCH 1/4] fix(schema): publish the creator, legislation, licence and access-rights rules The validator refuses an empty or oversized creator list or name, legislation list or licence, an accessRights outside the three EU IRIs, and oversized legalBasis or isReferencedBy lists, but the schemas allowed them, so code generated from them accepted manifests the node refuses. --- crates/core/src/model/manifest.rs | 33 +++++++ crates/core/src/model/metadata.rs | 4 + crates/core/src/model/overlay.rs | 35 ++++++- crates/core/src/validate_pkg.rs | 20 +++- .../tests/it/package_schema.rs | 94 ++++++++++++++++--- docs/manifest.schema.json | 38 ++++++-- docs/metadata-overlay.schema.json | 37 ++++++-- docs/package-format.md | 4 +- 8 files changed, 229 insertions(+), 36 deletions(-) diff --git a/crates/core/src/model/manifest.rs b/crates/core/src/model/manifest.rs index 1945ae1..48bf1df 100644 --- a/crates/core/src/model/manifest.rs +++ b/crates/core/src/model/manifest.rs @@ -138,12 +138,31 @@ pub struct ManifestMetadata { #[cfg_attr(feature = "schema", schemars(required))] pub description: Option, /// Access rights authority IRI (`PUBLIC` / `RESTRICTED` / `NON_PUBLIC`). + #[cfg_attr( + feature = "schema", + schemars(schema_with = "crate::validate_pkg::access_rights_schema") + )] pub access_rights: String, /// Legislation mandating the dataset (EU ELI IRIs, >= 1). + #[cfg_attr( + feature = "schema", + schemars( + length(min = 1, max = crate::validate_pkg::MAX_IRI_LIST_COUNT), + inner(length(max = crate::validate_pkg::MAX_IRI_LEN)) + ) + )] pub applicable_legislation: Vec, /// Reuse license for this dataset (IRI). + #[cfg_attr( + feature = "schema", + schemars(length(min = 1, max = crate::validate_pkg::MAX_IRI_LEN)) + )] pub license: String, /// Creating agents (>= 1). + #[cfg_attr( + feature = "schema", + schemars(length(min = 1, max = crate::validate_pkg::MAX_CREATORS_COUNT)) + )] pub creator: Vec, /// GDI health category IRIs (>= 1). #[cfg_attr( @@ -173,9 +192,23 @@ pub struct ManifestMetadata { pub type_: Option>, /// DPV legal basis IRIs (for real personal data). #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars( + length(max = crate::validate_pkg::MAX_IRI_LIST_COUNT), + inner(length(max = crate::validate_pkg::MAX_IRI_LEN)) + ) + )] pub legal_basis: Option>, /// Publication DOI IRIs. #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars( + length(max = crate::validate_pkg::MAX_IRI_LIST_COUNT), + inner(length(max = crate::validate_pkg::MAX_IRI_LEN)) + ) + )] pub is_referenced_by: Option>, /// Secondary identifiers. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/core/src/model/metadata.rs b/crates/core/src/model/metadata.rs index ac05555..f76c432 100644 --- a/crates/core/src/model/metadata.rs +++ b/crates/core/src/model/metadata.rs @@ -48,6 +48,10 @@ pub enum LocalizedText { #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct Agent { /// `foaf:name`. + #[cfg_attr( + feature = "schema", + schemars(length(min = 1, max = crate::validate_pkg::MAX_CREATOR_NAME_LEN)) + )] pub name: String, } diff --git a/crates/core/src/model/overlay.rs b/crates/core/src/model/overlay.rs index 6ed5504..c1819b3 100644 --- a/crates/core/src/model/overlay.rs +++ b/crates/core/src/model/overlay.rs @@ -24,16 +24,35 @@ pub struct MetadataOverlay { #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, /// Access rights authority IRI (`PUBLIC` / `RESTRICTED` / `NON_PUBLIC`). - #[serde(skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars(schema_with = "crate::validate_pkg::optional_access_rights_schema") + )] pub access_rights: Option, /// Legislation mandating the dataset (EU ELI IRIs, >= 1). #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars( + length(min = 1, max = crate::validate_pkg::MAX_IRI_LIST_COUNT), + inner(length(max = crate::validate_pkg::MAX_IRI_LEN)) + ) + )] pub applicable_legislation: Option>, /// Reuse license for this dataset (IRI). #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars(length(min = 1, max = crate::validate_pkg::MAX_IRI_LEN)) + )] pub license: Option, /// Creating agents (>= 1). #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars(length(min = 1, max = crate::validate_pkg::MAX_CREATORS_COUNT)) + )] pub creator: Option>, /// GDI health category IRIs (>= 1). #[serde(default, skip_serializing_if = "Option::is_none")] @@ -64,9 +83,23 @@ pub struct MetadataOverlay { pub type_: Option>, /// DPV legal basis IRIs (for real personal data). #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars( + length(max = crate::validate_pkg::MAX_IRI_LIST_COUNT), + inner(length(max = crate::validate_pkg::MAX_IRI_LEN)) + ) + )] pub legal_basis: Option>, /// Publication DOI IRIs. #[serde(skip_serializing_if = "Option::is_none")] + #[cfg_attr( + feature = "schema", + schemars( + length(max = crate::validate_pkg::MAX_IRI_LIST_COUNT), + inner(length(max = crate::validate_pkg::MAX_IRI_LEN)) + ) + )] pub is_referenced_by: Option>, /// Secondary identifiers. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/core/src/validate_pkg.rs b/crates/core/src/validate_pkg.rs index 4dd2339..c95157e 100644 --- a/crates/core/src/validate_pkg.rs +++ b/crates/core/src/validate_pkg.rs @@ -83,7 +83,7 @@ pub const MAX_KEYWORDS_COUNT: usize = 50; /// `creator name` cap. pub const MAX_CREATOR_NAME_LEN: usize = 255; /// `creator` count cap. -const MAX_CREATORS_COUNT: usize = 64; +pub const MAX_CREATORS_COUNT: usize = 64; /// `contactPoint fn` cap. pub const MAX_CONTACT_FN_LEN: usize = 255; /// `email` cap (RFC 5321). @@ -96,7 +96,7 @@ pub const MAILTO_EMAIL_PATTERN: &str = /// URL cap (`hasURL`, `afSourceReference`, ...). const MAX_URL_LEN: usize = 2048; /// IRI cap (every IRI metadata field). -const MAX_IRI_LEN: usize = 2048; +pub const MAX_IRI_LEN: usize = 2048; /// `otherIdentifier.notation` cap. const MAX_NOTATION_LEN: usize = 255; /// `otherIdentifier.schemaAgency` cap. @@ -113,7 +113,7 @@ const MAX_OTHER_IDS_COUNT: usize = 50; /// oversized manifest is re-serialised on every request. 64 is far above any real value (a /// dataset cites a handful of EU legislation IRIs) and far below the size at which /// re-serialising the list becomes one. -const MAX_IRI_LIST_COUNT: usize = 64; +pub const MAX_IRI_LIST_COUNT: usize = 64; /// `file category` / `reference` / `preciseReference` cap. const MAX_FILE_CATEGORY_LEN: usize = 32; /// File groups count cap. @@ -302,6 +302,20 @@ pub(crate) fn dataset_type_schema(_: &mut schemars::SchemaGenerator) -> schemars closed_list_schema(DATASET_TYPES, 1, true) } +/// `accessRights` in the manifest schema: one [`ACCESS_RIGHTS`] IRI. +#[cfg(feature = "schema")] +pub(crate) fn access_rights_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { + schemars::json_schema!({"type": "string", "enum": ACCESS_RIGHTS}) +} + +/// `accessRights` in the overlay schema, where it may be left out. +#[cfg(feature = "schema")] +pub(crate) fn optional_access_rights_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { + let mut allowed: Vec = ACCESS_RIGHTS.iter().map(|&iri| iri.into()).collect(); + allowed.push(serde_json::Value::Null); + schemars::json_schema!({"type": ["string", "null"], "enum": allowed}) +} + /// `conformsTo` IRIs: the closed set gdi-metadata's `DatasetShape` enumerates /// (`sh:in`, 3 values; vendored `Dataset.ttl`). /// diff --git a/crates/gdi-node-standalone/tests/it/package_schema.rs b/crates/gdi-node-standalone/tests/it/package_schema.rs index 4f61ef4..439dee4 100644 --- a/crates/gdi-node-standalone/tests/it/package_schema.rs +++ b/crates/gdi-node-standalone/tests/it/package_schema.rs @@ -8,7 +8,8 @@ use gdi_node_standalone_core::model::{Manifest, MetadataOverlay}; use gdi_node_standalone_core::validate_pkg::{ - CONFORMS_TO, DATASET_TYPES, HEALTH_CATEGORIES, MAX_CONTACT_FN_LEN, MAX_PROVENANCE_LEN, + ACCESS_RIGHTS, CONFORMS_TO, DATASET_TYPES, EHDS_ELI, HEALTH_CATEGORIES, MAX_CONTACT_FN_LEN, + MAX_CREATOR_NAME_LEN, MAX_CREATORS_COUNT, MAX_IRI_LEN, MAX_IRI_LIST_COUNT, MAX_PROVENANCE_LEN, MAX_TYPICAL_AGE, validate_overlay_result, validate_patch, }; use serde_json::{Value, json}; @@ -51,11 +52,22 @@ fn long(n: usize) -> String { "a".repeat(n) } +/// A valid IRI exactly `len` characters long. +fn iri(len: usize) -> String { + let base = "https://example.org/"; + format!("{base}{}", long(len - base.len())) +} + /// Field values the node refuses, each with a value at the limit that it accepts. +#[expect( + clippy::too_many_lines, + reason = "a data table, one row per rule; both tests loop over the same rows" +)] fn cases() -> Vec<(&'static str, &'static str, Value, Value)> { let contact = |fn_: Value, email: Value| json!({"fn": fn_, "hasEmail": email}); let email = json!("mailto:data@example.org"); - vec![ + let creators = |n: usize| json!(vec![json!({"name": "C"}); n]); + let mut cases = vec![ ( "contact point without a name or e-mail", "contactPoint", @@ -141,7 +153,61 @@ fn cases() -> Vec<(&'static str, &'static str, Value, Value)> { json!(["https://example.org/REAL_DATA"]), json!([DATASET_TYPES[0]]), ), - ] + ( + "access rights outside the vocabulary", + "accessRights", + json!("https://example.org/OPEN"), + json!(ACCESS_RIGHTS[0]), + ), + ( + "no legislation", + "applicableLegislation", + json!([]), + json!([EHDS_ELI]), + ), + ("no creator", "creator", json!([]), json!([{"name": "C"}])), + ( + "creator without a name", + "creator", + json!([{"name": ""}]), + json!([{"name": "C"}]), + ), + ( + "too many creators", + "creator", + creators(MAX_CREATORS_COUNT + 1), + creators(MAX_CREATORS_COUNT), + ), + ( + "creator name too long", + "creator", + json!([{"name": long(MAX_CREATOR_NAME_LEN + 1)}]), + json!([{"name": long(MAX_CREATOR_NAME_LEN)}]), + ), + ("empty licence", "license", json!(""), json!(iri(40))), + ( + "licence too long", + "license", + json!(iri(MAX_IRI_LEN + 1)), + json!(iri(MAX_IRI_LEN)), + ), + ]; + // One validator rule covers all three IRI lists. + for field in ["applicableLegislation", "legalBasis", "isReferencedBy"] { + cases.push(( + "too many IRIs", + field, + json!(vec![iri(40); MAX_IRI_LIST_COUNT + 1]), + json!(vec![iri(40); MAX_IRI_LIST_COUNT]), + )); + cases.push(( + "IRI too long", + field, + json!([iri(MAX_IRI_LEN + 1)]), + json!([iri(MAX_IRI_LEN)]), + )); + } + cases } #[test] @@ -155,18 +221,21 @@ fn the_manifest_schema_refuses_what_the_node_refuses() { let bad = manifest_with(field, refused); assert!( !node_accepts_manifest(&bad), - "{case}: the node must refuse it" + "{case} ({field}): the node must refuse it" + ); + assert!( + !schema.is_valid(&bad), + "{case} ({field}): the schema must refuse it" ); - assert!(!schema.is_valid(&bad), "{case}: the schema must refuse it"); let good = manifest_with(field, accepted); assert!( node_accepts_manifest(&good), - "{case}: the node accepts the limit" + "{case} ({field}): the node accepts the limit" ); assert!( schema.is_valid(&good), - "{case}: the schema accepts the limit" + "{case} ({field}): the schema accepts the limit" ); } } @@ -178,18 +247,21 @@ fn the_overlay_schema_refuses_what_the_node_refuses() { let bad = json!({ field: refused }); assert!( !node_accepts_overlay(&bad), - "{case}: the node must refuse it" + "{case} ({field}): the node must refuse it" + ); + assert!( + !schema.is_valid(&bad), + "{case} ({field}): the schema must refuse it" ); - assert!(!schema.is_valid(&bad), "{case}: the schema must refuse it"); let good = json!({ field: accepted }); assert!( node_accepts_overlay(&good), - "{case}: the node accepts the limit" + "{case} ({field}): the node accepts the limit" ); assert!( schema.is_valid(&good), - "{case}: the schema accepts the limit" + "{case} ({field}): the schema accepts the limit" ); } } diff --git a/docs/manifest.schema.json b/docs/manifest.schema.json index a76386c..bd59b9a 100644 --- a/docs/manifest.schema.json +++ b/docs/manifest.schema.json @@ -48,7 +48,9 @@ "properties": { "name": { "description": "`foaf:name`.", - "type": "string" + "type": "string", + "maxLength": 255, + "minLength": 1 } }, "required": [ @@ -501,14 +503,22 @@ "properties": { "accessRights": { "description": "Access rights authority IRI (`PUBLIC` / `RESTRICTED` / `NON_PUBLIC`).", - "type": "string" + "type": "string", + "enum": [ + "http://publications.europa.eu/resource/authority/access-right/PUBLIC", + "http://publications.europa.eu/resource/authority/access-right/RESTRICTED", + "http://publications.europa.eu/resource/authority/access-right/NON_PUBLIC" + ] }, "applicableLegislation": { "description": "Legislation mandating the dataset (EU ELI IRIs, >= 1).", "type": "array", "items": { - "type": "string" - } + "type": "string", + "maxLength": 2048 + }, + "maxItems": 64, + "minItems": 1 }, "catalog": { "description": "Catalog name; must match a catalog defined in the service config.", @@ -565,7 +575,9 @@ "type": "array", "items": { "$ref": "#/$defs/Agent" - } + }, + "maxItems": 64, + "minItems": 1 }, "datasetId": { "description": "Generated dataset ID (replaces `prefix`/`org`).", @@ -649,8 +661,10 @@ "null" ], "items": { - "type": "string" - } + "type": "string", + "maxLength": 2048 + }, + "maxItems": 64 }, "keywords": { "description": "Tags for discovery (recommended).", @@ -669,12 +683,16 @@ "null" ], "items": { - "type": "string" - } + "type": "string", + "maxLength": 2048 + }, + "maxItems": 64 }, "license": { "description": "Reuse license for this dataset (IRI).", - "type": "string" + "type": "string", + "maxLength": 2048, + "minLength": 1 }, "maxTypicalAge": { "description": "Typical age of the oldest people in the dataset, in years.", diff --git a/docs/metadata-overlay.schema.json b/docs/metadata-overlay.schema.json index 968b445..ac6d06d 100644 --- a/docs/metadata-overlay.schema.json +++ b/docs/metadata-overlay.schema.json @@ -9,6 +9,12 @@ "type": [ "string", "null" + ], + "enum": [ + "http://publications.europa.eu/resource/authority/access-right/PUBLIC", + "http://publications.europa.eu/resource/authority/access-right/RESTRICTED", + "http://publications.europa.eu/resource/authority/access-right/NON_PUBLIC", + null ] }, "applicableLegislation": { @@ -18,8 +24,11 @@ "null" ], "items": { - "type": "string" - } + "type": "string", + "maxLength": 2048 + }, + "maxItems": 64, + "minItems": 1 }, "conformsTo": { "description": "Standards-compliance IRIs.", @@ -57,7 +66,9 @@ ], "items": { "$ref": "#/$defs/Agent" - } + }, + "maxItems": 64, + "minItems": 1 }, "description": { "description": "Free-text description (plain string or language map).", @@ -129,8 +140,10 @@ "null" ], "items": { - "type": "string" - } + "type": "string", + "maxLength": 2048 + }, + "maxItems": 64 }, "keywords": { "description": "Tags for discovery (recommended).", @@ -149,15 +162,19 @@ "null" ], "items": { - "type": "string" - } + "type": "string", + "maxLength": 2048 + }, + "maxItems": 64 }, "license": { "description": "Reuse license for this dataset (IRI).", "type": [ "string", "null" - ] + ], + "maxLength": 2048, + "minLength": 1 }, "maxTypicalAge": { "description": "Typical age of the oldest people in the dataset, in years.", @@ -245,7 +262,9 @@ "properties": { "name": { "description": "`foaf:name`.", - "type": "string" + "type": "string", + "maxLength": 255, + "minLength": 1 } }, "required": [ diff --git a/docs/package-format.md b/docs/package-format.md index fc23986..67d1953 100644 --- a/docs/package-format.md +++ b/docs/package-format.md @@ -122,10 +122,10 @@ The package's first member. Serialized `camelCase`. Top level: | `catalog` | string | yes | Must match a catalog the node accepts. | | `title` | string \| lang-map | yes | Plain string or a `{lang: text}` map. | | `description` | string \| lang-map | yes | Ingest rejects a manifest without it; `manifest.schema.json` lists it under `required`. | -| `accessRights` | IRI string | yes | Access-right authority IRI. | +| `accessRights` | IRI string | yes | Access-right authority IRI, a closed set: `PUBLIC`, `RESTRICTED` and `NON_PUBLIC` under `http://publications.europa.eu/resource/authority/access-right/`. Any other value is rejected. | | `applicableLegislation` | IRI array (≥ 1) | yes | EU ELI IRIs. Omitting the EHDS ELI (`http://data.europa.eu/eli/reg/2025/327/oj`) is a warning, not a rejection. | | `license` | IRI string | yes | Reuse license IRI. | -| `creator` | `[{name}]` (≥ 1) | yes | Creating agents. | +| `creator` | `[{name}]` (≥ 1) | yes | Creating agents, each with a non-empty name. | | `healthCategory` | IRI array (≥ 1) | yes | GDI health-category IRIs, a closed set (`manifest.schema.json` lists it). Any other value is rejected. | | `contactPoint` | `{fn, hasEmail, hasURL?}` | yes | Who to write to about the dataset: a name and a `mailto:` e-mail, and optionally a web page. | | `keywords` | string array | no | Discovery tags. | From ab2722d5bd110a9d166a87266cf88cc184d0eb0f Mon Sep 17 00:00:00 2001 From: Erik Jaaniso Date: Tue, 20 Oct 2026 11:34:38 +0300 Subject: [PATCH 2/4] fix(tool): report a VCF without AF as an invalid VCF in build build's header check called it "invalid parquet" before any parquet existed. --- crates/gdi-dataset-tool/src/commands/cmd_build.rs | 2 +- crates/gdi-dataset-tool/tests/it/build_e2e.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/gdi-dataset-tool/src/commands/cmd_build.rs b/crates/gdi-dataset-tool/src/commands/cmd_build.rs index c3514a3..0f78310 100644 --- a/crates/gdi-dataset-tool/src/commands/cmd_build.rs +++ b/crates/gdi-dataset-tool/src/commands/cmd_build.rs @@ -1268,7 +1268,7 @@ fn preflight_vcf_headers(sources: &[PathBuf]) -> Result<(), ToolError> { let mut af_populations: BTreeSet = BTreeSet::new(); for source in sources { let pops = read_header_populations(source) - .map_err(|e| with_source(source, ToolError::from(&e)))?; + .map_err(|e| with_source(source, ToolError::from_vcf_stage(&e)))?; af_populations.extend(pops); if af_populations.len() > MAX_POPULATIONS { return Err(ToolError::user(format!( diff --git a/crates/gdi-dataset-tool/tests/it/build_e2e.rs b/crates/gdi-dataset-tool/tests/it/build_e2e.rs index 0430b2b..b6917c7 100644 --- a/crates/gdi-dataset-tool/tests/it/build_e2e.rs +++ b/crates/gdi-dataset-tool/tests/it/build_e2e.rs @@ -674,8 +674,8 @@ fn build_args_for(pkg: &Path, out: &Path) -> BuildArgs { } } -/// The header preflight rejects a VCF with no `AF` INFO field before conversion, with -/// the source path prefixed onto the error. +/// The header preflight rejects a VCF with no `AF` INFO field before conversion, as an +/// `invalid VCF` error prefixed with the source path. #[test] #[serial(env)] fn build_missing_af_field_is_rejected() { @@ -694,7 +694,8 @@ fn build_missing_af_field_is_rejected() { "no AF field", ); assert!( - err.message.contains("No AF INFO fields found") && err.message.contains("noaf.vcf"), + err.message.contains("invalid VCF: No AF INFO fields found") + && err.message.contains("noaf.vcf"), "expected a path-prefixed no-AF error, got: {}", err.message ); From f944148b1787fcc3b0b50a5687b712f3bc425345 Mon Sep 17 00:00:00 2001 From: Erik Jaaniso Date: Tue, 20 Oct 2026 11:35:31 +0300 Subject: [PATCH 3/4] fix(tool): make init ask for the catalog when several are allowed init filled in the first allowed catalog, so a package could land in whichever sorted first. The placeholder now lists the choices. --- .../gdi-dataset-tool/src/commands/cmd_init.rs | 44 ++++++++----------- crates/gdi-dataset-tool/tests/it/init_e2e.rs | 23 ++-------- docs/gdi-dataset-tool.md | 7 +-- 3 files changed, 27 insertions(+), 47 deletions(-) diff --git a/crates/gdi-dataset-tool/src/commands/cmd_init.rs b/crates/gdi-dataset-tool/src/commands/cmd_init.rs index 0854faf..23624cc 100644 --- a/crates/gdi-dataset-tool/src/commands/cmd_init.rs +++ b/crates/gdi-dataset-tool/src/commands/cmd_init.rs @@ -2,10 +2,10 @@ //! //! The generated template has the four sections (`metadata`, `files`, `internal`, //! `config`). Required fields are `REPLACE:`-prefixed placeholders the provider must fill -//! in **except** `applicableLegislation`, which is pre-filled with the EHDS ELI (the one -//! required field with a sensible default — removable, at the cost of a `build` warning -//! that `--strict` fails on, and extendable with the GDPR or a national act), and -//! `license`, which is a `REPLACE:` placeholder so the provider chooses explicit terms. +//! in **except** `applicableLegislation`, pre-filled with the EHDS ELI (removable, at the +//! cost of a `build` warning that `--strict` fails on, and extendable with the GDPR or a +//! national act), and `catalog` when the active profile allows a single one. `license` +//! stays a placeholder so the provider chooses explicit terms. //! Recommended fields (`keywords`, `numberOfUniqueIndividuals`) sit under their own //! header; optional fields are shown commented out with example values. //! @@ -64,13 +64,10 @@ fn conforms_to_comment() -> String { /// The scaffolded `package.yaml` template. /// /// Required fields are `REPLACE:` placeholders except `applicableLegislation` -/// (the EHDS ELI) and `license` (a `REPLACE:` placeholder). Recommended fields +/// (the EHDS ELI) and, with a single allowed catalog, `catalog`. Recommended fields /// are under their own header; optional fields are commented with examples. /// -/// `catalog_line` is the rendered `metadata.catalog` line (with any trailing -/// allow-list comment): either the static `REPLACE:` placeholder or, when the -/// active profile has a `catalogs` allow-list, the available catalog name(s) -/// (see [`scaffold_catalog_line`]). +/// `catalog_line` is the rendered `metadata.catalog` line (see [`scaffold_catalog_line`]). #[expect( clippy::too_many_lines, reason = "all but a handful of these lines are ONE string literal — the scaffold document \ @@ -262,20 +259,16 @@ pub fn run( Ok(()) } -/// Render the `metadata.catalog` line from the profile's allow-list names: the -/// first name becomes the value, any others are listed in a trailing comment. An -/// empty list yields the static `REPLACE:` placeholder. +/// Render the `metadata.catalog` line from the profile's allow-list names: one name is +/// filled in, several go into the `REPLACE:` placeholder for the provider to pick from +/// (a pre-filled guess would pass `build` unnoticed), and none gives the static +/// placeholder. fn scaffold_catalog_line<'a>(names: impl Iterator) -> String { let names: Vec<&str> = names.collect(); - match names.split_first() { - None => CATALOG_PLACEHOLDER.to_owned(), - Some((first, [])) => format!(r#"catalog: "{first}""#), - Some((first, rest)) => { - format!( - r#"catalog: "{first}" # other allowed catalogs: {}"#, - rest.join(", ") - ) - } + match names.as_slice() { + [] => CATALOG_PLACEHOLDER.to_owned(), + [only] => format!(r#"catalog: "{only}""#), + several => format!(r#"catalog: "REPLACE: one of {}""#, several.join(", ")), } } @@ -368,9 +361,10 @@ mod tests { scaffold_catalog_line(["gdi-aggregated"].into_iter()), r#"catalog: "gdi-aggregated""# ); - // Several names -> first is the value, the rest are listed in a comment. - let line = scaffold_catalog_line(["a", "b", "c"].into_iter()); - assert!(line.starts_with(r#"catalog: "a""#), "got: {line}"); - assert!(line.contains("other allowed catalogs: b, c"), "got: {line}"); + // Several names -> a placeholder listing them; the provider picks. + assert_eq!( + scaffold_catalog_line(["a", "b", "c"].into_iter()), + r#"catalog: "REPLACE: one of a, b, c""# + ); } } diff --git a/crates/gdi-dataset-tool/tests/it/init_e2e.rs b/crates/gdi-dataset-tool/tests/it/init_e2e.rs index b58c25a..28e2a05 100644 --- a/crates/gdi-dataset-tool/tests/it/init_e2e.rs +++ b/crates/gdi-dataset-tool/tests/it/init_e2e.rs @@ -120,9 +120,8 @@ fn init_refuses_existing_without_force() { #[test] #[serial(env)] fn init_scaffolds_catalog_names_from_profile_allow_list() { - // The profile `catalogs` allow-list scaffolds `init`'s - // `metadata.catalog`, offline. The first name becomes the value; others are - // listed in a trailing comment. + // `init` reads the profile's catalog allow-list offline. With several names the + // provider must choose, so the placeholder lists them. let tmp = tempfile::tempdir().unwrap(); let _config_dir = test_util::EnvGuard::set("GDI_CONFIG_DIR", tmp.path().join("cfg")); let cfg = tmp.path().join("tool.toml"); @@ -148,24 +147,10 @@ fn init_scaffolds_catalog_names_from_profile_allow_list() { gdi_dataset_tool::run(cli).expect("init --profile default succeeds"); let body = fs::read_to_string(&out).unwrap(); - // The first allow-listed name (BTreeMap-sorted) is the value, not a REPLACE. assert!( - body.contains(r#"catalog: "gdi-aggregated""#), - "the catalog value must be scaffolded from the allow-list; body:\n{body}" + body.contains(r#"catalog: "REPLACE: one of gdi-aggregated, synthetic-data""#), + "the placeholder must name every allowed catalog; body:\n{body}" ); - assert!( - !body.contains("REPLACE: catalog name"), - "the static catalog placeholder must be replaced; body:\n{body}" - ); - // The other catalog is offered in a comment. - assert!( - body.contains("other allowed catalogs: synthetic-data"), - "remaining catalogs must be listed in a comment; body:\n{body}" - ); - - // The scaffolded YAML still parses (the catalog comment is valid YAML). - let parsed: PackageYaml = serde_saphyr::from_str(&body.replace("REPLACE:", "filled-")).unwrap(); - assert_eq!(parsed.metadata.catalog, "gdi-aggregated"); } #[test] diff --git a/docs/gdi-dataset-tool.md b/docs/gdi-dataset-tool.md index 2a1740c..bcf12a8 100644 --- a/docs/gdi-dataset-tool.md +++ b/docs/gdi-dataset-tool.md @@ -1338,9 +1338,10 @@ gdi-dataset-tool init [-o ] [--force] The template carries all four sections with `REPLACE:`-prefixed placeholders for the required fields. The exception is `applicableLegislation`, pre-filled with the EHDS ELI: it is removable at the cost of a `build` warning that `--strict` fails on, and extendable -with the GDPR ELI or a national act. Recommended fields sit under their own header, and -optional fields are shown commented out. `init` mints no dataset ID and needs no profile -or country code. +with the GDPR ELI or a national act. `catalog` is filled in when the active profile allows +a single catalog; with several, its placeholder lists them. Recommended fields sit under +their own header, and optional fields are shown commented out. `init` mints no dataset ID +and needs no profile or country code. ```bash gdi-dataset-tool init -o my-dataset.yaml From 4842103171fdce11afbc38d4b7c02af8080d7914 Mon Sep 17 00:00:00 2001 From: Erik Jaaniso Date: Tue, 20 Oct 2026 11:36:21 +0300 Subject: [PATCH 4/4] fix(tool): find the pinned node key in doctor --offline Offline, doctor looked only at node_recipient_file and failed on the pin keys pin-recipient writes, which pack uses when the fetch fails. Profiles without a node URL keep reading node_recipient_file, as pack does. --- .../src/commands/cmd_doctor.rs | 18 +++++++-- .../gdi-dataset-tool/src/commands/cmd_keys.rs | 8 ++-- .../gdi-dataset-tool/src/commands/cmd_pack.rs | 7 ++-- .../gdi-dataset-tool/tests/it/doctor_e2e.rs | 40 +++++++++++++++++++ docs/gdi-dataset-tool.md | 5 ++- 5 files changed, 65 insertions(+), 13 deletions(-) diff --git a/crates/gdi-dataset-tool/src/commands/cmd_doctor.rs b/crates/gdi-dataset-tool/src/commands/cmd_doctor.rs index a920d7f..eef389a 100644 --- a/crates/gdi-dataset-tool/src/commands/cmd_doctor.rs +++ b/crates/gdi-dataset-tool/src/commands/cmd_doctor.rs @@ -5,7 +5,7 @@ //! It validates the active profile loads, the **provider identity** loads (without //! generating it) and its recipient derives, and the resolved **node recipient** //! (an explicit `--recipient` overrides in either mode; otherwise -//! `node_recipient_url` online or `node_recipient_file` offline) is a usable +//! `node_recipient_url` online, or offline the local copy `pack` would use) is a usable //! crypt4gh recipient. **Online** it additionally checks the FDP root //! responds, the catalog list is fetchable, and the S3 bucket is writable (a probe //! PUT/DELETE on a reserved key). It does not probe `/health/ready`, which is on the @@ -144,7 +144,7 @@ pub fn run( } else if online { "node_recipient_url" } else { - "node_recipient_file" + "the local recipient file" }; crate::output::note(&format!("checking node recipient via {recipient_src}")); checks.push(check_node_recipient(&active, args, online, config_path)); @@ -323,8 +323,20 @@ fn check_node_recipient( "no node_recipient_url and no service_url".to_owned(), )), } + } else if recipient::node_recipient_url(active).is_some() { + // Offline with a node URL: the local pin `pack` falls back to when the fetch fails. + let configured = active.node_recipient_path(config_path); + let default_pin = recipient::recipient_pin_path_for_read(active, config_path); + match recipient::offline_pin_fallback(configured.as_deref(), default_pin.as_deref()) { + Some(found) => found.map(|(pk, _)| pk), + None => Err(ToolError::user( + "no local node recipient: set node_recipient_file, or pin the node's key \ + with `keys pin-recipient`" + .to_owned(), + )), + } } else { - // Offline: the local recipient file is required. + // No node URL: the local recipient file is required, as it is for `pack`. match active.node_recipient_path(config_path) { Some(file) => recipient::read_node_recipient_file(&file), None => Err(ToolError::user( diff --git a/crates/gdi-dataset-tool/src/commands/cmd_keys.rs b/crates/gdi-dataset-tool/src/commands/cmd_keys.rs index 2aceab7..7f38504 100644 --- a/crates/gdi-dataset-tool/src/commands/cmd_keys.rs +++ b/crates/gdi-dataset-tool/src/commands/cmd_keys.rs @@ -406,10 +406,10 @@ fn recipient_oneline(pk: &PublicKey) -> String { .join("") } -/// Resolve the active profile's node recipient the way `pack`/`doctor` do: -/// `node_recipient_url` (online primary, defaulting to -/// `{service_url}/.well-known/c4gh-recipient`) if configured, else the offline -/// `node_recipient_file`. Returns an error (rendered by [`show`] as an +/// Resolve the active profile's node recipient: `node_recipient_url` (online primary, +/// defaulting to `{service_url}/.well-known/c4gh-recipient`) if configured, else the +/// offline `node_recipient_file`. Unlike `pack`, a failed fetch does not fall back to the +/// local pin. Returns an error (rendered by [`show`] as an /// unconditional `node recipient: ` stdout line) when neither is /// configured or the online fetch fails. fn resolve_node_recipient( diff --git a/crates/gdi-dataset-tool/src/commands/cmd_pack.rs b/crates/gdi-dataset-tool/src/commands/cmd_pack.rs index 9fe2de9..2406a32 100644 --- a/crates/gdi-dataset-tool/src/commands/cmd_pack.rs +++ b/crates/gdi-dataset-tool/src/commands/cmd_pack.rs @@ -299,12 +299,11 @@ pub fn resolve_recipients_readonly( Ok(vec![node, provider]) } -/// Resolve the node recipient with the spec precedence (mirrored from +/// Resolve the node recipient with the spec precedence (mirrored by /// `cmd_doctor::check_node_recipient`): explicit `--recipient ` /// wins; else the active profile's `node_recipient_url` (online primary, defaulting -/// to `{service_url}/.well-known/c4gh-recipient`) is fetched; else the offline -/// `node_recipient_file`. An online-fetch failure when a URL is configured is a hard error -/// — never a silent fall-through to a possibly-stale local file. +/// to `{service_url}/.well-known/c4gh-recipient`) is fetched, and when the fetch fails +/// the local pin is used with a warning; else the offline `node_recipient_file`. /// /// `--recipient` short-circuits before the profile is loaded, so it works with no /// profile configured. diff --git a/crates/gdi-dataset-tool/tests/it/doctor_e2e.rs b/crates/gdi-dataset-tool/tests/it/doctor_e2e.rs index b08478c..f69b2c8 100644 --- a/crates/gdi-dataset-tool/tests/it/doctor_e2e.rs +++ b/crates/gdi-dataset-tool/tests/it/doctor_e2e.rs @@ -156,6 +156,46 @@ fn offline_broken_recipient_fails() { ); } +/// Offline, `doctor` finds the pin `keys pin-recipient` writes when the profile names no +/// `node_recipient_file`, as `pack` does when the fetch fails. +#[test] +#[serial(env)] +fn offline_finds_the_url_keyed_pin() { + let tmp = tempfile::tempdir().unwrap(); + let _config_dir = make_provider_identity(tmp.path()); + let recipient = make_node_recipient(tmp.path()); + let cfg = tmp.path().join("tool.toml"); + fs::write( + &cfg, + "[profiles.default]\nservice_url = \"http://127.0.0.1:1\"\n", + ) + .unwrap(); + let cli = gdi_dataset_tool::cli::Cli::try_parse_from([ + "gdi-dataset-tool", + "--config", + cfg.to_str().unwrap(), + "keys", + "pin-recipient", + "--file", + recipient.to_str().unwrap(), + ]) + .unwrap(); + gdi_dataset_tool::run(cli).expect("keys pin-recipient --file"); + let pins = fs::read_dir(tmp.path().join("recipients")).unwrap().count(); + assert_eq!( + pins, 1, + "the pin must exist for the check below to mean anything" + ); + + let args = DoctorArgs { + offline: true, + recipient: None, + format: gdi_dataset_tool::cli::OutputFormat::Text, + }; + gdi_dataset_tool::commands::cmd_doctor::run(&args, None, Some(&cfg)) + .expect("doctor --offline finds the pinned recipient"); +} + #[test] #[serial(env)] fn online_unreachable_node_fails() { diff --git a/docs/gdi-dataset-tool.md b/docs/gdi-dataset-tool.md index bcf12a8..4e4aaad 100644 --- a/docs/gdi-dataset-tool.md +++ b/docs/gdi-dataset-tool.md @@ -2214,8 +2214,9 @@ world-readable key fails the check, because file permissions are the only protec unencrypted key has; fix it with `chmod 600`. Online it also checks that the FDP root responds, that the catalog list is fetchable, and, if an S3 block is set, that the bucket is writable, via a probe PUT and DELETE on a reserved key. It does not probe the -management plane's `/health/ready`. Offline it checks the local recipient file and the -`catalogs` allow-list. A failure exits non-zero. +management plane's `/health/ready`. Offline it checks the local copy of the node's key +(`node_recipient_file`, or else the pin `keys pin-recipient` or the first `pack` wrote) +and the `catalogs` allow-list. A failure exits non-zero. `--recipient ` is independent of `--offline`: passed on its own it points the node-recipient check at that local file while still running the online FDP, catalog and S3