From be002d67b048e51b9f3ee55ecd04f95b3c06bf89 Mon Sep 17 00:00:00 2001 From: fwazb Date: Wed, 22 Jul 2026 13:54:19 -0700 Subject: [PATCH] Normalize empty release context directories --- docs/workstreams/gcp-leoclean/goals.md | 47 ++-- ops/gcp_leoclean_nosend_package.py | 93 ++++++- tests/test_gcp_leoclean_nosend_package.py | 325 +++++++++++++++++++++- 3 files changed, 436 insertions(+), 29 deletions(-) diff --git a/docs/workstreams/gcp-leoclean/goals.md b/docs/workstreams/gcp-leoclean/goals.md index 97f0d1d..120baf9 100644 --- a/docs/workstreams/gcp-leoclean/goals.md +++ b/docs/workstreams/gcp-leoclean/goals.md @@ -24,23 +24,22 @@ until the GCP service has passed soak and restore testing. ## Current state -- Current main/base: `3569f4f2fb016e36f5ebeb27f01f3af91e1c360f`. -- PRs #220, #221, #226, #227, and #228 are merged. Main therefore contains the +- Current main/base: `f04956ce7c008009c284e1a779a359d20cd02f11`. +- PRs #220, #221, #226, #227, #228, and #229 are merged. Main therefore contains the source-first V3 rebuild foundation, the PostgreSQL large-object boundary, bounded Observatory prerequisite preservation, Cloud SQL provider-database residual handling, and the manual exact-revision no-send publication - workflow. -- A narrow current-main correction is in progress so rerunning - `scripts/kb_apply_prereqs.sql` preserves only the exact scoped runtime and - proposal-owner ACLs created by `ops/gcp_leoclean_runtime_role.sql` on the - prerequisite's protected table/column surface. Broader grants on that - surface and direct role membership continue to fail closed; the runtime - provisioner and verifier remain authoritative for role attributes, schema - privileges, routines, and the complete least-privilege posture. -- Local evidence for that correction: 230 focused tests passed; the - digest-pinned, network-disabled PostgreSQL 16.14 lifecycle canary passed; - two prerequisite reruns were stable; injected ACL and membership drift - failed closed; Ruff and `git diff --check` passed. + workflow plus the composable prerequisite/runtime ACL boundary. +- The exact-main GCP preflight passed for `f04956c`: the staging VM and private + PostgreSQL 16 Cloud SQL endpoint were reachable, the Artifact Registry + repository was immutable, and no public database attachment was present. +- Manual publication run #2 was bound to `f04956c` and failed closed before a + Docker build, tag, or push. The pinned Hermes archive contains one empty + `tinker-atropos/` directory; preparation copied it although the file-only + artifact manifest cannot bind it, so the exact directory allowlist rejected + the context. The current narrow correction removes only unmanifested empty + directories from the copied artifact, validates the complete context before + requesting GCP identity, and retains the later pre-push revalidation. - The GCP VM, private Cloud SQL instance, and scoped Secret Manager secret are known to exist, but no new current-main no-send image has yet been published by the merged #228 workflow. @@ -54,11 +53,10 @@ until the GCP service has passed soak and restore testing. ## Execution order -1. Review and merge the narrow PostgreSQL migration-order/composability - correction from current main. -2. Run the merged GCP preflight against that exact main revision, then manually - dispatch #228's publication workflow for the same revision. Verify the - resulting immutable image/config digests and publication receipt. +1. Review and merge the narrow copied-artifact empty-directory normalization. +2. Run the merged GCP preflight against the resulting exact main revision, then + manually dispatch the publication workflow for that same revision. Verify + the resulting immutable image/config digests and publication receipt. 3. Provision `leoclean_kb_runtime`, grant only the staging VM access to the scoped secret, and prove the Cloud SQL role's allowed reads and function-only proposal staging plus denied writes and escalation. @@ -91,8 +89,9 @@ until the GCP service has passed soak and restore testing. ## Human gates -The composability correction requires exact-revision database/security review -before merge. GCP role/IAM mutation, V3 canonical promotion, and Telegram -cutover remain distinct critical gates. Evidence from tests or agent review is -advisory; the exact revision and live receipts must be presented to the human -development lead at each gate. +The publication correction requires exact-revision package/release-CI review +before merge and a separate authorization before the manual workflow rerun. +GCP role/IAM mutation, V3 canonical promotion, and Telegram cutover remain +distinct critical gates. Evidence from tests or agent review is advisory; the +exact revision and live receipts must be presented to the human development +lead at each gate. diff --git a/ops/gcp_leoclean_nosend_package.py b/ops/gcp_leoclean_nosend_package.py index bfe2e48..4e1b02b 100644 --- a/ops/gcp_leoclean_nosend_package.py +++ b/ops/gcp_leoclean_nosend_package.py @@ -300,6 +300,79 @@ def _normalize_prepared_directory_modes(root: Path) -> None: path.chmod(0o700 if relative == "identity" else 0o755) +def _prune_empty_directories(root: Path) -> None: + """Remove directory entries that the file-only artifact manifest cannot bind.""" + + _safe_tree(root, "empty-directory pruning source") + directories = sorted( + (path for path in root.rglob("*") if path.is_dir()), + key=lambda path: len(path.relative_to(root).parts), + reverse=True, + ) + for path in directories: + try: + path.rmdir() + except OSError as exc: + if exc.errno in {errno.EEXIST, errno.ENOTEMPTY}: + continue + raise PackageError("cannot remove copied empty artifact directory") from exc + + +def _remove_prepared_context(root: Path, expected_identity: tuple[int, int]) -> None: + """Remove only the preparation directory created by this process.""" + + _require(root.is_absolute() and root.name not in {"", ".", ".."}, "prepared image context cleanup path is unsafe") + parent_descriptor = _open_private_operator_directory( + root.parent, + "prepared image context cleanup parent", + ) + cleanup_name = f".{root.name}.cleanup-{uuid.uuid4().hex}" + cleanup_root = root.with_name(cleanup_name) + try: + observed = _directory_entry_stat(parent_descriptor, root.name) + _require( + observed is not None + and stat.S_ISDIR(observed.st_mode) + and (observed.st_dev, observed.st_ino) == expected_identity, + "prepared image context cleanup refused to remove replaced output", + ) + _require( + _directory_entry_stat(parent_descriptor, cleanup_name) is None, + "prepared image context cleanup quarantine already exists", + ) + os.rename( + root.name, + cleanup_name, + src_dir_fd=parent_descriptor, + dst_dir_fd=parent_descriptor, + ) + quarantined = _directory_entry_stat(parent_descriptor, cleanup_name) + _require( + quarantined is not None + and stat.S_ISDIR(quarantined.st_mode) + and (quarantined.st_dev, quarantined.st_ino) == expected_identity, + "prepared image context cleanup quarantine identity drifted", + ) + cleanup_root.chmod(0o700) + _safe_tree(cleanup_root, "prepared image context cleanup") + _normalize_prepared_directory_modes(cleanup_root) + _safe_tree(cleanup_root, "normalized prepared image context cleanup") + quarantined = _directory_entry_stat(parent_descriptor, cleanup_name) + _require( + quarantined is not None + and (quarantined.st_dev, quarantined.st_ino) == expected_identity, + "prepared image context cleanup quarantine identity drifted", + ) + _require(shutil.rmtree.avoids_symlink_attacks, "prepared image context cleanup is not symlink safe") + shutil.rmtree(cleanup_name, dir_fd=parent_descriptor) + except OSError as exc: + raise PackageError("prepared image context cleanup failed") from exc + finally: + os.close(parent_descriptor) + _require(not cleanup_root.exists() and not cleanup_root.is_symlink(), "prepared image context cleanup did not pass") + _require(not root.exists() and not root.is_symlink(), "prepared image context cleanup detected replacement output") + + def tree_manifest(root: Path, *, excluded: frozenset[str] = frozenset()) -> dict[str, Any]: _safe_tree(root, "tree") files: list[dict[str, Any]] = [] @@ -1383,6 +1456,8 @@ def prepare_context( ) identity_manifest = load_json(identity / "identity-manifest.json", "identity manifest") output.mkdir(parents=True, mode=0o700) + output_stat = output.stat(follow_symlinks=False) + output_identity = (output_stat.st_dev, output_stat.st_ino) try: shutil.copytree(artifact, output / "artifact", symlinks=False) shutil.copytree(identity, output / "identity", symlinks=False) @@ -1420,10 +1495,20 @@ def prepare_context( }, ) _normalize_prepared_directory_modes(output) - _safe_tree(output, "prepared image context") - return image_input - except BaseException: - shutil.rmtree(output, ignore_errors=True) + _prune_empty_directories(output / "artifact") + prepared_input, _build_args = validate_prepared_image_context(output) + _require(prepared_input == image_input, "prepared image input changed during preparation") + validate_reviewed_context_source(repo_root, output, prepared_input) + return prepared_input + except BaseException as prepare_error: + try: + if output.exists() or output.is_symlink(): + _remove_prepared_context(output, output_identity) + except BaseException as cleanup_error: + raise PackageError( + "prepared image context failure cleanup did not pass after " + f"{type(prepare_error).__name__} ({prepare_error}): {cleanup_error}" + ) from cleanup_error raise diff --git a/tests/test_gcp_leoclean_nosend_package.py b/tests/test_gcp_leoclean_nosend_package.py index 79cc012..521fb99 100644 --- a/tests/test_gcp_leoclean_nosend_package.py +++ b/tests/test_gcp_leoclean_nosend_package.py @@ -531,6 +531,322 @@ def prepared_context_fixture(tmp_path: Path) -> tuple[Path, dict[str, object]]: return context, image_input +def test_prepare_prunes_unmanifested_empty_artifact_directories_only(tmp_path: Path) -> None: + artifact, receipt = artifact_fixture(tmp_path) + empty_leaf = artifact / "hermes-agent" / "tinker-atropos" / "nested-empty" + empty_leaf.mkdir(parents=True) + empty_leaf.parent.chmod(0o555) + source_tree = package.tree_manifest(artifact) + source_manifest = json.loads((artifact / "artifact-manifest.json").read_text(encoding="utf-8")) + identity, _contract = identity_fixture(tmp_path, artifact, receipt) + context = tmp_path / "prepared-context" + + image_input = package.prepare_context( + repo_root=ROOT, + artifact=artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=context, + source_revision=REVISION, + ) + + assert empty_leaf.is_dir() + assert empty_leaf.parent.stat().st_mode & 0o777 == 0o555 + assert package.tree_manifest(artifact) == source_tree + assert not (context / "artifact" / "hermes-agent" / "tinker-atropos").exists() + assert (context / "artifact" / "hermes-agent" / "uv.lock").is_file() + assert json.loads((context / "artifact" / "artifact-manifest.json").read_text(encoding="utf-8")) == ( + source_manifest + ) + observed, build_args = package.validate_prepared_image_context(context) + assert observed == image_input + assert build_args == json.loads((context / "build-args.json").read_text(encoding="utf-8")) + + clean_artifact = tmp_path / "artifact-without-empty-directories" + shutil.copytree(artifact, clean_artifact) + (clean_artifact / "hermes-agent" / "tinker-atropos").chmod(0o755) + (clean_artifact / "hermes-agent" / "tinker-atropos" / "nested-empty").rmdir() + (clean_artifact / "hermes-agent" / "tinker-atropos").rmdir() + clean_context = tmp_path / "prepared-clean-context" + clean_image_input = package.prepare_context( + repo_root=ROOT, + artifact=clean_artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=clean_context, + source_revision=REVISION, + ) + assert clean_image_input == image_input + + +@pytest.mark.parametrize("failure_stage", ["prepared_context", "reviewed_source"]) +def test_prepare_removes_partial_context_when_final_validation_fails( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + failure_stage: str, +) -> None: + artifact, receipt = artifact_fixture(tmp_path) + identity, _contract = identity_fixture(tmp_path, artifact, receipt) + context = tmp_path / "prepared-context" + source_tree = package.tree_manifest(artifact) + + def reject_prepared_context(_context: Path) -> tuple[dict[str, object], dict[str, str]]: + raise package.PackageError("injected final prepared-context validation failure") + + def reject_reviewed_source(_root: Path, _context: Path, _value: dict[str, object]) -> None: + raise package.PackageError("injected final reviewed-source validation failure") + + if failure_stage == "prepared_context": + monkeypatch.setattr(package, "validate_prepared_image_context", reject_prepared_context) + error = "injected final prepared-context validation failure" + else: + monkeypatch.setattr(package, "validate_reviewed_context_source", reject_reviewed_source) + error = "injected final reviewed-source validation failure" + + with pytest.raises(package.PackageError, match=error): + package.prepare_context( + repo_root=ROOT, + artifact=artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=context, + source_revision=REVISION, + ) + + assert not context.exists() + assert package.tree_manifest(artifact) == source_tree + + +def test_prepare_removes_read_only_partial_context_after_early_copy_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + artifact, receipt = artifact_fixture(tmp_path) + copied_parent = artifact / "hermes-agent" + payload = copied_parent / "uv.lock" + empty_parent = copied_parent / "tinker-atropos" + empty_leaf = empty_parent / "nested-empty" + empty_leaf.mkdir(parents=True) + empty_leaf.chmod(0o555) + empty_parent.chmod(0o555) + copied_parent.chmod(0o555) + source_tree = package.tree_manifest(artifact) + identity, _contract = identity_fixture(tmp_path, artifact, receipt) + context = tmp_path / "prepared-context" + original_copytree = package.shutil.copytree + + def fail_identity_copy( + source: Path, + destination: Path, + *args: object, + **kwargs: object, + ) -> Path: + if Path(destination).name == "identity": + raise OSError("injected identity copy failure") + return original_copytree(source, destination, *args, **kwargs) + + monkeypatch.setattr(package.shutil, "copytree", fail_identity_copy) + + with pytest.raises(OSError, match="injected identity copy failure"): + package.prepare_context( + repo_root=ROOT, + artifact=artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=context, + source_revision=REVISION, + ) + + assert not context.exists() + assert empty_leaf.is_dir() + assert 'name = "pyyaml"' in payload.read_text(encoding="utf-8") + assert copied_parent.stat().st_mode & 0o777 == 0o555 + assert empty_parent.stat().st_mode & 0o777 == 0o555 + assert empty_leaf.stat().st_mode & 0o777 == 0o555 + assert package.tree_manifest(artifact) == source_tree + + +@pytest.mark.parametrize("replacement_kind", ["symlink", "directory"]) +def test_prepare_cleanup_refuses_replaced_output_without_touching_victim( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + replacement_kind: str, +) -> None: + artifact, receipt = artifact_fixture(tmp_path) + identity, _contract = identity_fixture(tmp_path, artifact, receipt) + context = tmp_path / "prepared-context" + retained = tmp_path / "retained-original-context" + victim = tmp_path / "victim" + victim.mkdir() + victim_marker = victim / "marker.txt" + victim_marker.write_text("must survive\n", encoding="utf-8") + original_copytree = package.shutil.copytree + + def replace_before_failure( + source: Path, + destination: Path, + *args: object, + **kwargs: object, + ) -> Path: + if Path(destination).name == "identity": + context.rename(retained) + if replacement_kind == "symlink": + context.symlink_to(victim, target_is_directory=True) + else: + context.mkdir() + (context / "replacement-marker.txt").write_text("replacement\n", encoding="utf-8") + raise OSError("injected failure after output replacement") + return original_copytree(source, destination, *args, **kwargs) + + monkeypatch.setattr(package.shutil, "copytree", replace_before_failure) + + with pytest.raises( + package.PackageError, + match="cleanup refused to remove replaced output", + ): + package.prepare_context( + repo_root=ROOT, + artifact=artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=context, + source_revision=REVISION, + ) + + assert victim_marker.read_text(encoding="utf-8") == "must survive\n" + assert (retained / "artifact" / "bootstrap.py").is_file() + if replacement_kind == "symlink": + assert context.is_symlink() + else: + assert (context / "replacement-marker.txt").read_text(encoding="utf-8") == "replacement\n" + + +@pytest.mark.parametrize("cleanup_failure", ["normalize", "remove"]) +def test_prepare_cleanup_failure_is_never_swallowed( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + cleanup_failure: str, +) -> None: + artifact, receipt = artifact_fixture(tmp_path) + identity, _contract = identity_fixture(tmp_path, artifact, receipt) + context = tmp_path / "prepared-context" + original_copytree = package.shutil.copytree + + def fail_identity_copy( + source: Path, + destination: Path, + *args: object, + **kwargs: object, + ) -> Path: + if Path(destination).name == "identity": + raise OSError("injected identity copy failure") + return original_copytree(source, destination, *args, **kwargs) + + with monkeypatch.context() as patch: + patch.setattr(package.shutil, "copytree", fail_identity_copy) + if cleanup_failure == "normalize": + patch.setattr( + package, + "_normalize_prepared_directory_modes", + lambda _root: (_ for _ in ()).throw(OSError("injected cleanup normalization failure")), + ) + else: + def fail_cleanup_removal(_root: str, *, dir_fd: int) -> None: + raise OSError("injected cleanup removal failure") + + fail_cleanup_removal.avoids_symlink_attacks = True # type: ignore[attr-defined] + patch.setattr( + package.shutil, + "rmtree", + fail_cleanup_removal, + ) + + with pytest.raises( + package.PackageError, + match="prepared image context failure cleanup did not pass", + ) as raised: + package.prepare_context( + repo_root=ROOT, + artifact=artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=context, + source_revision=REVISION, + ) + + chain_messages: list[str] = [] + chained: BaseException | None = raised.value + while chained is not None and str(chained) not in chain_messages: + chain_messages.append(str(chained)) + chained = chained.__cause__ or chained.__context__ + assert "injected identity copy failure" in " | ".join(chain_messages) + expected_cleanup_error = ( + "injected cleanup normalization failure" + if cleanup_failure == "normalize" + else "injected cleanup removal failure" + ) + assert expected_cleanup_error in " | ".join(chain_messages) + + quarantines = list(tmp_path.glob(".prepared-context.cleanup-*")) + assert not context.exists() + assert len(quarantines) == 1 + package._normalize_prepared_directory_modes(quarantines[0]) + shutil.rmtree(quarantines[0]) + + +def test_prepare_cleanup_preserves_replacement_created_during_quarantined_removal( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + artifact, receipt = artifact_fixture(tmp_path) + identity, _contract = identity_fixture(tmp_path, artifact, receipt) + context = tmp_path / "prepared-context" + replacement_marker = context / "replacement-marker.txt" + original_copytree = package.shutil.copytree + original_rmtree = package.shutil.rmtree + + def fail_identity_copy( + source: Path, + destination: Path, + *args: object, + **kwargs: object, + ) -> Path: + if Path(destination).name == "identity": + raise OSError("injected identity copy failure") + return original_copytree(source, destination, *args, **kwargs) + + def create_replacement_then_remove_quarantine(root: str, *, dir_fd: int) -> None: + context.mkdir() + replacement_marker.write_text("must survive\n", encoding="utf-8") + original_rmtree(root, dir_fd=dir_fd) + + create_replacement_then_remove_quarantine.avoids_symlink_attacks = True # type: ignore[attr-defined] + monkeypatch.setattr(package.shutil, "copytree", fail_identity_copy) + monkeypatch.setattr(package.shutil, "rmtree", create_replacement_then_remove_quarantine) + + with pytest.raises( + package.PackageError, + match="cleanup detected replacement output", + ): + package.prepare_context( + repo_root=ROOT, + artifact=artifact, + receipt=receipt, + identity=identity, + identity_source_root=ROOT, + output=context, + source_revision=REVISION, + ) + + assert replacement_marker.read_text(encoding="utf-8") == "must survive\n" + + def mocked_build_push_docker( image_input: dict[str, object], *, @@ -1246,7 +1562,13 @@ def test_build_push_derives_receipt_and_retains_all_daemon_references( gcloud_binary, outcome_journal = publish_outcome_authority(tmp_path, image_input) fake_run, state, calls = mocked_build_push_docker(image_input) reviewed: list[tuple[Path, Path, dict[str, object]]] = [] - monkeypatch.setattr(package, "validate_prepared_image_context", lambda _context: (image_input, build_args)) + validated: list[Path] = [] + + def validate_context(observed_context: Path) -> tuple[dict[str, object], dict[str, str]]: + validated.append(observed_context) + return image_input, build_args + + monkeypatch.setattr(package, "validate_prepared_image_context", validate_context) monkeypatch.setattr( package, "validate_reviewed_context_source", @@ -1282,6 +1604,7 @@ def test_build_push_derives_receipt_and_retains_all_daemon_references( "action": "fresh_push", "observed_digest": f"sha256:{IMAGE_DIGEST}", } + assert validated == [context, context] assert reviewed == [(repo_root, context, image_input), (repo_root, context, image_input)] assert state == {"build": True, "candidate": True, "digests": True, "pushes": 1} build = next(operation for operation, _kwargs in calls if operation and operation[0] == "build")