From 6b52ed399ba41dd0c1fcc97a897b3e9302611b92 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 13 Aug 2026 00:25:02 -0700 Subject: [PATCH] [eric] outputs: deleting a published app takes it off the internet first, through one shared teardown path (ENG-282) --- backend/apps/outputs/outputs.py | 28 +++-- backend/apps/outputs/release_publication.py | 32 ++++++ .../tests/test_delete_releases_publication.py | 106 ++++++++++++++++++ 3 files changed, 156 insertions(+), 10 deletions(-) create mode 100644 backend/apps/outputs/release_publication.py create mode 100644 backend/tests/test_delete_releases_publication.py diff --git a/backend/apps/outputs/outputs.py b/backend/apps/outputs/outputs.py index 1f1b1e74..76a49ad5 100644 --- a/backend/apps/outputs/outputs.py +++ b/backend/apps/outputs/outputs.py @@ -23,7 +23,8 @@ from backend.apps.outputs.publish_capability import check_publish_capability from backend.apps.outputs.publish_common import slugify, PublishError from backend.apps.outputs.publish_scan import scan_for_publish, quick_ast_gate from backend.apps.outputs.publish_build import build_static, collect_bundle -from backend.apps.outputs.publish_cloud import upload_to_cloud, unpublish_from_cloud +from backend.apps.outputs.publish_cloud import upload_to_cloud +from backend.apps.outputs.release_publication import release_publication from backend.apps.outputs.view_builder_templates import ( VIEW_TEMPLATE_FILES, load_app_builder_skill, @@ -673,6 +674,18 @@ async def delete_orphan(workspace_id: str): @outputs.router.delete("/{output_id}") async def delete_output(output_id: str): output = load(output_id) + # Take the app off the internet BEFORE destroying anything local, and refuse to continue if that + # fails. The record is the only thing that still knows the slug, so deleting first and releasing + # second leaves an app anyone can reach and its owner cannot (measured: delete returned 200 and + # the public URL kept serving byte-identical content). + try: + if await release_publication(output, load_settings()): + save(output) + except PublishError as e: + raise HTTPException( + status_code=502, + detail=f"Couldn't take this app off the internet, so it was not deleted: {e}", + ) # Delete the source tree too. Dropping only the record left the whole app (source, dist, .env) # on disk with nothing in the UI pointing at it: measured 9 orphans and ~0.85GB on one machine, # and a deleted app is supposed to be GONE, not merely hidden. Worse, the orphan recoverer @@ -920,15 +933,10 @@ async def publish_output(body: PublishRequest): async def unpublish_output(body: PublishPreflightRequest): """Take the app offline and clear its publish state.""" output = load(body.output_id) - if output.published_slug: - try: - await unpublish_from_cloud(load_settings(), output.published_slug) - except PublishError as e: - return {"ok": False, "error": str(e)} - output.published_slug = None - output.published_url = None - output.publish_status = None - output.publish_error = None + try: + await release_publication(output, load_settings()) + except PublishError as e: + return {"ok": False, "error": str(e)} save(output) return {"ok": True} diff --git a/backend/apps/outputs/release_publication.py b/backend/apps/outputs/release_publication.py new file mode 100644 index 00000000..c0452e93 --- /dev/null +++ b/backend/apps/outputs/release_publication.py @@ -0,0 +1,32 @@ +"""The one place an app stops being published (ENG-282). + +Deleting an app and unpublishing it are different user actions that must reach the +same end state, and when they were written separately only one of them released the +publication: delete rmtree'd the workspace and dropped the record while the cloud row +stayed active, so the app was live at a URL its owner could no longer see. Measured on +1.7.8-exp.1 against prod, delete returned HTTP 200 and the public URL kept serving +byte-identical content. + +Both paths now go through here, so a third deletion path cannot reintroduce the split. +Callers must treat a raised PublishError as fatal and destroy nothing: the record is +the only thing that still knows the slug. +""" +from typing import Any + +from typeguard import typechecked + +from backend.apps.outputs.models import Output +from backend.apps.outputs.publish_cloud import unpublish_from_cloud + + +@typechecked +async def release_publication(output: Output, settings: Any) -> bool: + """Take the app off the internet and clear its publish state. False when it was never published.""" + if not output.published_slug: + return False + await unpublish_from_cloud(settings, output.published_slug) + output.published_slug = None + output.published_url = None + output.publish_status = None + output.publish_error = None + return True diff --git a/backend/tests/test_delete_releases_publication.py b/backend/tests/test_delete_releases_publication.py new file mode 100644 index 00000000..9d081fe2 --- /dev/null +++ b/backend/tests/test_delete_releases_publication.py @@ -0,0 +1,106 @@ +"""Deleting a published app must take its public URL down (ENG-282). + +Measured live on 1.7.8-exp.1 against prod before this test existed. Same app, +same URL, same probe, minutes apart, only the endpoint differing: + + unpublish -> HTTP 200 marker=1 becomes HTTP 404 marker=0 + delete -> HTTP 200 marker=1 becomes HTTP 200 marker=1 + +`delete_output` stopped every runtime, rmtree'd the workspace and removed the +record, and never once looked at `published_slug`. The record holding that slug +was destroyed first, so the app ended up reachable by anyone with the link and +unreachable by its owner. Pressing Delete published your app forever. + +The seal is ordering plus a single owner: release the publication BEFORE any +local destruction, and refuse to destroy anything if the release failed, so +"deleted locally, live publicly" cannot be reached from any deletion path. + +Run: + backend/.venv/bin/python -m pytest backend/tests/test_delete_releases_publication.py -v +""" + +import os +from typing import Any, List + +import pytest +from fastapi import HTTPException + +from backend.apps.outputs import outputs as outputs_module +from backend.apps.outputs import release_publication as release_module +from backend.apps.outputs.models import Output +from backend.apps.outputs.publish_common import PublishError + + +class P_Recorder: + """Stands in for the cloud so the test never touches the network.""" + + def __init__(self, fail: bool = False) -> None: + self.slugs: List[str] = [] + self.fail = fail + + async def __call__(self, settings: Any, slug: str) -> None: + self.slugs.append(slug) + if self.fail: + raise PublishError("cloud said no") + + +def p_make_output(tmp_path: Any, published: bool) -> Output: + out = Output(name="probe", description="", files={"index.html": "

x

"}) + if published: + out.published_slug = "probe-slug" + out.published_url = "https://probe-slug.openswarm.host" + outputs_module.save(out) + return out + + +@pytest.fixture +def p_isolated(tmp_path: Any, monkeypatch: Any) -> Any: + monkeypatch.setattr(outputs_module, "DATA_DIR", str(tmp_path), raising=False) + os.makedirs(str(tmp_path), exist_ok=True) + return tmp_path + + +@pytest.mark.asyncio +async def test_deleting_a_published_app_releases_the_publication(p_isolated: Any, monkeypatch: Any) -> None: + """The bug, stated as an assertion. Fails before the fix: zero slugs released.""" + rec = P_Recorder() + monkeypatch.setattr(release_module, "unpublish_from_cloud", rec) + out = p_make_output(p_isolated, published=True) + + await outputs_module.delete_output(out.id) + + assert rec.slugs == ["probe-slug"], ( + f"delete never released the publication; cloud saw {rec.slugs}. " + "The app is now live at a URL its owner can no longer reach." + ) + + +@pytest.mark.asyncio +async def test_a_failed_release_leaves_the_record_intact(p_isolated: Any, monkeypatch: Any) -> None: + """Ordering is the other half. If the release fails, nothing local may be destroyed, + because the record is the only thing that still knows the slug.""" + rec = P_Recorder(fail=True) + monkeypatch.setattr(release_module, "unpublish_from_cloud", rec) + out = p_make_output(p_isolated, published=True) + + with pytest.raises(HTTPException) as caught: + await outputs_module.delete_output(out.id) + assert caught.value.status_code >= 400 + + still_there = outputs_module.load(out.id) + assert still_there.published_slug == "probe-slug", ( + "the record was destroyed after a failed release, so the slug is unrecoverable" + ) + + +@pytest.mark.asyncio +async def test_an_unpublished_app_still_deletes_cleanly(p_isolated: Any, monkeypatch: Any) -> None: + """The other direction, so a fix that merely blocks deletion is caught too.""" + rec = P_Recorder() + monkeypatch.setattr(release_module, "unpublish_from_cloud", rec) + out = p_make_output(p_isolated, published=False) + + await outputs_module.delete_output(out.id) + + assert rec.slugs == [], "an unpublished app must not call the cloud at all" + assert not os.path.exists(os.path.join(str(p_isolated), f"{out.id}.json"))