fix(supervise): get bot-bottle.db off the data plane (supervise + egress proposals over RPC) #471
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: didericis/bot-bottle#471
Reference in New Issue
Block a user
Delete Branch "fix/db-off-data-plane-469"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #469.
What
Enforces PRD 0070's rule — only the orchestrator opens
bot-bottle.db; the data plane reaches state through the control-plane RPC, never a direct file handle. Three data-plane daemons held a direct read-write handle on the shared SQLite file: the supervise MCP server, the egress DLP addon (the most attack-exposed process — TLS-bumping and body-scanning hostile traffic), and the git-gate pre-receive hook (the third writer the issue didn't enumerate; it runs in the same gateway container, so the DB mount couldn't be dropped without migrating it too).Control plane — agent half of the supervise flow
Two new endpoints on
orchestrator/control_plane.py, both attributed by(source_ip, identity_token)exactly like/resolve(never a caller-supplied slug), so a bottle can only ever queue or read its own proposals even if the data plane is compromised:POST /supervise/propose→201 {proposal_id}POST /supervise/poll→200 {status, notes?, final_file?}(pending|unknown| terminal decision). A decided poll archives server-side, preserving the archive-after-read contract.Backed by
Orchestrator.supervise_queue_proposal/supervise_poll_response, and reached through newPolicyResolver.propose_supervise/poll_superviseclient methods.Data plane — no more DB handle
supervise_server.py: queues via RPC and keeps its ~30s grace window by polling the RPC (nowait_for_response/write_proposal/read_response/archive_proposal).egress_addon.py: token-allow block queues/polls over RPC; safelist still keyed by the resolved bottle.git_gate_render.py: the gitleaks-allow hook queues/polls over RPC using(source_ip, identity_token)threaded into the CGI env bygit_http_backend.py.Packaging
Drop the DB bind-mount and
SUPERVISE_DB_PATHfrom every data-plane container/VM (docker gateway + infra, macOS infra, firecracker infra). Each consolidated backend'sSUPERVISE_DB_PATHequaledhost_db_path(), so the orchestrator falls back to the same file viaBOT_BOTTLE_ROOTand remains the sole opener.Docs
PRD 0070: the rule is now in force; transitional caveat removed.
Tests
Rewrote the supervise-server and egress supervise tests around a fake resolver backed by the real queue (contracts preserved through the RPC seam); added propose/poll coverage for the control plane, service, and
PolicyResolver, including cross-bottle isolation. Full unit suite green except 10 pre-existingtest_cli_start_headlesserrors (environment: sandbox stdin isn't a PTY; identical on the base branch).🤖 Generated with Claude Code
8a82861459to2acaa082e42acaa082e4to385cca72a7385cca72a7to30d74cbf2b30d74cbf2bto72fdb1d14bFound two blocking issues:
High — data-plane compromise can bypass supervision entirely. The gateway receives
BOT_BOTTLE_CONTROL_PLANE_TOKEN, andPolicyResolversends it with the new supervise RPCs. That same token authorizes every control-plane route, including/supervise/respond. A compromised egress or git-gate process can queue a proposal and approve it directly—or modify policies and inspect other proposals—without operator involvement. The new(source_ip, identity_token)checks do not protect trusted operator routes. Use a separate, least-privilege data-plane credential restricted to/resolve,/supervise/propose, and/supervise/poll. Seebot_bottle/policy_resolver.py:69-77andbot_bottle/orchestrator/control_plane.py:100-114.Medium — polling can permanently lose an operator decision.
supervise_poll_response()archives the proposal before the HTTP response is successfully delivered. If the connection drops after archiving, the retry receivesunknown; egress, supervise, and git-gate eventually fail or time out despite approval. The old flow archived only after the caller received the response. Keep terminal decisions available for idempotent polls, or add an explicit acknowledgement before archiving. Seebot_bottle/orchestrator/service.py:225-250.I would request changes before merging.
@didericis-claude RE first issue @didericis-codex mentioned, implement roles embedded into JWT tokens the orchestrator issues and decrypts. Roles should be based on service type (
gateway,cli, etc)@didericis @didericis-codex — done. Pushed
feat(control-plane): role-scoped signed tokensaddressing codex's High finding via role JWTs.What changed
bot_bottle/control_auth.py: stdlib HS256 (no dependency)mint/verifyof compact{"role": ...}tokens. Roles:gateway(data plane) andcli(host operator/launcher).dispatchnow gates each route by role —gatewayreaches/resolve+/supervise/{propose,poll}, everything else iscli-only. Wrong role → 403, missing token → 401.gatewaytoken (newBOT_BOTTLE_CONTROL_AUTH_JWT) it can't rewrite intocli; the host CLI mints its ownclitoken from the host key. So a compromised egress/git-gate can no longer approve its own proposals, rewrite policy, or read injected upstream tokens.gateway_initscopes the signing key to the orchestrator process and the gateway token to the data-plane daemons, so even in the combined infra container a compromised data-plane daemon never sees the key. Standalone gateway containers never get the key at all.cliaccess as the fail-visible test fallback.Decisions (happy to revisit): tokens are signed, not encrypted (the role isn't a secret) and long-lived (parity with the static token they replace — the win is the unforgeable role claim, not rotation).
cliis a superset that reaches every route;gatewayis strictly the three data-plane lookups.Residual: in the combined infra container the signing key still lives in the orchestrator process's env, so a root-level compromise of a co-located data-plane daemon could read
/proc/<orch>/environ. That's the "run daemons as distinct non-root UIDs" hardening already noted as a follow-up in #469; the standalone gateway (the attack-exposed egress path) is fully isolated.On the Medium finding (poll archives the decision before the HTTP response is delivered): per @didericis's direction I scoped this PR to the JWT roles only and left that one for a follow-up — it's a real idempotency bug, and the fix is small (stop archiving on
supervise_poll_responseand return the decision idempotently; decided proposals already drop off the operator's pending list). Say the word and I'll fold it in here instead.New tests:
test_control_auth(sign/verify, tamper, alg-confusion, alg=none), role gating intest_orchestrator_control_plane+ the docker auth integration test, per-process env scoping intest_gateway_init, and the CLI/resolver token wiring. Lint + pyright clean locally; CI running.@didericis-claude make the poll idempotent and raise the interval to 250 ms
Done — poll is idempotent (no archive on read; a re-poll returns the same decision) and the interval is now 250ms. Decided proposals still drop off the pending list, and the rows are reaped on teardown/reconcile.
Blocking finding:
BOT_BOTTLE_CONTROL_PLANE_TOKENand starts the gateway daemons with no gateway JWT (bot_bottle/backend/firecracker/infra_vm.py:488-501). ConsequentlyControlPlaneServerenters open mode and grants every unauthenticated request theclirole (bot_bottle/orchestrator/control_plane.py:399-430). The nft boundary only prevents an agent from reaching the port; it does not constrain egress, supervise, or git-http, which run in the same infra VM and already reach the service at127.0.0.1. If one of those data-plane daemons is compromised, it can still call/supervise/respond, policy mutation, and the other operator routes directly. Please provision a signing key and a pre-mintedgatewayJWT for the Firecracker processes too, with the same per-process scoping used by the other combined launcher.The previous polling/idempotency finding is resolved, and the unit suite is passing in my re-review run.
Good catch — you're right, open mode reopened the exact hole for the daemons co-located in the infra VM (the nft boundary only fences off the separate agent VM). Fixed in
aac27d8: the infra VM now generates the control-plane signing key on its persistent volume, hands it only to the orchestrator process, mints agatewayJWT for the data-plane daemons, and the host mirrors the key back into its token file so the CLI signsclitokens the VM verifies. Same per-process scoping the docker/macOS launchers use — no more open mode. Watching the firecracker integration job.The original Firecracker role-bypass finding is fixed, but the fix introduces a new blocking regression:
_with_signing_key()unconditionally overwrites the single host-widecontrol-plane-tokenwith that guest key (bot_bottle/backend/firecracker/infra_vm.py:193-213). Docker and macOS orchestrators are launched with the value of that same host file and retain it in their process environment, while every newOrchestratorClientmints itscliJWT from the file’s current value. If another backend is already running,ensure_running()for Firecracker therefore changes the key used by all subsequent host clients without rotating the other live orchestrators; supervise, teardown, policy updates, and cleanup against those backends begin returning 401. Cross-backend operation is supported elsewhere in the CLI, so the signing key needs to remain host-canonical/shared, or token selection/storage must be scoped per control-plane endpoint rather than clobbering the global file.Also,
_with_signing_key()treats failure to retrieve the key as best-effort and returns a usable-looking infra handle (infra_vm.py:193-207), even though every authenticated operation in the immediately following launch will then fail with the stale/missing host key. Once key synchronization is required for correctness, this path should retry or fail explicitly rather than proceeding.Targeted authentication, Firecracker-infra, gateway-init, and supervise tests pass; they do not cover coexistence with an already-running control plane using the previous host key.
You're right — the mirror-into-the-shared-host-file approach was wrong; it clobbers the host-canonical key and 401s a co-running Docker/macOS control plane. Fixed in
66409c7by keeping the host token file as the single source of truth: the launcher now pushes the host key into the freshly booted infra VM over SSH (atomic write, same transport aspersist_env_var_secret), and the VM's init waits for it and refuses to start the control plane — rather than run open — if it never arrives. Nothing writes back to the host file, so other backends are untouched, and the best-effort silent path is gone (both the push and the guest fail loudly). Watching the firecracker integration job.66409c770bto4a83f45d9d4a83f45d9dtoaac27d8a40Separate the gateway (data plane) from the orchestrator (control plane) at the module level. The gateway runtime files move out of the package root — and the backend-neutral Gateway lifecycle ABC + GATEWAY_* constants move out of orchestrator/ — into a new bot_bottle/gateway/ package: gateway/__init__.py (was orchestrator/gateway.py: Gateway ABC + consts + rotate_gateway_ca) gateway/gateway_init.py (the PID-1 daemon supervisor) gateway/egress_addon.py, egress_addon_core.py, egress_dlp_config.py, dlp_detectors.py (the egress mitmproxy daemon) gateway/git_http_backend.py (the git-http daemon) gateway/git_gate_render.py (the git-gate pre-receive rendering) gateway/supervise_server.py (the supervise MCP daemon) gateway/policy_resolver.py (the data-plane control-plane RPC client) orchestrator/ now holds only control-plane files. The shared plan/types/auth layer (egress.py=EgressPlan, git_gate.py=GitGatePlan, supervise.py, supervise_types.py, control_auth.py) and the launch-time git-gate provisioning helpers stay at root, so orchestrator/ and backend/ still own them. Because these daemons are invoked as `python3 -m bot_bottle.<name>`, loaded flat by mitmproxy, and referenced in Dockerfile.gateway, the move updates more than Python imports: the `-m` invocations (firecracker/macOS infra scripts), the Dockerfile.gateway addon shim + ENTRYPOINT, gateway_init's _DAEMONS module paths, and the git-gate CGI heredocs all now point at bot_bottle.gateway.*. No behavior change; full unit suite green (2251). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Give each service its own store package + manager, and cut the supervise module along the control/data-plane boundary so nothing in the shared layer reaches up into the orchestrator. Stores, by owner: - bot_bottle/store/ keeps only the shared base (DbStore, migrations) and the concrete stores that aren't service-owned (audit_store, config_store). - bot_bottle/orchestrator/store/ now houses the orchestrator-owned stores — queue_store (supervise queue), secret_store, config_store — plus a new orchestrator store_manager that migrates them (composing audit/config downward from the base). The old shared store_manager is gone. Supervise plane, by tier: - bot_bottle/supervisor/ (NEUTRAL, importable by every tier including the gateway): types.py (the Proposal/Response/AuditEntry wire types + the tool/ status/poll constants + the shared daemon constants moved out of supervise.py) and plan.py (SupervisePlan, a pure DTO). - bot_bottle/orchestrator/supervisor/ (orchestrator-only): queue.py (the queue/audit I/O wrappers + render_diff + sha256_hex) and supervise.py (the Supervise lifecycle that stages the DB via the store manager). Its __init__ re-exports the neutral vocabulary so orchestrator-side callers import from one place. The gateway now imports only bot_bottle.supervisor.types (never bot_bottle.supervise), so the data plane holds no code dependency on the orchestrator — it reaches the queue over the control-plane RPC. This removes the circular import that moving queue_store under orchestrator introduced (supervise -> orchestrator -> service -> supervise). supervise_types.py -> supervisor/types.py; supervise.py deleted (split). Full unit suite green (2251). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Remove the supervise queue helpers that no live path uses anymore — they're mechanisms the PRD-0070 / idempotent-poll migration superseded: - archive_proposal (+ QueueStore.archive_proposal): the poll stopped archiving on read (#469 review), so single-proposal archiving has no caller; decided proposals drop off the pending list via their response row and are reaped in bulk by archive_all_proposals on teardown/reconcile. - wait_for_response: the data plane polls the queue over the control-plane RPC now, so nothing block-waits on it. - audit_dir / audit_log_path: file-based audit paths, replaced by the SQLite AuditStore. Also drops the tests that covered those obsolete helpers. Kept the prod-unused read accessors list_pending_proposals and read_audit_entries: those aren't dead mechanisms — they're the read side of live write paths that many tests use to verify real queue/audit behavior. Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Turn the loose queue/audit functions in orchestrator/supervisor/queue.py into methods on a concrete Supervisor service. The Orchestrator now owns an injectable `self._supervisor` and calls `self._supervisor.write_proposal(...)` etc., instead of module-level free functions — the supervise dependency is explicit and mockable, and a Supervisor can be scoped to a `db_path` (defaults to the host DB) so tests can point it at a temp database. - Supervisor (in the package __init__) drops the ABC and gains write_proposal, read_proposal, list_pending_proposals, list_all_pending_proposals, write_response, read_response, archive_all_proposals, write_audit_entry, read_audit_entries, plus the launch-time prepare(). - render_diff / sha256_hex are pure and stateless, so they move to orchestrator/supervisor/util.py (re-exported from the facade) rather than becoming methods. - queue.py is deleted; service.py + the supervise tests call through a Supervisor instance. Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>backend/__init__.py eagerly imported the whole framework (manifest, egress, git-gate, env, workspace, agent-provider), so importing ANY backend.* submodule paid ~32 modules just to run the package init. Split it: - backend/base.py — the abstract contract (BottleSpec, BottlePlan, BottleCleanupPlan, ExecResult, ActiveAgent, Bottle, BottleImages, BottleBackend). Carries the framework imports; loaded only when a caller needs the contract. - backend/selection.py — backend registry / selection / enumeration (get_bottle_backend, _get_backends, _auto_select_backend, has_backend, known_backend_names, enumerate_active_agents). Concrete backends still imported lazily inside. - backend/__init__.py — thin: a __getattr__ that resolves the public names from those submodules on first access (+ a TYPE_CHECKING block for checkers). Existing `from bot_bottle.backend import X` and patch.object call-sites keep working. `import bot_bottle.backend` drops from 32 -> 2 bot_bottle modules. Repointed the backend-selection test's patch targets to the `selection` module (the functions reference each other there now; the FirecrackerBottleBackend class-method patches are unchanged — same class object). Note: backend.docker.util is still heavy because backend/docker/__init__ eagerly imports DockerBottleBackend — slimming the three sub-package inits is the follow-up that makes the leaves cheap. Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Turn the git_gate module into a package with GitGate as a concrete service class (dropping the ABC), mirroring the Supervisor shape. The host-side git-gate operations are now methods the backend drives: git_gate/ __init__.py — thin __getattr__ facade (keeps `from bot_bottle.git_gate import …` working; render/hook names lazily forwarded to gateway.git_gate_render) plan.py — GitGatePlan (the launch DTO the backend contract references) service.py — GitGate: prepare / provision_dynamic_keys / revoke_provisioned_keys / preflight_host_keys provision.py — deploy-key lifecycle (was git_gate_provision.py) host_key.py — host-key preflight (was git_gate_host_key.py) The backend launch + base.py preflight now call GitGate() methods instead of the free functions. The runtime rendering / in-gateway hook execution stay in gateway/git_gate_render.py (data plane) — only the host-side service moved. Because the facade forwards names lazily, the ~25 `from bot_bottle.git_gate import GitGatePlan` call-sites are unchanged, and importing git_gate.plan (the contract's dependency) no longer drags in the provisioning / forge-API code. Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>_common had rotted into a junk drawer. Trim it to what actually justifies a shared leaf module and rename for legibility: * REPO_DIR: deleted (dead — nothing imported it). * read_tty_line: dropped the pointless re-export; cleanup/start/init now import it straight from bot_bottle.util. * USER_CWD: deleted. It captured os.getcwd() at import time, but nothing chdirs and no test patched it, so it was equivalent to a live os.getcwd() — inlined at the six call sites. * PROG: kept, now the sole member. It's still a leaf (both the dispatcher and the commands it imports need it) so it can't move into __init__ without a circular import. _common.py -> constants.py. Full unit suite green (2243); CLI dispatch verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Split the cli package's two responsibilities out of __init__: * commands/__init__.py now assembles the COMMANDS registry and NO_MIGRATION_COMMANDS from the per-command modules — the command surface lives entirely under commands/. * __main__.py now owns main() (dispatch + migration gate + exit-code mapping) alongside the runnable entry guard. cli/__init__.py shrinks to a shim that re-exports main / COMMANDS / NO_MIGRATION_COMMANDS, so bot_bottle.cli.main (repo-root cli.py entry) and the tests' bot_bottle.cli.COMMANDS keep working unchanged. Verified `python -m bot_bottle.cli` (runpy) and `cli.py` both dispatch; full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>The codebase used "control plane" both as an architectural role term AND as an identifier alias for the orchestrator component, producing duplicate names for one thing (control_plane_url vs orchestrator_url, CONTROL_PLANE_PORT, host_control_plane_token, …). Going forward the concrete component is always named for what it is — Gateway or Orchestrator — and the plane vocabulary is reserved for prose (module descriptions, the security argument). Renamed (identifiers + the in-repo env/wire/file string values, all setters/getters are in this repo so the change is atomic): ControlPlaneServer -> OrchestratorServer control_plane_url -> orchestrator_url probe_control_plane_url -> probe_orchestrator_url host_control_plane_token -> host_orchestrator_token CONTROL_PLANE_PORT -> ORCHESTRATOR_PORT CONTROL_PLANE_TOKEN_ENV/FILE -> ORCHESTRATOR_TOKEN_ENV/FILENAME BOT_BOTTLE_CONTROL_PLANE_TOKEN-> BOT_BOTTLE_ORCHESTRATOR_TOKEN control-plane-token (file) -> orchestrator-token control_auth (module) -> orchestrator_auth (stays top-level; the gateway imports it and must not import the orchestrator/ package) CONTROL_AUTH_HEADER -> ORCHESTRATOR_AUTH_HEADER x-bot-bottle-control-auth -> x-bot-bottle-orchestrator-auth CONTROL_AUTH_JWT_ENV -> ORCHESTRATOR_AUTH_JWT_ENV BOT_BOTTLE_CONTROL_AUTH_JWT -> BOT_BOTTLE_ORCHESTRATOR_AUTH_JWT _control_auth_headers -> _orchestrator_auth_headers Prose plane-terms ("control plane", "data plane") are preserved, including the test name test_data_plane_daemons_get_jwt_not_key (it names the security invariant). Gateway and orchestrator verified to agree on the renamed wire header; full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Two facade issues from the reorg: * orchestrator/__init__.py used `__all__ = list(_LAZY)`, which pyright strict can't analyze (reportUnsupportedDunderAll) — and because it isn't a static literal, the TYPE_CHECKING re-export imports read as unused. Replaced with the explicit literal list the other lazy facades already use. * manifest/index.py imported six piece types (ManifestAgentProvider, EGRESS_AUTH_SCHEMES, ManifestEgressConfig/Route, ManifestGitEntry, ManifestKeyConfig) it never referenced — the package facade re-exports those straight from their submodules, so index.py needn't import them. Dropped the dead imports. pyright: 0 errors (was 24). Full unit suite green (2243). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.