- config_store.py imported host_db_path from supervise_types (wrong);
it lives in paths.py, matching all other stores (audit, queue, etc.)
- test_stale_checks: two tests expected check_stale called twice
(agent + sidecar) — consolidated arch has no sidecar, so once is
correct; update assertions and remove unused Path import
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pyright strict reportUnusedImport flagged BottleImages in
test_docker_launch_committed_image.py and test_macos_container_launch.py;
neither file references the type by name (they only use the returned
value's attributes).
Addresses review comments 3098, 3099, 3100 on PR #336:
- Add BottleImages(agent, sidecar) dataclass to backend/__init__.py.
Docker/macOS backends use str image refs; smolmachines uses Path
artifacts. Replaces the singular `image` variable from the canonical
pattern in comment 3100.
- Replace _image_stale_checks/skip_stale with public prelaunch_checks().
CLI now calls backend.prelaunch_checks(plan) before backend.launch(plan);
if StaleImageError is raised and the operator confirms, launch proceeds
without re-checking. Removes the while-True/skip_stale retry loop.
- Add abstract _build_or_load_images(plan) -> BottleImages to
BottleBackend. launch() calls it then passes images to _launch_impl.
Each backend implements both methods.
- Fix comment 3098 (macos-container): _build_images is removed.
build_or_load_images() has separate fresh/cached code paths — the
cached path never calls a build helper.
- Update _start_bundle (smolmachines) to accept sidecar_artifact: Path
directly. Sidecar artifact resolution moves to _sidecar_from_path(),
called by build_or_load_images alongside _agent_from_path().
- image_cache: StaleImageError exception + check_stale/check_stale_path (raise instead of warn)
- BottleBackend.launch: template method (skip_stale flag) that calls _image_stale_checks then _launch_impl
- Each backend: _image_stale_checks delegates to a stale_checks() function in its launch module; _launch_impl replaces launch override
- macos_container: adds image_created_at to util, cached-image support in _build_images, stale_checks
- cli/start.py: catches StaleImageError, prompts interactively, retries with skip_stale=True; headless mode dies on it
_auto_select_backend gains a prompt parameter (default True). When
prompt=False the docker-fallback [i/d/q] menu is skipped and the call
dies immediately with an actionable message ("set
BOT_BOTTLE_BACKEND=docker or install a VM backend"), preventing hangs
in CI, webhook dispatch, and orchestrator launches.
prepare_with_preflight passes prompt=not spec.headless so the headless
start path can never block waiting for TTY input it cannot receive.
Remove the private _read_tty_line duplicate from backend/__init__.py and
the local definition from cli/_common.py. Both now import from the
shared bot_bottle.util module.
- Remove hardcoded --backend=macos-container flag reference in
firecracker/util.py require_firecracker() error message
- Remove --backend flag from cli.py start; backend selection now
driven exclusively by BOT_BOTTLE_BACKEND env var or auto-selection
- Skip unavailable backends in cli.py cleanup (fixes crash on Linux
when macos-container.prepare_cleanup calls require_container())
- Add two-tier auto-selection: VM backend first (macos-container on
macOS, firecracker on Linux+KVM); fall back to docker with a
security warning and interactive i/d/q prompt; exit if docker
also unavailable and print VM install instructions
Closes#344
The macOS infra container's init script still spawned the supervisor as
`python3 /app/gateway_init.py`. 5ad3449 moved bot_bottle from flat files
under /app into a pip-installed package, so that file no longer exists:
the supervisor never started, mitmdump never generated its CA, and the
launch failed downstream with a misleading "gateway CA not available"
error pointing at TLS rather than at the supervisor.
This is the same defect 127ba49 fixed for the firecracker backend. It
survived on macOS because CI has no Apple Container runner — the KVM
runner added in c193b04 covers firecracker, and ubuntu-latest covers
docker, but nothing exercises macos_container.
test_macos_infra asserted the substring "gateway_init.py", which the
broken path satisfied; it now asserts the module form, matching the
assertion 127ba49 already made in test_firecracker_infra_vm.
Verified end to end on the macos-container backend: image builds, CA is
generated, agent registers, and a headless bottle runs a prompt.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
test_python_imports_supervise_module probed the image with
`import supervise; import supervise_server`, the flat sibling modules that
5ad3449 replaced. They now live in the installed package, so the test fails
with ModuleNotFoundError when actually run.
It went unnoticed because the whole class carries a GITEA_ACTIONS skip — it
is interactive-only, so CI never executes it.
All 4 tests in the class now pass against a freshly built gateway image.
Closes#429.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review follow-ups for the infra candidate-artifact flow:
1. publish-infra would fail on the first merge to main. The rootfs version now
hashes the dropbear bytes, and build-infra sets BOT_BOTTLE_FC_DROPBEAR to the
staged dropbear — but publish-infra (ubuntu-latest) set none, so _publish_
bundle re-derived the version with a "<missing>" dropbear and rejected the
candidate as "does not match checkout". Download the same firecracker-inputs
dropbear and export BOT_BOTTLE_FC_DROPBEAR in publish-infra (and add
stage-firecracker-inputs to its needs, since it now consumes that artifact).
2. Guard stage-firecracker-inputs with the same fork-PR check as the other
KVM-runner jobs, so a fork PR can't spin the privileged runner (it only
copies a static binary, but keep the posture consistent; it gates the whole
Firecracker chain via needs).
3. ensure_artifact_gz / _publish_bundle read version.txt before checking it
exists — a bundle missing it raised a raw FileNotFoundError instead of the
intended "bundle is incomplete" die. Guard the read (kept before the gz/sha
completeness check so a wrong-version bundle still reports the version
mismatch, not "incomplete").
4. test_infra_artifact.setUp didn't isolate BOT_BOTTLE_INFRA_ARTIFACT_DIR, so
an ambient candidate dir (the coverage job exports one) would send the
registry-pull tests down the local-bundle path. Pin it off in setUp; the
candidate-path cases set it explicitly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
The coverage CI job exports BOT_BOTTLE_INFRA_BUILD=local so the integration
suite builds infra/agent images from source. But that job also runs the unit
suite, and the ambient =local leaked into test_default_pulls_artifact_without_
docker — it drove ensure_built() down the local Docker-build branch, so the
"build_image not called / artifact pulled" assertion failed (build_image
called 3 times).
Pin the var off in that test so it exercises the default pull path regardless
of the runner's ambient env (mirrors test_local_mode_builds_deps_before_infra,
which pins it on). Full unit suite verified green under BOT_BOTTLE_INFRA_BUILD
=local.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
Dockerfile.gateway COPYs pyproject.toml into /src and runs pip install
/src, so it is a real input to the baked rootfs. A dependency-only change
previously reused stale artifact versions, potentially booting a rootfs
whose installed packages differed from the current checkout.
Also adds _fake_repo fixture support and a regression test so this input
can't silently drop out of the hash again.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two lint fixes, both test-only (no effect on the infra artifact version):
- annotate the TestAdoptable / TestKillInfraFirecrackers helper params
(reportMissingParameterType).
- test_docker_test_helpers: read __unittest_skip__ via getattr on the
dynamically-built Case type, matching the sibling assertion — pyright can't
see the attribute the skip decorator adds (reportAttributeAccessIssue).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
build_agent_rootfs_dir cached the built rootfs by Dockerfile content alone,
but util.inject_guest_boot then writes util._GUEST_INIT into it. So a fix to
the init — making /tmp world-writable (1777) so the agent can create scratch
dirs / git worktrees there — did NOT bust the cache: the KVM runner kept
reusing a stale agent-<dockerfilehash> rootfs built with the old init, and the
sandbox-escape README-push test kept failing at `git init /tmp/...` with
"Permission denied".
Key the cache on Dockerfile content AND the injected init (_rootfs_digest), so
an init change rebuilds. Self-busting: the new key yields a fresh cache dir, so
no manual cache clear on the runner.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
The KVM CI runner now sets BOT_BOTTLE_FC_* for its isolated pool (distinct
iface prefix / orch iface / nft table / IP base), and that env leaks into the
coverage job's test process. Five netpool tests hardcoded the default names
(bbfc*, bot_bottle_fc) and so failed there with e.g. ['bbfc1'] != ['bbci1'].
Assert against netpool's env-driven config instead — slot(i).iface / the
configured prefix — so the tests check the LOGIC regardless of which pool the
host is configured for. The single-source test now compares the parsed
defaults (netpool._DEFAULTS, env-independent) for the module constants, since
IFACE_PREFIX/NFT_TABLE legitimately layer an env override on top.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
The infra VM is a per-host singleton that outlives short-lived launchers, so
`ensure_running` adopts it when its control plane is healthy. But it adopted
ANY healthy VM regardless of the code that built it — so after an infra-code
change the old VM kept being adopted and the new code never booted. The only
way to dislodge it was an out-of-band `kill`, which then raced whatever
launched next. On CI this meant every infra change needed a manual VM kill.
Make adoption version-aware:
* boot records the infra-artifact version it booted from in a `booted-version`
marker beside the singleton; `stop` clears it.
* `ensure_running` adopts only when the marker matches the current version
(`_adoptable`); a missing/mismatched marker falls through to stop + reboot.
So a stale VM is replaced automatically on the next launch — no manual kill,
and it's concurrency-safe (the reboot happens under the singleton flock).
Also harden teardown: the PID file drifts after crashes / out-of-band kills,
so `stop` now also reaps any orphaned firecracker still bound to the infra
config path (scoped to that path, so interactive-pool VMs are untouched) —
otherwise a survivor holds the orchestrator TAP and the fresh boot dies with
"tap … Resource busy".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
The rootless agent rootfs build can land /tmp as 0755/root-owned, so the
agent (uid 1000 node) can't create scratch dirs there. The sandbox-escape
suite's README-push test does `cd /tmp && git init sandbox-escape-repo` and
died with "cannot mkdir sandbox-escape-repo: Permission denied" — before the
git-gate gitleaks hook could run — so the test read it as a missing hook.
On docker the agent inherits node:22-slim's 1777 /tmp, which is why only the
Firecracker path was affected (and only now that the suite runs end-to-end).
Set /tmp to 1777 in the guest PID-1 init, so every agent VM boots with a
usable /tmp regardless of rootfs perm drift.
Also update the infra-init unit test to assert the gateway launches via the
`bot_bottle.gateway_init` module (matching the prior fix), not a file path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
Two diff-coverage gaps on the ci-kvm-runner branch:
1. bot_bottle/backend/docker/setup.py: the try/except TimeoutExpired
block added in a prior commit had no tests reaching the subprocess
path. Add two tests to TestDockerSetupStatus: one for the success
path (subprocess returns 0) and one for the TimeoutExpired fallback.
2. bot_bottle/db_store.py: the _connection() context manager change in
is_migrated() was never exercised by unit tests (all callers mock
is_migrated() directly). Add test_db_store.py covering the absent-DB,
missing-schema-table, migrated, and behind-schema cases.
On a self-hosted KVM runner the process has a real controlling terminal
so name_color_modal successfully opens /dev/tty and enters a curses
loop waiting for keyboard input, hanging the test indefinitely.
Docker containers (ubuntu-latest runners) don't have a real /dev/tty,
causing an OSError that triggers the existing fallback — this is why
the hang was invisible in ubuntu-latest CI.
Also add timeout=5 to _daemon_reachable() to match the same defensive
fix already applied to docker_available() in tests/_docker.py.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Docker integration tests are already covered by the integration-docker
job on ubuntu-latest. On the KVM runner the Firecracker TAP/nftables
pool conflicts with Docker networking, causing those tests to hang
and the coverage job to never complete.
Add SKIP_DOCKER_TESTS env-var support to docker_available() and set
it for the integration phase of coverage.sh so only Firecracker
integration tests run there.
On the self-hosted KVM runner Docker is on PATH but the daemon socket
is unreachable (firewalled/dropped). subprocess.run(["docker", "info"])
with no timeout hangs indefinitely on a dropped connection, stalling the
coverage job for hours — one hang per @skip_unless_docker()-decorated
class, ~8 per integration suite run.
Add timeout=5 with a TimeoutExpired → False fallback so the check
resolves quickly to "unreachable" rather than blocking.
The sandbox-escape test was unconditionally skipped when GITEA_ACTIONS=true,
which prevented Firecracker orchestration coverage from being measured even
when BOT_BOTTLE_BACKEND=firecracker is set on the KVM runner.
Narrow the skip to: GITEA_ACTIONS=true AND BOT_BOTTLE_BACKEND != firecracker.
When BOT_BOTTLE_BACKEND=firecracker the test is explicitly opted in to run on
the self-hosted KVM runner where the required /dev/kvm + TAP pool exist.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NixOS doesn't populate /bin (no /bin/sleep), so the gateway-init
end-to-end tests that spawn a real `sleep` errored with
FileNotFoundError. Add tests/_bin.py with a PATH-resolved SLEEP
constant (falling back to /bin/sleep on FHS hosts) and import it in
test_gateway_init.py instead of hardcoding the path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9qa3xoavjQScufDfZaXKR
Closes#412.
The supervise MCP server blocked the agent's tool call polling for the
operator's decision, and on timeout returned `status: pending` with no
proposal id and no way to poll a specific proposal — so the only way to
learn a late decision was to re-propose (a duplicate).
- `handle_tools_call` pending timeout now returns the `proposal_id` and
points the agent at `check-proposal`.
- New `check-proposal` MCP tool: non-blocking status lookup by proposal id
(pending | approved | modified | rejected | unknown). Reuses the queue's
FileNotFoundError semantics; archives a decided proposal exactly like the
synchronous path, so a pending proposal stays visible to the operator
until it's both decided and polled.
- `TOOL_CHECK_PROPOSAL` constant, re-exported from supervise; kept out of
TOOLS since it never becomes a Proposal.tool.
Enforcement is unchanged — the tools only propose policy; the egress proxy
and git-gate still enforce — so returning early opens no hole. Follow-ups
(git-gate reject-requeue, backpressure, notifications, web console) are in
the PRD.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBCHap11yGAKuKfsehNPaD
The pre-receive hook scanned existing-branch updates with the delta range
$old..$new. On a rebase / non-fast-forward force-push onto an advanced main,
$old is no longer an ancestor of $new, so $old..$new expands to all of main's
new history — including the deliberate sandbox-escape gitleaks fixtures — and
the push is rejected on commits that belong to main, not the branch.
Unify the range on `$new --not --all` for every non-delete push (this is the
deferred open question from PRD 0028, which already applied it to new refs
for #106). It scans only the commits the push introduces and is
security-equivalent: the bare repo's refs come only from trusted upstream
mirror-fetch and gitleaks-gated pushes, so an excluded commit is
already-upstream or already-scanned. It is also more correct for
non-fast-forward pushes, where $old..$new can skip commits off the direct path.
Fixes#421
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1qRZTJC6qgBsUSjNrBdkX
`backend setup/status/teardown` manage host prerequisites only and never
open the store, but the CLI dispatcher ran the schema-migration gate before
every command. On a non-TTY runner the gate's `Migrate now? [y/N]` prompt
reads EOF and refuses, so `backend status --backend=firecracker` exits 1 —
breaking the Firecracker CI preflight on any host without a pre-migrated DB.
Exempt `backend` from the gate; store-touching commands stay gated.
Fixes#419
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1qRZTJC6qgBsUSjNrBdkX
- Rename _BACKENDS → _backends: pyright treats uppercase module-level
names as constants and flags the reassignment in _get_backends() as
reportConstantRedefinition; lowercase avoids this.
- Add TYPE_CHECKING guard importing CommitCancelled/Freezer/get_freezer
from .freeze: pyright cannot see module-level __getattr__ bindings, so
reportUnsupportedDunderAll fired for those three __all__ entries; the
guard makes them visible to the type checker without running at import
time.
- Update test_backend_selection.py to patch _backends (lowercase).
Importing backend.docker.util previously triggered eager loading of all
three backend packages (~76 modules) because backend/__init__.py imported
DockerBottleBackend, FirecrackerBottleBackend, and MacosContainerBottleBackend
at module scope. This made the module prohibitively expensive to import
from the orchestrator layer and elsewhere.
The three backend imports are now deferred into _get_backends(), which
loads all three on first call and caches the result in the module-level
_BACKENDS variable (initially None). Module-level __getattr__ exposes
backend classes and freeze symbols lazily for existing import/patch sites.
backend/docker/util.py raw subprocess.run(["docker", ...]) calls are
replaced with the shared run_docker primitive from docker_cmd, eliminating
the duplication between the backend and orchestrator implementations.
_silent_run() is removed; image_exists() is inlined directly onto
run_docker. The commit_container test is updated to patch run_docker
instead of subprocess.run.
Install bot_bottle via pip in Dockerfile.gateway instead of COPYing
individual .py files flat under /app/. This eliminates the try/except
import shims in egress_addon_core, dlp_detectors, egress_addon,
supervise, supervise_server, and git_http_backend that existed only
to support the flat-bundle layout.
Adds bot_bottle/constants.py as a single source of truth for
IDENTITY_HEADER and GIT_GATE_TIMEOUT_SECS, removing the duplicated
literal definitions in egress_addon.py, supervise_server.py,
git_http_backend.py, and git_gate_render.py.
Test files updated to match: test_supervise_server.py drops the
sys.path.insert hack in favour of direct package imports; the
egress_addon test shims no longer pre-populate sys.modules with a
bare egress_addon_core alias.
b1850be's fail-closed-503 test (rebased in from main) built a git-http
server with a flat repo.git and no policy_resolver. The resolver-only
data plane on this branch denies an unattributed request with 404 before
it reaches the access-hook path the test exercises, so it saw 404 != 503.
Nest the bare repo under <root>/<_BID>/ and set _FixedResolver(_BID) on
the server, matching every other test in this module, so the request is
attributed and reaches the access-hook (mocked to raise PermissionError)
that the 503 fail-closed behavior guards.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
The egress_entrypoint.sh fail-closed guard (this branch) exits 1 when
BOT_BOTTLE_ORCHESTRATOR_URL is unset, which broke the argv-construction
tests that ran the script without it. Set the URL in the shared
_run_entrypoint helper (a precondition for reaching mitmdump now, like
PATH) and add a test asserting the guard fails closed when it's absent.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
Follow-ups from the #402 review of the single-tenant data-plane teardown.
- egress_entrypoint.sh: refuse to launch mitmdump when
BOT_BOTTLE_ORCHESTRATOR_URL is unset, so the fail-closed guarantee no
longer rests solely on mitmproxy's errorcheck addon exiting on the
addon's load-time raise. A misconfigured gateway can never come up as
a bare TLS-bumping open proxy with no policy.
- orchestrator/gateway.py: ensure_running() raises GatewayError on an
empty orchestrator URL — a URL-less launch would only crash-loop the
now-resolver-only daemons (egress raises, git-http exits 1, supervise
exits 2). The env-injection branch is now unconditional.
- Drop stale "single-tenant" / "reads routes.yaml" comments in
gateway.py and egress_entrypoint.sh, and the /etc/egress/routes.yaml
layout line in Dockerfile.gateway.
- Tests: gateway fixtures supply an orchestrator URL; add a
refuse-without-URL test and assert the URL env is injected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
All three backends (docker, firecracker, macos-container) now launch through
the consolidated orchestrator, and every production gateway sets
BOT_BOTTLE_ORCHESTRATOR_URL — so the legacy single-tenant (`resolver is None`)
branches in the shared gateway's data plane were unreachable dead code, a second
security-relevant path to keep correct in parallel with the live one. Make the
orchestrator resolver mandatory and delete the single-tenant paths from the
three data-plane modules.
egress_addon.py: drop the static routes file entirely — EGRESS_ROUTES, _reload,
the SIGHUP handler, self.config, and the SUPERVISE_BOTTLE_SLUG env slug. The
per-request /resolve is the only policy source; __init__ fail-closes if
BOT_BOTTLE_ORCHESTRATOR_URL is unset. Introspection (`_egress.local/allowlist`)
now reports the calling bottle's *resolved* routes. The block/redact log gates
and _req_ctx redaction now read the per-flow config/env from the request-time
stash, so they use each bottle's log level and token set (they silently used the
empty static config before). Nothing sends `docker kill --signal HUP` to the
gateway in the consolidated model (the egress applicators fail closed), so
removing the SIGHUP reload is safe.
git_http_backend.py: resolver mandatory; no flat-root fallback. main() refuses
to start without an orchestrator URL; a request whose source resolves to no
bottle 404s.
supervise_server.py: resolver mandatory; every proposal is attributed to the
source-IP-resolved bottle. Remove handle_list_egress_routes (the proxy-fetch
introspection that only worked when the proxy carried one bottle's identity) —
list-egress-routes is answered from the resolved policy. main() refuses to start
without an orchestrator URL.
Tests: a host-side fake resolver serves each test's Config through the real
parse path (a small YAML-subset emitter round-trips route_to_yaml_dict); the
response/websocket hooks stash it as request() would. Deletes the tests for the
removed static-config, SIGHUP-reload, and single-tenant-passthrough paths; adds
fail-closed-without-orchestrator coverage.
Follow-up: gateway_init still forwards SIGHUP to the egress child (now dormant —
no one sends it); the README still describes the docker backend's per-bottle
topology. Both are outside the data-plane teardown.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
Cloning/fetching from the git-gate on the Apple-container backend failed with
"empty reply from server" (curl exit 52). Root cause: the git-http handler
crashed on every upload-pack with
PermissionError: [Errno 13] Permission denied: '/etc/git-gate/access-hook'
The access-hook is exec'd directly, so it needs the x bit. prepare() stages it
0o700 and trusted the gateway copy to carry that mode. `docker cp` does; the
Apple `container cp` (AppleGatewayTransport) does not, landing the hook 0o644 →
EACCES. The unhandled exception killed the handler thread, closing the socket
with no HTTP response — which the client sees as the opaque empty reply.
- provision_git_gate now `chmod +x`es the access-hook on the gateway side after
the copy, so it's executable under every transport (docker/apple/firecracker).
- git-http handler wraps the access-hook subprocess.run: an OSError /
SubprocessError (un-execable, timed out) now fails closed with a 503 instead
of crashing the thread into an empty reply — a gate that can't run its hook
should deny, visibly.
- Updates the now-misleading "docker cp preserves source mode" comment in
git_gate.prepare().
Regression tests: provisioning applies +x to the access-hook; the handler
returns 503 (not an empty reply) when the hook can't be exec'd.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Self-review of this PR: making `response()` run in the consolidated gateway
also activates its `LOG_FULL` `_log_response` call there — previously
unreachable, since the empty static config made `response()` return early. That
logger redacted with `os.environ`, which in multi-tenant mode does NOT hold the
bottle's per-request `/resolve` tokens (only the resolved `env` overlay does),
so a non-token-shaped provisioned secret appearing in a response could be logged
in the clear.
Thread the resolved per-flow `env` into `_log_request` / `_log_response` so the
LOG_FULL redaction scrubs the calling bottle's secrets. Adds a regression test
(a non-token-shaped `/resolve` secret, absent from os.environ, must not appear
in the response log) and updates the redaction-test helpers for the new arg.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
In the consolidated (multi-tenant) gateway the addon's static `self.config`
is empty — each request's real policy comes from the per-request `/resolve`.
`response()` and `websocket_message()` still matched routes against that empty
config, so inbound prompt-injection DLP and WebSocket credential/injection DLP
silently skipped every scan (fail-open) whenever the gateway ran multi-tenant.
This is backend-agnostic: the gateway image (and this addon) is shared by the
Firecracker, macOS, and docker consolidated backends.
Resolve the per-flow (config, slug, env) once in `request()`, stash it on
`flow.metadata`, and have both hooks read it back — falling back to the static
single-tenant values for a flow that never passed through `request()`. Reusing
the request's one `/resolve` avoids a round-trip per response and per WebSocket
frame.
Adds multi-tenant regression tests for both hooks that fail against the old
fall-open behaviour.
Refs: audit issue #400 (finding #2)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
Addresses the 5 lower-priority findings left as follow-up in the earlier
review, now that each has a concrete answer:
- Add gateway_name: str = GATEWAY_NAME to OrchestratorService.__init__
(mirrors the existing orchestrator_name param) and thread it through
_gateway(). Deletes the test's _IsolatedOrchestratorService subclass,
which existed only to override a private method for this one kwarg —
any caller needing gateway-name isolation can now use the public
constructor. Backward compatible: every existing caller constructs
OrchestratorService with keyword args and a sensible default is kept.
- Give the test its own fixed image tags (bot-bottle-orchestrator:itest,
bot-bottle-gateway:itest) instead of the production :latest ones.
_running_image_is_current() keys gateway staleness off the image tag's
ID, not per-instance identity, so rebuilding the shared :latest tag from
whatever's on disk during a test run could make a real host's running
production gateway look stale and get force-recreated. Fixed tags (not
per-run-suffixed, so they don't accumulate) fully decouple the two.
- setUp -> setUpClass/tearDownClass: all 5 tests are read-only checks
against the same running control plane, so one shared container
lifecycle replaces 5 (each of which paid its own container-start +
image-build + health-poll cycle). Cuts the file's wall-clock roughly
4x (11.5s -> 2.9-4.3s) and, combined with the network-rm cleanup from
the previous commit, means one cleanup instead of five.
- Reuse OrchestratorClient (bot_bottle/orchestrator/client.py) instead of
a hand-rolled urllib helper — the test now exercises the same
request/response code path the real host CLI uses, rather than a
private copy that could silently drift from it.
- Add the chown workaround test_multitenant_isolation.py already needed
for this exact bind-mount: the orchestrator container has no USER
directive, so it writes the registry DB as root into the throwaway
host_root; chown it back before tempdir cleanup so that doesn't raise
PermissionError on native Linux Docker (no UID remap, unlike Docker
Desktop's macOS VM).
Verified: ran the suite twice in a row (idempotency — fixed image tags
don't accumulate, 5/5 pass both times, 2.99-4.33s each), the real
~/.bot-bottle/control-plane-token is untouched, zero leaked networks or
containers after either run, exactly 2 :itest images (not growing), the
full orchestrator unit suite (93 tests) and the sibling docker
gateway/broker integration tests still pass. pyright clean, pylint
10.00/10 on both changed files.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Pushing the previous two fixes surfaced a real, currently-failing CI run
(gitea actions run 2164, jobs "integration" and "coverage"): every one of
the 5 new tests errored in setUp with
docker: Error response from daemon: error while creating mount source
path '/workspace/didericis/bot-bottle': mkdir /workspace: read-only
file system
This is finding #4 from the review of the previous commit, now confirmed
live rather than just plausible: act_runner's job-container topology
can't satisfy the orchestrator's host-path bind mount, the same
constraint test_multitenant_isolation.py, test_gateway_image.py, and
test_sandbox_escape.py already skip around. Add the identical
skip_unless_docker + GITEA_ACTIONS guard so this test degrades the same
way its siblings do instead of failing the job.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two bugs surfaced by a review of the previous commit:
- host_control_plane_token() resolves its path via the ambient
BOT_BOTTLE_ROOT env var, not the host_root kwarg passed to
OrchestratorService (that kwarg only controls the DB bind-mount
destination). The test's isolation claim was false as a result: running
it read/wrote the developer's real ~/.bot-bottle/control-plane-token
instead of the throwaway temp dir — confirmed directly on disk. Fixed
by pointing the env var at the same temp dir for the test's duration
and restoring it via addCleanup.
- ensure_running() creates a per-bottle Docker network but stop() only
ever removes containers, never the network — every run of this test
leaked one bridge network permanently (found and removed 5 from prior
runs via `docker network ls`). Fixed with an explicit `docker network
rm` in addCleanup.
Verified: re-ran the suite twice: 5/5 pass both times, the real
~/.bot-bottle/control-plane-token timestamp is unchanged across both runs
(proving isolation), and `docker network ls` shows zero leaked
bot-bottle-net-itest-* networks afterward. pyright clean, pylint 10.00/10.
Remaining findings from the same review (missing GITEA_ACTIONS skip
guard, root-owned bind-mount cleanup on native Linux, no setUpClass,
reinvented OrchestratorClient, gateway_name should be a constructor
param rather than a subclassed private-method override) are left for a
follow-up — each is a real, separate design/scope call, not a
mechanical fix like these two.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
didericis-codex
didericis-claude
didericis