feat(purl): guard emitted PURL validity, and build source RPM PURLs

.github/workflows/check.yml

@@ -401,6 +401,16 @@ jobs:
         if: matrix.shard == 'integration'
         run: cargo test --test output_format_golden --profile ci-release --verbose
 
+      # Expected fixtures are generated from the parsers, so they double as a
+      # broad sample of the PURLs the parsers emit. Nothing at runtime enforces
+      # PURL validity — `normalize_purl` returns unparsable input unchanged, and
+      # dropping such a value at the output boundary would erase the component
+      # from SBOM output, where the PURL is its identity — so the guard lives
+      # here instead.
+      - name: Run emitted PURL validity guard
+        if: matrix.shard == 'integration'
+        run: cargo test --test emitted_purl_validity_guard --profile ci-release --verbose
+
       # The progress-CLI suite above already builds `provenant` at
       # `target/ci-release/provenant` (it exercises `CARGO_BIN_EXE_provenant`),
       # so this reuses that binary instead of paying for a second CLI build in

src/parsers/rpm_db.rs

@@ -444,7 +444,11 @@ fn build_package_data(pkg: RpmQueryPackage, datasource_id: DatasourceId) -> Pack
                 )
             })
             .unwrap_or_else(empty_declared_license_data);
-    let source_packages = source_rpm.clone().into_iter().collect();
+    let source_packages = source_rpm
+        .as_deref()
+        .and_then(source_rpm_purl)
+        .into_iter()
+        .collect();
     let file_references = {
         let from_dir_components =
             build_file_references(&pkg.base_names, &pkg.dir_indexes, &pkg.dir_names);
@@ -540,6 +544,34 @@ fn build_dependency(require: &str) -> Option<Dependency> {
     })
 }
 
+/// Builds the PURL for a binary RPM's source RPM, from its `name-version-release.arch.rpm`
+/// filename.
+///
+/// `source_packages` is specified as a list of *PURLs* — an SRPM is the source
+/// package of a binary RPM — so storing the filename verbatim put a value there
+/// that no consumer can parse. The release is part of the version, matching how
+/// the binary package's own PURL is built, and `arch` stays a qualifier.
+///
+/// Returns `None` when the filename does not decompose, rather than falling back
+/// to the raw string: a non-PURL in this field is exactly the problem.
+pub(super) fn source_rpm_purl(source_rpm: &str) -> Option<String> {
+    let stem = source_rpm.strip_suffix(".rpm")?;
+    let (name_version_release, arch) = stem.rsplit_once('.')?;
+    let (name_version, release) = name_version_release.rsplit_once('-')?;
+    let (name, version) = name_version.rsplit_once('-')?;
+    if name.is_empty() || version.is_empty() || release.is_empty() {
+        return None;
+    }
+
+    build_package_purl(
+        Some(name),
+        None,
+        Some(&format!("{version}-{release}")),
+        Some(arch),
+        None,
+    )
+}
+
 fn build_package_purl(
     name: Option<&str>,
     namespace: Option<&str>,
@@ -602,6 +634,33 @@ fn infer_platform_architecture(platform: Option<&str>) -> Option<String> {
 
 #[cfg(test)]
 mod tests {
+    #[test]
+    fn test_source_rpm_purl_decomposes_a_nevra_filename() {
+        // `source_packages` is specified as PURLs — an SRPM is the source package
+        // of a binary RPM — so the filename was a value no consumer could parse.
+        assert_eq!(
+            super::source_rpm_purl("gcc-13.1.1-2.fc38.src.rpm").as_deref(),
+            Some("pkg:rpm/gcc@13.1.1-2.fc38?arch=src")
+        );
+        // A name containing `-` still splits correctly: only the last two
+        // hyphen-separated fields are version and release.
+        assert_eq!(
+            super::source_rpm_purl("fedora-modular-repos-26-0.4.module_39876f37.src.rpm")
+                .as_deref(),
+            Some("pkg:rpm/fedora-modular-repos@26-0.4.module_39876f37?arch=src")
+        );
+        assert_eq!(
+            super::source_rpm_purl("fping-2.4b2-10.fc12.src.rpm").as_deref(),
+            Some("pkg:rpm/fping@2.4b2-10.fc12?arch=src")
+        );
+
+        // Anything that does not decompose yields no PURL rather than falling
+        // back to the raw string, which is the defect being fixed.
+        for malformed in ["gcc.rpm", "gcc-13.src.rpm", "not-an-rpm", ""] {
+            assert_eq!(super::source_rpm_purl(malformed), None, "{malformed:?}");
+        }
+    }
+
     use super::*;
 
     use crate::models::DatasourceId;

src/parsers/rpm_parser.rs

@@ -508,7 +508,12 @@ fn build_salvaged_rpm_package(path: &Path, fields: SalvagedRpmFields) -> Option<
         declared_license_expression_spdx,
         license_detections,
         extracted_license_statement,
-        source_packages: fields.source_rpm.into_iter().collect(),
+        source_packages: fields
+            .source_rpm
+            .as_deref()
+            .and_then(super::rpm_db::source_rpm_purl)
+            .into_iter()
+            .collect(),
         extra_data: (!extra_data.is_empty()).then_some(extra_data),
         purl: build_rpm_purl(
             &name,
@@ -808,7 +813,11 @@ fn parse_rpm_package(metadata: &PackageMetadata, path: &Path) -> PackageData {
         license_detections,
         extracted_license_statement,
         dependencies,
-        source_packages: source_rpm.into_iter().collect(),
+        source_packages: source_rpm
+            .as_deref()
+            .and_then(super::rpm_db::source_rpm_purl)
+            .into_iter()
+            .collect(),
         vcs_url,
         extra_data: (!extra_data.is_empty()).then_some(extra_data),
         purl: name.as_ref().and_then(|n| {

testdata/rpm/fedora-bdb-rootfs.expected.json

@@ -59,7 +59,7 @@
     "other_license_detections": [],
     "extracted_license_statement": "MIT",
     "notice_text": null,
-    "source_packages": ["fedora-modular-repos-26-0.4.module_39876f37.src.rpm"],
+    "source_packages": ["pkg:rpm/fedora-modular-repos@26-0.4.module_39876f37?arch=src"],
     "file_references": [
       {
         "path": "/etc/pki/rpm-gpg",

testdata/rpm/fping-2.4b2-10.fc12.x86_64.rpm.expected.json

@@ -29,7 +29,7 @@
     "keywords": ["Applications/Internet"],
     "homepage_url": "http://www.fping.com/",
     "extracted_license_statement": "BSD with advertising",
-    "source_packages": ["fping-2.4b2-10.fc12.src.rpm"],
+    "source_packages": ["pkg:rpm/fping@2.4b2-10.fc12?arch=src"],
     "extra_data": {
       "distribution": "Koji"
     },

testdata/rpm/rpmdb.sqlite.expected.json

@@ -59,7 +59,7 @@
     "other_license_detections": [],
     "extracted_license_statement": "GPLv3+ and GPLv3+ with exceptions and GPLv2+ with exceptions and LGPLv2+ and BSD",
     "notice_text": null,
-    "source_packages": ["gcc-13.1.1-2.fc38.src.rpm"],
+    "source_packages": ["pkg:rpm/gcc@13.1.1-2.fc38?arch=src"],
     "file_references": [
       {
         "path": "/lib64/libgcc_s-13-20230511.so.1",

tests/emitted_purl_validity_guard.rs

@@ -0,0 +1,206 @@
+// SPDX-FileCopyrightText: Provenant contributors
+// SPDX-License-Identifier: Apache-2.0
+
+//! Guards every PURL-shaped string in the checked-in expected fixtures against
+//! being unparsable.
+//!
+//! Expected fixtures are generated from the parsers, so they are a broad sample
+//! of what the parsers actually emit. Nothing else enforces PURL validity:
+//! `normalize_purl` is a normalizer and returns unparsable input unchanged, and
+//! parsers historically assembled PURLs with `format!`, splicing unvalidated
+//! names straight in — which is how `pkg:pypi/::`, `pkg:apk/alpine/musl>=1.2.0`
+//! and `pkg:osgi/my bundle?x@1.0.0` came to ship.
+//!
+//! This is deliberately a check rather than a runtime filter. Dropping an
+//! unparsable PURL at the output boundary would erase the component from SBOM
+//! output entirely — the PURL doubles as its identity there — and would discard
+//! caller-supplied values on the `--from-json` path. The parsers are the right
+//! place to be correct; this is the guard that keeps them so.
+
+use std::collections::BTreeSet;
+use std::fs;
+use std::path::{Path, PathBuf};
+use std::str::FromStr;
+
+use packageurl::PackageUrl;
+use serde_json::Value;
+
+/// Fields whose values are PURLs, or PURLs carrying a `uuid` qualifier.
+const PURL_FIELDS: &[&str] = &[
+    "purl",
+    "package_uid",
+    "dependency_uid",
+    "for_package_uid",
+    "for_packages",
+    "source_packages",
+];
+
+/// The one identity Provenant emits that is deliberately not a PURL: the
+/// fallback used when a package has no resolvable coordinates. It is prefixed so
+/// that a consumer fails loudly rather than mis-parsing it as one.
+const NON_PURL_IDENTITY_PREFIX: &str = "generated-package:";
+
+fn expected_fixture_files(root: &Path) -> Vec<PathBuf> {
+    let mut found = Vec::new();
+    let Ok(entries) = fs::read_dir(root) else {
+        return found;
+    };
+
+    for entry in entries.flatten() {
+        let path = entry.path();
+        if path.is_dir() {
+            found.extend(expected_fixture_files(&path));
+            continue;
+        }
+        let Some(name) = path.file_name().and_then(|name| name.to_str()) else {
+            continue;
+        };
+        if name.contains("expected") && (name.ends_with(".json") || name.ends_with(".expected")) {
+            found.push(path);
+        }
+    }
+
+    found
+}
+
+fn collect_purls(value: &Value, into: &mut BTreeSet<String>) {
+    match value {
+        Value::Object(map) => {
+            for (key, child) in map {
+                if PURL_FIELDS.contains(&key.as_str()) {
+                    match child {
+                        Value::String(purl) => {
+                            into.insert(purl.clone());
+                        }
+                        Value::Array(items) => {
+                            for item in items.iter().filter_map(Value::as_str) {
+                                into.insert(item.to_string());
+                            }
+                        }
+                        _ => {}
+                    }
+                }
+                collect_purls(child, into);
+            }
+        }
+        Value::Array(items) => {
+            for item in items {
+                collect_purls(item, into);
+            }
+        }
+        _ => {}
+    }
+}
+
+#[test]
+fn every_emitted_purl_in_expected_fixtures_parses() {
+    let fixtures = expected_fixture_files(Path::new("testdata"));
+    assert!(
+        fixtures.len() > 100,
+        "expected to find the fixture corpus, found {} files",
+        fixtures.len()
+    );
+
+    let mut purls = BTreeSet::new();
+    for fixture in &fixtures {
+        let Ok(contents) = fs::read_to_string(fixture) else {
+            continue;
+        };
+        let Ok(value) = serde_json::from_str::<Value>(&contents) else {
+            // Not every `.expected` file is JSON; those carry no PURL fields.
+            continue;
+        };
+        collect_purls(&value, &mut purls);
+    }
+
+    assert!(
+        purls.len() > 500,
+        "expected a broad PURL sample, found {}",
+        purls.len()
+    );
+
+    let unparsable: Vec<&String> = purls
+        .iter()
+        .filter(|purl| !purl.is_empty())
+        .filter(|purl| !purl.starts_with(NON_PURL_IDENTITY_PREFIX))
+        .filter(|purl| PackageUrl::from_str(purl).is_err())
+        .collect();
+
+    assert!(
+        unparsable.is_empty(),
+        "these emitted PURLs cannot be parsed:\n  {}",
+        unparsable
+            .iter()
+            .map(|purl| purl.as_str())
+            .collect::<Vec<_>>()
+            .join("\n  ")
+    );
+}
+
+#[test]
+fn every_emitted_purl_in_expected_fixtures_keeps_its_components() {
+    // Component stability rather than string equality: re-emitting a parsed PURL
+    // must yield the same type, namespace, name, version, qualifiers and
+    // subpath. This catches a component landing in the wrong slot — a `#` in a
+    // name becoming a subpath, or a `uuid` qualifier swallowed into one — while
+    // tolerating the two places Provenant deliberately encodes differently from
+    // the crate (`$`/`'` in versions, and golang namespace case).
+    let mut purls = BTreeSet::new();
+    for fixture in expected_fixture_files(Path::new("testdata")) {
+        let Ok(contents) = fs::read_to_string(&fixture) else {
+            continue;
+        };
+        let Ok(value) = serde_json::from_str::<Value>(&contents) else {
+            continue;
+        };
+        collect_purls(&value, &mut purls);
+    }
+
+    let mut unstable = Vec::new();
+    for purl in purls
+        .iter()
+        .filter(|purl| !purl.is_empty())
+        .filter(|purl| !purl.starts_with(NON_PURL_IDENTITY_PREFIX))
+    {
+        let Ok(parsed) = PackageUrl::from_str(purl) else {
+            continue; // reported by the parse test above
+        };
+        let reparsed = match PackageUrl::from_str(&parsed.to_string()) {
+            Ok(reparsed) => reparsed,
+            Err(error) => {
+                unstable.push(format!("{purl} -> re-parse failed: {error}"));
+                continue;
+            }
+        };
+
+        if parsed.ty() != reparsed.ty()
+            || parsed.namespace() != reparsed.namespace()
+            || parsed.name() != reparsed.name()
+            || parsed.version() != reparsed.version()
+            || parsed.subpath() != reparsed.subpath()
+        {
+            unstable.push(format!("{purl} -> {}", reparsed));
+            continue;
+        }
+
+        let qualifiers: BTreeSet<(&str, &str)> = parsed
+            .qualifiers()
+            .iter()
+            .map(|(key, value)| (key.as_ref(), value.as_ref()))
+            .collect();
+        let reparsed_qualifiers: BTreeSet<(&str, &str)> = reparsed
+            .qualifiers()
+            .iter()
+            .map(|(key, value)| (key.as_ref(), value.as_ref()))
+            .collect();
+        if qualifiers != reparsed_qualifiers {
+            unstable.push(format!("{purl} -> qualifiers changed"));
+        }
+    }
+
+    assert!(
+        unstable.is_empty(),
+        "these PURLs do not survive a parse/emit round trip with their components intact:\n  {}",
+        unstable.join("\n  ")
+    );
+}