fix: enforce shared storage permissions
lint / lint (push) Failing after 1m2s
test / image-input-builds (pull_request) Successful in 1m1s
test / unit (pull_request) Successful in 55s
test / integration-docker (pull_request) Failing after 1m3s
test / coverage (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Failing after 14m45s

This commit is contained in:
2026-07-27 04:45:08 +00:00
parent dbbb185d0a
commit 38a67d2767
5 changed files with 124 additions and 18 deletions
+13 -2
View File
@@ -63,12 +63,23 @@ def provision_git_gate(
transport.exec(["chmod", "+x", "/etc/git-gate/access-hook"]) transport.exec(["chmod", "+x", "/etc/git-gate/access-hook"])
creds = _creds_dir(bottle_id) creds = _creds_dir(bottle_id)
transport.exec(["mkdir", "-p", creds]) transport.exec(["mkdir", "-p", creds])
transport.exec(["chmod", "700", creds])
credential_paths: list[str] = []
for u in plan.upstreams: for u in plan.upstreams:
if u.identity_file: 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) known_hosts = str(u.known_hosts_file)
if known_hosts and known_hosts != ".": 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. # Init the bare repos + per-repo credential config for this namespace.
script = git_gate_render_provision(bottle_id, plan.upstreams) script = git_gate_render_provision(bottle_id, plan.upstreams)
transport.exec(["sh", "-c", script]) transport.exec(["sh", "-c", script])
+40 -9
View File
@@ -2,7 +2,9 @@
from __future__ import annotations from __future__ import annotations
import os
import sqlite3 import sqlite3
import stat
from contextlib import contextmanager from contextlib import contextmanager
from pathlib import Path from pathlib import Path
@@ -19,9 +21,45 @@ class DbStore:
def __init__(self, db_path: Path, migrations: TableMigrations) -> None: def __init__(self, db_path: Path, migrations: TableMigrations) -> None:
self.db_path = db_path self.db_path = db_path
self._migrations = migrations 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: def _connect(self) -> sqlite3.Connection:
self._secure_db_file()
conn = sqlite3.connect(self.db_path) conn = sqlite3.connect(self.db_path)
conn.row_factory = sqlite3.Row conn.row_factory = sqlite3.Row
return conn return conn
@@ -51,16 +89,9 @@ class DbStore:
return version == len(self._migrations.migrations) return version == len(self._migrations.migrations)
def migrate(self) -> None: 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: with self._connection() as conn:
self._migrations.apply(conn) self._migrations.apply(conn)
self._chmod()
def _chmod(self) -> None:
try:
self.db_path.chmod(0o600)
except OSError:
pass
__all__ = ["DbStore", "DbVersionError"] __all__ = ["DbStore", "DbVersionError"]
+30
View File
@@ -3,9 +3,11 @@
from __future__ import annotations from __future__ import annotations
import sqlite3 import sqlite3
import stat
import tempfile import tempfile
import unittest import unittest
from pathlib import Path from pathlib import Path
from unittest.mock import patch
from bot_bottle.store.db_store import DbStore from bot_bottle.store.db_store import DbStore
from bot_bottle.store.migrations import TableMigrations from bot_bottle.store.migrations import TableMigrations
@@ -22,6 +24,34 @@ class TestDbStoreIsMigrated(unittest.TestCase):
store = _store(Path(d)) store = _store(Path(d))
self.assertFalse(store.is_migrated()) 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): def test_returns_false_when_schema_versions_missing(self):
# DB file exists but has no schema_versions table → OperationalError → False. # DB file exists but has no schema_versions table → OperationalError → False.
with tempfile.TemporaryDirectory() as d: with tempfile.TemporaryDirectory() as d:
+35 -3
View File
@@ -55,7 +55,11 @@ class TestProvisionGitGate(unittest.TestCase):
def test_copies_creds_and_runs_namespaced_init(self) -> None: def test_copies_creds_and_runs_namespaced_init(self) -> None:
calls: list[list[str]] = [] calls: list[list[str]] = []
with patch(_RUN, side_effect=_recorder(calls)): 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"]] 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) 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, ["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: def test_omits_known_hosts_copy_when_absent(self) -> None:
calls: list[list[str]] = [] calls: list[list[str]] = []
with patch(_RUN, side_effect=_recorder(calls)): 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.assertEqual(1, len(creds_cps)) # only the key, not known_hosts
self.assertTrue(creds_cps[0][3].endswith("/foo-key")) self.assertTrue(creds_cps[0][3].endswith("/foo-key"))
+6 -4
View File
@@ -136,12 +136,13 @@ class TestStoreGuardBranches(unittest.TestCase):
db.unlink() db.unlink()
self.assertEqual([], store.list_all_pending_proposals()) 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: with tempfile.TemporaryDirectory() as d:
db = Path(d) / "q.db" db = Path(d) / "q.db"
store = QueueStore("key", db_path=db) store = QueueStore("key", db_path=db)
with patch("pathlib.Path.chmod", side_effect=OSError("ro")): 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): def test_audit_store_missing_db_read_returns_empty(self):
with tempfile.TemporaryDirectory() as d: with tempfile.TemporaryDirectory() as d:
@@ -151,12 +152,13 @@ class TestStoreGuardBranches(unittest.TestCase):
db.unlink() db.unlink()
self.assertEqual([], store.read_audit_entries("egress", "slug")) 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: with tempfile.TemporaryDirectory() as d:
db = Path(d) / "a.db" db = Path(d) / "a.db"
store = AuditStore(db_path=db) store = AuditStore(db_path=db)
with patch("pathlib.Path.chmod", side_effect=OSError("ro")): with patch("pathlib.Path.chmod", side_effect=OSError("ro")):
store.migrate() # must not raise with self.assertRaisesRegex(OSError, "ro"):
store.migrate()
if __name__ == "__main__": if __name__ == "__main__":