fix(site-publish): close split migration boundaries

Authored-By: OpenAI (GPT-5) <noreply@openai.com>

README.md

@@ -133,10 +133,18 @@ any route's mutable objects change.
 Mutable default and override partitions receive their final cache policy before
 the matching prefix-scoped stale deletion, so publication never exposes a
 provisional cache policy or a pointer to a missing immutable target.
+Generated Ingress annotations retain each bucket's prior route and immutable paths. When a move
+places that retired prefix inside the new sync scope, only its declared
+immutable subtrees are excluded; a current-file collision fails publication.
+The bucket-keyed history rejects a protected-to-public transition even when the
+artifact is renamed; publishing that artifact publicly requires a new bucket.
+Legacy single-surface is public for this downgrade check. Removing or renaming
+an immutable rule preserves its prior URLs; current mutable content at one of
+those paths is rejected instead of replacing it.
 
 Artifact input directories must be pairwise disjoint after filesystem
 resolution. Publication stops before build or upload if one contains another or
-escapes the repository. Descendant symlinks are also rejected, preventing
+escapes the repository. Symlinked roots, components, and descendants are also rejected, preventing
 protected input from entering a public artifact through dereference. Split
 storage endpoints are pinned to Garage, and each website
 authority is derived from its bucket; a site cannot expose an arbitrary backend.

scripts/deploy.py

@@ -12,6 +12,8 @@ from pathlib import Path
 from urllib.error import HTTPError, URLError
 from urllib.request import Request, urlopen
 
+import yaml
+
 from utils import (
     NAMESPACE,
     clone_apps,
@@ -176,7 +178,37 @@ def publish_route_immutables(artifact, route, site_dir, credential_env_names=Non
             publish_immutable_rule(artifact, route, rule, html_dir, aws_env)
 
 
-def s3_sync(artifact, route, site_dir, credential_env_names=None):
+def retired_immutable_filters(artifact, route, html_dir, previous_contract):
+    """Protect immutable keys only when an old route falls inside the new scope."""
+    if not previous_contract:
+        return []
+    current_prefix = route["path"].strip("/")
+    previous_prefix = previous_contract["path"].strip("/")
+    if current_prefix:
+        marker = f"{current_prefix}/"
+        if previous_prefix == current_prefix:
+            previous_prefix = ""
+        elif not previous_prefix.startswith(marker):
+            return []
+        else:
+            previous_prefix = previous_prefix[len(marker):]
+    filters = []
+    current_immutable = {
+        rule["path"] for rule in artifact["cache_rules"] if _is_immutable(rule)
+    }
+    for immutable_path in previous_contract["immutable_paths"]:
+        retired_path = "/".join(part for part in (previous_prefix, immutable_path) if part)
+        collision = html_dir / retired_path
+        if (immutable_path not in current_immutable and collision.exists()
+                and any(path.is_file() for path in collision.rglob("*"))):
+            raise RuntimeError(
+                f"current artifact collides with retired immutable partition: {retired_path}"
+            )
+        filters.extend(("--exclude", f"{retired_path}/*"))
+    return filters
+
+
+def s3_sync(artifact, route, site_dir, credential_env_names=None, previous_contract=None):
     endpoint = artifact["s3_endpoint"]
     html_dir = site_dir / artifact["build_dir"]
     aws_env = publication_aws_env(artifact, credential_env_names)
@@ -204,12 +236,13 @@ def s3_sync(artifact, route, site_dir, credential_env_names=None):
     # so a fresh upload always carries the right MIME type.
     specific_paths = [rule["path"] for rule in artifact["cache_rules"] if rule["path"]]
     default_filters = [arg for path in specific_paths for arg in ("--exclude", f"{path}/*")]
+    retired_filters = retired_immutable_filters(artifact, route, html_dir, previous_contract)
     run(["aws", "--endpoint-url", endpoint, "s3", "cp", f"{html_dir}/", destination,
          "--recursive", "--only-show-errors", "--cache-control", default_cache,
-         *default_filters, *exclude_args], env=aws_env)
+         *default_filters, *retired_filters, *exclude_args], env=aws_env)
     run(["aws", "--endpoint-url", endpoint, "s3", "sync", f"{html_dir}/", destination,
          "--delete", "--only-show-errors", "--cache-control", default_cache,
-         *default_filters, *exclude_args], env=aws_env)
+         *default_filters, *retired_filters, *exclude_args], env=aws_env)
     for rule in artifact["cache_rules"]:
         if not rule["path"]:
             continue
@@ -284,7 +317,13 @@ def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg):
     for route in cfg["routes"]:
         artifact = artifact_by_name[route["artifact"]]
         resource_name = k8s_name(site_name) if cfg["compatibility"] else f"{k8s_name(site_name)}-{route['name']}"
-        routes.append({**route, "resource_name": resource_name, "artifact_config": artifact})
+        immutable_paths = [
+            rule["path"] for rule in artifact["cache_rules"] if _is_immutable(rule)
+        ]
+        routes.append({
+            **route, "resource_name": resource_name, "artifact_config": artifact,
+            "immutable_paths_json": json.dumps(immutable_paths, separators=(",", ":")),
+        })
     template_vars = {
         "site": site_name,
         "site_k8s": k8s_name(site_name),
@@ -297,6 +336,55 @@ def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg):
     render_templates(action_dir, template_vars, app_dir, manifests_dir)
 
 
+def previous_route_contracts(app_dir):
+    """Read bucket-keyed route history from generated Ingresses."""
+    contracts = {}
+    manifests = app_dir / "manifests"
+    if not manifests.exists():
+        return contracts
+    for path in sorted(manifests.glob("ingress*.yaml")):
+        document = yaml.safe_load(path.read_text()) or {}
+        annotations = document.get("metadata", {}).get("annotations", {})
+        artifact = annotations.get("site-publish.fritzlab.net/artifact")
+        access = annotations.get("site-publish.fritzlab.net/access")
+        bucket = annotations.get("site-publish.fritzlab.net/bucket")
+        immutable_paths_json = annotations.get("site-publish.fritzlab.net/immutable-paths")
+        route_path = annotations.get("site-publish.fritzlab.net/route-path")
+        values = (artifact, access, bucket, immutable_paths_json, route_path)
+        if all(value is None for value in values):
+            continue
+        if (not all(isinstance(value, str) for value in values)
+                or access not in {"public", "protected"} or not route_path.startswith("/")):
+            raise RuntimeError(f"invalid site-publish route history in {path}")
+        try:
+            immutable_paths = json.loads(immutable_paths_json)
+        except json.JSONDecodeError as exc:
+            raise RuntimeError(f"invalid site-publish route history in {path}") from exc
+        if not isinstance(immutable_paths, list) or any(not isinstance(item, str) for item in immutable_paths):
+            raise RuntimeError(f"invalid site-publish route history in {path}")
+        if bucket in contracts:
+            raise RuntimeError(f"duplicate site-publish route history for bucket {bucket}")
+        contracts[bucket] = {
+            "path": route_path, "access": access, "artifact": artifact,
+            "immutable_paths": immutable_paths,
+        }
+    return contracts
+
+
+def validate_route_migrations(cfg, previous_contracts):
+    artifacts = {artifact["name"]: artifact for artifact in cfg["artifacts"]}
+    for route in cfg["routes"]:
+        artifact = artifacts[route["artifact"]]
+        previous = previous_contracts.get(artifact["bucket"])
+        if (previous and previous["access"] == "protected"
+                and route["access"] in {"public", "legacy"}
+        ):
+            raise RuntimeError(
+                f"artifact {route['artifact']} cannot become public while reusing protected "
+                f"bucket {artifact['bucket']}"
+            )
+
+
 def deploy_static(site_name, site_dir, action_dir, token, cfg):
     artifact_by_name = {artifact["name"]: artifact for artifact in cfg["artifacts"]}
     credential_env_names = {
@@ -305,6 +393,11 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
     validate_publication_environment(cfg)
     for artifact in cfg["artifacts"]:
         validate_artifact_output(site_dir, artifact)
+    apps_dir = clone_apps(token)
+    app_dir = apps_dir / "sjc001" / "websites" / site_name
+    manifests_dir = app_dir / "manifests"
+    previous_contracts = previous_route_contracts(app_dir)
+    validate_route_migrations(cfg, previous_contracts)
     # Complete immutable work across the whole publication before any route's
     # mutable pointers can change. Partial immutable success is safe; mixing a
     # new route with an old route after a later immutable failure is not.
@@ -313,14 +406,13 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg):
             artifact_by_name[route["artifact"]], route, site_dir, credential_env_names,
         )
     for route in cfg["routes"]:
-        s3_sync(artifact_by_name[route["artifact"]], route, site_dir, credential_env_names)
+        s3_sync(
+            artifact_by_name[route["artifact"]], route, site_dir, credential_env_names,
+            previous_contracts.get(artifact_by_name[route["artifact"]]["bucket"]),
+        )
     if cfg["compatibility"]:
         ensure_bucket_aliases(site_name, cfg["aliases"], os.environ.get("GARAGE_ADMIN_TOKEN"))
 
-    apps_dir = clone_apps(token)
-    app_dir = apps_dir / "sjc001" / "websites" / site_name
-    manifests_dir = app_dir / "manifests"
-
     render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg)
 
     commit_and_push(apps_dir, f"Deploy {site_name}", token)

scripts/utils.py

@@ -432,7 +432,15 @@ def validate_artifact_inputs(site_dir, cfg):
     root = Path(site_dir).resolve()
     sources = []
     for artifact in cfg["artifacts"]:
-        source = (root / artifact["content_dir"]).resolve()
+        declared = root
+        for component in Path(artifact["content_dir"]).parts:
+            declared /= component
+            if declared.is_symlink():
+                raise ConfigError(
+                    f"artifact {artifact['name']} content_dir contains symlink component: "
+                    f"{declared.relative_to(root)}"
+                )
+        source = declared.resolve()
         if source != root and root not in source.parents:
             raise ConfigError(
                 f"artifact {artifact['name']} content_dir resolves outside the repository"

templates/ingress.yaml.j2

@@ -4,6 +4,13 @@ metadata:
   name: {{ route.resource_name }}
   namespace: {{ namespace }}
   annotations:
+{%- if not compatibility %}
+    site-publish.fritzlab.net/artifact: {{ route.artifact | tojson }}
+    site-publish.fritzlab.net/access: {{ route.access | tojson }}
+    site-publish.fritzlab.net/bucket: {{ route.artifact_config.bucket | tojson }}
+    site-publish.fritzlab.net/immutable-paths: {{ route.immutable_paths_json | tojson }}
+    site-publish.fritzlab.net/route-path: {{ route.path | tojson }}
+{%- endif %}
     traefik.ingress.kubernetes.io/router.middlewares: https-redirect@file,retry-upstream@file{% if route.access_middleware %},{{ route.access_middleware }}@file{% endif %}{% for m in route.middlewares %},{{ m }}@file{% endfor %}
 spec:
   ingressClassName: traefik

tests/test_contract.py

@@ -230,6 +230,16 @@ class ConfigContractTests(unittest.TestCase):
             with self.assertRaisesRegex(ConfigError, "build input contains symlink"):
                 validate_artifact_inputs(root, cfg)
 
+    def test_artifact_root_symlink_is_rejected_before_resolution(self):
+        cfg = normalize_site_config(self.raw, "baseline.fritzlab.net")
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            (root / "dist-real").mkdir()
+            (root / "dist").symlink_to(root / "dist-real")
+            (root / "portal" / "build").mkdir(parents=True)
+            with self.assertRaisesRegex(ConfigError, "content_dir contains symlink component"):
+                validate_artifact_inputs(root, cfg)
+
 
 class GenerationTests(unittest.TestCase):
     def render(self, raw):
@@ -263,6 +273,17 @@ class GenerationTests(unittest.TestCase):
         self.assertNotIn("passhostheader", files["manifests/ingress-portal.yaml"])
         self.assertIn("baseline-dist.web.sjc001.fritzlab.net", files["manifests/service-distributions.yaml"])
 
+    def test_yaml_ambiguous_artifact_name_stays_a_string_annotation(self):
+        raw = fixture("split-site.yaml")
+        raw["artifacts"][0]["name"] = "yes"
+        raw["routes"][0]["artifact"] = "yes"
+        tmp, _, files = self.render(raw)
+        self.addCleanup(tmp.cleanup)
+        ingress = yaml.safe_load(files["manifests/ingress-portal.yaml"])
+        self.assertEqual(
+            "yes", ingress["metadata"]["annotations"]["site-publish.fritzlab.net/artifact"],
+        )
+
     def test_generation_is_deterministic_when_input_lists_are_reversed(self):
         raw = fixture("split-site.yaml")
         first_tmp, _, first = self.render(raw)
@@ -286,12 +307,14 @@ class GenerationTests(unittest.TestCase):
             self.assertFalse(stale.exists())
 
     def test_legacy_names_and_garage_s3_target_are_preserved(self):
-        tmp, _, files = self.render(fixture("legacy-site.yaml"))
+        tmp, app_dir, files = self.render(fixture("legacy-site.yaml"))
         self.addCleanup(tmp.cleanup)
         self.assertIn("manifests/service.yaml", files)
         self.assertIn("manifests/ingress.yaml", files)
         self.assertIn("garage-s3.storage.svc.k8s.sjc001.fritzlab.net", files["manifests/service.yaml"])
         self.assertNotIn("passhostheader", files["manifests/ingress.yaml"])
+        self.assertNotIn("site-publish.fritzlab.net", files["manifests/ingress.yaml"])
+        self.assertEqual({}, deploy.previous_route_contracts(app_dir))
 
 
 class BuildTests(unittest.TestCase):
@@ -429,12 +452,93 @@ class PublishingTests(unittest.TestCase):
                 immutable_publish.call_args.args[2]["cache_control"],
             )
 
+    def test_root_move_preserves_only_actual_retired_immutable_prefix(self):
+        cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
+        artifact = next(item for item in cfg["artifacts"] if item["name"] == "distributions")
+        route = {**next(item for item in cfg["routes"] if item["artifact"] == "distributions"),
+                 "path": "/"}
+        with tempfile.TemporaryDirectory() as tmp:
+            root = Path(tmp)
+            html = root / artifact["build_dir"]
+            (html / "releases").mkdir(parents=True)
+            (html / "channels").mkdir()
+            (html / "docs" / "releases").mkdir(parents=True)
+            (html / "channels" / "stable.json").write_text("channel")
+            (html / "docs" / "releases" / "index.html").write_text("mutable")
+            commands = []
+            with patch.dict(os.environ, {
+                "DIST_S3_ACCESS_KEY": "dist-key", "DIST_S3_SECRET_KEY": "dist-secret"
+            }, clear=False), patch.object(
+                deploy, "run", side_effect=lambda command, **_: commands.append(command)
+            ):
+                deploy.s3_sync(artifact, route, root, previous_contract={
+                    "path": "/foo", "access": "public", "artifact": "distributions",
+                    "immutable_paths": ["releases"],
+                })
+            rendered = [" ".join(command) for command in commands]
+            self.assertTrue(all("foo/releases/*" in command for command in rendered[:2]))
+            self.assertTrue(all("*/releases/*" not in command for command in rendered))
+
+    def test_protected_bucket_cannot_become_public_across_deployments(self):
+        cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
+        previous = {
+            "baseline-dist": {
+                "path": "/dist", "access": "protected", "artifact": "old-name",
+                "immutable_paths": ["releases"],
+            }
+        }
+        with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
+            deploy.validate_route_migrations(cfg, previous)
+        renamed = copy.deepcopy(cfg)
+        artifact = next(item for item in renamed["artifacts"] if item["name"] == "distributions")
+        artifact["name"] = "downloads"
+        route = next(item for item in renamed["routes"] if item["artifact"] == "distributions")
+        route["artifact"] = "downloads"
+        with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
+            deploy.validate_route_migrations(renamed, previous)
+        previous = {"retired-protected-bucket": previous["baseline-dist"]}
+        deploy.validate_route_migrations(cfg, previous)
+
+    def test_protected_split_bucket_cannot_become_legacy_public(self):
+        cfg = normalize_site_config(fixture("legacy-site.yaml"), "baseline.fritzlab.net")
+        previous = {
+            "baseline.fritzlab.net": {
+                "path": "/portal", "access": "protected", "artifact": "portal",
+                "immutable_paths": [],
+            }
+        }
+        with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"):
+            deploy.validate_route_migrations(cfg, previous)
+
+    def test_removed_immutable_rule_preserves_prior_keys(self):
+        cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
+        artifact = next(item for item in cfg["artifacts"] if item["name"] == "distributions")
+        artifact["cache_rules"] = [
+            rule for rule in artifact["cache_rules"] if rule["path"] != "releases"
+        ]
+        route = next(item for item in cfg["routes"] if item["artifact"] == "distributions")
+        with tempfile.TemporaryDirectory() as tmp:
+            html = Path(tmp)
+            filters = deploy.retired_immutable_filters(artifact, route, html, {
+                "path": "/dist", "access": "public", "artifact": "distributions",
+                "immutable_paths": ["releases"],
+            })
+            self.assertEqual(["--exclude", "releases/*"], filters)
+            (html / "releases").mkdir()
+            (html / "releases" / "replacement.js").write_text("mutable")
+            with self.assertRaisesRegex(RuntimeError, "collides with retired immutable"):
+                deploy.retired_immutable_filters(artifact, route, html, {
+                    "path": "/dist", "access": "public", "artifact": "distributions",
+                    "immutable_paths": ["releases"],
+                })
+
     def test_later_route_immutable_failure_stops_all_mutable_publication(self):
         cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")
         with tempfile.TemporaryDirectory() as tmp:
             root = Path(tmp)
             with patch.object(deploy, "validate_publication_environment"), \
-                    patch.object(deploy, "validate_artifact_output"), patch.object(
+                    patch.object(deploy, "validate_artifact_output"), \
+                    patch.object(deploy, "clone_apps", return_value=root / "apps"), patch.object(
                         deploy, "publish_route_immutables",
                         side_effect=[None, RuntimeError("immutable failed")],
                     ) as immutable_publish, patch.object(deploy, "s3_sync") as mutable_sync, \