fix(git-gate): review — sandbox naming, ResolverLike Protocol, token-required note

Addresses PR #365 review:
- Type `resolve_sandbox_root`'s resolver param as a `ResolverLike` Protocol
  (structural, `resolve_bottle_id` only) — fixes the pyright errors from
  passing a duck-typed fake resolver in tests; mirrors
  egress_addon_core.PolicyResolverLike.
- Rename `_project_root`/`project_root`/`resolve_repo_root`/
  `DEFAULT_PROJECT_ROOT` -> `_sandbox_root`/`sandbox_root`/
  `resolve_sandbox_root`/`DEFAULT_REPO_ROOT`: "sandbox" names the per-tenant
  scope; "project" was too amorphous. `GIT_PROJECT_ROOT` keeps git's own
  env-var name.
- Note the single-tenant path is transitional (stripped once every backend
  runs the consolidated gateway).
- policy_resolver: document that the optional identity token is
  transitional — the consolidated end state requires it (flips at the
  /resolve boundary once token *delivery* lands, a PRD 0070 open question).
- Split the long line in main() (was 105 cols).

pyright 0 errors; pylint 9.95/10; unit suite green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck

bot_bottle/git_http_backend.py

@@ -13,7 +13,8 @@ calling bottle's repo namespace (`<root>/<bottle_id>`), attributed from
 the unspoofable source IP via the orchestrator. Per-repo credentials +
 hooks scope by repo directory, so isolating the *root* per bottle isolates
 its creds too. Unattributed clients fail closed (404). Unset → the legacy
-per-bottle single-tenant flat root, unchanged.
+per-bottle single-tenant flat root, unchanged — a transitional path that
+gets stripped out once every backend runs the consolidated gateway.
 """
 
 from __future__ import annotations
@@ -21,6 +22,7 @@ from __future__ import annotations
 import os
 import subprocess
 import sys
+import typing
 from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
 from pathlib import Path
 from urllib.parse import urlsplit
@@ -54,24 +56,38 @@ ORCHESTRATOR_URL_ENV = "BOT_BOTTLE_ORCHESTRATOR_URL"
 IDENTITY_HEADER = "x-bot-bottle-identity"
 
 # Default flat repo root (single-tenant, and the base under which
-# consolidated mode nests each bottle's namespace).
-DEFAULT_PROJECT_ROOT = "/git"
+# consolidated mode nests each sandbox's namespace).
+DEFAULT_REPO_ROOT = "/git"
 
 
-def resolve_repo_root(
-    resolver: "PolicyResolver | None",
+class ResolverLike(typing.Protocol):
+    """Structural type for the resolver `resolve_sandbox_root` needs — just
+    `resolve_bottle_id`. A Protocol so tests can pass a fake without
+    importing PolicyResolver (mirrors egress_addon_core.PolicyResolverLike)."""
+
+    def resolve_bottle_id(
+        self, source_ip: str, identity_token: str = ...,
+    ) -> "str | None": ...
+
+
+def resolve_sandbox_root(
+    resolver: "ResolverLike | None",
     base_root: Path,
     source_ip: str,
     identity_token: str = "",
 ) -> Path | None:
-    """The repo root to serve this request from, or None to deny (404).
+    """The per-sandbox repo root to serve this request from, or None to
+    deny (404).
 
     Single-tenant (`resolver is None`): the flat `base_root`, unchanged.
+    NOTE: this legacy per-bottle single-tenant path is transitional — it
+    will be stripped out once every backend runs the consolidated gateway
+    (PRD 0070), leaving only the source-IP-attributed path below.
 
-    Consolidated: `base_root/<bottle_id>`, where the bottle is attributed
+    Consolidated: `base_root/<bottle_id>`, where the sandbox is attributed
     from the source IP via the orchestrator. Fail-closed — an unattributed
     client, a resolver error, or a namespace that would escape `base_root`
-    all deny, so one bottle can never reach another's repos."""
+    all deny, so one sandbox can never reach another's repos."""
     if resolver is None:
         return base_root
     try:
@@ -106,24 +122,25 @@ class GitHttpHandler(BaseHTTPRequestHandler):
     def do_POST(self) -> None:
         self._run_backend()
 
-    def _project_root(self) -> Path | None:
-        """This request's repo root, or None to deny. Single-tenant unless
-        the server was started with a resolver (consolidated mode), in which
-        case the root is the calling bottle's source-IP-selected namespace."""
-        base = Path(os.environ.get("GIT_PROJECT_ROOT", DEFAULT_PROJECT_ROOT))
+    def _sandbox_root(self) -> Path | None:
+        """This request's per-sandbox repo root, or None to deny. Single-tenant
+        unless the server was started with a resolver (consolidated mode), in
+        which case the root is the calling sandbox's source-IP-selected
+        namespace. `GIT_PROJECT_ROOT` keeps git's own env-var name."""
+        base = Path(os.environ.get("GIT_PROJECT_ROOT", DEFAULT_REPO_ROOT))
         resolver = getattr(self.server, "policy_resolver", None)
         token = self.headers.get(IDENTITY_HEADER, "")
-        return resolve_repo_root(resolver, base, self.client_address[0], token)
+        return resolve_sandbox_root(resolver, base, self.client_address[0], token)
 
     def _run_backend(self) -> None:
-        project_root = self._project_root()
-        if project_root is None:
+        sandbox_root = self._sandbox_root()
+        if sandbox_root is None:
             # Unattributed / resolver error: deny before touching any repo.
             self.send_error(404)
             return
         parsed = urlsplit(self.path)
         if self._is_upload_pack(parsed.path, parsed.query):
-            repo_dir = self._repo_dir(project_root, parsed.path)
+            repo_dir = self._repo_dir(sandbox_root, parsed.path)
             if repo_dir is None:
                 self.send_error(404)
                 return
@@ -157,7 +174,7 @@ class GitHttpHandler(BaseHTTPRequestHandler):
                 return
         env = os.environ.copy()
         env.update({
-            "GIT_PROJECT_ROOT": str(project_root),
+            "GIT_PROJECT_ROOT": str(sandbox_root),
             "GIT_HTTP_EXPORT_ALL": "1",
             "REQUEST_METHOD": self.command,
             "PATH_INFO": parsed.path,
@@ -203,8 +220,8 @@ class GitHttpHandler(BaseHTTPRequestHandler):
         )
         self._write_cgi_response(proc.stdout)
 
-    def _repo_dir(self, project_root: Path, path: str) -> Path | None:
-        root = project_root.resolve()
+    def _repo_dir(self, sandbox_root: Path, path: str) -> Path | None:
+        root = sandbox_root.resolve()
         relative = path.lstrip("/").split(".git", 1)[0] + ".git"
         candidate = (root / relative).resolve()
         if root not in (candidate, *candidate.parents):
@@ -262,9 +279,10 @@ def main() -> int:
     port = int(os.environ.get("GIT_HTTP_PORT", str(DEFAULT_PORT)))
     server = ThreadingHTTPServer(("0.0.0.0", port), GitHttpHandler)
     orch_url = os.environ.get(ORCHESTRATOR_URL_ENV, "").strip()
-    # Consolidated mode: resolve each request's bottle namespace by source IP.
-    # Absent → single-tenant (flat repo root); behaviour unchanged.
-    server.policy_resolver = PolicyResolver(orch_url) if orch_url else None  # type: ignore[attr-defined]
+    # Consolidated mode: resolve each request's sandbox namespace by source
+    # IP. Absent → single-tenant (flat repo root); behaviour unchanged.
+    resolver = PolicyResolver(orch_url) if orch_url else None
+    server.policy_resolver = resolver  # type: ignore[attr-defined]
     mode = "multi-tenant" if orch_url else "single-tenant"
     sys.stdout.write(f"git-http listening on 0.0.0.0:{port} ({mode})\n")
     sys.stdout.flush()

bot_bottle/policy_resolver.py

@@ -73,8 +73,15 @@ class PolicyResolver:
     def resolve(self, source_ip: str, identity_token: str = "") -> str | None:
         """The calling bottle's policy blob, or None if unattributed. Always
         fetches from the orchestrator so revocations / changes / teardowns
-        are honored immediately. `identity_token` is optional — omit it to
-        resolve by source IP alone (the network-layer attribution).
+        are honored immediately.
+
+        `identity_token` is optional *transitionally* — omitting it resolves
+        by source IP alone (the network-layer attribution, sound by
+        construction on Firecracker's `/31`+nft, weaker elsewhere). The
+        consolidated end state makes the token mandatory so the app-layer
+        defense is always enforced, not silently degraded; that flips at the
+        `/resolve` boundary once identity-token *delivery* lands (a PRD 0070
+        open question — we can't require what the agent can't yet present).
         Raises `PolicyResolveError` if the orchestrator can't be reached."""
         payload = self._post_resolve(source_ip, identity_token)
         if payload is None:

tests/unit/test_git_http_multitenant.py

@@ -1,4 +1,4 @@
-"""Unit: resolve_repo_root — source-IP-keyed git-gate repo namespace (PRD 0070).
+"""Unit: resolve_sandbox_root — source-IP-keyed git-gate repo namespace (PRD 0070).
 
 The consolidated git-http backend serves every bottle from one process and
 selects each request's repo root by the calling bottle's source IP. This is
@@ -10,7 +10,7 @@ from __future__ import annotations
 import unittest
 from pathlib import Path
 
-from bot_bottle.git_http_backend import resolve_repo_root
+from bot_bottle.git_http_backend import resolve_sandbox_root
 from bot_bottle.policy_resolver import PolicyResolveError
 
 _BASE = Path("/git")
@@ -32,32 +32,32 @@ class _FakeResolver:
 class TestResolveRepoRoot(unittest.TestCase):
     def test_single_tenant_passthrough(self) -> None:
         # No resolver → the flat base root, unchanged (legacy per-bottle mode).
-        self.assertEqual(_BASE, resolve_repo_root(None, _BASE, "10.243.0.1"))
+        self.assertEqual(_BASE, resolve_sandbox_root(None, _BASE, "10.243.0.1"))
 
     def test_attributed_bottle_gets_namespaced_root(self) -> None:
-        root = resolve_repo_root(_FakeResolver(bottle_id="ab12cd34"), _BASE, "10.243.0.1")
+        root = resolve_sandbox_root(_FakeResolver(bottle_id="ab12cd34"), _BASE, "10.243.0.1")
         self.assertEqual(Path("/git/ab12cd34"), root)
 
     def test_unattributed_denies(self) -> None:
-        self.assertIsNone(resolve_repo_root(_FakeResolver(bottle_id=None), _BASE, "10.243.0.9"))
+        self.assertIsNone(resolve_sandbox_root(_FakeResolver(bottle_id=None), _BASE, "10.243.0.9"))
 
     def test_empty_bottle_id_denies(self) -> None:
-        self.assertIsNone(resolve_repo_root(_FakeResolver(bottle_id=""), _BASE, "10.243.0.1"))
+        self.assertIsNone(resolve_sandbox_root(_FakeResolver(bottle_id=""), _BASE, "10.243.0.1"))
 
     def test_resolver_error_denies(self) -> None:
         # Orchestrator unreachable/errored must never serve repos.
-        self.assertIsNone(resolve_repo_root(_FakeResolver(raises=True), _BASE, "10.243.0.1"))
+        self.assertIsNone(resolve_sandbox_root(_FakeResolver(raises=True), _BASE, "10.243.0.1"))
 
     def test_namespace_escape_denies(self) -> None:
         # A bottle_id that would climb out of the root is rejected (defense in
         # depth — registry ids are token_hex, but never trust the namespace).
         self.assertIsNone(
-            resolve_repo_root(_FakeResolver(bottle_id="../etc"), _BASE, "10.243.0.1")
+            resolve_sandbox_root(_FakeResolver(bottle_id="../etc"), _BASE, "10.243.0.1")
         )
 
     def test_forwards_source_ip_and_token(self) -> None:
         r = _FakeResolver(bottle_id="b1")
-        resolve_repo_root(r, _BASE, "10.243.0.1", "tok")
+        resolve_sandbox_root(r, _BASE, "10.243.0.1", "tok")
         self.assertEqual(("10.243.0.1", "tok"), r.calls[0])