From 85a0b41380abc22e8683cc9236e0ee1bc336ae0a Mon Sep 17 00:00:00 2001 From: Evelyn Chen Date: Sat, 29 Aug 2026 22:42:37 +0000 Subject: [PATCH 1/4] fix(site-publish): close split migration boundaries Authored-By: OpenAI (GPT-5) --- README.md | 10 +++- scripts/deploy.py | 110 ++++++++++++++++++++++++++++++++++---- scripts/utils.py | 10 +++- templates/ingress.yaml.j2 | 7 +++ tests/test_contract.py | 108 ++++++++++++++++++++++++++++++++++++- 5 files changed, 232 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 6a2c886..9f0ca29 100644 --- a/README.md +++ b/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. diff --git a/scripts/deploy.py b/scripts/deploy.py index 025189d..b298349 100644 --- a/scripts/deploy.py +++ b/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) diff --git a/scripts/utils.py b/scripts/utils.py index 8376cd5..b12a079 100644 --- a/scripts/utils.py +++ b/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" diff --git a/templates/ingress.yaml.j2 b/templates/ingress.yaml.j2 index 30ab0b9..daa1c07 100644 --- a/templates/ingress.yaml.j2 +++ b/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 diff --git a/tests/test_contract.py b/tests/test_contract.py index a8c239f..b202901 100644 --- a/tests/test_contract.py +++ b/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, \ From 892b6e6441f188074cd8457dc75a8c1a05940575 Mon Sep 17 00:00:00 2001 From: Evelyn Chen Date: Sat, 29 Aug 2026 23:15:29 +0000 Subject: [PATCH 2/4] fix(site-publish): retain immutable route history Authored-By: OpenAI (GPT-5) --- README.md | 6 +++-- scripts/deploy.py | 57 ++++++++++++++++++++++++++---------------- tests/test_contract.py | 43 ++++++++++++++++++++++++++++--- 3 files changed, 79 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 9f0ca29..3fd696b 100644 --- a/README.md +++ b/README.md @@ -133,8 +133,10 @@ 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 +Generated Ingress annotations retain each bucket's prior route and all immutable +key prefixes. The prefixes are bucket-relative and carried forward after a rule +is removed or renamed, so later deployments and route moves cannot forget them. +When a move places a 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. diff --git a/scripts/deploy.py b/scripts/deploy.py index b298349..56cda67 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -178,29 +178,40 @@ def publish_route_immutables(artifact, route, site_dir, credential_env_names=Non publish_immutable_rule(artifact, route, rule, html_dir, aws_env) +def immutable_key_prefixes(artifact, route): + """Return immutable partitions as bucket-relative key prefixes.""" + route_prefix = route["path"].strip("/") + return [ + "/".join(part for part in (route_prefix, rule["path"]) if part) + for rule in artifact["cache_rules"] if _is_immutable(rule) + ] + + +def retained_immutable_paths(artifact, route, previous_contract): + """Carry all bucket history forward so later route moves cannot delete it.""" + previous_paths = previous_contract["immutable_paths"] if previous_contract else [] + return sorted(set(previous_paths) | set(immutable_key_prefixes(artifact, route))) + + def retired_immutable_filters(artifact, route, html_dir, previous_contract): - """Protect immutable keys only when an old route falls inside the new scope.""" + """Protect historical immutable keys that fall inside the current sync 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) - } + current_immutable = set(immutable_key_prefixes(artifact, route)) for immutable_path in previous_contract["immutable_paths"]: - retired_path = "/".join(part for part in (previous_prefix, immutable_path) if part) + if immutable_path in current_immutable: + continue + if current_prefix: + marker = f"{current_prefix}/" + if not immutable_path.startswith(marker): + continue + retired_path = immutable_path[len(marker):] + else: + retired_path = immutable_path 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("*"))): + if collision.exists() and any(path.is_file() for path in collision.rglob("*")): raise RuntimeError( f"current artifact collides with retired immutable partition: {retired_path}" ) @@ -308,18 +319,20 @@ def ensure_bucket_aliases(site_name, aliases, admin_token): raise -def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg): +def render_site_manifests( + site_name, action_dir, app_dir, manifests_dir, cfg, previous_contracts=None, +): """Always re-render manifests from current site.yaml. Templates own domain + aliases, so changes propagate without manual edits.""" manifests_dir.mkdir(parents=True, exist_ok=True) artifact_by_name = {artifact["name"]: artifact for artifact in cfg["artifacts"]} + previous_contracts = previous_contracts or {} routes = [] 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']}" - immutable_paths = [ - rule["path"] for rule in artifact["cache_rules"] if _is_immutable(rule) - ] + previous = previous_contracts.get(artifact["bucket"]) + immutable_paths = retained_immutable_paths(artifact, route, previous) routes.append({ **route, "resource_name": resource_name, "artifact_config": artifact, "immutable_paths_json": json.dumps(immutable_paths, separators=(",", ":")), @@ -413,7 +426,9 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg): if cfg["compatibility"]: ensure_bucket_aliases(site_name, cfg["aliases"], os.environ.get("GARAGE_ADMIN_TOKEN")) - render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg) + render_site_manifests( + site_name, action_dir, app_dir, manifests_dir, cfg, previous_contracts, + ) commit_and_push(apps_dir, f"Deploy {site_name}", token) diff --git a/tests/test_contract.py b/tests/test_contract.py index b202901..73f4e91 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -473,7 +473,7 @@ class PublishingTests(unittest.TestCase): ): deploy.s3_sync(artifact, route, root, previous_contract={ "path": "/foo", "access": "public", "artifact": "distributions", - "immutable_paths": ["releases"], + "immutable_paths": ["foo/releases"], }) rendered = [" ".join(command) for command in commands] self.assertTrue(all("foo/releases/*" in command for command in rendered[:2])) @@ -484,7 +484,7 @@ class PublishingTests(unittest.TestCase): previous = { "baseline-dist": { "path": "/dist", "access": "protected", "artifact": "old-name", - "immutable_paths": ["releases"], + "immutable_paths": ["dist/releases"], } } with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"): @@ -521,7 +521,7 @@ class PublishingTests(unittest.TestCase): html = Path(tmp) filters = deploy.retired_immutable_filters(artifact, route, html, { "path": "/dist", "access": "public", "artifact": "distributions", - "immutable_paths": ["releases"], + "immutable_paths": ["dist/releases"], }) self.assertEqual(["--exclude", "releases/*"], filters) (html / "releases").mkdir() @@ -529,9 +529,44 @@ class PublishingTests(unittest.TestCase): with self.assertRaisesRegex(RuntimeError, "collides with retired immutable"): deploy.retired_immutable_filters(artifact, route, html, { "path": "/dist", "access": "public", "artifact": "distributions", - "immutable_paths": ["releases"], + "immutable_paths": ["dist/releases"], }) + def test_retired_immutable_history_survives_the_following_deployment(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") + route["path"] = "/" + previous = { + "baseline-dist": { + "path": "/foo", "access": "public", "artifact": "distributions", + "immutable_paths": ["foo/releases"], + } + } + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + app_dir = root / "app" + manifests = app_dir / "manifests" + app_dir.mkdir() + deploy.render_site_manifests( + "baseline.fritzlab.net", ROOT, app_dir, manifests, cfg, previous, + ) + following = deploy.previous_route_contracts(app_dir) + self.assertEqual(["foo/releases"], following["baseline-dist"]["immutable_paths"]) + html = root / "html" + html.mkdir() + first_filters = deploy.retired_immutable_filters( + artifact, route, html, previous["baseline-dist"], + ) + following_filters = deploy.retired_immutable_filters( + artifact, route, html, following["baseline-dist"], + ) + self.assertEqual(["--exclude", "foo/releases/*"], first_filters) + self.assertEqual(first_filters, following_filters) + 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: From 898816db16c8e27a6c2cb2ee30bf303a15436790 Mon Sep 17 00:00:00 2001 From: Evelyn Chen Date: Sat, 29 Aug 2026 22:42:37 +0000 Subject: [PATCH 3/4] fix(site-publish): close split migration boundaries Authored-By: OpenAI (GPT-5) --- README.md | 12 ++- scripts/deploy.py | 162 +++++++++++++++++++++++++++++++++--- scripts/utils.py | 10 ++- templates/ingress.yaml.j2 | 7 ++ tests/test_contract.py | 170 +++++++++++++++++++++++++++++++++++++- 5 files changed, 346 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 6a2c886..a0b4767 100644 --- a/README.md +++ b/README.md @@ -133,10 +133,20 @@ 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 and `site-publish-history.yaml` retain every +seen bucket's access, prior route, and cumulative bucket-relative immutable +paths, including while an artifact is absent. 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. diff --git a/scripts/deploy.py b/scripts/deploy.py index 025189d..2a7a1fe 100644 --- a/scripts/deploy.py +++ b/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,34 @@ 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 recorded bucket keys when they fall inside the current sync scope.""" + if not previous_contract: + return [] + current_prefix = route["path"].strip("/") + filters = [] + current_immutable = { + "/".join(part for part in (current_prefix, rule["path"]) if part) + for rule in artifact["cache_rules"] if _is_immutable(rule) + } + for immutable_path in previous_contract["immutable_paths"]: + retired_path = immutable_path + if current_prefix: + marker = f"{current_prefix}/" + if not retired_path.startswith(marker): + continue + retired_path = retired_path[len(marker):] + 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 +233,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 @@ -275,16 +305,37 @@ def ensure_bucket_aliases(site_name, aliases, admin_token): raise -def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg): +def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg, + previous_contracts=None): """Always re-render manifests from current site.yaml. Templates own domain + aliases, so changes propagate without manual edits.""" manifests_dir.mkdir(parents=True, exist_ok=True) artifact_by_name = {artifact["name"]: artifact for artifact in cfg["artifacts"]} routes = [] + previous_contracts = previous_contracts or {} + next_contracts = {bucket: dict(contract) + for bucket, contract in previous_contracts.items()} 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}) + route_prefix = route["path"].strip("/") + immutable_paths = { + "/".join(part for part in (route_prefix, rule["path"]) if part) + for rule in artifact["cache_rules"] if _is_immutable(rule) + } + previous = previous_contracts.get(artifact["bucket"]) + if previous: + immutable_paths.update(previous["immutable_paths"]) + next_contracts[artifact["bucket"]] = { + "path": route["path"], + "access": "public" if route["access"] == "legacy" else route["access"], + "artifact": route["artifact"], + "immutable_paths": sorted(immutable_paths), + } + routes.append({ + **route, "resource_name": resource_name, "artifact_config": artifact, + "immutable_paths_json": json.dumps(sorted(immutable_paths), separators=(",", ":")), + }) template_vars = { "site": site_name, "site_k8s": k8s_name(site_name), @@ -295,6 +346,89 @@ def render_site_manifests(site_name, action_dir, app_dir, manifests_dir, cfg): "routes": routes, } render_templates(action_dir, template_vars, app_dir, manifests_dir) + if not cfg["compatibility"] or previous_contracts: + (app_dir / "site-publish-history.yaml").write_text(yaml.safe_dump( + {"version": 1, "buckets": next_contracts}, sort_keys=True, + )) + + +def _validate_route_contract(bucket, contract, path): + expected = {"path", "access", "artifact", "immutable_paths"} + if (not isinstance(bucket, str) or not bucket or not isinstance(contract, dict) + or set(contract) != expected + or contract.get("access") not in {"public", "protected"} + or not isinstance(contract.get("path"), str) + or not contract["path"].startswith("/") + or not isinstance(contract.get("artifact"), str) or not contract["artifact"] + or not isinstance(contract.get("immutable_paths"), list) + or any(not isinstance(item, str) or not item or item.startswith("/") + or any(part in {"", ".", ".."} for part in item.split("/")) + for item in contract["immutable_paths"])): + raise RuntimeError(f"invalid site-publish route history in {path}") + return { + "path": contract["path"], "access": contract["access"], + "artifact": contract["artifact"], + "immutable_paths": sorted(set(contract["immutable_paths"])), + } + + +def previous_route_contracts(app_dir): + """Read bucket-keyed route history from generated Ingresses.""" + history_path = app_dir / "site-publish-history.yaml" + if history_path.exists(): + document = yaml.safe_load(history_path.read_text()) + if (not isinstance(document, dict) or set(document) != {"version", "buckets"} + or document["version"] != 1 or not isinstance(document["buckets"], dict)): + raise RuntimeError(f"invalid site-publish route history in {history_path}") + return { + bucket: _validate_route_contract(bucket, contract, history_path) + for bucket, contract in document["buckets"].items() + } + 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] = _validate_route_contract(bucket, { + "path": route_path, "access": access, "artifact": artifact, + "immutable_paths": immutable_paths, + }, path) + 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): @@ -305,6 +439,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,15 +452,16 @@ 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) + render_site_manifests( + site_name, action_dir, app_dir, manifests_dir, cfg, previous_contracts, + ) commit_and_push(apps_dir, f"Deploy {site_name}", token) diff --git a/scripts/utils.py b/scripts/utils.py index 8376cd5..b12a079 100644 --- a/scripts/utils.py +++ b/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" diff --git a/templates/ingress.yaml.j2 b/templates/ingress.yaml.j2 index 30ab0b9..daa1c07 100644 --- a/templates/ingress.yaml.j2 +++ b/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 diff --git a/tests/test_contract.py b/tests/test_contract.py index a8c239f..a64c65b 100644 --- a/tests/test_contract.py +++ b/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): @@ -253,6 +263,7 @@ class GenerationTests(unittest.TestCase): "app.yaml", "manifests/certificate.yaml", "manifests/ingress-distributions.yaml", "manifests/ingress-portal.yaml", "manifests/kustomization.yaml", "manifests/service-distributions.yaml", "manifests/service-portal.yaml", + "site-publish-history.yaml", }, set(files)) for content in files.values(): self.assertIsNotNone(yaml.safe_load(content)) @@ -263,6 +274,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 +308,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 +453,154 @@ 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": ["foo/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": ["dist/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": ["dist/releases"], + }) + + def test_removed_immutable_rule_remains_in_next_manifest_history(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" + ] + previous = { + "baseline-dist": { + "path": "/dist", "access": "public", "artifact": "distributions", + "immutable_paths": ["dist/releases"], + } + } + with tempfile.TemporaryDirectory() as tmp: + app_dir = Path(tmp) / "app" + manifests = app_dir / "manifests" + deploy.render_site_manifests( + "baseline.fritzlab.net", ROOT, app_dir, manifests, cfg, previous, + ) + first = deploy.previous_route_contracts(app_dir) + self.assertEqual(["dist/releases"], first["baseline-dist"]["immutable_paths"]) + deploy.render_site_manifests( + "baseline.fritzlab.net", ROOT, app_dir, manifests, cfg, first, + ) + second = deploy.previous_route_contracts(app_dir) + self.assertEqual(first, second) + + def test_removed_artifact_keeps_bucket_access_history(self): + raw = fixture("split-site.yaml") + raw["artifacts"] = [item for item in raw["artifacts"] if item["name"] == "portal"] + raw["routes"] = [item for item in raw["routes"] if item["artifact"] == "portal"] + cfg = normalize_site_config(raw, "baseline.fritzlab.net") + previous = { + "baseline-dist": { + "path": "/dist", "access": "protected", "artifact": "distributions", + "immutable_paths": ["dist/releases"], + } + } + with tempfile.TemporaryDirectory() as tmp: + app_dir = Path(tmp) / "app" + deploy.render_site_manifests( + "baseline.fritzlab.net", ROOT, app_dir, app_dir / "manifests", cfg, previous, + ) + retained = deploy.previous_route_contracts(app_dir) + self.assertEqual(previous["baseline-dist"], retained["baseline-dist"]) + readded = normalize_site_config( + fixture("split-site.yaml"), "baseline.fritzlab.net", + ) + with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"): + deploy.validate_route_migrations(readded, retained) + + def test_malformed_persistent_history_fails_closed(self): + with tempfile.TemporaryDirectory() as tmp: + app_dir = Path(tmp) + (app_dir / "site-publish-history.yaml").write_text( + "version: 1\nbuckets:\n bucket:\n path: /\n" + " access: public\n artifact: site\n" + " immutable_paths: [../releases]\n" + ) + with self.assertRaisesRegex(RuntimeError, "invalid site-publish route history"): + deploy.previous_route_contracts(app_dir) + 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, \ From deccc4e1777d2492bc4bf4b5f62b2b8b9f5aac0c Mon Sep 17 00:00:00 2001 From: Evelyn Chen Date: Sat, 29 Aug 2026 23:19:43 +0000 Subject: [PATCH 4/4] test(site-publish): prove absent bucket tombstone Authored-By: OpenAI (GPT-5) --- tests/test_contract.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/tests/test_contract.py b/tests/test_contract.py index 02a7b64..7fcb972 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -559,15 +559,20 @@ class PublishingTests(unittest.TestCase): second = deploy.previous_route_contracts(app_dir) self.assertEqual(first, second) - def test_removed_artifact_keeps_bucket_access_history(self): + def test_protected_split_bucket_survives_absence_and_blocks_legacy(self): raw = fixture("split-site.yaml") - raw["artifacts"] = [item for item in raw["artifacts"] if item["name"] == "portal"] - raw["routes"] = [item for item in raw["routes"] if item["artifact"] == "portal"] + raw["artifacts"] = [ + item for item in raw["artifacts"] if item["name"] == "distributions" + ] + raw["routes"] = [ + item for item in raw["routes"] if item["artifact"] == "distributions" + ] + raw["routes"][0]["path"] = "/" cfg = normalize_site_config(raw, "baseline.fritzlab.net") previous = { - "baseline-dist": { - "path": "/dist", "access": "protected", "artifact": "distributions", - "immutable_paths": ["dist/releases"], + "baseline.fritzlab.net": { + "path": "/", "access": "protected", "artifact": "portal", + "immutable_paths": [], } } with tempfile.TemporaryDirectory() as tmp: @@ -576,12 +581,14 @@ class PublishingTests(unittest.TestCase): "baseline.fritzlab.net", ROOT, app_dir, app_dir / "manifests", cfg, previous, ) retained = deploy.previous_route_contracts(app_dir) - self.assertEqual(previous["baseline-dist"], retained["baseline-dist"]) - readded = normalize_site_config( - fixture("split-site.yaml"), "baseline.fritzlab.net", + self.assertEqual( + previous["baseline.fritzlab.net"], retained["baseline.fritzlab.net"], + ) + legacy = normalize_site_config( + fixture("legacy-site.yaml"), "baseline.fritzlab.net", ) with self.assertRaisesRegex(RuntimeError, "cannot become public while reusing protected"): - deploy.validate_route_migrations(readded, retained) + deploy.validate_route_migrations(legacy, retained) def test_malformed_persistent_history_fails_closed(self): with tempfile.TemporaryDirectory() as tmp: