diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ae7996..a1e9dc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). without changing the centrally managed review-agent credential contract. ### Security +- Canonicalize the public manifest writer's optional `forbidden_root` before any + output-parent creation or output-path access. Missing, non-directory, + symlinked, unresolvable, or otherwise noncanonical roots now fail with one + stable non-leaking error, and every pre-write, descriptor-bound, and post-sync + containment check reuses the same resolved directory authority. - Revalidate the complete canonical evidence set and the closed owner-only manifest after publication but before reporting success. A second independent bounded evidence pass must reproduce the exact strict manifest bytes, while a diff --git a/docs/sealed-release-evidence.md b/docs/sealed-release-evidence.md index e5ad7e9..bbba050 100644 --- a/docs/sealed-release-evidence.md +++ b/docs/sealed-release-evidence.md @@ -128,6 +128,22 @@ any path whose lexical absolute form differs from its strict filesystem-resolved form. This makes the directory authority used for payload verification identical to the root excluded from manifest output. +Direct public API callers using +`write_evidence_manifest(..., forbidden_root=...)` have the same fail-closed +precondition. The optional root must be one existing real directory reached +through a lexical path with no symbolic-link component. Each named lexical +component is inspected before parent traversal is normalized, so an intermediate +symbolic link cannot be hidden by a later `..`; a `..` segment through only real +non-symlink components remains valid when the resulting canonical directory +satisfies the same root contract. Validation finishes before the output parent is +created or the output path is touched; a missing, non-directory, symlinked, +unresolvable, or otherwise noncanonical value raises the stable error +`evidence manifest forbidden root is missing or unsafe`. The writer stores the +resulting canonical path once and reuses that exact authority for the pre-open, +descriptor-bound, and post-`fsync` containment checks. The CLI already supplies +its previously canonicalized evidence root, so this public-API guard does not +broaden or weaken the command-line contract. + The evidence directory should already be sealed against concurrent writes by the build system or artifact service. Descriptor and repeated-digest checks are a fail-closed verification boundary, not a substitute for immutable storage or an diff --git a/src/egressweave/release_evidence.py b/src/egressweave/release_evidence.py index 7f3365d..3c4e016 100644 --- a/src/egressweave/release_evidence.py +++ b/src/egressweave/release_evidence.py @@ -626,20 +626,54 @@ def _require_output_outside_verified_set( raise SystemExit("evidence manifest output must remain outside the verified set") +def _require_canonical_forbidden_root(forbidden_root: Path) -> Path: + """Return one canonical excluded directory or raise one stable public error. + + Every named lexical component is inspected before any ``..`` normalization, + so a symbolic link cannot be hidden by later parent traversal. Public callers + receive one dedicated non-leaking failure, then the existing canonical-root + contract supplies the single authority reused by the writer afterward. + """ + try: + current = Path(forbidden_root.anchor) + for component in forbidden_root.parts[len(current.parts) :]: + current /= component + if component != ".." and current.is_symlink(): + raise SystemExit( + "release evidence directory path must not traverse symlinks" + ) + return _require_canonical_evidence_root(forbidden_root) + except SystemExit: + raise SystemExit( + "evidence manifest forbidden root is missing or unsafe" + ) from None + + def write_evidence_manifest( manifest: dict[str, Any], output_path: Path, *, forbidden_root: Path | None = None, ) -> None: - """Create one private manifest and optionally exclude one verified directory.""" + """Create one private manifest while optionally excluding one real directory. + + When supplied, ``forbidden_root`` must name an existing real directory through + a lexical path with no symbolic-link component. The writer validates it before + creating the output parent, stores the canonical result once, and reuses that + same authority for every pre-write, descriptor-bound, and post-sync check. + """ payload = _encode_evidence_manifest(manifest) + canonical_forbidden_root = ( + _require_canonical_forbidden_root(forbidden_root) + if forbidden_root is not None + else None + ) try: output_path.parent.mkdir(parents=True, exist_ok=True) except OSError as error: raise SystemExit("evidence manifest parent directory is unavailable") from error - if forbidden_root is not None: - _require_output_outside_verified_set(output_path, forbidden_root) + if canonical_forbidden_root is not None: + _require_output_outside_verified_set(output_path, canonical_forbidden_root) try: with open(output_path, "xb", opener=_open_exclusive_manifest) as stream: @@ -648,8 +682,11 @@ def write_evidence_manifest( stream, label="evidence manifest output", ) - if forbidden_root is not None: - _require_output_outside_verified_set(output_path, forbidden_root) + if canonical_forbidden_root is not None: + _require_output_outside_verified_set( + output_path, + canonical_forbidden_root, + ) stream.write(payload) stream.flush() os.fsync(stream.fileno()) @@ -658,8 +695,11 @@ def write_evidence_manifest( stream, label="evidence manifest output", ) - if forbidden_root is not None: - _require_output_outside_verified_set(output_path, forbidden_root) + if canonical_forbidden_root is not None: + _require_output_outside_verified_set( + output_path, + canonical_forbidden_root, + ) except FileExistsError: raise SystemExit("evidence manifest output already exists") from None except OSError as error: @@ -738,4 +778,4 @@ def main() -> int: if __name__ == "__main__": - raise SystemExit(main()) + raise SystemExit(main()) \ No newline at end of file diff --git a/tests/test_sealed_release_evidence_forbidden_root.py b/tests/test_sealed_release_evidence_forbidden_root.py new file mode 100644 index 0000000..3144da6 --- /dev/null +++ b/tests/test_sealed_release_evidence_forbidden_root.py @@ -0,0 +1,167 @@ +"""Regression tests for the public manifest writer's forbidden-root boundary.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from test_sealed_release_evidence_output_boundary import MANIFEST + +from egressweave import release_evidence + + +def test_public_writer_rejects_symlinked_forbidden_root(tmp_path: Path) -> None: + """Reject a symlink alias before writing inside its canonical target.""" + real_evidence_root = tmp_path / "real-evidence" + real_evidence_root.mkdir() + forbidden_root_alias = tmp_path / "evidence-alias" + try: + forbidden_root_alias.symlink_to(real_evidence_root, target_is_directory=True) + except OSError: + pytest.skip("directory symbolic links are unavailable on this platform") + output_path = real_evidence_root / "manifest.json" + + with pytest.raises(SystemExit, match="missing or unsafe"): + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=forbidden_root_alias, + ) + + assert not output_path.exists() + + +def test_public_writer_rejects_intermediate_symlink_erased_by_parent_traversal( + tmp_path: Path, +) -> None: + """Reject a lexical symlink component even when a later ``..`` hides it.""" + real_evidence_root = tmp_path / "real-evidence" + child = real_evidence_root / "child" + child.mkdir(parents=True) + alias = real_evidence_root / "alias" + try: + alias.symlink_to(child, target_is_directory=True) + except OSError: + pytest.skip("directory symbolic links are unavailable on this platform") + forbidden_root = alias / ".." + output_path = tmp_path / "new-parent" / "manifest.json" + + with pytest.raises(SystemExit, match="missing or unsafe"): + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=forbidden_root, + ) + + assert not output_path.parent.exists() + + +def test_public_writer_accepts_real_forbidden_root_outside_output( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Keep the documented safe-root success path and all three checks working.""" + forbidden_root = tmp_path / "real-evidence" + forbidden_root.mkdir() + output_path = tmp_path / "manifest-parent" / "manifest.json" + original_check = release_evidence._require_output_outside_verified_set + observed_roots: list[Path] = [] + + def record_containment_check(path: Path, verified_root: Path) -> None: + observed_roots.append(verified_root) + original_check(path, verified_root) + + monkeypatch.setattr( + release_evidence, + "_require_output_outside_verified_set", + record_containment_check, + ) + + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=forbidden_root, + ) + + assert json.loads(output_path.read_text(encoding="utf-8")) == MANIFEST + assert observed_roots == [forbidden_root.resolve()] * 3 + + +def test_public_writer_accepts_real_parent_traversal_without_symlinks( + tmp_path: Path, +) -> None: + """Permit lexical parent traversal when every named component is a real path.""" + forbidden_root = tmp_path / "real-evidence" + child = forbidden_root / "child" + child.mkdir(parents=True) + output_path = tmp_path / "manifest-parent" / "manifest.json" + + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=child / "..", + ) + + assert json.loads(output_path.read_text(encoding="utf-8")) == MANIFEST + + +def test_public_writer_rejects_missing_forbidden_root_before_parent_creation( + tmp_path: Path, +) -> None: + """Reject a missing exclusion root before creating the output directory.""" + output_path = tmp_path / "new-parent" / "manifest.json" + + with pytest.raises(SystemExit, match="missing or unsafe"): + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=tmp_path / "missing-evidence", + ) + + assert not output_path.parent.exists() + + +def test_public_writer_rejects_file_forbidden_root_before_parent_creation( + tmp_path: Path, +) -> None: + """Reject a non-directory exclusion root before creating output storage.""" + forbidden_root = tmp_path / "not-a-directory" + forbidden_root.write_text("not an evidence directory", encoding="utf-8") + output_path = tmp_path / "new-parent" / "manifest.json" + + with pytest.raises(SystemExit, match="missing or unsafe"): + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=forbidden_root, + ) + + assert not output_path.parent.exists() + + +def test_public_writer_normalizes_forbidden_root_resolution_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Normalize a canonicalization failure before creating output storage.""" + forbidden_root = tmp_path / "evidence" + forbidden_root.mkdir() + output_path = tmp_path / "new-parent" / "manifest.json" + original_resolve = Path.resolve + + def fail_forbidden_root(path: Path, *args, **kwargs): + if path == forbidden_root: + raise OSError("blocked") + return original_resolve(path, *args, **kwargs) + + monkeypatch.setattr(Path, "resolve", fail_forbidden_root) + + with pytest.raises(SystemExit, match="missing or unsafe"): + release_evidence.write_evidence_manifest( + MANIFEST, + output_path, + forbidden_root=forbidden_root, + ) + + assert not output_path.parent.exists() diff --git a/tests/test_sealed_release_evidence_resource_limits.py b/tests/test_sealed_release_evidence_resource_limits.py index 541bd43..fed3f38 100644 --- a/tests/test_sealed_release_evidence_resource_limits.py +++ b/tests/test_sealed_release_evidence_resource_limits.py @@ -72,12 +72,19 @@ def fail_open(path: Path, *args, **kwargs): ) -def test_deeply_nested_json_is_masked_by_the_strict_evidence_boundary( +def test_json_parser_recursion_failure_is_masked_by_the_strict_evidence_boundary( tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, ) -> None: """Normalize parser recursion failure instead of leaking an exception.""" - sbom = tmp_path / "deep.cdx.json" - sbom.write_text("[" * 10_000 + "0" + "]" * 10_000, encoding="utf-8") + sbom = tmp_path / "recursive.cdx.json" + sbom.write_text("{}", encoding="utf-8") + + def fail_with_recursion_error(*args: object, **kwargs: object) -> object: + """Model a parser recursion failure independent of interpreter depth.""" + raise RecursionError("synthetic parser recursion failure") + + monkeypatch.setattr(release_evidence.json, "loads", fail_with_recursion_error) with pytest.raises(SystemExit, match="not strict JSON"): release_evidence._load_strict_json(sbom)