fix: enforce shared storage permissions

bot_bottle/backend/cleanup_control.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import os
 import shutil
 import subprocess
 from collections.abc import Sequence
@@ -19,11 +20,19 @@ class CleanupFailures:
         self._messages: list[str] = []
 
     def run(self, argv: Sequence[str], description: str) -> None:
+        raw_timeout = os.environ.get(
+            "BOT_BOTTLE_CLEANUP_COMMAND_TIMEOUT_SECONDS", "120",
+        )
+        try:
+            timeout = float(raw_timeout)
+        except ValueError:
+            timeout = 120.0
         try:
             result = subprocess.run(
                 list(argv), capture_output=True, text=True, check=False,
+                timeout=max(timeout, 1.0),
             )
-        except OSError as exc:
+        except (OSError, subprocess.SubprocessError) as exc:
             self._messages.append(f"{description}: {exc}")
             return
         if result.returncode != 0:
@@ -40,6 +49,9 @@ class CleanupFailures:
         except OSError as exc:
             self._messages.append(f"{description}: {exc}")
 
+    def record(self, message: str) -> None:
+        self._messages.append(message)
+
     def raise_if_any(self) -> None:
         if self._messages:
             raise CleanupError("; ".join(self._messages))

bot_bottle/backend/firecracker/cleanup.py

@@ -29,7 +29,7 @@ from pathlib import Path
 
 from ...log import info
 from .. import EnumerationError
-from ..cleanup_control import CleanupFailures
+from ..cleanup_control import CleanupError, CleanupFailures
 from . import lifecycle_lock, util
 from .bottle_cleanup_plan import FirecrackerBottleCleanupPlan
 
@@ -153,7 +153,10 @@ def cleanup(plan: FirecrackerBottleCleanupPlan) -> None:
         approved_dirs = set(plan.run_dirs).intersection(fresh.run_dirs)
         failures = CleanupFailures()
         for pid in sorted(approved_pids):
-            _terminate_orphan(pid, _run_root())
+            try:
+                _terminate_orphan(pid, _run_root())
+            except CleanupError as exc:
+                failures.record(str(exc))
         for path in sorted(approved_dirs):
             info(f"rm -rf {path}")
             failures.remove_tree(Path(path), f"removing Firecracker run dir {path}")
@@ -184,6 +187,13 @@ def _terminate_orphan(pid: int, run_root: Path) -> None:
         if run_dir is None or run_dir.is_dir():
             return
         info(f"kill firecracker VM pid {pid}")
-        signal.pidfd_send_signal(pidfd, signal.SIGTERM)
+        try:
+            signal.pidfd_send_signal(pidfd, signal.SIGTERM)
+        except ProcessLookupError:
+            return
+        except OSError as exc:
+            raise CleanupError(
+                f"could not signal Firecracker pid {pid}: {exc}"
+            ) from exc
     finally:
         os.close(pidfd)

bot_bottle/backend/provision_gateway.py

@@ -63,12 +63,23 @@ def provision_git_gate(
     transport.exec(["chmod", "+x", "/etc/git-gate/access-hook"])
     creds = _creds_dir(bottle_id)
     transport.exec(["mkdir", "-p", creds])
+    transport.exec(["chmod", "700", creds])
+    credential_paths: list[str] = []
     for u in plan.upstreams:
         if u.identity_file:
-            transport.cp_into(u.identity_file, f"{creds}/{u.name}-key")
+            key_path = f"{creds}/{u.name}-key"
+            transport.cp_into(u.identity_file, key_path)
+            credential_paths.append(key_path)
         known_hosts = str(u.known_hosts_file)
         if known_hosts and known_hosts != ".":
-            transport.cp_into(known_hosts, f"{creds}/{u.name}-known_hosts")
+            known_hosts_path = f"{creds}/{u.name}-known_hosts"
+            transport.cp_into(known_hosts, known_hosts_path)
+            credential_paths.append(known_hosts_path)
+    # Copy-mode behavior differs across Docker, Apple Container, and SSH.
+    # Apply the security contract inside the gateway so every backend produces
+    # the same private credential namespace.
+    if credential_paths:
+        transport.exec(["chmod", "600", *credential_paths])
     # Init the bare repos + per-repo credential config for this namespace.
     script = git_gate_render_provision(bottle_id, plan.upstreams)
     transport.exec(["sh", "-c", script])

bot_bottle/gateway/bounded_http.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import http.server
+import io
 import socket
 import threading
 import time
@@ -14,6 +15,10 @@ class Readable(Protocol):
     def read(self, size: int = -1, /) -> bytes: ...
 
 
+class Writable(Protocol):
+    def write(self, data: bytes, /) -> object: ...
+
+
 @dataclass(frozen=True)
 class BodyReadError(Exception):
     status: int
@@ -30,6 +35,25 @@ def read_declared_body(
     require_length: bool,
 ) -> bytes:
     """Validate and read exactly one declared body under a read deadline."""
+    output = io.BytesIO()
+    copy_declared_body(
+        stream, output, connection, raw_length, maximum=maximum,
+        timeout_seconds=timeout_seconds, require_length=require_length,
+    )
+    return output.getvalue()
+
+
+def copy_declared_body(
+    stream: Readable,
+    output: Writable,
+    connection: socket.socket,
+    raw_length: str | None,
+    *,
+    maximum: int,
+    timeout_seconds: float,
+    require_length: bool,
+) -> int:
+    """Copy one declared body to a sink without retaining it in memory."""
     if raw_length is None:
         if require_length:
             raise BodyReadError(411, "Content-Length required")
@@ -44,7 +68,6 @@ def read_declared_body(
         raise BodyReadError(413, "request body too large")
     previous_timeout = connection.gettimeout()
     deadline = time.monotonic() + timeout_seconds
-    chunks: list[bytes] = []
     remaining = length
     try:
         while remaining:
@@ -55,13 +78,13 @@ def read_declared_body(
             chunk = stream.read(min(remaining, 64 * 1024))
             if not chunk:
                 raise BodyReadError(400, "incomplete request body")
-            chunks.append(chunk)
+            output.write(chunk)
             remaining -= len(chunk)
     except TimeoutError as exc:
         raise BodyReadError(408, "request body read timed out") from exc
     finally:
         connection.settimeout(previous_timeout)
-    return b"".join(chunks)
+    return length
 
 
 class BoundedThreadingHTTPServer(http.server.ThreadingHTTPServer):
@@ -106,4 +129,9 @@ class BoundedThreadingHTTPServer(http.server.ThreadingHTTPServer):
             self._request_slots.release()
 
 
-__all__ = ["BodyReadError", "BoundedThreadingHTTPServer", "read_declared_body"]
+__all__ = [
+    "BodyReadError",
+    "BoundedThreadingHTTPServer",
+    "copy_declared_body",
+    "read_declared_body",
+]

bot_bottle/gateway/git_gate/http_backend.py

@@ -21,6 +21,8 @@ from __future__ import annotations
 import os
 import subprocess
 import sys
+import tempfile
+import threading
 import typing
 from http.server import BaseHTTPRequestHandler
 from pathlib import Path
@@ -30,7 +32,7 @@ from bot_bottle.constants import GIT_GATE_TIMEOUT_SECS, IDENTITY_HEADER
 from bot_bottle.gateway.bounded_http import (
     BodyReadError,
     BoundedThreadingHTTPServer,
-    read_declared_body,
+    copy_declared_body,
 )
 from bot_bottle.gateway.policy_resolver import PolicyResolveError, PolicyResolver
 
@@ -84,6 +86,8 @@ def resolve_sandbox_root(
 MAX_BODY_BYTES = 100 * 1024 * 1024
 REQUEST_BODY_TIMEOUT_SECONDS = 30.0
 MAX_REQUEST_WORKERS = 16
+MAX_BODY_WORKERS = 2
+_BODY_WORK_SLOTS = threading.BoundedSemaphore(MAX_BODY_WORKERS)
 
 
 class GitHttpHandler(BaseHTTPRequestHandler):
@@ -191,31 +195,40 @@ class GitHttpHandler(BaseHTTPRequestHandler):
             value = self.headers.get(header)
             if value:
                 env[variable] = value
-        try:
+        if not _BODY_WORK_SLOTS.acquire(blocking=False):
-            body = read_declared_body(
+            self.send_error(503, "git request capacity exhausted")
-                self.rfile,
-                self.connection,
-                self.headers.get("content-length"),
-                maximum=MAX_BODY_BYTES,
-                timeout_seconds=REQUEST_BODY_TIMEOUT_SECONDS,
-                require_length=False,
-            )
-        except BodyReadError as exc:
-            self.send_error(exc.status, exc.message)
             return
         try:
-            proc = subprocess.run(
+            with tempfile.TemporaryFile() as body:
-                ["git", "http-backend"],
+                try:
-                input=body,
+                    copy_declared_body(
-                env=env,
+                        self.rfile,
-                capture_output=True,
+                        body,
-                check=False,
+                        self.connection,
-                timeout=GIT_GATE_TIMEOUT_SECS,
+                        self.headers.get("content-length"),
-            )
+                        maximum=MAX_BODY_BYTES,
-        except (OSError, subprocess.SubprocessError) as exc:
+                        timeout_seconds=REQUEST_BODY_TIMEOUT_SECONDS,
-            self.log_message("git http-backend unavailable: %s", exc)
+                        require_length=False,
-            self.send_error(503, "git backend unavailable")
+                    )
-            return
+                except BodyReadError as exc:
+                    self.send_error(exc.status, exc.message)
+                    return
+                body.seek(0)
+                try:
+                    proc = subprocess.run(
+                        ["git", "http-backend"],
+                        stdin=body,
+                        env=env,
+                        capture_output=True,
+                        check=False,
+                        timeout=GIT_GATE_TIMEOUT_SECS,
+                    )
+                except (OSError, subprocess.SubprocessError) as exc:
+                    self.log_message("git http-backend unavailable: %s", exc)
+                    self.send_error(503, "git backend unavailable")
+                    return
+        finally:
+            _BODY_WORK_SLOTS.release()
         self._write_cgi_response(proc.stdout)
 
     def _repo_dir(self, sandbox_root: Path, path: str) -> Path | None:

bot_bottle/store/db_store.py

@@ -2,7 +2,9 @@
 
 from __future__ import annotations
 
+import os
 import sqlite3
+import stat
 from contextlib import contextmanager
 from pathlib import Path
 
@@ -19,9 +21,45 @@ class DbStore:
     def __init__(self, db_path: Path, migrations: TableMigrations) -> None:
         self.db_path = db_path
         self._migrations = migrations
-        self.db_path.parent.mkdir(parents=True, exist_ok=True)
+        self._secure_parent()
+        if self.db_path.exists():
+            self._chmod()
+
+    def _secure_parent(self) -> None:
+        """Create and verify the private parent directory."""
+        parent = self.db_path.parent
+        parent.mkdir(mode=0o700, parents=True, exist_ok=True)
+        parent.chmod(0o700)
+        if stat.S_IMODE(parent.stat().st_mode) != 0o700:
+            raise PermissionError(f"database directory is not mode 0700: {parent}")
+
+    def _secure_db_file(self) -> None:
+        """Create the database without a permissive filesystem window.
+
+        SQLite otherwise creates a missing database using the process umask.
+        This store contains control-plane identity tokens, so both creation and
+        repair are fail-closed rather than best-effort.
+        """
+        try:
+            fd = os.open(
+                self.db_path,
+                os.O_WRONLY | os.O_CREAT | os.O_EXCL,
+                stat.S_IRUSR | stat.S_IWUSR,
+            )
+        except FileExistsError:
+            pass
+        else:
+            os.close(fd)
+        self._chmod()
+
+    def _chmod(self) -> None:
+        """Enforce and verify the private database mode after every write."""
+        self.db_path.chmod(0o600)
+        if stat.S_IMODE(self.db_path.stat().st_mode) != 0o600:
+            raise PermissionError(f"database is not mode 0600: {self.db_path}")
 
     def _connect(self) -> sqlite3.Connection:
+        self._secure_db_file()
         conn = sqlite3.connect(self.db_path)
         conn.row_factory = sqlite3.Row
         return conn
@@ -51,16 +89,9 @@ class DbStore:
         return version == len(self._migrations.migrations)
 
     def migrate(self) -> None:
-        """Apply any pending migrations and set permissions on the DB file."""
+        """Apply any pending migrations to the already-secured DB file."""
         with self._connection() as conn:
             self._migrations.apply(conn)
-        self._chmod()
-
-    def _chmod(self) -> None:
-        try:
-            self.db_path.chmod(0o600)
-        except OSError:
-            pass
 
 
 __all__ = ["DbStore", "DbVersionError"]

tests/unit/test_db_store.py

@@ -3,9 +3,11 @@
 from __future__ import annotations
 
 import sqlite3
+import stat
 import tempfile
 import unittest
 from pathlib import Path
+from unittest.mock import patch
 
 from bot_bottle.store.db_store import DbStore
 from bot_bottle.store.migrations import TableMigrations
@@ -22,6 +24,34 @@ class TestDbStoreIsMigrated(unittest.TestCase):
             store = _store(Path(d))
             self.assertFalse(store.is_migrated())
 
+    def test_creates_private_directory_and_database_before_first_open(self):
+        with tempfile.TemporaryDirectory() as d:
+            parent = Path(d) / "store"
+            store = _store(parent)
+            self.assertEqual(0o700, stat.S_IMODE(parent.stat().st_mode))
+            self.assertFalse(store.db_path.exists())
+            store.migrate()
+            self.assertEqual(0o600, stat.S_IMODE(store.db_path.stat().st_mode))
+
+    def test_repairs_existing_permissions(self):
+        with tempfile.TemporaryDirectory() as d:
+            parent = Path(d) / "store"
+            parent.mkdir(mode=0o755)
+            db_path = parent / "test.db"
+            db_path.touch(mode=0o644)
+            store = _store(parent)
+            self.assertEqual(db_path, store.db_path)
+            self.assertEqual(0o700, stat.S_IMODE(parent.stat().st_mode))
+            self.assertEqual(0o600, stat.S_IMODE(db_path.stat().st_mode))
+
+    def test_permission_repair_failure_is_not_suppressed(self):
+        with tempfile.TemporaryDirectory() as d:
+            parent = Path(d) / "store"
+            parent.mkdir()
+            with patch.object(Path, "chmod", side_effect=OSError("denied")):
+                with self.assertRaisesRegex(OSError, "denied"):
+                    _store(parent)
+
     def test_returns_false_when_schema_versions_missing(self):
         # DB file exists but has no schema_versions table → OperationalError → False.
         with tempfile.TemporaryDirectory() as d:

tests/unit/test_provision_gateway.py

@@ -55,7 +55,11 @@ class TestProvisionGitGate(unittest.TestCase):
     def test_copies_creds_and_runs_namespaced_init(self) -> None:
         calls: list[list[str]] = []
         with patch(_RUN, side_effect=_recorder(calls)):
-            provision_git_gate(DockerGatewayTransport("gw"), "bottle1", _plan(_up("foo", known_hosts="/host/kh")))
+            provision_git_gate(
+                DockerGatewayTransport("gw"),
+                "bottle1",
+                _plan(_up("foo", known_hosts="/host/kh")),
+            )
 
         cps = [c for c in calls if c[:2] == ["docker", "cp"]]
         self.assertIn(["docker", "cp", "/host/keys/id", "gw:/git-gate/creds/bottle1/foo-key"], cps)
@@ -81,11 +85,39 @@ class TestProvisionGitGate(unittest.TestCase):
             ["docker", "exec", "gw", "chmod", "+x", "/etc/git-gate/access-hook"], calls,
         )
 
+    def test_applies_private_modes_inside_gateway(self) -> None:
+        calls: list[list[str]] = []
+        with patch(_RUN, side_effect=_recorder(calls)):
+            provision_git_gate(
+                DockerGatewayTransport("gw"),
+                "bottle1",
+                _plan(_up("foo", known_hosts="/host/kh")),
+            )
+        self.assertIn(
+            [
+                "docker", "exec", "gw", "chmod", "700",
+                "/git-gate/creds/bottle1",
+            ],
+            calls,
+        )
+        self.assertIn(
+            [
+                "docker", "exec", "gw", "chmod", "600",
+                "/git-gate/creds/bottle1/foo-key",
+                "/git-gate/creds/bottle1/foo-known_hosts",
+            ],
+            calls,
+        )
+
     def test_omits_known_hosts_copy_when_absent(self) -> None:
         calls: list[list[str]] = []
         with patch(_RUN, side_effect=_recorder(calls)):
-            provision_git_gate(DockerGatewayTransport("gw"), "b1", _plan(_up("foo")))  # no known_hosts
+            # No known-hosts file: only the identity key is copied.
-        creds_cps = [c for c in calls if c[:2] == ["docker", "cp"] and "/git-gate/creds/" in c[3]]
+            provision_git_gate(DockerGatewayTransport("gw"), "b1", _plan(_up("foo")))
+        creds_cps = [
+            c for c in calls
+            if c[:2] == ["docker", "cp"] and "/git-gate/creds/" in c[3]
+        ]
         self.assertEqual(1, len(creds_cps))  # only the key, not known_hosts
         self.assertTrue(creds_cps[0][3].endswith("/foo-key"))
 

tests/unit/test_supervise_edge.py

@@ -136,12 +136,13 @@ class TestStoreGuardBranches(unittest.TestCase):
             db.unlink()
             self.assertEqual([], store.list_all_pending_proposals())
 
-    def test_queue_store_chmod_oserror_is_swallowed(self):
+    def test_queue_store_chmod_oserror_fails_closed(self):
         with tempfile.TemporaryDirectory() as d:
             db = Path(d) / "q.db"
             store = QueueStore("key", db_path=db)
             with patch("pathlib.Path.chmod", side_effect=OSError("ro")):
-                store.migrate()  # must not raise
+                with self.assertRaisesRegex(OSError, "ro"):
+                    store.migrate()
 
     def test_audit_store_missing_db_read_returns_empty(self):
         with tempfile.TemporaryDirectory() as d:
@@ -151,12 +152,13 @@ class TestStoreGuardBranches(unittest.TestCase):
             db.unlink()
             self.assertEqual([], store.read_audit_entries("egress", "slug"))
 
-    def test_audit_store_chmod_oserror_is_swallowed(self):
+    def test_audit_store_chmod_oserror_fails_closed(self):
         with tempfile.TemporaryDirectory() as d:
             db = Path(d) / "a.db"
             store = AuditStore(db_path=db)
             with patch("pathlib.Path.chmod", side_effect=OSError("ro")):
-                store.migrate()  # must not raise
+                with self.assertRaisesRegex(OSError, "ro"):
+                    store.migrate()
 
 
 if __name__ == "__main__":