From b6543c9c5547fa05433d37f5721d1e4ce764f412 Mon Sep 17 00:00:00 2001
From: ZdenekSrotyr <zdenek.srotyr@keboola.com>
Date: Tue, 5 May 2026 19:47:12 +0200
Subject: [PATCH] =?UTF-8?q?fix:=20Devin=20Review=20on=20#194=20=E2=80=94?=
 =?UTF-8?q?=202=20BUG-class=20findings?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

1. .env_overlay write paths now match read path under STATE_DIR.
   app/main.py:343 reads via _state_dir() (post-PR #194), but two
   write sites still hardcoded ${DATA_DIR}/state/.env_overlay:
     - app/api/admin.py:2687 — configure endpoint secrets persistence
     - app/api/marketplaces.py:152 — marketplace PAT persistence
   Under flat-mount layout (STATE_DIR=/data-state) the admin UI wrote
   secrets to /data/state/.env_overlay while the app read from
   /data-state/.env_overlay, silently dropping the value on next
   restart. Both write sites now go through _state_dir().

2. host-mount.yml: caddy inherits data:/srv:ro from base, but with
   no service populating the data: named volume (other services
   switched to direct /data binds), the inherited mount points at an
   empty Docker volume — try_files finds nothing, every parquet
   download falls through to uvicorn, defeating the v0.36.0
   file_server bypass under the host-mount layout. Added a caddy
   override that restates all mounts including a direct /data:/srv:ro
   bind. Mirrors the comment + treatment already in flat-mount.yml.
---
 app/api/admin.py              |  9 +++++++--
 app/api/marketplaces.py       | 14 +++++++++++---
 docker-compose.host-mount.yml | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/app/api/admin.py b/app/api/admin.py
index fe43805..7fd1f82 100644
--- a/app/api/admin.py
+++ b/app/api/admin.py
@@ -2683,8 +2683,13 @@ async def configure_instance(
         secrets_to_persist["KEBOOLA_STACK_URL"] = request.keboola_url
 
     if secrets_to_persist:
-        data_dir = Path(os.environ.get("DATA_DIR", "./data"))
-        overlay_path = data_dir / "state" / ".env_overlay"
+        # Resolve via _state_dir() so the path matches app/main.py's
+        # startup-time read of the same overlay. Without this, an operator
+        # on the flat-mount layout (STATE_DIR=/data-state) would write
+        # secrets to /data/state/.env_overlay here while the app reads
+        # from /data-state/.env_overlay — silent loss on next restart.
+        from app.secrets import _state_dir
+        overlay_path = _state_dir() / ".env_overlay"
         overlay_path.parent.mkdir(parents=True, exist_ok=True)
 
         # Merge with existing overlay
diff --git a/app/api/marketplaces.py b/app/api/marketplaces.py
index b802726..f910259 100644
--- a/app/api/marketplaces.py
+++ b/app/api/marketplaces.py
@@ -147,9 +147,17 @@ def _token_env_name(slug: str) -> str:
 
 
 def _persist_token(env_name: str, value: str) -> None:
-    """Write (or update) a single key in data/state/.env_overlay and os.environ."""
-    data_dir = Path(os.environ.get("DATA_DIR", "./data"))
-    overlay_path = data_dir / "state" / ".env_overlay"
+    """Write (or update) a single key in ``${STATE_DIR}/.env_overlay`` and ``os.environ``.
+
+    Path resolution matches ``app/main.py``'s startup-time read; without
+    this alignment, marketplace PATs persisted under the flat-mount
+    layout (``STATE_DIR=/data-state``) would land at
+    ``/data/state/.env_overlay`` while the app reads from
+    ``/data-state/.env_overlay``, silently dropping the token on the
+    next restart.
+    """
+    from app.secrets import _state_dir
+    overlay_path = _state_dir() / ".env_overlay"
     overlay_path.parent.mkdir(parents=True, exist_ok=True)
 
     existing: dict[str, str] = {}
diff --git a/docker-compose.host-mount.yml b/docker-compose.host-mount.yml
index af03c33..fa1bb27 100644
--- a/docker-compose.host-mount.yml
+++ b/docker-compose.host-mount.yml
@@ -75,3 +75,22 @@ services:
   ws-gateway:
     volumes: !override
       - /data:/data
+
+  caddy:
+    # Caddy was originally inheriting `data:/srv:ro` from the base
+    # service. Once the other services switch to direct binds and
+    # nothing populates the `data:` named volume, that inherited
+    # mount points at an empty Docker-managed volume — and the
+    # @download `try_files /bigquery/data/<id>.parquet …` block
+    # in Caddyfile finds nothing, so every parquet download falls
+    # through to the app's uvicorn worker, defeating the v0.36.0
+    # file_server bypass.
+    #
+    # Restate every mount the base caddy service depends on; mirror
+    # the same caveat that lives in flat-mount.yml.
+    volumes: !override
+      - ./Caddyfile:/etc/caddy/Caddyfile:ro
+      - /data/state/certs:/certs:ro
+      - caddy_data:/data
+      - caddy_config:/config
+      - /data:/srv:ro