Merge pull request #230 from living-ip/fix/nosend-empty-directory-normalization

Normalize empty no-send release directories

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.

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
 
 

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")