From 7bc292e7e8318ffd56a2bbd05e821367df869e05 Mon Sep 17 00:00:00 2001 From: HARISH SESHADRI Date: Fri, 28 Aug 2026 23:21:39 -0700 Subject: [PATCH] fix(pages): support existing direct-upload projects Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01WhrjTokoEF6Zax8EASdGsv --- .../.dagger/src/cloudflare_pages/api.py | 48 +++-- .../.dagger/tests/test_api.py | 44 ++++- .../.dagger/tests/test_deploy_contract.py | 175 +++++++++++++++++- 3 files changed, 248 insertions(+), 19 deletions(-) diff --git a/modules/cloudflare-pages/.dagger/src/cloudflare_pages/api.py b/modules/cloudflare-pages/.dagger/src/cloudflare_pages/api.py index eb99a05..b76a6df 100644 --- a/modules/cloudflare-pages/.dagger/src/cloudflare_pages/api.py +++ b/modules/cloudflare-pages/.dagger/src/cloudflare_pages/api.py @@ -109,12 +109,12 @@ def parse_deployments_response(raw: str) -> DeploymentsResponse: def require_project_binding(project: PagesProject, target: PagesTarget) -> None: """Require repository, project, production branch, and domain coherence.""" _require_project_identity(project, target) + _require_domains(project.domains, target) source = project.source if source is None: - raise CloudflarePolicyError("Cloudflare project target binding differs") + return _require_source_binding(source.config.owner, source.config.repo_name, target) _require_source_policy(source.type, source.config.production_branch, target) - _require_domains(project.domains, target) def _require_project_identity(project: PagesProject, target: PagesTarget) -> None: @@ -180,8 +180,8 @@ async def deploy_verified_artifact[ArtifactT]( require_evidence_binding(target, github, attempt) await operations.wrangler_preflight() project = await _read_preflight(operations, target) - await _disable_git(operations, target, project.id) - await _revalidate_disabled_project(operations, target, project.id) + await _disable_git(operations, target, project) + await _revalidate_disabled_project(operations, target, project) created = await operations.upload(artifact, github.commit_sha) deployment = await _converge( operations, target, github.commit_sha, project.id, created.deployment_id @@ -224,28 +224,44 @@ async def _read_preflight[ArtifactT]( async def _disable_git[ArtifactT]( - operations: PagesOperations[ArtifactT], target: PagesTarget, expected_project_id: str + operations: PagesOperations[ArtifactT], target: PagesTarget, expected: PagesProject ) -> None: + if expected.source is None: + return project = parse_project_response(await operations.disable_git()) require_project_binding(project, target) - if project.id != expected_project_id: - raise CloudflarePolicyError("Cloudflare project identity changed before upload") - source = project.source - if source is None or source.config.production_deployments_enabled: - raise CloudflarePolicyError("Cloudflare Git production deployment remains enabled") - if source.config.preview_deployment_setting != "none": - raise CloudflarePolicyError("Cloudflare Git preview deployment remains enabled") + _require_same_project_state(project, expected) + _require_same_delivery_mode(project, expected) + _require_git_disabled(project) async def _revalidate_disabled_project[ArtifactT]( - operations: PagesOperations[ArtifactT], target: PagesTarget, project_id: str + operations: PagesOperations[ArtifactT], target: PagesTarget, expected: PagesProject ) -> None: project = parse_project_response(await operations.get_project()) require_project_binding(project, target) - if project.id != project_id: + _require_same_project_state(project, expected) + _require_same_delivery_mode(project, expected) + _require_git_disabled(project) + + +def _require_same_project_state(project: PagesProject, expected: PagesProject) -> None: + if project.id != expected.id: raise CloudflarePolicyError("Cloudflare project identity changed before upload") + if frozenset(project.domains) != frozenset(expected.domains): + raise CloudflarePolicyError("Cloudflare project domains changed before upload") + + +def _require_same_delivery_mode(project: PagesProject, expected: PagesProject) -> None: + if (project.source is None) != (expected.source is None): + raise CloudflarePolicyError("Cloudflare project delivery mode changed before upload") + + +def _require_git_disabled(project: PagesProject) -> None: source = project.source - if source is None or source.config.production_deployments_enabled: + if source is None: + return + if source.config.production_deployments_enabled: raise CloudflarePolicyError("Cloudflare Git production deployment remains enabled") if source.config.preview_deployment_setting != "none": raise CloudflarePolicyError("Cloudflare Git preview deployment remains enabled") @@ -427,7 +443,7 @@ def _require_success(success: bool, errors: tuple[ApiProblem, ...]) -> None: def _project_result(payload: dict[str, JsonValue]) -> dict[str, JsonValue]: project = _object(_required(payload, "result")) fields = _project(project, ("id", "name", "production_branch", "domains")) - source = project.get("source") + source = _required(project, "source") return fields | {"source": None if source is None else _source_result(source)} diff --git a/modules/cloudflare-pages/.dagger/tests/test_api.py b/modules/cloudflare-pages/.dagger/tests/test_api.py index 9cac5a7..cf5d076 100644 --- a/modules/cloudflare-pages/.dagger/tests/test_api.py +++ b/modules/cloudflare-pages/.dagger/tests/test_api.py @@ -139,6 +139,36 @@ def test_should_bind_read_only_project_preflight() -> None: require_project_binding(project, _target()) +def test_should_bind_existing_direct_upload_project() -> None: + # Given + payload = json.loads(_project_payload()) + payload["result"]["source"] = None + project = parse_project_response(json.dumps(payload)) + + # When / Then + require_project_binding(project, _target()) + + +def test_should_reject_missing_project_source() -> None: + # Given + payload = json.loads(_project_payload()) + payload["result"].pop("source") + + # When / Then + with pytest.raises(CloudflarePolicyError, match="schema"): + parse_project_response(json.dumps(payload)) + + +def test_should_reject_wrong_project_source_type() -> None: + # Given + payload = json.loads(_project_payload()) + payload["result"]["source"] = "github" + + # When / Then + with pytest.raises(CloudflarePolicyError, match="schema"): + parse_project_response(json.dumps(payload)) + + def test_should_reject_malformed_project_schema() -> None: # Given payload = json.loads(_project_payload()) @@ -271,9 +301,9 @@ def test_should_reject_failed_deployment_stage(status: str) -> None: @pytest.mark.parametrize( ("mutation", "value"), ( - ("source", None), ("domains", ["edge-reco.pages.dev"]), ("production_branch", "release"), + ("name", "almamesh"), ), ) def test_should_reject_foreign_project_preflight(mutation: str, value: object) -> None: @@ -291,6 +321,7 @@ def test_should_reject_foreign_project_preflight(mutation: str, value: object) - ("mutation", "value"), ( ("owner", "foreign"), + ("repo_name", "foreign"), ("production_branch", "release"), ), ) @@ -305,6 +336,17 @@ def test_should_reject_foreign_git_source(mutation: str, value: str) -> None: require_project_binding(project, _target()) +def test_should_reject_foreign_git_source_type() -> None: + # Given + payload = json.loads(_project_payload()) + payload["result"]["source"]["type"] = "gitlab" + project = parse_project_response(json.dumps(payload)) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="target binding"): + require_project_binding(project, _target()) + + def test_should_reject_malformed_account_reference() -> None: # Given / When / Then with pytest.raises(CloudflarePolicyError, match="account identity"): diff --git a/modules/cloudflare-pages/.dagger/tests/test_deploy_contract.py b/modules/cloudflare-pages/.dagger/tests/test_deploy_contract.py index c1cd410..f9dcde2 100644 --- a/modules/cloudflare-pages/.dagger/tests/test_deploy_contract.py +++ b/modules/cloudflare-pages/.dagger/tests/test_deploy_contract.py @@ -256,6 +256,33 @@ def _project_payload(project: str = "edge-reco") -> str: ) +def _direct_upload_project_payload() -> str: + payload = json.loads(_project_payload()) + payload["result"]["source"] = None + return json.dumps(payload) + + +def _project_payload_with_foreign_domain() -> str: + payload = json.loads(_direct_upload_project_payload()) + payload["result"]["domains"] = ["edge-reco.pages.dev"] + return json.dumps(payload) + + +def _direct_upload_project_payload_with_domain_drift() -> str: + payload = json.loads(_direct_upload_project_payload()) + payload["result"]["domains"] = ["edge-reco.com", "attacker.example"] + return json.dumps(payload) + + +def _git_project_payload_with_domain_drift() -> str: + payload = json.loads(_project_payload()) + payload["result"]["domains"] = ["edge-reco.com", "attacker.example"] + config = payload["result"]["source"]["config"] + config["production_deployments_enabled"] = False + config["preview_deployment_setting"] = "none" + return json.dumps(payload) + + def _deployment_payload(status: str = "success") -> str: result = [] if status == "absent" else [_deployment(status)] return json.dumps( @@ -333,10 +360,12 @@ class FakeOperations: events: list[str] = field(default_factory=list) sleeps: list[int] = field(default_factory=list) uploaded_artifact: object | None = None + project_reads: int = 0 async def get_project(self) -> str: self.events.append("get-project") - if "disable-git" in self.events and self.revalidated_project is not None: + self.project_reads += 1 + if self.project_reads > 1 and self.revalidated_project is not None: return self.revalidated_project return self.project @@ -347,7 +376,11 @@ async def get_deployments(self) -> str: async def disable_git(self) -> str: self.events.append("disable-git") payload = json.loads(self.patched_project or self.project) - config = payload["result"]["source"]["config"] + source = payload["result"]["source"] + if source is None: + self.project = json.dumps(payload) + return self.project + config = source["config"] if self.disable_production: config["production_deployments_enabled"] = False if self.disable_preview: @@ -580,6 +613,144 @@ async def test_should_deploy_one_verified_artifact_in_required_order() -> None: assert evidence.attempt_identity == AttemptIdentity("44", 2) +@pytest.mark.asyncio +async def test_should_deploy_existing_direct_upload_without_git_patch() -> None: + # Given + project = _direct_upload_project_payload() + operations = FakeOperations( + [_deployment_payload("absent"), _deployment_payload()], project=project + ) + + # When + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + + # Then + assert operations.events == [ + "wrangler-preflight", + "get-project", + "get-deployments", + "get-project", + f"upload:{FULL_SHA}", + "get-deployments", + ] + + +@pytest.mark.asyncio +async def test_should_reject_direct_upload_identity_drift_before_upload() -> None: + # Given + operations = FakeOperations( + [_deployment_payload("absent")], + project=_direct_upload_project_payload(), + revalidated_project=_project_payload_with_foreign_domain(), + ) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="target binding"): + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + assert operations.events == [ + "wrangler-preflight", + "get-project", + "get-deployments", + "get-project", + ] + + +@pytest.mark.asyncio +async def test_should_reject_direct_upload_domain_set_drift_before_upload() -> None: + # Given + operations = FakeOperations( + [_deployment_payload("absent")], + project=_direct_upload_project_payload(), + revalidated_project=_direct_upload_project_payload_with_domain_drift(), + ) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="domains changed"): + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + assert not any(event.startswith("upload:") for event in operations.events) + + +@pytest.mark.asyncio +async def test_should_reject_direct_upload_source_added_before_upload() -> None: + # Given + operations = FakeOperations( + [_deployment_payload("absent")], + project=_direct_upload_project_payload(), + revalidated_project=_project_payload(), + ) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="delivery mode changed"): + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + assert not any(event.startswith("upload:") for event in operations.events) + + +@pytest.mark.asyncio +async def test_should_reject_git_source_removed_by_patch() -> None: + # Given + operations = FakeOperations( + [_deployment_payload("absent")], patched_project=_direct_upload_project_payload() + ) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="delivery mode changed"): + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + assert operations.events == [ + "wrangler-preflight", + "get-project", + "get-deployments", + "disable-git", + ] + + +@pytest.mark.asyncio +async def test_should_reject_git_source_removed_before_upload() -> None: + # Given + operations = FakeOperations( + [_deployment_payload("absent")], + revalidated_project=_direct_upload_project_payload(), + ) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="delivery mode changed"): + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + assert operations.events == [ + "wrangler-preflight", + "get-project", + "get-deployments", + "disable-git", + "get-project", + ] + + +@pytest.mark.asyncio +async def test_should_reject_git_domain_set_drift_before_upload() -> None: + # Given + operations = FakeOperations( + [_deployment_payload("absent")], + revalidated_project=_git_project_payload_with_domain_drift(), + ) + + # When / Then + with pytest.raises(CloudflarePolicyError, match="domains changed"): + await deploy_verified_artifact( + operations, object(), _target(), _github_evidence(), AttemptIdentity("44", 2) + ) + assert not any(event.startswith("upload:") for event in operations.events) + + @pytest.mark.asyncio async def test_should_ignore_old_same_sha_and_wait_for_created_id() -> None: old = _deployment("success", "11111111-fccd-4d4a-a28a-cb84f88f6")