Deploy 1bd20861 from GitHub Actions

app/api/v1/upload.py

@@ -119,6 +119,32 @@ async def upload_document(
             pass
         raise
 
+    # Mirror the bytes into durable storage. UPLOAD_DIR is on the container
+    # filesystem, which is wiped on every restart in the deployed environment,
+    # so the local copy alone is a cache rather than storage. A failure here is
+    # logged and tolerated: the upload still works for this container's
+    # lifetime, and rejecting it outright would be a worse outcome than storing
+    # it with reduced durability.
+    from app.services import object_storage
+
+    if object_storage.is_configured():
+        with open(local_path, "rb") as f:
+            stored = await object_storage.put_object(
+                local_filename, f.read(), content_type
+            )
+        if not stored:
+            logger.warning(
+                "Upload %s is on local disk only and will not survive a "
+                "restart: durable storage write failed.",
+                local_filename,
+            )
+    else:
+        logger.warning(
+            "Durable storage is not configured; upload %s will be lost when "
+            "the container restarts. Set SUPABASE_URL and SUPABASE_SERVICE_KEY.",
+            local_filename,
+        )
+
     # Internal storage reference — resolved only against UPLOAD_DIR. Storing a
     # bare key rather than an absolute path keeps the row portable across
     # containers and gives the SSRF guard a value it can safely confine.

app/config.py

@@ -150,6 +150,32 @@ class Settings(BaseSettings):
         description="Root directory for locally stored uploads. All storage:// "
         "URLs resolve inside this directory and may not escape it.",
     )
+    # ── Durable object storage ────────────────────────────────────────────────
+    # UPLOAD_DIR lives on the container filesystem, which is ephemeral on
+    # HuggingFace Spaces: a restart returns it to the state baked into the
+    # image, i.e. empty, while the Postgres rows describing those files
+    # survive. Documents then appear healthy until something needs the
+    # original bytes, at which point extraction fails with "Stored file no
+    # longer exists". Setting these moves the bytes somewhere that outlives
+    # the container; leaving them unset keeps the previous local-disk
+    # behaviour so local development needs no cloud credentials.
+    SUPABASE_URL: str = Field(
+        default="",
+        description="Supabase project URL, e.g. https://xxxx.supabase.co. "
+        "Enables durable upload storage when set together with "
+        "SUPABASE_SERVICE_KEY.",
+    )
+    SUPABASE_SERVICE_KEY: str = Field(
+        default="",
+        description="Supabase service role key. The uploads bucket is private "
+        "and only this backend reads it, so the anon key is not sufficient.",
+    )
+    SUPABASE_STORAGE_BUCKET: str = Field(
+        default="documents",
+        description="Bucket name for uploaded documents. Create it as a "
+        "private bucket in the Supabase dashboard.",
+    )
+
     MAX_FILE_SIZE_MB: int = Field(default=50)
     ALLOWED_FILE_TYPES: List[str] = Field(
         default=[

app/core/url_guard.py

@@ -239,3 +239,30 @@ def resolve_storage_path(file_url: str) -> str:
 def is_internal_storage_url(file_url: str) -> bool:
     """True if this URL refers to a file this backend stored itself."""
     return urlparse(file_url).scheme in (STORAGE_SCHEME, "file")
+
+
+def storage_key_from_url(file_url: str) -> str:
+    """
+    Extract the bare object key from an internal storage URL.
+
+    This is the name used both for the file inside UPLOAD_DIR and for the
+    object in durable storage, so the two stay addressable by the same value.
+    Legacy file:// rows hold an absolute path; only its basename is meaningful
+    as a key.
+    """
+    parsed = urlparse(file_url)
+
+    if parsed.scheme == STORAGE_SCHEME:
+        raw = f"{parsed.netloc}{parsed.path}"
+    elif parsed.scheme == "file":
+        raw = parsed.path
+        if os.name == "nt" and raw.startswith("/"):
+            raw = raw[1:]
+    else:
+        raise UnsafeURLError(f"Not an internal storage URL: {parsed.scheme}")
+
+    key = os.path.basename(raw)
+    if not key:
+        raise UnsafeURLError("Storage URL is missing a key")
+
+    return key

app/services/extractors.py

@@ -19,14 +19,61 @@ import httpx
 
 from app.config import settings
 from app.core.url_guard import (
+    UnsafeURLError,
     fetch_remote_file,
     is_internal_storage_url,
     resolve_storage_path,
+    storage_key_from_url,
 )
 
 logger = logging.getLogger(__name__)
 
 
+async def _restore_from_durable_storage(file_url: str, max_bytes: int) -> bytes:
+    """
+    Recover a document whose local copy has been lost, and re-cache it.
+
+    Raises UnsafeURLError with the original message when no durable copy
+    exists, so callers upstream see the same failure they always did.
+    """
+    from app.services import object_storage
+
+    key = storage_key_from_url(file_url)
+
+    if not object_storage.is_configured():
+        raise UnsafeURLError(
+            "Stored file no longer exists, and durable storage is not "
+            "configured. Set SUPABASE_URL and SUPABASE_SERVICE_KEY so uploads "
+            "survive a container restart."
+        )
+
+    data = await object_storage.get_object(key)
+    if data is None:
+        raise UnsafeURLError("Stored file no longer exists")
+
+    if len(data) > max_bytes:
+        raise UnsafeURLError("Stored file exceeds the maximum allowed size")
+
+    logger.info("Restored %s from durable storage after local copy was lost", key)
+
+    # Repopulate the local cache; failure here is not fatal because the bytes
+    # needed for this request are already in hand.
+    try:
+        os.makedirs(settings.UPLOAD_DIR, exist_ok=True)
+        await asyncio.to_thread(
+            _write_bytes, os.path.join(settings.UPLOAD_DIR, key), data
+        )
+    except OSError as exc:
+        logger.warning("Could not re-cache %s locally: %s", key, exc)
+
+    return data
+
+
+def _write_bytes(path: str, data: bytes) -> None:
+    with open(path, "wb") as f:
+        f.write(data)
+
+
 def _read_capped(path: str, max_bytes: int) -> bytes:
     """Read a local file, refusing anything over the size ceiling."""
     if os.path.getsize(path) > max_bytes:
@@ -311,8 +358,14 @@ async def download_and_extract(file_url: str, file_type: str) -> ExtractedDocume
 
     if is_internal_storage_url(file_url):
         # Path is confined to UPLOAD_DIR by the guard; traversal is rejected.
-        local_path = await asyncio.to_thread(resolve_storage_path, file_url)
-        file_bytes = await asyncio.to_thread(_read_capped, local_path, max_bytes)
+        try:
+            local_path = await asyncio.to_thread(resolve_storage_path, file_url)
+            file_bytes = await asyncio.to_thread(_read_capped, local_path, max_bytes)
+        except UnsafeURLError:
+            # The local copy is gone — the usual cause is a container restart
+            # wiping UPLOAD_DIR. Fall back to the durable copy and repopulate
+            # the local cache so subsequent reads are local again.
+            file_bytes = await _restore_from_durable_storage(file_url, max_bytes)
     else:
         # User-supplied URL: https only, public addresses only, size-capped.
         file_bytes = await fetch_remote_file(file_url, max_bytes=max_bytes)

app/services/object_storage.py

@@ -0,0 +1,150 @@
+"""
+Durable object storage for uploaded documents.
+
+Why this exists
+---------------
+Uploads were written to UPLOAD_DIR on the container's own filesystem. That
+filesystem is ephemeral on HuggingFace Spaces: every rebuild or restart starts
+from the image, so the directory comes back empty. The database rows survive
+(Postgres is external), which makes the failure especially confusing — the
+document still lists, still shows its analysis, and still answers questions
+from its stored embeddings, right up until something needs the original bytes
+again. Then:
+
+    app.core.url_guard.UnsafeURLError: Stored file no longer exists
+
+That is exactly what happened in production: a deploy restarted the Space,
+every uploaded PDF went with it, and the next re-analysis destroyed the
+document's chunks before discovering the source was gone.
+
+The fix is to keep the bytes somewhere that outlives the container. Supabase
+is already a dependency of this project for Postgres, and its Storage API is
+plain HTTP, so this needs no new package — just httpx, which is already here.
+
+Degrading gracefully
+--------------------
+If SUPABASE_URL / SUPABASE_SERVICE_KEY are not set, every function here
+reports "not configured" and callers fall back to local disk exactly as
+before. Nothing breaks; durability is simply not gained. This keeps local
+development and the test suite working without cloud credentials.
+"""
+
+import logging
+from typing import Optional
+
+import httpx
+
+from app.config import settings
+
+logger = logging.getLogger(__name__)
+
+# Uploads are capped at MAX_FILE_SIZE_MB, so a generous per-request timeout is
+# still bounded. Cold Supabase connections occasionally take a few seconds.
+_TIMEOUT = httpx.Timeout(30.0, connect=10.0)
+
+
+def is_configured() -> bool:
+    """True when a durable bucket is available to store objects in."""
+    return bool(settings.SUPABASE_URL and settings.SUPABASE_SERVICE_KEY)
+
+
+def _object_url(key: str) -> str:
+    base = settings.SUPABASE_URL.rstrip("/")
+    bucket = settings.SUPABASE_STORAGE_BUCKET
+    return f"{base}/storage/v1/object/{bucket}/{key}"
+
+
+def _headers() -> dict:
+    # The service key is used rather than the anon key: this bucket is private
+    # and the backend is the only thing that should read or write it.
+    return {
+        "Authorization": f"Bearer {settings.SUPABASE_SERVICE_KEY}",
+        "apikey": settings.SUPABASE_SERVICE_KEY,
+    }
+
+
+async def put_object(key: str, data: bytes, content_type: str) -> bool:
+    """
+    Store bytes under `key`. Returns True on success.
+
+    Failure is logged and reported, never raised: an upload that reached local
+    disk is still usable for the current container's lifetime, and refusing the
+    whole upload because the durable copy failed would be a worse outcome than
+    storing it with reduced durability.
+    """
+    if not is_configured():
+        return False
+
+    try:
+        async with httpx.AsyncClient(timeout=_TIMEOUT) as client:
+            response = await client.post(
+                _object_url(key),
+                content=data,
+                headers={
+                    **_headers(),
+                    "Content-Type": content_type,
+                    # Objects are keyed by UUID and never rewritten, so an
+                    # existing key means a retry of the same upload.
+                    "x-upsert": "true",
+                },
+            )
+
+        if response.status_code in (200, 201):
+            logger.info("Stored %s in durable storage (%d bytes)", key, len(data))
+            return True
+
+        logger.error(
+            "Durable storage rejected %s: %s %s",
+            key,
+            response.status_code,
+            response.text[:200],
+        )
+        return False
+
+    except Exception as exc:
+        logger.error("Durable storage write failed for %s: %s", key, exc)
+        return False
+
+
+async def get_object(key: str) -> Optional[bytes]:
+    """
+    Fetch bytes for `key`, or None when unavailable.
+
+    None covers both "not configured" and "not found", because the caller does
+    the same thing in either case: fall back to local disk and, failing that,
+    report the file as missing.
+    """
+    if not is_configured():
+        return None
+
+    try:
+        async with httpx.AsyncClient(timeout=_TIMEOUT) as client:
+            response = await client.get(_object_url(key), headers=_headers())
+
+        if response.status_code == 200:
+            return response.content
+
+        if response.status_code != 404:
+            logger.error(
+                "Durable storage read failed for %s: %s %s",
+                key,
+                response.status_code,
+                response.text[:200],
+            )
+        return None
+
+    except Exception as exc:
+        logger.error("Durable storage read failed for %s: %s", key, exc)
+        return None
+
+
+async def delete_object(key: str) -> None:
+    """Best-effort removal, so deleting a document does not leak storage."""
+    if not is_configured():
+        return
+
+    try:
+        async with httpx.AsyncClient(timeout=_TIMEOUT) as client:
+            await client.delete(_object_url(key), headers=_headers())
+    except Exception as exc:
+        logger.warning("Durable storage delete failed for %s: %s", key, exc)

tests/unit/test_object_storage.py

@@ -0,0 +1,175 @@
+"""
+Unit tests for durable upload storage.
+
+These exist because of a production incident. Uploads were written only to
+UPLOAD_DIR, which lives on the container filesystem. HuggingFace Spaces
+restarts the container on every deploy and the filesystem comes back as it was
+baked into the image — empty — while the Postgres rows describing those files
+survive. A document therefore kept listing, kept showing its analysis and kept
+answering questions from its stored embeddings, right up until something asked
+for the original bytes:
+
+    app.core.url_guard.UnsafeURLError: Stored file no longer exists
+
+A re-analysis then destroyed the document's chunks before discovering the
+source was gone, which turned a recoverable situation into data loss.
+"""
+
+import os
+
+import pytest
+
+from app.core.url_guard import UnsafeURLError, storage_key_from_url
+from app.services import object_storage
+
+pytestmark = pytest.mark.unit
+
+
+# ── Key extraction ─────────────────────────────────────────────────────────────
+
+
+def test_storage_key_from_storage_url():
+    assert storage_key_from_url("storage://abc123.pdf") == "abc123.pdf"
+
+
+def test_storage_key_from_legacy_file_url():
+    """Older rows hold an absolute path; only the basename is a usable key."""
+    assert storage_key_from_url("file:///app/uploads/abc123.pdf") == "abc123.pdf"
+
+
+def test_storage_key_rejects_external_url():
+    with pytest.raises(UnsafeURLError):
+        storage_key_from_url("https://example.com/abc123.pdf")
+
+
+def test_storage_key_rejects_empty_key():
+    with pytest.raises(UnsafeURLError):
+        storage_key_from_url("storage://")
+
+
+# ── Configuration gating ───────────────────────────────────────────────────────
+
+
+def test_not_configured_without_credentials(monkeypatch):
+    monkeypatch.setattr(object_storage.settings, "SUPABASE_URL", "")
+    monkeypatch.setattr(object_storage.settings, "SUPABASE_SERVICE_KEY", "")
+    assert object_storage.is_configured() is False
+
+
+def test_not_configured_with_only_a_url(monkeypatch):
+    """A URL without a key cannot authenticate, so it is not usable."""
+    monkeypatch.setattr(
+        object_storage.settings, "SUPABASE_URL", "https://x.supabase.co"
+    )
+    monkeypatch.setattr(object_storage.settings, "SUPABASE_SERVICE_KEY", "")
+    assert object_storage.is_configured() is False
+
+
+def test_configured_with_both(monkeypatch):
+    monkeypatch.setattr(
+        object_storage.settings, "SUPABASE_URL", "https://x.supabase.co"
+    )
+    monkeypatch.setattr(object_storage.settings, "SUPABASE_SERVICE_KEY", "svc-key")
+    assert object_storage.is_configured() is True
+
+
+@pytest.mark.asyncio
+async def test_unconfigured_calls_are_inert(monkeypatch):
+    """
+    With no credentials the module must not attempt any network call, so local
+    development and CI keep working exactly as before.
+    """
+    monkeypatch.setattr(object_storage.settings, "SUPABASE_URL", "")
+    monkeypatch.setattr(object_storage.settings, "SUPABASE_SERVICE_KEY", "")
+
+    assert await object_storage.put_object("k.pdf", b"data", "application/pdf") is False
+    assert await object_storage.get_object("k.pdf") is None
+    await object_storage.delete_object("k.pdf")  # must not raise
+
+
+# ── Recovery after the local copy is lost ──────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_missing_local_file_is_restored_from_durable_storage(
+    monkeypatch, tmp_path
+):
+    """The exact scenario that lost a document in production."""
+    from app.services import extractors
+
+    monkeypatch.setattr(extractors.settings, "UPLOAD_DIR", str(tmp_path))
+    monkeypatch.setattr(object_storage, "is_configured", lambda: True)
+
+    async def fake_get(key):
+        assert key == "abc123.pdf"
+        return b"%PDF-1.4 recovered"
+
+    monkeypatch.setattr(object_storage, "get_object", fake_get)
+
+    data = await extractors._restore_from_durable_storage(
+        "storage://abc123.pdf", max_bytes=1024
+    )
+
+    assert data == b"%PDF-1.4 recovered"
+    # The local cache is repopulated so the next read does not go over the wire.
+    assert (tmp_path / "abc123.pdf").read_bytes() == b"%PDF-1.4 recovered"
+
+
+@pytest.mark.asyncio
+async def test_missing_everywhere_reports_the_original_failure(monkeypatch, tmp_path):
+    from app.services import extractors
+
+    monkeypatch.setattr(extractors.settings, "UPLOAD_DIR", str(tmp_path))
+    monkeypatch.setattr(object_storage, "is_configured", lambda: True)
+
+    async def fake_get(key):
+        return None
+
+    monkeypatch.setattr(object_storage, "get_object", fake_get)
+
+    with pytest.raises(UnsafeURLError, match="no longer exists"):
+        await extractors._restore_from_durable_storage(
+            "storage://gone.pdf", max_bytes=1024
+        )
+
+
+@pytest.mark.asyncio
+async def test_unconfigured_recovery_explains_how_to_fix_it(monkeypatch, tmp_path):
+    """
+    An operator reading this error should learn why the file vanished and what
+    to set so it stops happening, not just that it is absent.
+    """
+    from app.services import extractors
+
+    monkeypatch.setattr(extractors.settings, "UPLOAD_DIR", str(tmp_path))
+    monkeypatch.setattr(object_storage, "is_configured", lambda: False)
+
+    with pytest.raises(UnsafeURLError) as exc:
+        await extractors._restore_from_durable_storage(
+            "storage://gone.pdf", max_bytes=1024
+        )
+
+    message = str(exc.value)
+    assert "SUPABASE_URL" in message
+    assert "restart" in message
+
+
+@pytest.mark.asyncio
+async def test_oversized_restored_file_is_rejected(monkeypatch, tmp_path):
+    """The size cap must hold on the recovery path too, not just on upload."""
+    from app.services import extractors
+
+    monkeypatch.setattr(extractors.settings, "UPLOAD_DIR", str(tmp_path))
+    monkeypatch.setattr(object_storage, "is_configured", lambda: True)
+
+    async def fake_get(key):
+        return b"x" * 5000
+
+    monkeypatch.setattr(object_storage, "get_object", fake_get)
+
+    with pytest.raises(UnsafeURLError, match="maximum allowed size"):
+        await extractors._restore_from_durable_storage(
+            "storage://big.pdf", max_bytes=1024
+        )
+
+    assert not os.path.exists(tmp_path / "big.pdf")