From 848d2caae51deece6bbc7d1d6bd96a45502e29bf Mon Sep 17 00:00:00 2001 From: iron-prog Date: Fri, 14 Aug 2026 18:39:27 +0000 Subject: [PATCH 1/6] feat(deps): SBOM ingestion + a standalone coverage-measurement script (#338) First slice of #338 (repo dependency network chart) -- data layer only, per the design agreed with exploreriii: measure real SBOM coverage before building the resolution heuristic or the chart around a guess. - models.py: DependencyManifestRecord (one parsed SBOM package, purl-based ecosystem+name+version, not the raw SPDX name field) and SbomCoverageRecord (per-repo fetch outcome: ok/disabled/error -- kept distinct from the package list itself, so 'no edges' is distinguishable from 'no data', per the issue's coverage-honesty requirement). - github_rest.py: fetch_repo_sbom (REST SBOM endpoint, 403/404 -> disabled mirroring has_codeowners_file's exact contract, any other error -> status='error' rather than raising or silently dropping), _parse_purl (percent-decoded, handles npm scopes/Maven groupIds/Cargo/PyPI/Go), fetch_org_sbom_data (org-wide fan-out via fetch_all_with_retry, always exactly one coverage row per input repo). - data/dependency_repo_map.yaml: the curated-override file, following affiliations.yaml's '# manual' convention but explicitly documented as a different *kind* of file -- corrections-only, not a comprehensive regenerated map. Empty by design until real resolution data exists. - measure_sbom_coverage.py: standalone script (not a registered pipeline yet -- that's gated on this measurement) to answer the actual open question: how many hiero-ledger repos have a readable SBOM at all. Bug caught before committing: the first working version of _parse_purl didn't percent-decode the namespace/name segment, so a scoped npm package like '@org/pkg' parsed back out as the literal '%40org/pkg'. Caught by testing against a realistic synthetic purl, not by inspection. No resolution heuristic, no chart, no pipeline registration yet -- those depend on what the coverage script finds when run against real hiero-ledger data. Deliberately small first slice, per the PR-split plan agreed in the design comment. Verified: 738 passed, 95.36% coverage, ruff clean. Data-source and fan-out logic verified interactively against realistic synthetic SBOM payloads (SPDX shape, documentDescribes root-exclusion, mixed ok/disabled/error repos) before writing the formal tests. Signed-off-by: iron-prog --- measure_sbom_coverage.py | 75 ++++++++++ .../data/dependency_repo_map.yaml | 28 ++++ .../data_sources/github_rest.py | 132 +++++++++++++++++- src/hiero_analytics/data_sources/models.py | 33 +++++ tests/data_sources/test_github_rest.py | 127 +++++++++++++++++ 5 files changed, 391 insertions(+), 4 deletions(-) create mode 100644 measure_sbom_coverage.py create mode 100644 src/hiero_analytics/data/dependency_repo_map.yaml diff --git a/measure_sbom_coverage.py b/measure_sbom_coverage.py new file mode 100644 index 000000000..f7c0517de --- /dev/null +++ b/measure_sbom_coverage.py @@ -0,0 +1,75 @@ +"""Measure real SBOM coverage for an org — the gating question on #338. + +Not a registered pipeline; this is throwaway measurement code to answer +"how many repos have a readable dependency-graph SBOM" before investing in +the resolution heuristic or the chart. Run directly: + + uv run python measure_sbom_coverage.py + +Requires GITHUB_TOKEN in the environment (same as any other pipeline run). +Prints a coverage breakdown and writes the raw per-repo results to +sbom_coverage_raw.csv for closer inspection. +""" + +from __future__ import annotations + +import csv + +from hiero_analytics.config.paths import ORG +from hiero_analytics.data_sources.github_client import GitHubClient +from hiero_analytics.data_sources.github_ingest import fetch_org_repos_graphql +from hiero_analytics.data_sources.github_rest import fetch_org_sbom_data + + +def main(org: str = ORG) -> None: + """Fetch SBOM coverage for every repo in ``org`` and print a summary.""" + client = GitHubClient() + + repos = fetch_org_repos_graphql(client, org) + if not repos: + print(f"No repositories found for org: {org}") + return + + repo_names = [r.name for r in repos] + print(f"Fetching SBOM data for {len(repo_names)} repos in {org}...") + + coverage, packages = fetch_org_sbom_data(client, org, repo_names) + + by_status: dict[str, list] = {} + for c in coverage: + by_status.setdefault(c.status, []).append(c) + + print() + print("=== Coverage summary ===") + for status in ("ok", "disabled", "error"): + rows = by_status.get(status, []) + print(f"{status:>10}: {len(rows)} repos") + + ok_rows = by_status.get("ok", []) + nonzero = [c for c in ok_rows if c.package_count > 0] + zero = [c for c in ok_rows if c.package_count == 0] + print() + print(f"Of the {len(ok_rows)} repos with a readable SBOM:") + print(f" {len(nonzero)} have at least one dependency ({sum(c.package_count for c in nonzero)} packages total)") + print(f" {len(zero)} have a readable but empty manifest") + + print() + print(f"Total dependency edges (all ecosystems, unresolved to org repos yet): {len(packages)}") + ecosystems: dict[str, int] = {} + for p in packages: + ecosystems[p.ecosystem] = ecosystems.get(p.ecosystem, 0) + 1 + for eco, n in sorted(ecosystems.items(), key=lambda kv: -kv[1]): + print(f" {eco}: {n}") + + with open("sbom_coverage_raw.csv", "w", newline="", encoding="utf-8") as f: + writer = csv.writer(f) + writer.writerow(["repo", "status", "package_count"]) + for c in sorted(coverage, key=lambda c: c.repo): + writer.writerow([c.repo, c.status, c.package_count]) + + print() + print("Wrote per-repo detail to sbom_coverage_raw.csv") + + +if __name__ == "__main__": + main() diff --git a/src/hiero_analytics/data/dependency_repo_map.yaml b/src/hiero_analytics/data/dependency_repo_map.yaml new file mode 100644 index 000000000..c634fd915 --- /dev/null +++ b/src/hiero_analytics/data/dependency_repo_map.yaml @@ -0,0 +1,28 @@ +# Package -> org-repo overrides for the dependency network chart (#338). +# +# Different in kind from affiliations.yaml, despite the shared '# manual' +# convention: affiliations.yaml is a comprehensively regenerated mapping of +# every known login. This file is corrections-only. The normalization +# heuristics in analysis/dependency_network.py resolve the overwhelming +# majority of packages on their own; nothing needs an entry here unless the +# heuristic gets it wrong. +# +# Two kinds of entry: +# package: "repo-name" -- the heuristic missed a real intra-org dependency +# (e.g. a published package name that doesn't +# resemble its source repo's name at all). +# package: null -- a false-positive suppression: a package whose +# name coincidentally resembles an org repo but +# is NOT actually published by this org (a public +# npm/PyPI/crates.io package with a similar name). +# Without this, the resolver would wrongly draw +# an intra-org edge. +# +# Key format: "ecosystem:package_name" (as parsed from the purl -- see +# DependencyManifestRecord), lowercased. Append '# manual' to any entry you +# hand-correct, same as affiliations.yaml, so it's visible and won't be +# silently overwritten if this file ever grows a regeneration step. +# +# Empty by design at PR-1 time -- no real hiero-ledger SBOM data has been +# resolved against yet. Populate once the coverage/resolution numbers from +# the initial run show which packages the heuristic actually misses. diff --git a/src/hiero_analytics/data_sources/github_rest.py b/src/hiero_analytics/data_sources/github_rest.py index eff98fb57..74a9c6372 100644 --- a/src/hiero_analytics/data_sources/github_rest.py +++ b/src/hiero_analytics/data_sources/github_rest.py @@ -1,7 +1,8 @@ -"""GitHub REST helpers for CODEOWNERS presence and Actions-runner classification. +"""GitHub REST helpers for CODEOWNERS presence, Actions-runner classification, and dependency-graph SBOM fetching. -CODEOWNERS existence checks and workflow-YAML runner scanning via the REST API -(the GraphQL ingestion lives in ``github_ingest``). +CODEOWNERS existence checks, workflow-YAML runner scanning, and per-repo SBOM +package lists, all via the REST API (the GraphQL ingestion lives in +``github_ingest``). """ from __future__ import annotations @@ -9,13 +10,14 @@ import base64 import logging import re +from urllib.parse import unquote import requests import yaml from .github_client import GitHubClient from .github_ingest._common import fetch_all_with_retry -from .models import RunnerRecord +from .models import DependencyManifestRecord, RunnerRecord, SbomCoverageRecord logger = logging.getLogger(__name__) @@ -60,6 +62,89 @@ def has_codeowners_file(client: GitHubClient, org: str, repo: str) -> bool: return False +def _parse_purl(purl: str) -> tuple[str, str, str | None] | None: + """Parse a package URL (purl) into (ecosystem, name, version). + + Format: ``pkg:type/namespace/name@version`` or ``pkg:type/name@version`` + (namespace optional; npm scopes and Maven groupIds arrive as the + namespace segment, percent-encoded per the purl spec — e.g. an npm scope + is ``%40scope``, not ``@scope``). Qualifiers (``?...``) and subpath + (``#...``) are dropped — irrelevant for repo resolution. Returns + ``None`` for anything that doesn't parse as ``pkg:...`` rather than + raising, since a single malformed purl shouldn't fail the whole repo's + SBOM. + """ + if not purl.startswith("pkg:"): + return None + body = purl[len("pkg:") :].split("?", 1)[0].split("#", 1)[0] + if "/" not in body: + return None + ecosystem, rest = body.split("/", 1) + name_and_version, _, version = rest.rpartition("@") + if not name_and_version: + # No '@version' segment at all -- treat the whole remainder as the name. + name_and_version, version = rest, None + return ecosystem.lower(), unquote(name_and_version), unquote(version) if version else None + + +def fetch_repo_sbom( + client: GitHubClient, org: str, repo: str +) -> tuple[SbomCoverageRecord, list[DependencyManifestRecord]]: + """Fetch and parse one repo's dependency-graph SBOM. + + Only a 404 or 403 counts as "dependency graph disabled" (status + ``"disabled"``, empty package list) — mirroring ``has_codeowners_file``'s + exact treatment, since GitHub returns 404/403 for repos where the + dependency graph feature is off, not for genuine failures. Any other + error propagates as status ``"error"`` with the repo's coverage row + still returned (never silently dropped), so a network/permission issue + reads as "we don't know," not "this repo has no dependencies." + + NOTE: package parsing is based on the documented SPDX/purl shape of the + dependency-graph SBOM endpoint; verify against a real response before + relying on this for production coverage numbers — see the design note + on #338. + """ + url = f"https://api.github.com/repos/{org}/{repo}/dependency-graph/sbom" + try: + payload = client.get(url) + except requests.HTTPError as exc: + if exc.response is not None and exc.response.status_code in (403, 404): + logger.debug("Dependency graph disabled for %s", repo) + return SbomCoverageRecord(repo=repo, status="disabled", package_count=0), [] + logger.error("SBOM fetch failed for %s: %s", repo, exc) + return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + except requests.RequestException as exc: + logger.error("SBOM fetch failed for %s: %s", repo, exc) + return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + + sbom = (payload or {}).get("sbom") or {} + packages = sbom.get("packages") or [] + # The document's own "described" package(s) represent the repo itself, + # not a dependency -- exclude by SPDXID via the SPDX-standard + # documentDescribes list rather than guessing from the name. + described_ids = set(sbom.get("documentDescribes") or []) + + records: list[DependencyManifestRecord] = [] + for pkg in packages: + if not isinstance(pkg, dict) or pkg.get("SPDXID") in described_ids: + continue + purls = [ + ref.get("referenceLocator", "") + for ref in (pkg.get("externalRefs") or []) + if isinstance(ref, dict) and ref.get("referenceType") == "purl" + ] + parsed = next((p for purl in purls if (p := _parse_purl(purl)) is not None), None) + if parsed is None: + continue + ecosystem, package_name, version = parsed + records.append( + DependencyManifestRecord(repo=repo, package_name=package_name, ecosystem=ecosystem, version=version) + ) + + return SbomCoverageRecord(repo=repo, status="ok", package_count=len(records)), records + + def _is_self_hosted(label: str) -> bool | None: """ Determines if a runner is self-hosted. @@ -166,3 +251,42 @@ def fetch_repo_workflows(client: GitHubClient, org: str, repo: str) -> list[Runn task_desc=f"workflow files ({repo})", describe=lambda wf: str(wf.get("name", "?")), ) + + +# Concurrency for the org-wide SBOM fan-out (one call per repo). +_SBOM_FETCH_WORKERS = 8 + + +def fetch_org_sbom_data( + client: GitHubClient, + org: str, + repo_names: list[str], + max_workers: int = _SBOM_FETCH_WORKERS, +) -> tuple[list[SbomCoverageRecord], list[DependencyManifestRecord]]: + """Fetch SBOM coverage + packages for every repo in ``repo_names``. + + Returns ``(coverage, packages)`` — always exactly one coverage row per + input repo (``fetch_repo_sbom`` never raises; disabled/error states are + returned, not thrown), and zero or more package rows per repo. + + ``fetch_all_with_retry`` expects a flat per-item list, so each repo's + ``(coverage, packages)`` pair is flattened into one list (coverage row + first) and split back apart here by type after the fan-out completes — + simpler than teaching the shared retry helper a second return shape for + one caller. + """ + + def per_repo(repo: str) -> list[SbomCoverageRecord | DependencyManifestRecord]: + coverage, packages = fetch_repo_sbom(client, org, repo) + return [coverage, *packages] + + combined = fetch_all_with_retry( + repo_names, + max_workers, + per_repo, + task_desc="SBOM data", + describe=str, + ) + coverage = [r for r in combined if isinstance(r, SbomCoverageRecord)] + packages = [r for r in combined if isinstance(r, DependencyManifestRecord)] + return coverage, packages diff --git a/src/hiero_analytics/data_sources/models.py b/src/hiero_analytics/data_sources/models.py index 7ffb804bb..09989e8f9 100644 --- a/src/hiero_analytics/data_sources/models.py +++ b/src/hiero_analytics/data_sources/models.py @@ -442,6 +442,39 @@ class ScorecardRecord: date: datetime +@dataclass(frozen=True) +class DependencyManifestRecord: + """One SBOM package entry for a repo, before org-repo resolution. + + ``package_name`` and ``ecosystem`` are parsed from the package URL + (purl) in the SBOM's ``externalRefs``, not the raw SPDX ``name`` field — + purl gives a consistent ecosystem+name split across npm/PyPI/Maven/ + Cargo/Go, where the raw name field's shape varies per ecosystem. + """ + + repo: str + package_name: str + ecosystem: str + version: str | None + + +@dataclass(frozen=True) +class SbomCoverageRecord: + """Per-repo SBOM fetch outcome — kept distinct from the package list itself. + + ``status`` is one of ``"ok"`` (SBOM available, packages parsed — + ``package_count`` may still be 0 for a repo with a genuinely empty + manifest), ``"disabled"`` (dependency graph off for this repo, or the + endpoint 404/403'd), or ``"error"`` (fetch failed for a reason other + than disablement; surfaced rather than silently dropped). This is what + makes "no edges" distinguishable from "no data" downstream. + """ + + repo: str + status: str + package_count: int + + @dataclass(frozen=True) class CodeOwnersRecord: """Represents the presence of a CODEOWNERS file in a repository.""" diff --git a/tests/data_sources/test_github_rest.py b/tests/data_sources/test_github_rest.py index 63a6aedb8..0c841a4b1 100644 --- a/tests/data_sources/test_github_rest.py +++ b/tests/data_sources/test_github_rest.py @@ -15,10 +15,14 @@ from hiero_analytics.data_sources.github_rest import ( _is_self_hosted, + _parse_purl, _process_workflow_file, + fetch_org_sbom_data, + fetch_repo_sbom, fetch_repo_workflows, has_codeowners_file, ) +from hiero_analytics.data_sources.models import DependencyManifestRecord, SbomCoverageRecord def _http_error(status_code: int) -> requests.HTTPError: @@ -141,3 +145,126 @@ def test_fetch_repo_workflows_scans_only_yaml_files(): assert [r.workflow_file for r in records] == ["ci.yml"] assert records[0].is_self_hosted is True + + +# -- _parse_purl -------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("purl", "expected"), + [ + ("pkg:npm/left-pad@1.0.1", ("npm", "left-pad", "1.0.1")), + ("pkg:npm/%40scope/pkg@2.0.0", ("npm", "@scope/pkg", "2.0.0")), + ("pkg:maven/com.example/thing@2.0", ("maven", "com.example/thing", "2.0")), + ("pkg:pypi/requests@2.31.0", ("pypi", "requests", "2.31.0")), + ("pkg:cargo/serde@1.0.0?extra=1", ("cargo", "serde", "1.0.0")), # qualifiers dropped + ("pkg:golang/github.com/org/mod@v1.2.3", ("golang", "github.com/org/mod", "v1.2.3")), + ("pkg:npm/no-version", ("npm", "no-version", None)), # no '@version' segment at all + ], +) +def test_parse_purl_handles_each_ecosystem_shape(purl, expected): + """Each ecosystem's purl shape parses to (ecosystem, name, version), scope/groupId decoded.""" + assert _parse_purl(purl) == expected + + +@pytest.mark.parametrize("malformed", ["not-a-purl", "pkg:", "pkg:npm-no-slash"]) +def test_parse_purl_returns_none_for_malformed_input(malformed): + """Malformed input returns None rather than raising -- one bad purl shouldn't fail a repo's SBOM.""" + assert _parse_purl(malformed) is None + + +# -- fetch_repo_sbom: the 404/403-vs-error contract, and SBOM parsing --------- + + +def test_fetch_repo_sbom_parses_packages_and_excludes_the_described_root(): + """Packages parse to DependencyManifestRecord; the repo's own root package is excluded.""" + client = Mock() + client.get.return_value = { + "sbom": { + "documentDescribes": ["SPDXRef-root"], + "packages": [ + {"SPDXID": "SPDXRef-root", "name": "org/repo"}, # the repo itself -- must be excluded + { + "SPDXID": "SPDXRef-1", + "name": "lodash", + "externalRefs": [{"referenceType": "purl", "referenceLocator": "pkg:npm/lodash@4.17.21"}], + }, + ], + } + } + + coverage, records = fetch_repo_sbom(client, "org", "repo") + + assert coverage == SbomCoverageRecord(repo="repo", status="ok", package_count=1) + assert records == [DependencyManifestRecord(repo="repo", package_name="lodash", ecosystem="npm", version="4.17.21")] + + +def test_fetch_repo_sbom_skips_packages_without_a_parseable_purl(): + """A package with no purl external ref is skipped, not fabricated from the raw name.""" + client = Mock() + client.get.return_value = { + "sbom": { + "documentDescribes": [], + "packages": [{"SPDXID": "SPDXRef-1", "name": "mystery-pkg", "externalRefs": []}], + } + } + + coverage, records = fetch_repo_sbom(client, "org", "repo") + + assert records == [] + assert coverage.status == "ok" + assert coverage.package_count == 0 + + +@pytest.mark.parametrize("status_code", [403, 404]) +def test_fetch_repo_sbom_treats_403_and_404_as_disabled(status_code): + """Both 403 and 404 mean 'dependency graph off for this repo', not an error.""" + client = Mock() + client.get.side_effect = _http_error(status_code) + + coverage, records = fetch_repo_sbom(client, "org", "repo") + + assert coverage == SbomCoverageRecord(repo="repo", status="disabled", package_count=0) + assert records == [] + + +def test_fetch_repo_sbom_reports_other_errors_as_error_status_not_disabled(): + """A genuine failure (5xx, auth, etc.) is status='error', distinguishable from 'disabled'.""" + client = Mock() + client.get.side_effect = _http_error(500) + + coverage, records = fetch_repo_sbom(client, "org", "repo") + + assert coverage.status == "error" + assert records == [] + + +# -- fetch_org_sbom_data: the org-wide fan-out -------------------------------- + + +def test_fetch_org_sbom_data_returns_one_coverage_row_per_repo(): + """Every input repo gets exactly one coverage row, regardless of outcome.""" + + def fake_get(url): + if "repo-a" in url: + return { + "sbom": { + "documentDescribes": [], + "packages": [ + { + "SPDXID": "x", + "externalRefs": [{"referenceType": "purl", "referenceLocator": "pkg:npm/left-pad@1.0.0"}], + } + ], + } + } + raise _http_error(404) + + client = Mock() + client.get.side_effect = fake_get + + coverage, packages = fetch_org_sbom_data(client, "org", ["repo-a", "repo-b"], max_workers=2) + + assert {c.repo for c in coverage} == {"repo-a", "repo-b"} + assert {c.repo: c.status for c in coverage} == {"repo-a": "ok", "repo-b": "disabled"} + assert [p.repo for p in packages] == ["repo-a"] From 049f2991fb360bfe718f527700bc7fdfa7fcb8b2 Mon Sep 17 00:00:00 2001 From: iron-prog Date: Mon, 17 Aug 2026 12:40:07 +0530 Subject: [PATCH 2/6] style(deps): shorten SBOM fetch comments Signed-off-by: iron-prog --- .../data_sources/github_rest.py | 67 +++++++------------ 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/src/hiero_analytics/data_sources/github_rest.py b/src/hiero_analytics/data_sources/github_rest.py index 74a9c6372..3464c2ce6 100644 --- a/src/hiero_analytics/data_sources/github_rest.py +++ b/src/hiero_analytics/data_sources/github_rest.py @@ -15,6 +15,7 @@ import requests import yaml +from .dataset_store import PartialOrgFetchError from .github_client import GitHubClient from .github_ingest._common import fetch_all_with_retry from .models import DependencyManifestRecord, RunnerRecord, SbomCoverageRecord @@ -90,33 +91,13 @@ def _parse_purl(purl: str) -> tuple[str, str, str | None] | None: def fetch_repo_sbom( client: GitHubClient, org: str, repo: str ) -> tuple[SbomCoverageRecord, list[DependencyManifestRecord]]: - """Fetch and parse one repo's dependency-graph SBOM. + """Fetch and parse one repository's dependency-graph SBOM. - Only a 404 or 403 counts as "dependency graph disabled" (status - ``"disabled"``, empty package list) — mirroring ``has_codeowners_file``'s - exact treatment, since GitHub returns 404/403 for repos where the - dependency graph feature is off, not for genuine failures. Any other - error propagates as status ``"error"`` with the repo's coverage row - still returned (never silently dropped), so a network/permission issue - reads as "we don't know," not "this repo has no dependencies." - - NOTE: package parsing is based on the documented SPDX/purl shape of the - dependency-graph SBOM endpoint; verify against a real response before - relying on this for production coverage numbers — see the design note - on #338. + Treat 403/404 as disabled and other failures as errors so missing data + is not confused with an empty dependency set. """ url = f"https://api.github.com/repos/{org}/{repo}/dependency-graph/sbom" - try: - payload = client.get(url) - except requests.HTTPError as exc: - if exc.response is not None and exc.response.status_code in (403, 404): - logger.debug("Dependency graph disabled for %s", repo) - return SbomCoverageRecord(repo=repo, status="disabled", package_count=0), [] - logger.error("SBOM fetch failed for %s: %s", repo, exc) - return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] - except requests.RequestException as exc: - logger.error("SBOM fetch failed for %s: %s", repo, exc) - return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + payload = client.get(url) sbom = (payload or {}).get("sbom") or {} packages = sbom.get("packages") or [] @@ -263,30 +244,30 @@ def fetch_org_sbom_data( repo_names: list[str], max_workers: int = _SBOM_FETCH_WORKERS, ) -> tuple[list[SbomCoverageRecord], list[DependencyManifestRecord]]: - """Fetch SBOM coverage + packages for every repo in ``repo_names``. - - Returns ``(coverage, packages)`` — always exactly one coverage row per - input repo (``fetch_repo_sbom`` never raises; disabled/error states are - returned, not thrown), and zero or more package rows per repo. + """Fetch and parse one repo's dependency-graph SBOM. - ``fetch_all_with_retry`` expects a flat per-item list, so each repo's - ``(coverage, packages)`` pair is flattened into one list (coverage row - first) and split back apart here by type after the fan-out completes — - simpler than teaching the shared retry helper a second return shape for - one caller. + HTTP failures propagate so the org-wide retry layer can retry transient + failures. Final failures are recorded as ``error`` by the org fan-out; + they must not be classified as ``disabled`` from the status code alone. """ - def per_repo(repo: str) -> list[SbomCoverageRecord | DependencyManifestRecord]: coverage, packages = fetch_repo_sbom(client, org, repo) return [coverage, *packages] - - combined = fetch_all_with_retry( - repo_names, - max_workers, - per_repo, - task_desc="SBOM data", - describe=str, - ) + + try: + combined = fetch_all_with_retry( + repo_names, + max_workers, + per_repo, + task_desc="SBOM data", + describe=str, + ) + except PartialOrgFetchError as exc: + combined = list(exc.records) + combined.extend( + SbomCoverageRecord(repo=repo, status="error", package_count=0) + for repo in exc.failed_repos + ) coverage = [r for r in combined if isinstance(r, SbomCoverageRecord)] packages = [r for r in combined if isinstance(r, DependencyManifestRecord)] return coverage, packages From 51fe1f931e45dc9114e6e2ab06ecad1bca00f6a3 Mon Sep 17 00:00:00 2001 From: iron-prog Date: Fri, 21 Aug 2026 00:18:30 +0530 Subject: [PATCH 3/6] fix: address PR review feedback Signed-off-by: iron-prog --- .../data_sources/github_rest.py | 27 +++++++++++++------ src/hiero_analytics/data_sources/models.py | 5 ++-- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/hiero_analytics/data_sources/github_rest.py b/src/hiero_analytics/data_sources/github_rest.py index 3464c2ce6..7ae741c62 100644 --- a/src/hiero_analytics/data_sources/github_rest.py +++ b/src/hiero_analytics/data_sources/github_rest.py @@ -93,11 +93,19 @@ def fetch_repo_sbom( ) -> tuple[SbomCoverageRecord, list[DependencyManifestRecord]]: """Fetch and parse one repository's dependency-graph SBOM. - Treat 403/404 as disabled and other failures as errors so missing data - is not confused with an empty dependency set. + Treat 403/404 as ``"disabled"`` (dependency graph off for this repo) and + any other HTTP failure as ``"error"``, so missing data is never confused + with an empty dependency set. Non-HTTP request failures (timeouts, + connection errors) still propagate so the org-wide fan-out in + ``fetch_org_sbom_data`` can retry them. """ url = f"https://api.github.com/repos/{org}/{repo}/dependency-graph/sbom" - payload = client.get(url) + try: + payload = client.get(url) + except requests.HTTPError as exc: + status_code = exc.response.status_code if exc.response is not None else None + status = "disabled" if status_code in (403, 404) else "error" + return SbomCoverageRecord(repo=repo, status=status, package_count=0), [] sbom = (payload or {}).get("sbom") or {} packages = sbom.get("packages") or [] @@ -244,11 +252,14 @@ def fetch_org_sbom_data( repo_names: list[str], max_workers: int = _SBOM_FETCH_WORKERS, ) -> tuple[list[SbomCoverageRecord], list[DependencyManifestRecord]]: - """Fetch and parse one repo's dependency-graph SBOM. - - HTTP failures propagate so the org-wide retry layer can retry transient - failures. Final failures are recorded as ``error`` by the org fan-out; - they must not be classified as ``disabled`` from the status code alone. + """Fetch and parse dependency-graph SBOMs for every repo in ``repo_names``. + + Fans ``fetch_repo_sbom`` out across the org with retry. ``fetch_repo_sbom`` + already turns 403/404 into ``"disabled"`` and any other HTTP failure into + ``"error"`` for that repo, so those never raise here. Non-HTTP failures + (timeouts, connection errors) do propagate, and this layer retries them; + a repo that is still failing after retries gets an ``"error"`` coverage + row from the fan-out itself instead of being silently dropped. """ def per_repo(repo: str) -> list[SbomCoverageRecord | DependencyManifestRecord]: coverage, packages = fetch_repo_sbom(client, org, repo) diff --git a/src/hiero_analytics/data_sources/models.py b/src/hiero_analytics/data_sources/models.py index 09989e8f9..96762c49b 100644 --- a/src/hiero_analytics/data_sources/models.py +++ b/src/hiero_analytics/data_sources/models.py @@ -12,6 +12,7 @@ from collections.abc import Mapping from dataclasses import dataclass from datetime import datetime +from typing import Literal from hiero_analytics.domain.bots import is_bot_login from hiero_analytics.domain.hip_references import extract_hip_mentions @@ -455,7 +456,7 @@ class DependencyManifestRecord: repo: str package_name: str ecosystem: str - version: str | None + version: str | None = None @dataclass(frozen=True) @@ -471,7 +472,7 @@ class SbomCoverageRecord: """ repo: str - status: str + status: Literal["ok", "disabled", "error"] package_count: int From ac144ef743173f448a1d821c09db66dd201beb6f Mon Sep 17 00:00:00 2001 From: iron-prog Date: Sun, 6 Sep 2026 11:55:41 +0530 Subject: [PATCH 4/6] fix: only 404 means SBOM disabled; validate SBOM schema before ok Signed-off-by: iron-prog --- .../data_sources/github_rest.py | 39 ++++++++++-------- tests/data_sources/test_github_rest.py | 40 +++++++++++++++---- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/src/hiero_analytics/data_sources/github_rest.py b/src/hiero_analytics/data_sources/github_rest.py index 7ae741c62..ab846e77b 100644 --- a/src/hiero_analytics/data_sources/github_rest.py +++ b/src/hiero_analytics/data_sources/github_rest.py @@ -93,25 +93,32 @@ def fetch_repo_sbom( ) -> tuple[SbomCoverageRecord, list[DependencyManifestRecord]]: """Fetch and parse one repository's dependency-graph SBOM. - Treat 403/404 as ``"disabled"`` (dependency graph off for this repo) and - any other HTTP failure as ``"error"``, so missing data is never confused - with an empty dependency set. Non-HTTP request failures (timeouts, - connection errors) still propagate so the org-wide fan-out in - ``fetch_org_sbom_data`` can retry them. + Only a 404 counts as ``"disabled"`` (dependency graph off for this repo) — + the same 404-vs-error contract ``has_codeowners_file``/``fetch_repo_workflows`` + use. A 403 is ambiguous (rate limiting, an insufficiently-scoped token, a + private repo) rather than an unambiguous disabled-state signal, so it is + reported as ``"error"`` like any other HTTP failure, so the org-wide + fan-out in ``fetch_org_sbom_data`` can retry it instead of a transient or + permission failure being silently misclassified as "no dependency graph". + Non-HTTP request failures (timeouts, connection errors) still propagate + so ``fetch_org_sbom_data`` can retry them too. """ url = f"https://api.github.com/repos/{org}/{repo}/dependency-graph/sbom" try: payload = client.get(url) except requests.HTTPError as exc: status_code = exc.response.status_code if exc.response is not None else None - status = "disabled" if status_code in (403, 404) else "error" + status = "disabled" if status_code == 404 else "error" return SbomCoverageRecord(repo=repo, status=status, package_count=0), [] - sbom = (payload or {}).get("sbom") or {} - packages = sbom.get("packages") or [] - # The document's own "described" package(s) represent the repo itself, - # not a dependency -- exclude by SPDXID via the SPDX-standard - # documentDescribes list rather than guessing from the name. + sbom = payload.get("sbom") if isinstance(payload, dict) else None + if not isinstance(sbom, dict): + return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + + raw_packages = sbom.get("packages") + if raw_packages is not None and not isinstance(raw_packages, list): + return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + packages = raw_packages or [] described_ids = set(sbom.get("documentDescribes") or []) records: list[DependencyManifestRecord] = [] @@ -261,10 +268,11 @@ def fetch_org_sbom_data( a repo that is still failing after retries gets an ``"error"`` coverage row from the fan-out itself instead of being silently dropped. """ + def per_repo(repo: str) -> list[SbomCoverageRecord | DependencyManifestRecord]: coverage, packages = fetch_repo_sbom(client, org, repo) return [coverage, *packages] - + try: combined = fetch_all_with_retry( repo_names, @@ -272,13 +280,10 @@ def per_repo(repo: str) -> list[SbomCoverageRecord | DependencyManifestRecord]: per_repo, task_desc="SBOM data", describe=str, - ) + ) except PartialOrgFetchError as exc: combined = list(exc.records) - combined.extend( - SbomCoverageRecord(repo=repo, status="error", package_count=0) - for repo in exc.failed_repos - ) + combined.extend(SbomCoverageRecord(repo=repo, status="error", package_count=0) for repo in exc.failed_repos) coverage = [r for r in combined if isinstance(r, SbomCoverageRecord)] packages = [r for r in combined if isinstance(r, DependencyManifestRecord)] return coverage, packages diff --git a/tests/data_sources/test_github_rest.py b/tests/data_sources/test_github_rest.py index 0c841a4b1..5294dfcc1 100644 --- a/tests/data_sources/test_github_rest.py +++ b/tests/data_sources/test_github_rest.py @@ -173,7 +173,7 @@ def test_parse_purl_returns_none_for_malformed_input(malformed): assert _parse_purl(malformed) is None -# -- fetch_repo_sbom: the 404/403-vs-error contract, and SBOM parsing --------- +# -- fetch_repo_sbom: the 404-vs-error contract, and SBOM parsing ------------ def test_fetch_repo_sbom_parses_packages_and_excludes_the_described_root(): @@ -216,11 +216,10 @@ def test_fetch_repo_sbom_skips_packages_without_a_parseable_purl(): assert coverage.package_count == 0 -@pytest.mark.parametrize("status_code", [403, 404]) -def test_fetch_repo_sbom_treats_403_and_404_as_disabled(status_code): - """Both 403 and 404 mean 'dependency graph off for this repo', not an error.""" +def test_fetch_repo_sbom_treats_404_as_disabled(): + """404 is the only unambiguous 'dependency graph off for this repo' signal.""" client = Mock() - client.get.side_effect = _http_error(status_code) + client.get.side_effect = _http_error(404) coverage, records = fetch_repo_sbom(client, "org", "repo") @@ -228,10 +227,11 @@ def test_fetch_repo_sbom_treats_403_and_404_as_disabled(status_code): assert records == [] -def test_fetch_repo_sbom_reports_other_errors_as_error_status_not_disabled(): - """A genuine failure (5xx, auth, etc.) is status='error', distinguishable from 'disabled'.""" +@pytest.mark.parametrize("status_code", [403, 500]) +def test_fetch_repo_sbom_reports_403_and_other_errors_as_error_status_not_disabled(status_code): + """403 is ambiguous (rate limit, scope, private repo) -- must be retryable 'error', not 'disabled'.""" client = Mock() - client.get.side_effect = _http_error(500) + client.get.side_effect = _http_error(status_code) coverage, records = fetch_repo_sbom(client, "org", "repo") @@ -239,6 +239,30 @@ def test_fetch_repo_sbom_reports_other_errors_as_error_status_not_disabled(): assert records == [] +@pytest.mark.parametrize("malformed_sbom", [{"sbom": "not-an-object"}, {"sbom": None}, "not-an-object", None]) +def test_fetch_repo_sbom_reports_error_when_sbom_is_not_an_object(malformed_sbom): + """A malformed payload/sbom must not be reported as an available SBOM with zero packages.""" + client = Mock() + client.get.return_value = malformed_sbom + + coverage, records = fetch_repo_sbom(client, "org", "repo") + + assert coverage == SbomCoverageRecord(repo="repo", status="error", package_count=0) + assert records == [] + + +@pytest.mark.parametrize("malformed_packages", [{"not": "a list"}, "not-a-list", 5]) +def test_fetch_repo_sbom_reports_error_when_packages_is_not_a_list(malformed_packages): + """'packages' present but the wrong type must not silently resolve to an empty manifest.""" + client = Mock() + client.get.return_value = {"sbom": {"documentDescribes": [], "packages": malformed_packages}} + + coverage, records = fetch_repo_sbom(client, "org", "repo") + + assert coverage == SbomCoverageRecord(repo="repo", status="error", package_count=0) + assert records == [] + + # -- fetch_org_sbom_data: the org-wide fan-out -------------------------------- From ed55e512ba3da55908060ef7680c6601a6a0804e Mon Sep 17 00:00:00 2001 From: iron-prog Date: Thu, 24 Sep 2026 18:07:04 +0530 Subject: [PATCH 5/6] fix(deps): address SBOM review feedback Signed-off-by: iron-prog --- .../measure_sbom_coverage.py | 0 .../data/dependency_repo_map.yaml | 28 --------- .../data_sources/github_rest.py | 62 +++++++++++++++---- src/hiero_analytics/data_sources/models.py | 4 +- tests/data_sources/test_github_rest.py | 58 +++++++++++++++-- 5 files changed, 104 insertions(+), 48 deletions(-) rename measure_sbom_coverage.py => scripts/measure_sbom_coverage.py (100%) delete mode 100644 src/hiero_analytics/data/dependency_repo_map.yaml diff --git a/measure_sbom_coverage.py b/scripts/measure_sbom_coverage.py similarity index 100% rename from measure_sbom_coverage.py rename to scripts/measure_sbom_coverage.py diff --git a/src/hiero_analytics/data/dependency_repo_map.yaml b/src/hiero_analytics/data/dependency_repo_map.yaml deleted file mode 100644 index c634fd915..000000000 --- a/src/hiero_analytics/data/dependency_repo_map.yaml +++ /dev/null @@ -1,28 +0,0 @@ -# Package -> org-repo overrides for the dependency network chart (#338). -# -# Different in kind from affiliations.yaml, despite the shared '# manual' -# convention: affiliations.yaml is a comprehensively regenerated mapping of -# every known login. This file is corrections-only. The normalization -# heuristics in analysis/dependency_network.py resolve the overwhelming -# majority of packages on their own; nothing needs an entry here unless the -# heuristic gets it wrong. -# -# Two kinds of entry: -# package: "repo-name" -- the heuristic missed a real intra-org dependency -# (e.g. a published package name that doesn't -# resemble its source repo's name at all). -# package: null -- a false-positive suppression: a package whose -# name coincidentally resembles an org repo but -# is NOT actually published by this org (a public -# npm/PyPI/crates.io package with a similar name). -# Without this, the resolver would wrongly draw -# an intra-org edge. -# -# Key format: "ecosystem:package_name" (as parsed from the purl -- see -# DependencyManifestRecord), lowercased. Append '# manual' to any entry you -# hand-correct, same as affiliations.yaml, so it's visible and won't be -# silently overwritten if this file ever grows a regeneration step. -# -# Empty by design at PR-1 time -- no real hiero-ledger SBOM data has been -# resolved against yet. Populate once the coverage/resolution numbers from -# the initial run show which packages the heuristic actually misses. diff --git a/src/hiero_analytics/data_sources/github_rest.py b/src/hiero_analytics/data_sources/github_rest.py index ab846e77b..3c765cca3 100644 --- a/src/hiero_analytics/data_sources/github_rest.py +++ b/src/hiero_analytics/data_sources/github_rest.py @@ -104,20 +104,50 @@ def fetch_repo_sbom( so ``fetch_org_sbom_data`` can retry them too. """ url = f"https://api.github.com/repos/{org}/{repo}/dependency-graph/sbom" + logger.info("Fetching SBOM for %s/%s", org, repo) + try: payload = client.get(url) except requests.HTTPError as exc: status_code = exc.response.status_code if exc.response is not None else None - status = "disabled" if status_code == 404 else "error" - return SbomCoverageRecord(repo=repo, status=status, package_count=0), [] + + if status_code == 404: + logger.info( + "SBOM unavailable for %s/%s (404)", + org, + repo, + ) + return SbomCoverageRecord( + repo=repo, + status="disabled", + package_count=0, + ), [] + + logger.warning( + "SBOM request failed for %s/%s with HTTP %s; propagating for retry", + org, + repo, + status_code, + ) + raise sbom = payload.get("sbom") if isinstance(payload, dict) else None if not isinstance(sbom, dict): - return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + logger.error("Malformed SBOM response for %s/%s", org, repo) + return SbomCoverageRecord( + repo=repo, + status="error", + package_count=0, + ), [] raw_packages = sbom.get("packages") if raw_packages is not None and not isinstance(raw_packages, list): - return SbomCoverageRecord(repo=repo, status="error", package_count=0), [] + logger.error("Malformed SBOM package list for %s/%s", org, repo) + return SbomCoverageRecord( + repo=repo, + status="error", + package_count=0, + ), [] packages = raw_packages or [] described_ids = set(sbom.get("documentDescribes") or []) @@ -138,8 +168,17 @@ def fetch_repo_sbom( DependencyManifestRecord(repo=repo, package_name=package_name, ecosystem=ecosystem, version=version) ) - return SbomCoverageRecord(repo=repo, status="ok", package_count=len(records)), records - + logger.info( + "SBOM fetched for %s/%s: %d dependency packages", + org, + repo, + len(records), + ) + return SbomCoverageRecord( + repo=repo, + status="ok", + package_count=len(records), + ), records def _is_self_hosted(label: str) -> bool | None: """ @@ -261,12 +300,11 @@ def fetch_org_sbom_data( ) -> tuple[list[SbomCoverageRecord], list[DependencyManifestRecord]]: """Fetch and parse dependency-graph SBOMs for every repo in ``repo_names``. - Fans ``fetch_repo_sbom`` out across the org with retry. ``fetch_repo_sbom`` - already turns 403/404 into ``"disabled"`` and any other HTTP failure into - ``"error"`` for that repo, so those never raise here. Non-HTTP failures - (timeouts, connection errors) do propagate, and this layer retries them; - a repo that is still failing after retries gets an ``"error"`` coverage - row from the fan-out itself instead of being silently dropped. + Fans ``fetch_repo_sbom`` out across the organization with retry handling. + A 404 is treated as an unavailable dependency graph. Other HTTP and + network failures propagate through the fan-out so transient failures can + be retried. Repositories that still fail after the retry are represented + as ``"error"`` coverage rows rather than being silently dropped. """ def per_repo(repo: str) -> list[SbomCoverageRecord | DependencyManifestRecord]: diff --git a/src/hiero_analytics/data_sources/models.py b/src/hiero_analytics/data_sources/models.py index 96762c49b..7a677d736 100644 --- a/src/hiero_analytics/data_sources/models.py +++ b/src/hiero_analytics/data_sources/models.py @@ -465,8 +465,8 @@ class SbomCoverageRecord: ``status`` is one of ``"ok"`` (SBOM available, packages parsed — ``package_count`` may still be 0 for a repo with a genuinely empty - manifest), ``"disabled"`` (dependency graph off for this repo, or the - endpoint 404/403'd), or ``"error"`` (fetch failed for a reason other + manifest), ``"disabled"`` (dependency graph unavilable for this repo, or the + endpoint 404 response), or ``"error"`` (fetch failed for a reason other than disablement; surfaced rather than silently dropped). This is what makes "no edges" distinguishable from "no data" downstream. """ diff --git a/tests/data_sources/test_github_rest.py b/tests/data_sources/test_github_rest.py index 5294dfcc1..0f80e70e4 100644 --- a/tests/data_sources/test_github_rest.py +++ b/tests/data_sources/test_github_rest.py @@ -228,15 +228,13 @@ def test_fetch_repo_sbom_treats_404_as_disabled(): @pytest.mark.parametrize("status_code", [403, 500]) -def test_fetch_repo_sbom_reports_403_and_other_errors_as_error_status_not_disabled(status_code): - """403 is ambiguous (rate limit, scope, private repo) -- must be retryable 'error', not 'disabled'.""" +def test_fetch_repo_sbom_propagates_http_errors(status_code): + """HTTP failures must propagate so the org-level retry can handle them.""" client = Mock() client.get.side_effect = _http_error(status_code) - coverage, records = fetch_repo_sbom(client, "org", "repo") - - assert coverage.status == "error" - assert records == [] + with pytest.raises(requests.HTTPError): + fetch_repo_sbom(client, "org", "repo") @pytest.mark.parametrize("malformed_sbom", [{"sbom": "not-an-object"}, {"sbom": None}, "not-an-object", None]) @@ -292,3 +290,51 @@ def fake_get(url): assert {c.repo for c in coverage} == {"repo-a", "repo-b"} assert {c.repo: c.status for c in coverage} == {"repo-a": "ok", "repo-b": "disabled"} assert [p.repo for p in packages] == ["repo-a"] + + +def test_fetch_org_sbom_data_retries_http_failures(): + """HTTP failures from a repo fetch reach the org-level retry mechanism.""" + calls = {"repo-a": 0} + + def fake_get(url): + if "repo-a" in url: + calls["repo-a"] += 1 + + if calls["repo-a"] == 1: + raise _http_error(500) + + return { + "sbom": { + "documentDescribes": [], + "packages": [ + { + "SPDXID": "x", + "externalRefs": [ + { + "referenceType": "purl", + "referenceLocator": "pkg:npm/left-pad@1.0.0", + } + ], + } + ], + } + } + + raise _http_error(404) + + client = Mock() + client.get.side_effect = fake_get + + coverage, packages = fetch_org_sbom_data( + client, + "org", + ["repo-a", "repo-b"], + max_workers=1, + ) + + assert calls["repo-a"] == 2 + assert {c.repo: c.status for c in coverage} == { + "repo-a": "ok", + "repo-b": "disabled", + } + assert len(packages) == 1 \ No newline at end of file From 6a76efe16922fc9c5141f366ff9929f21ac04694 Mon Sep 17 00:00:00 2001 From: iron-prog Date: Fri, 25 Sep 2026 21:36:39 +0530 Subject: [PATCH 6/6] lint fixes Signed-off-by: iron-prog --- src/hiero_analytics/data_sources/github_rest.py | 1 + tests/data_sources/test_github_rest.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hiero_analytics/data_sources/github_rest.py b/src/hiero_analytics/data_sources/github_rest.py index 3c765cca3..35685f6e0 100644 --- a/src/hiero_analytics/data_sources/github_rest.py +++ b/src/hiero_analytics/data_sources/github_rest.py @@ -180,6 +180,7 @@ def fetch_repo_sbom( package_count=len(records), ), records + def _is_self_hosted(label: str) -> bool | None: """ Determines if a runner is self-hosted. diff --git a/tests/data_sources/test_github_rest.py b/tests/data_sources/test_github_rest.py index 0f80e70e4..b6cf78df2 100644 --- a/tests/data_sources/test_github_rest.py +++ b/tests/data_sources/test_github_rest.py @@ -337,4 +337,4 @@ def fake_get(url): "repo-a": "ok", "repo-b": "disabled", } - assert len(packages) == 1 \ No newline at end of file + assert len(packages) == 1