diff --git a/README.md b/README.md index 7a21f77..cf534f8 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ artifacts: - name: distributions type: static content_dir: dist + cors_origins: ['*'] publish: bucket: baseline-dist credentials: @@ -151,6 +152,12 @@ 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. +`cors_origins` is reconciled as a bucket policy on every split publication. Values are either `*` +or HTTPS origins; browser access is limited to `GET` and `HEAD`. Omitting the field removes stale +CORS from that bucket. Protected artifacts cannot allow wildcard CORS. All immutable objects and +all bucket CORS policies complete before mutable channels change; if any policy write fails, the +policies already attempted are restored to their pre-publication values. + Each split route gets a bucket-specific `.web.sjc001.fritzlab.net` ExternalName Service annotated to disable pass-host-header and a separate Ingress. Route Ingresses share the hostname's certificate Secret. The access middleware and diff --git a/scripts/deploy.py b/scripts/deploy.py index 9900d17..45e25bb 100644 --- a/scripts/deploy.py +++ b/scripts/deploy.py @@ -8,6 +8,7 @@ import os import re import shutil import subprocess +import tempfile from pathlib import Path from urllib.error import HTTPError, URLError from urllib.request import Request, urlopen @@ -169,6 +170,98 @@ def publication_aws_env(artifact, credential_env_names=None): return aws_env +def configure_cors(bucket, origins, endpoint, aws_env): + """Reconcile read-only browser access without exposing publication credentials.""" + if origins is None: + return + config = None + if origins: + config = { + "CORSRules": [{ + "AllowedOrigins": origins, + "AllowedMethods": ["GET", "HEAD"], + "AllowedHeaders": ["*"], + "ExposeHeaders": ["ETag"], + "MaxAgeSeconds": 3600, + }], + } + set_cors_configuration(bucket, config, endpoint, aws_env) + + +def set_cors_configuration(bucket, config, endpoint, aws_env): + """Apply an exact bucket CORS configuration, or remove it when absent.""" + if config is None: + run([ + "aws", "--endpoint-url", endpoint, "s3api", "delete-bucket-cors", + "--bucket", bucket, + ], env=aws_env) + return + with tempfile.NamedTemporaryFile("w", suffix=".json", encoding="utf-8") as handle: + json.dump(config, handle) + handle.flush() + run([ + "aws", "--endpoint-url", endpoint, "s3api", "put-bucket-cors", + "--bucket", bucket, "--cors-configuration", f"file://{handle.name}", + ], env=aws_env) + + +def get_cors_configuration(bucket, endpoint, aws_env): + """Read the exact bucket CORS configuration for rollback.""" + result = _aws_capture([ + "aws", "--endpoint-url", endpoint, "s3api", "get-bucket-cors", + "--bucket", bucket, "--output", "json", + ], aws_env) + if result.returncode == 0: + try: + config = json.loads(result.stdout) + except json.JSONDecodeError as error: + raise RuntimeError(f"get-bucket-cors returned invalid JSON for {bucket}") from error + if not isinstance(config, dict) or not isinstance(config.get("CORSRules"), list): + raise RuntimeError(f"get-bucket-cors returned an invalid policy for {bucket}") + return config + error = f"{result.stdout}\n{result.stderr}" + if "NoSuchCORSConfiguration" in error: + return None + raise RuntimeError(f"get-bucket-cors failed for {bucket}: {error.strip()}") + + +def reconcile_artifact_cors(artifacts, credential_env_names=None): + """Reconcile all policies, restoring the prior set if any write fails.""" + snapshots = [] + for artifact in artifacts: + if artifact["cors_origins"] is None: + continue + aws_env = publication_aws_env(artifact, credential_env_names) + snapshots.append(( + artifact, + aws_env, + get_cors_configuration(artifact["bucket"], artifact["s3_endpoint"], aws_env), + )) + + attempted = [] + try: + for artifact, aws_env, previous in snapshots: + attempted.append((artifact, aws_env, previous)) + configure_cors( + artifact["bucket"], artifact["cors_origins"], artifact["s3_endpoint"], aws_env, + ) + except Exception as error: + rollback_errors = [] + for artifact, aws_env, previous in reversed(attempted): + try: + set_cors_configuration( + artifact["bucket"], previous, artifact["s3_endpoint"], aws_env, + ) + except Exception as rollback_error: + rollback_errors.append(f"{artifact['bucket']}: {rollback_error}") + if rollback_errors: + raise RuntimeError( + f"CORS reconciliation failed ({error}); rollback also failed for " + f"{'; '.join(rollback_errors)}" + ) from error + raise + + def publish_route_immutables(artifact, route, site_dir, credential_env_names=None): """Publish one route's immutable partitions during the global preflight.""" html_dir = site_dir / artifact["build_dir"] @@ -460,6 +553,10 @@ def deploy_static(site_name, site_dir, action_dir, token, cfg): publish_route_immutables( artifact_by_name[route["artifact"]], route, site_dir, credential_env_names, ) + # Reconcile every browser-read policy before publishing mutable content. + # A CORS failure therefore cannot leave a new channel pointing at a release + # whose cross-origin assets browsers cannot consume. + reconcile_artifact_cors(cfg["artifacts"], credential_env_names) for route in cfg["routes"]: s3_sync( artifact_by_name[route["artifact"]], route, site_dir, credential_env_names, diff --git a/scripts/utils.py b/scripts/utils.py index b12a079..f499731 100644 --- a/scripts/utils.py +++ b/scripts/utils.py @@ -1,5 +1,6 @@ """Shared utilities for the site-publish action.""" +import ipaddress import os import re import shutil @@ -104,6 +105,55 @@ def _strings(value, label): return values +def _cors_origins(value, label): + origins = _list(value, label) + if any(not isinstance(item, str) or not item for item in origins): + raise ConfigError(f"{label} must be a list of non-empty strings") + canonical = [] + for origin in origins: + if origin == "*": + canonical.append(origin) + continue + parsed = urlparse(origin) + try: + port = parsed.port + except ValueError: + raise ConfigError(f"{label} must contain '*' or canonical HTTPS origins") from None + if parsed.scheme != "https" or not parsed.hostname or parsed.path or parsed.params or ( + parsed.query or parsed.fragment or parsed.username or parsed.password + ): + raise ConfigError(f"{label} must contain '*' or canonical HTTPS origins") + try: + address = ipaddress.ip_address(parsed.hostname) + except ValueError: + try: + hostname = parsed.hostname.encode("idna").decode("ascii") + except UnicodeError: + raise ConfigError( + f"{label} must contain '*' or canonical HTTPS origins" + ) from None + try: + _hostname(hostname, f"{label} hostname") + except ConfigError: + raise ConfigError( + f"{label} must contain '*' or canonical HTTPS origins" + ) from None + else: + if getattr(address, "scope_id", None) is not None: + raise ConfigError( + f"{label} must contain '*' or canonical HTTPS origins" + ) + hostname = f"[{address.compressed}]" if address.version == 6 else address.compressed + canonical.append(f"https://{hostname}{f':{port}' if port not in (None, 443) else ''}") + if len(canonical) != len(set(canonical)): + raise ConfigError(f"{label} must not contain duplicate canonical origins") + if "*" in canonical and len(canonical) != 1: + raise ConfigError(f"{label} wildcard must be the only origin") + if origins != canonical: + raise ConfigError(f"{label} must contain '*' or canonical HTTPS origins") + return origins + + def _hostname(value, label): if not isinstance(value, str) or len(value) > 253 or value.endswith("."): raise ConfigError(f"{label} must be a lowercase DNS hostname without a trailing dot") @@ -215,6 +265,7 @@ def _legacy_config(raw, site_name): "website_authority": "garage-s3.storage.svc.k8s.sjc001.fritzlab.net", "credentials": {"access_key_env": "AWS_ACCESS_KEY_ID", "secret_key_env": "AWS_SECRET_ACCESS_KEY"}, "cache_rules": [{"path": "", "cache_control": DEFAULT_CACHE_CONTROL}], + "cors_origins": None, } return { "version": 1, @@ -234,7 +285,11 @@ def _legacy_config(raw, site_name): def _artifact(item, index): label = f"artifacts[{index}]" item = _mapping(item, label) - _known_keys(item, {"name", "type", "content_dir", "tidy", "excludes", "publish", "cache"}, label) + _known_keys( + item, + {"name", "type", "content_dir", "tidy", "excludes", "publish", "cache", "cors_origins"}, + label, + ) name = item.get("name") if not isinstance(name, str) or not NAME_RE.fullmatch(name): raise ConfigError(f"{label}.name must be a DNS label") @@ -303,6 +358,10 @@ def _artifact(item, index): "website_authority": authority, "credentials": normalized_credentials, "cache_rules": cache_rules, + "cors_origins": ( + _cors_origins(item["cors_origins"], f"{label}.cors_origins") + if "cors_origins" in item else [] + ), } @@ -389,6 +448,10 @@ def _validate_multi(cfg): raise ConfigError(f"protected route {route['name']} cannot use shared-cache max-age") if route["access"] == "public" and "private" in directives: raise ConfigError(f"public route {route['name']} cannot use private cache policy") + if route["access"] == "protected" and "*" in artifact["cors_origins"]: + raise ConfigError( + f"protected route {route['name']} cannot allow wildcard CORS" + ) root = next(route for route in routes if route["path"] == "/") if any(route["access"] == "protected" for route in routes) and root["access"] == "public": raise ConfigError("a public '/' catch-all would expose unmatched protected content") diff --git a/tests/fixtures/split-site.yaml b/tests/fixtures/split-site.yaml index bbb8ec8..51907b7 100644 --- a/tests/fixtures/split-site.yaml +++ b/tests/fixtures/split-site.yaml @@ -15,6 +15,7 @@ artifacts: - name: distributions type: static content_dir: dist + cors_origins: ['*'] publish: bucket: baseline-dist credentials: diff --git a/tests/test_contract.py b/tests/test_contract.py index 7fcb972..f09deb9 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -113,6 +113,69 @@ class ConfigContractTests(unittest.TestCase): self.assertEqual(["/dist", "/"], [route["path"] for route in cfg["routes"]]) credentials = {artifact["name"]: artifact["credentials"] for artifact in cfg["artifacts"]} self.assertNotEqual(credentials["distributions"], credentials["portal"]) + distributions = next( + artifact for artifact in cfg["artifacts"] if artifact["name"] == "distributions" + ) + self.assertEqual(["*"], distributions["cors_origins"]) + + def test_cors_origins_are_https_origins_or_wildcard(self): + self.assert_invalid( + lambda raw: raw["artifacts"][1].__setitem__( + "cors_origins", ["http://consumer.example"] + ), + "must contain '\\*' or canonical HTTPS origins", + ) + for origin in ( + "https://consumer example", + "https://Consumer.example", + "https://consumer.example:443", + "https://consumer.example/", + "https://consumer.example:invalid", + "https://[fe80::1%eth0]", + "https://[fe80::1%25eth0]", + ): + with self.subTest(origin=origin): + self.assert_invalid( + lambda raw, origin=origin: raw["artifacts"][1].__setitem__( + "cors_origins", [origin] + ), + "must contain '\\*' or canonical HTTPS origins", + ) + self.assert_invalid( + lambda raw: raw["artifacts"][1].__setitem__( + "cors_origins", ["https://consumer.example", "https://consumer.example:443"] + ), + "must not contain duplicate canonical origins", + ) + self.assert_invalid( + lambda raw: raw["artifacts"][1].__setitem__( + "cors_origins", ["*", "https://consumer.example"] + ), + "wildcard must be the only origin", + ) + raw = copy.deepcopy(self.raw) + raw["artifacts"][1]["cors_origins"] = ["https://[2602:817:3000::1]:8443"] + cfg = normalize_site_config(raw, "baseline.fritzlab.net") + self.assertEqual( + ["https://[2602:817:3000::1]:8443"], + next(a for a in cfg["artifacts"] if a["name"] == "distributions")["cors_origins"], + ) + + def test_declared_cors_origins_must_be_a_list(self): + for value in (None, False, 0, "", {}): + with self.subTest(value=value): + self.assert_invalid( + lambda raw, value=value: raw["artifacts"][1].__setitem__( + "cors_origins", value + ), + "cors_origins must be a list", + ) + + def test_protected_route_rejects_wildcard_cors(self): + self.assert_invalid( + lambda raw: raw["artifacts"][0].__setitem__("cors_origins", ["*"]), + "protected route portal cannot allow wildcard CORS", + ) def test_equivalent_route_paths_are_ambiguous(self): self.assert_invalid(lambda raw: raw["routes"].append({ @@ -131,6 +194,7 @@ class ConfigContractTests(unittest.TestCase): raw["routes"][1]["access"] = { "mode": "protected", "middleware": "authentik-forwardauth" } + raw["artifacts"][1]["cors_origins"] = [] raw["artifacts"][0]["cache"]["rules"][0]["cache_control"] = ( "public, max-age=0, must-revalidate" ) @@ -341,6 +405,75 @@ class BuildTests(unittest.TestCase): class PublishingTests(unittest.TestCase): + def test_cors_policy_is_reconciled_as_read_only_browser_access(self): + captured = {} + + def capture(command, **_kwargs): + config_path = command[command.index("--cors-configuration") + 1] + captured["command"] = command + captured["config"] = json.loads(Path(config_path.removeprefix("file://")).read_text()) + + with patch.object(deploy, "run", side_effect=capture): + deploy.configure_cors( + "baseline-dist", ["*"], "http://garage-s3.storage.svc:3900", {} + ) + + self.assertIn("put-bucket-cors", captured["command"]) + self.assertEqual(["GET", "HEAD"], captured["config"]["CORSRules"][0]["AllowedMethods"]) + self.assertEqual(["*"], captured["config"]["CORSRules"][0]["AllowedOrigins"]) + + def test_empty_cors_policy_removes_stale_bucket_cors(self): + with patch.object(deploy, "run") as request: + deploy.configure_cors( + "baseline-catalogue", [], "http://garage-s3.storage.svc:3900", {} + ) + self.assertIn("delete-bucket-cors", request.call_args.args[0]) + + def test_cors_reconciliation_restores_prior_policies_on_failure(self): + artifacts = [ + { + "bucket": "first", "cors_origins": ["https://new.example"], + "s3_endpoint": "http://garage-s3.storage.svc:3900", "credentials": {}, + }, + { + "bucket": "second", "cors_origins": [], + "s3_endpoint": "http://garage-s3.storage.svc:3900", "credentials": {}, + }, + ] + prior = [ + {"CORSRules": [{"AllowedOrigins": ["https://old.example"]}]}, + None, + ] + writes = [] + + def write(bucket, config, *_args): + writes.append((bucket, config)) + if bucket == "second" and len(writes) == 2: + raise RuntimeError("write failed") + + with patch.object(deploy, "publication_aws_env", return_value={}), patch.object( + deploy, "get_cors_configuration", side_effect=prior, + ) as read, patch.object(deploy, "set_cors_configuration", side_effect=write), \ + self.assertRaisesRegex(RuntimeError, "write failed"): + deploy.reconcile_artifact_cors(artifacts) + + self.assertEqual(2, read.call_count) + self.assertEqual( + [ + ("first", {"CORSRules": [{ + "AllowedOrigins": ["https://new.example"], + "AllowedMethods": ["GET", "HEAD"], + "AllowedHeaders": ["*"], + "ExposeHeaders": ["ETag"], + "MaxAgeSeconds": 3600, + }]}), + ("second", None), + ("second", None), + ("first", prior[0]), + ], + writes, + ) + def test_apps_clone_never_places_token_in_argv_or_log(self): calls = [] secret = "clone-secret-must-not-appear" @@ -645,15 +778,42 @@ class PublishingTests(unittest.TestCase): 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, \ + ) as immutable_publish, patch.object( + deploy, "reconcile_artifact_cors" + ) as cors_reconcile, patch.object(deploy, "s3_sync") as mutable_sync, \ self.assertRaisesRegex( RuntimeError, "immutable failed" ): deploy.deploy_static("baseline", root, root, "token", cfg) self.assertEqual(2, immutable_publish.call_count) + cors_reconcile.assert_not_called() mutable_sync.assert_not_called() + def test_all_cors_policies_complete_before_mutable_publication(self): + cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net") + events = [] + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + apps = root / "apps" + apps.mkdir() + with patch.object(deploy, "validate_publication_environment"), patch.object( + deploy, "validate_artifact_output" + ), patch.object( + deploy, "publish_route_immutables", side_effect=lambda *_args: events.append("immutable") + ), patch.object( + deploy, "reconcile_artifact_cors", side_effect=lambda *_args: events.append("cors") + ), patch.object( + deploy, "s3_sync", side_effect=lambda *_args: events.append("mutable") + ), patch.object(deploy, "clone_apps", return_value=apps), patch.object( + deploy, "render_site_manifests" + ), patch.object(deploy, "commit_and_push"): + deploy.deploy_static("baseline", root, root, "token", cfg) + + self.assertEqual( + ["immutable", "immutable", "cors", "mutable", "mutable"], events + ) + def test_absent_artifact_is_detected_before_publish(self): cfg = normalize_site_config(fixture("split-site.yaml"), "baseline.fritzlab.net")