fix(sbom): preserve Markdown report integrity #932
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # SBOM Markdown data-integrity boundary | ||
|
|
||
| ## Incident | ||
|
|
||
| The organization SBOM inventory treated repository names, component names, | ||
| versions, license expressions, API failure details, and the generation label as | ||
| trusted Markdown. Newlines, table delimiters, link brackets, code delimiters, | ||
| or raw-HTML delimiters in dependency metadata could therefore create forged | ||
| rows, headings, links, or presentation markup in the governance-facing report. | ||
| The machine-readable JSON remained structurally valid, but reviewers could be | ||
| misled by its human-readable companion. | ||
|
|
||
| ## Decision | ||
|
|
||
| The renderer now passes every externally derived string through one bounded | ||
| text encoder before interpolation. It collapses CR/LF line structure and emits | ||
| numeric or named character references for ampersands, backslashes, table pipes, | ||
| angle brackets, brackets, backticks, URI punctuation, mention markers, and issue | ||
| reference markers. Encoding dots, colons, at-signs, and number signs, plus | ||
| asterisks, underscores, and tildes, additionally prevents emphasis and | ||
| strikethrough presentation. The existing punctuation encoding prevents GFM | ||
| bare URLs, email autolinks, GitHub mentions, and issue references | ||
| from becoming active while browsers still render the intended text. Counts and | ||
| fixed policy labels remain native values. The JSON inventory deliberately | ||
| retains the original data so machine consumers and incident investigators do | ||
| not lose evidence. | ||
|
|
||
| This is a rendering-integrity control, not license verification. A component | ||
| with an unknown or policy-relevant license remains flagged by the existing | ||
| policy logic after its display text is neutralized. | ||
|
|
||
| The report summary also publishes `error_count` and an explicit completeness | ||
| state. Missing repository SBOM evidence therefore cannot be interpreted as a | ||
| clean zero-finding inventory merely because the unavailable repository has no | ||
| components in the roll-up. | ||
|
|
||
| Repository unavailability uses `error is not None` in both the JSON summary and | ||
| Markdown per-repository rendering. An empty error string is still unavailable | ||
| evidence rather than an empty repository, so the human and machine channels | ||
| cannot disagree about completeness. | ||
|
|
||
| ## Test-first evidence | ||
|
|
||
| `tests/test_sbom_markdown_integrity.py` first demonstrated that crafted SBOM | ||
| metadata produced a second-level heading and a forged table row. A follow-up | ||
| RED fixture proved that bare URLs, email addresses, mentions, and issue numbers | ||
| remained active without brackets. The accepted contract rejects active row, | ||
| heading, link, autolink, mention, issue-reference, and raw-HTML structure while | ||
| keeping the corresponding text visibly represented through character | ||
| references. | ||
|
|
||
| ## Failure, recovery, and rollback | ||
|
|
||
| Unexpected display text should be compared with `inventory.json`, which is the | ||
| lossless evidence channel. Rollback requires an independently reviewed renderer | ||
| that proves all externally derived strings remain text in every Markdown | ||
| context. Removing the encoder or escaping only table pipes is not acceptable | ||
| because headings, links, code spans, and raw HTML are separate parse surfaces. | ||
|
|
||
| ## APA 7th references | ||
|
|
||
| GitHub. (2019). *GitHub Flavored Markdown specification* (Version 0.29-gfm). | ||
| https://github.github.com/gfm/ | ||
|
|
||
| MacFarlane, J. (2024, January 28). *CommonMark specification* (Version 0.31.2). | ||
| https://spec.commonmark.org/0.31.2/ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| import argparse | ||
| import concurrent.futures | ||
| import json | ||
| import re | ||
| import subprocess | ||
| import sys | ||
| from dataclasses import dataclass, field | ||
|
|
@@ -51,6 +52,11 @@ | |
| # Sentinel emitted by SBOM tooling when it cannot determine a license. | ||
| NOASSERTION = "NOASSERTION" | ||
|
|
||
| _OWNER_LOGIN_PATTERN = re.compile( | ||
| r"[A-Za-z0-9](?:[A-Za-z0-9]|-(?=[A-Za-z0-9])){0,38}" | ||
| ) | ||
| _REPOSITORY_NAME_PATTERN = re.compile(r"[A-Za-z0-9._-]{1,100}") | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class Component: | ||
|
|
@@ -202,8 +208,10 @@ def build_inventory(repo_inventories: Sequence[RepoInventory]) -> dict[str, Any] | |
| license_totals: dict[str, int] = {} | ||
| flagged: list[dict[str, str]] = [] | ||
| total_components = 0 | ||
| error_count = 0 | ||
|
|
||
| for repo_inventory in sorted(repo_inventories, key=lambda r: r.repo.lower()): | ||
| error_count += int(repo_inventory.error is not None) | ||
| components_payload = [ | ||
| { | ||
| "name": component.name, | ||
|
|
@@ -242,6 +250,8 @@ def build_inventory(repo_inventories: Sequence[RepoInventory]) -> dict[str, Any] | |
| "repo_count": len(repos_payload), | ||
| "component_count": total_components, | ||
| "flagged_count": len(flagged), | ||
| "error_count": error_count, | ||
| "complete": error_count == 0, | ||
| "policy": "commercial-license-only", | ||
| }, | ||
| "license_totals": dict(sorted(license_totals.items())), | ||
|
|
@@ -250,13 +260,37 @@ def build_inventory(repo_inventories: Sequence[RepoInventory]) -> dict[str, Any] | |
| } | ||
|
|
||
|
|
||
| def _markdown_text(value: Any) -> str: | ||
| """Return untrusted inventory text without active Markdown structure.""" | ||
| text = str(value).replace("\r\n", " ").replace("\r", " ").replace("\n", " ") | ||
| replacements = { | ||
| "&": "&", | ||
| "\\": "\", | ||
| "|": "|", | ||
| "<": "<", | ||
| ">": ">", | ||
| "[": "[", | ||
| "]": "]", | ||
| "`": "`", | ||
| "*": "*", | ||
| "_": "_", | ||
| "~": "~", | ||
| ":": ":", | ||
| "@": "@", | ||
| "#": "#", | ||
| ".": ".", | ||
| "$": "$", | ||
| } | ||
|
Comment on lines
+263
to
+283
Contributor
There was a problem hiding this comment. 📝 Info: Version/license text is now entity-encoded in the Markdown report Because 
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| return "".join(replacements.get(character, character) for character in text) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| def render_inventory_markdown(inventory: dict[str, Any], *, generated_at: str) -> str: | ||
| """Render the consolidated inventory as a governance-facing markdown page.""" | ||
| summary = inventory["summary"] | ||
| lines: list[str] = [ | ||
| "# Organization SBOM inventory", | ||
| "", | ||
| f"Generated: {_markdown_text(generated_at)}", | ||
| "", | ||
| "One central view of every managed repository's software components,", | ||
| "versions, and licenses. Feeds license and vulnerability governance", | ||
|
|
@@ -268,6 +302,8 @@ def render_inventory_markdown(inventory: dict[str, Any], *, generated_at: str) - | |
| f"- Components: {summary['component_count']}", | ||
| f"- Policy: {summary['policy']}", | ||
| f"- Flagged licenses: {summary['flagged_count']}", | ||
| f"- SBOMs unavailable: {summary['error_count']}", | ||
| f"- Evidence completeness: {'complete' if summary['complete'] else 'incomplete'}", | ||
| "", | ||
| "## License roll-up", | ||
| "", | ||
|
|
@@ -276,7 +312,7 @@ def render_inventory_markdown(inventory: dict[str, Any], *, generated_at: str) - | |
| ] | ||
| for license_key, count in inventory["license_totals"].items(): | ||
| flag = " ⚠️" if is_flagged_license(license_key) else "" | ||
| lines.append(f"| {_markdown_text(license_key)}{flag} | {count} |") | ||
|
|
||
| lines.extend(["", "## Flagged components (policy violations)", ""]) | ||
| if inventory["flagged_licenses"]: | ||
|
|
@@ -288,17 +324,19 @@ def render_inventory_markdown(inventory: dict[str, Any], *, generated_at: str) - | |
| ) | ||
| for item in inventory["flagged_licenses"]: | ||
| lines.append( | ||
| f"| {_markdown_text(item['repo'])} | {_markdown_text(item['name'])} | " | ||
| f"{_markdown_text(item['version'] or '—')} | " | ||
| f"{_markdown_text(item['license'])} |" | ||
| ) | ||
| else: | ||
| lines.append("No copyleft or NOASSERTION components detected.") | ||
|
|
||
| lines.extend(["", "## Per-repository components", ""]) | ||
| for repo in inventory["repos"]: | ||
| lines.append(f"### {_markdown_text(repo['repo'])}") | ||
| lines.append("") | ||
| if repo["error"] is not None: | ||
| lines.append(f"SBOM unavailable: {_markdown_text(repo['error'])}") | ||
|
Comment on lines
+338
to
+339
Contributor
There was a problem hiding this comment. 📝 Info: Empty-error repo now treated as unavailable in both channels The render guard changed to
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| lines.append("") | ||
| continue | ||
| if not repo["components"]: | ||
|
|
@@ -311,19 +349,39 @@ def render_inventory_markdown(inventory: dict[str, Any], *, generated_at: str) - | |
| for component in repo["components"]: | ||
| flag = "yes" if component["flagged"] else "no" | ||
| lines.append( | ||
| f"| {_markdown_text(component['name'])} | " | ||
| f"{_markdown_text(component['version'] or '—')} | " | ||
| f"{_markdown_text(component['license'])} | {flag} |" | ||
| ) | ||
| lines.append("") | ||
| return "\n".join(lines).rstrip() + "\n" | ||
|
|
||
|
|
||
| def _resolve_output_dir(output_dir: Path, *, base_dir: Path | None = None) -> Path: | ||
| """Resolve an output directory while preventing writes outside the workspace.""" | ||
| base = (base_dir or Path.cwd()).resolve() | ||
| resolved = (base / output_dir if not output_dir.is_absolute() else output_dir).resolve() | ||
| try: | ||
| resolved.relative_to(base) | ||
| except ValueError as exc: | ||
| raise ValueError("output directory must stay within the workspace") from exc | ||
| return resolved | ||
|
Comment on lines
+360
to
+368
Contributor
There was a problem hiding this comment. 📝 Info: Output dir resolution fails closed for absolute paths outside cwd
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
|
|
||
| def write_inventory( | ||
| inventory: dict[str, Any], | ||
| markdown: str, | ||
| output_dir: Path, | ||
| *, | ||
| base_dir: Path | None = None, | ||
| ) -> None: | ||
| """Write inventory artifacts only inside the configured workspace.""" | ||
| safe_output_dir = _resolve_output_dir(output_dir, base_dir=base_dir) | ||
| safe_output_dir.mkdir(parents=True, exist_ok=True) | ||
| (safe_output_dir / "inventory.json").write_text( | ||
| json.dumps(inventory, indent=2, sort_keys=False) + "\n", encoding="utf-8" | ||
| ) | ||
| (safe_output_dir / "inventory.md").write_text(markdown, encoding="utf-8") | ||
|
|
||
|
|
||
| def _run(args: Sequence[str]) -> str: # pragma: no cover - thin subprocess wrapper | ||
|
|
@@ -332,26 +390,54 @@ def _run(args: Sequence[str]) -> str: # pragma: no cover - thin subprocess wrap | |
| return process.stdout | ||
|
|
||
|
|
||
| def _validate_owner_login(value: str) -> str: | ||
| """Return a canonical GitHub owner login or reject an unsafe operand.""" | ||
| if not isinstance(value, str) or _OWNER_LOGIN_PATTERN.fullmatch(value) is None: | ||
| raise ValueError("invalid GitHub organization login") | ||
| return value | ||
|
|
||
|
|
||
| def _validate_repo_full_name(value: str) -> str: | ||
| """Return a canonical ``owner/repository`` name or reject it.""" | ||
| if not isinstance(value, str) or value.count("/") != 1: | ||
| raise ValueError("invalid GitHub repository full name") | ||
| owner, repository = value.split("/", 1) | ||
| if ( | ||
| _OWNER_LOGIN_PATTERN.fullmatch(owner) is None | ||
| or _REPOSITORY_NAME_PATTERN.fullmatch(repository) is None | ||
| or repository in {".", ".."} | ||
| or repository.startswith("-") | ||
| ): | ||
| raise ValueError("invalid GitHub repository full name") | ||
| return value | ||
|
|
||
|
|
||
| def list_org_repos(org: str) -> list[str]: # pragma: no cover - network | ||
| """List non-archived repositories for an organization via gh.""" | ||
| validated_org = _validate_owner_login(org) | ||
| raw = _run( | ||
| [ | ||
| "gh", | ||
| "repo", | ||
| "list", | ||
| "--no-archived", | ||
| "--limit", | ||
| "500", | ||
| "--json", | ||
| "nameWithOwner", | ||
| "--", | ||
| validated_org, | ||
| ] | ||
| ) | ||
| return [ | ||
| _validate_repo_full_name(entry["nameWithOwner"]) | ||
| for entry in json.loads(raw or "[]") | ||
|
Comment on lines
431
to
+434
Contributor
There was a problem hiding this comment. 📝 Info: list_org_repos now fails closed on any unexpected repo name
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| ] | ||
|
|
||
|
|
||
| def fetch_repo_sbom(repo: str) -> RepoInventory: # pragma: no cover - network | ||
| """Fetch and parse one repository's dependency-graph SBOM via gh.""" | ||
| repo = _validate_repo_full_name(repo) | ||
| try: | ||
| raw = _run(["gh", "api", f"/repos/{repo}/dependency-graph/sbom"]) | ||
| except subprocess.CalledProcessError as exc: | ||
|
|
@@ -448,7 +534,11 @@ def main(argv: list[str]) -> int: # pragma: no cover - CLI orchestration | |
| if args.self_test: | ||
| self_test() | ||
| return 0 | ||
| repos = ( | ||
| [_validate_repo_full_name(repo) for repo in args.repos] | ||
| if args.repos | ||
| else list_org_repos(args.org) | ||
| ) | ||
| inventories = collect_inventories(repos) | ||
| inventory = build_inventory(inventories) | ||
| generated_at = args.generated_at or "unspecified" | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: Minor Markdown formatting quirks in ARCHITECTURE.md
The new
## SBOM Markdown integrityheading is placed with no blank line separating it from the preceding paragraph (ARCHITECTURE.md), and two consecutive blank lines were inserted between list items in the 'Related durable documents' list (ARCHITECTURE.md). CommonMark/GFM still renders the ATX heading (headings may interrupt paragraphs) and still renders the trailing bullet, so these are cosmetic only and do not break the rendered document.Was this helpful? React with 👍 or 👎 to provide feedback.