diff --git a/.gitea/workflows/canaries.yml b/.gitea/workflows/canaries.yml index 9d064d37..fe242f54 100644 --- a/.gitea/workflows/canaries.yml +++ b/.gitea/workflows/canaries.yml @@ -2,7 +2,7 @@ # digest, etc.) without coupling every dev push to upstream registry # availability. # -# Opt-in via CLAUDE_BOTTLE_RUN_CANARIES=1 so the same files can be run +# Opt-in via BOT_BOTTLE_RUN_CANARIES=1 so the same files can be run # locally with the same gating. name: canaries @@ -17,7 +17,7 @@ jobs: canaries: runs-on: ubuntu-latest env: - CLAUDE_BOTTLE_RUN_CANARIES: "1" + BOT_BOTTLE_RUN_CANARIES: "1" steps: - name: Checkout uses: actions/checkout@v4 @@ -25,4 +25,7 @@ jobs: # No actions/setup-python: canaries are stdlib unittest on the image's # system Python 3.12 (older act_runner mishandles setup-python's PATH). - name: Run canaries - run: python3 -m unittest discover -t . -s tests/canaries -v + run: | + python3 -m scripts.unittest_gate \ + -t . -s tests/canaries -v \ + --minimum-executed 1 --fail-on-skip diff --git a/.gitea/workflows/pre-release-test.yml b/.gitea/workflows/pre-release-test.yml index 99f87bad..4263914f 100644 --- a/.gitea/workflows/pre-release-test.yml +++ b/.gitea/workflows/pre-release-test.yml @@ -60,6 +60,9 @@ jobs: integration-docker: runs-on: ubuntu-latest + concurrency: + group: integration-docker-infra + cancel-in-progress: false steps: - name: Checkout uses: actions/checkout@v4 @@ -84,7 +87,30 @@ jobs: env: BOT_BOTTLE_BACKEND: docker COVERAGE_FILE: ${{ github.workspace }}/.coverage.docker - run: python3 -m coverage run -m unittest discover -t . -s tests/integration -v + run: | + set -euo pipefail + DOCKER_CLIENT_NETWORK=$( + docker inspect "$(hostname)" | + python3 -c 'import json,sys; n=json.load(sys.stdin)[0]["NetworkSettings"]["Networks"]; print(next(iter(n)))' + ) + test -n "$DOCKER_CLIENT_NETWORK" + RUN_KEY="${GITHUB_RUN_ID:-${GITHUB_RUN_NUMBER:-0}}" + export NO_PROXY="*" + export no_proxy="*" + export BOT_BOTTLE_DOCKER_CLIENT_NETWORK="$DOCKER_CLIENT_NETWORK" + export BOT_BOTTLE_DOCKER_ROOT_MOUNT="bot-bottle-ci-root-$RUN_KEY" + export BOT_BOTTLE_DOCKER_CA_MOUNT="bot-bottle-ci-ca-$RUN_KEY" + python3 -m coverage run -m scripts.unittest_gate \ + -t . -s tests/integration -v \ + --minimum-executed 22 --fail-on-skip + + - name: Clean Docker integration volumes + if: always() + run: | + RUN_KEY="${GITHUB_RUN_ID:-${GITHUB_RUN_NUMBER:-0}}" + docker volume rm --force \ + "bot-bottle-ci-root-$RUN_KEY" \ + "bot-bottle-ci-ca-$RUN_KEY" 2>/dev/null || true # Non-dot name so upload-artifact's dotfile-skipping glob picks it up. - name: Stage docker coverage for upload diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 23a00e3b..963e7d0f 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -12,10 +12,14 @@ on: - 'bot_bottle/**' - 'tests/**/*.py' - 'cli.py' + - 'install.sh' + - 'setup.py' + - 'MANIFEST.in' + - 'flake.nix' + - 'nix/firecracker-netpool.nix' - 'scripts/coverage.sh' - 'scripts/critical-modules.txt' - - 'scripts/diff_coverage.py' - - 'scripts/tracker_policy.py' + - 'scripts/**/*.py' - 'scripts/firecracker-netpool.sh' - 'Dockerfile*' - 'pyproject.toml' @@ -23,15 +27,20 @@ on: - '.coveragerc' - '.dockerignore' - '.gitea/workflows/test.yml' + - '.gitea/workflows/pre-release-test.yml' pull_request: paths: - 'bot_bottle/**' - 'tests/**/*.py' - 'cli.py' + - 'install.sh' + - 'setup.py' + - 'MANIFEST.in' + - 'flake.nix' + - 'nix/firecracker-netpool.nix' - 'scripts/coverage.sh' - 'scripts/critical-modules.txt' - - 'scripts/diff_coverage.py' - - 'scripts/tracker_policy.py' + - 'scripts/**/*.py' - 'scripts/firecracker-netpool.sh' - 'Dockerfile*' - 'pyproject.toml' @@ -39,6 +48,7 @@ on: - '.coveragerc' - '.dockerignore' - '.gitea/workflows/test.yml' + - '.gitea/workflows/pre-release-test.yml' jobs: unit: @@ -71,6 +81,9 @@ jobs: integration-docker: runs-on: ubuntu-latest + concurrency: + group: integration-docker-infra + cancel-in-progress: false steps: - name: Checkout uses: actions/checkout@v4 @@ -87,7 +100,33 @@ jobs: env: BOT_BOTTLE_BACKEND: docker COVERAGE_FILE: ${{ github.workspace }}/.coverage.docker - run: python3 -m coverage run -m unittest discover -t . -s tests/integration -v + run: | + set -euo pipefail + # act_runner executes this job in a container while sharing the host + # Docker socket. Attach control-plane siblings to the job's network, + # and use named volumes for state the host daemon must mount. + DOCKER_CLIENT_NETWORK=$( + docker inspect "$(hostname)" | + python3 -c 'import json,sys; n=json.load(sys.stdin)[0]["NetworkSettings"]["Networks"]; print(next(iter(n)))' + ) + test -n "$DOCKER_CLIENT_NETWORK" + RUN_KEY="${GITHUB_RUN_ID:-${GITHUB_RUN_NUMBER:-0}}" + export NO_PROXY="*" + export no_proxy="*" + export BOT_BOTTLE_DOCKER_CLIENT_NETWORK="$DOCKER_CLIENT_NETWORK" + export BOT_BOTTLE_DOCKER_ROOT_MOUNT="bot-bottle-ci-root-$RUN_KEY" + export BOT_BOTTLE_DOCKER_CA_MOUNT="bot-bottle-ci-ca-$RUN_KEY" + python3 -m coverage run -m scripts.unittest_gate \ + -t . -s tests/integration -v \ + --minimum-executed 22 --fail-on-skip + + - name: Clean Docker integration volumes + if: always() + run: | + RUN_KEY="${GITHUB_RUN_ID:-${GITHUB_RUN_NUMBER:-0}}" + docker volume rm --force \ + "bot-bottle-ci-root-$RUN_KEY" \ + "bot-bottle-ci-ca-$RUN_KEY" 2>/dev/null || true - name: Stage docker coverage for upload run: cp .coverage.docker coverage-docker.dat diff --git a/.gitea/workflows/update-badges.yml b/.gitea/workflows/update-badges.yml index 94ec1a98..223de9de 100644 --- a/.gitea/workflows/update-badges.yml +++ b/.gitea/workflows/update-badges.yml @@ -33,19 +33,28 @@ jobs: - name: Run coverage and extract percentage id: coverage run: | - python3 -m coverage run -m unittest discover -t . -s tests/unit > /dev/null 2>&1 || true - PERCENT=$(python3 -m coverage report 2>/dev/null | grep '^TOTAL' | grep -oP '\d+(?=%)' | tail -1) + set -euo pipefail + # Never publish a badge from a failed or partial test run. + python3 -m coverage run -m unittest discover -t . -s tests/unit + REPORT=$(python3 -m coverage report) + printf '%s\n' "$REPORT" + PERCENT=$(printf '%s\n' "$REPORT" | awk '$1 == "TOTAL" {gsub("%", "", $NF); print $NF}') + test -n "$PERCENT" echo "percent=$PERCENT" >> $GITHUB_OUTPUT echo "Coverage: $PERCENT%" - name: Extract core (critical-module) coverage percentage id: core_coverage run: | + set -euo pipefail # Reuses the .coverage data from the previous step. The core list is - # the single source of truth in scripts/critical-modules.txt; every - # core module is unit-tested, so the unit-only run is accurate for it. - INCLUDE=$(grep -vE '^[[:space:]]*(#|$)' scripts/critical-modules.txt | paste -sd, -) - PERCENT=$(python3 -m coverage report --include="$INCLUDE" 2>/dev/null | grep '^TOTAL' | grep -oP '\d+(?=%)' | tail -1) + # validated single source of truth. Fail if a listed path disappeared + # or if the measured core falls below ADR 0004's 90% minimum. + INCLUDE=$(python3 scripts/critical_modules.py) + REPORT=$(python3 -m coverage report --include="$INCLUDE" --fail-under=90) + printf '%s\n' "$REPORT" + PERCENT=$(printf '%s\n' "$REPORT" | awk '$1 == "TOTAL" {gsub("%", "", $NF); print $NF}') + test -n "$PERCENT" echo "percent=$PERCENT" >> $GITHUB_OUTPUT echo "Core coverage: $PERCENT%" diff --git a/README.md b/README.md index dae91e9b..da908a87 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,7 @@ On compatible macOS hosts, the default backend requires Apple's `container` CLI Use `BOT_BOTTLE_BACKEND=docker ./cli.py start ` on hosts where neither Apple Container nor KVM is available and Docker is the desired backend. -> **CI (macOS Apple Container):** the `integration-macos` job (`.gitea/workflows/test.yml`) runs the integration suite against `BOT_BOTTLE_BACKEND=macos-container` on a self-hosted macOS runner labelled `macos`, because Apple Container needs the host virtualization framework and cannot run in a Linux container (so it can't reuse the `kvm` runner). Provision an Apple Silicon host with the `container` CLI on `PATH` and `container system status` running, then register the runner in **host mode** (not docker mode) with the `macos` label — `brew install gitea-runner` (the `act_runner` rename). Give it a Python ≥ 3.11 with `coverage` importable on the launchd service's `PATH` (a launchd service doesn't inherit your shell profile, so pin `node` and the Python env explicitly). The job is **advisory** — `workflow_dispatch` (manual) only, never triggered by push or PR — since a single laptop that sleeps/roams must not block merges or churn on every push to main; its coverage doesn't feed the gate. The infra container is a singleton (`bot-bottle-mac-infra`), so keep runner concurrency at 1. +> **CI (macOS Apple Container):** the advisory `integration-macos` job in `.gitea/workflows/pre-release-test.yml` runs only on manual dispatch. It targets a self-hosted host-mode runner labelled `macos`; Apple Container cannot run inside the Linux pull-request runner. Provision an Apple Silicon host with the `container` CLI running and Python ≥ 3.11 plus `coverage` on the launchd service's explicit `PATH`. The infra container is a singleton (`bot-bottle-mac-infra`), so keep runner concurrency at 1. Its coverage is reported separately and never feeds the required pull-request gate. ### Containers inside a bottle @@ -174,7 +174,7 @@ BOT_BOTTLE_BACKEND=firecracker ./cli.py start > **NixOS:** enable `virtualisation.docker`, ensure the KVM module is loaded (`boot.kernelModules = [ "kvm-intel" ];` or `kvm-amd`), and add your user to the `kvm` and `docker` groups. For the network pool, consume the flake module — `imports = [ inputs.bot-bottle.nixosModules.firecracker-netpool ]; services.bot-bottle-firecracker = { enable = true; owner = "you"; };` — then `nixos-rebuild switch` (imperative nft/TAP rules don't survive a rebuild; channel users can `imports = [ /nix/firecracker-netpool.nix ]`). `firecracker` isn't in nixpkgs by default as a user binary — install the release binary (pin the version) and put it on `PATH`. -> **CI:** the coverage gate (`.gitea/workflows/test.yml` → `coverage` job) runs on a self-hosted runner labelled `kvm`, because the Firecracker backend's VM/SSH orchestration is exercised only by the integration suite, which needs `/dev/kvm` + the provisioned pool (a container runner would skip it and read as uncovered). Provision that runner exactly like a normal Firecracker host — `firecracker` on `PATH`, `/dev/kvm`, the cached guest kernel + static dropbear, and the pool installed as the persistent systemd unit — then register it with the `kvm` label. A Docker-capable hosted job builds the candidate once; KVM tests boot those exact bytes, and a successful main run publishes them. The unit/lint jobs still run on `ubuntu-latest`. +> **CI:** Firecracker integration runs in the manually dispatched `.gitea/workflows/pre-release-test.yml` on a self-hosted runner labelled `kvm`; privileged KVM hosts never execute unreviewed PR code automatically. Provision it like a normal Firecracker host: `firecracker` on `PATH`, `/dev/kvm`, the cached guest kernel and static dropbear, and the persistent TAP/nft pool. The required pull-request workflow runs unit plus the complete Docker integration suite on `ubuntu-latest`; see `docs/ci.md`. ```sh ./cli.py start # builds the image on first run, drops you into claude diff --git a/bot_bottle/backend/docker/gateway.py b/bot_bottle/backend/docker/gateway.py index 612b3d77..689ba5f0 100644 --- a/bot_bottle/backend/docker/gateway.py +++ b/bot_bottle/backend/docker/gateway.py @@ -17,6 +17,10 @@ from ...gateway import ( DEFAULT_CA_TIMEOUT_SECONDS, CA_POLL_SECONDS, GATEWAY_CA_CERT, GatewayError ) +DEFAULT_GATEWAY_SUBNET = "10.242.255.0/24" +_GATEWAY_SUBNET_LABEL = "bot-bottle.gateway-subnet" + + class DockerGateway(Gateway): """The consolidated gateway as a single, fixed-name Docker container. @@ -35,6 +39,8 @@ class DockerGateway(Gateway): build_context: Path | None = None, dockerfile: str | None = GATEWAY_DOCKERFILE, host_port_bindings: tuple[int, ...] = (), + ca_mount_source: str | Path | None = None, + subnet: str | None = None, ) -> None: self.image_ref = image_ref self.name = name @@ -59,6 +65,15 @@ class DockerGateway(Gateway): # backend's dev-harness gateway so VMs can reach it via their TAP link; # Docker's DNAT + the nft `ct status dnat accept` rule handle the rest. self._host_port_bindings = host_port_bindings + self._subnet = ( + subnet + or os.environ.get("BOT_BOTTLE_DOCKER_GATEWAY_SUBNET", "").strip() + or DEFAULT_GATEWAY_SUBNET + ) + configured_ca = os.environ.get("BOT_BOTTLE_DOCKER_CA_MOUNT", "").strip() + self._ca_mount_source = str( + ca_mount_source or configured_ca or host_gateway_ca_dir() + ) def image_exists(self) -> bool: return run_docker(["docker", "image", "inspect", self.image_ref]).returncode == 0 @@ -109,10 +124,34 @@ class DockerGateway(Gateway): def _ensure_network(self) -> None: """Create the shared gateway network if it doesn't exist. Idempotent — a concurrent create loses harmlessly (the loser sees 'already exists'). - Docker picks the subnet; the launcher reads it back to allocate IPs.""" - if run_docker(["docker", "network", "inspect", self.network]).returncode == 0: - return - proc = run_docker(["docker", "network", "create", self.network]) + The explicit subnet is required because bottle attribution pins source + IPs; Docker rejects static endpoint addresses on an auto-IPAM network.""" + inspected = run_docker([ + "docker", "network", "inspect", + "--format", f'{{{{index .Labels "{_GATEWAY_SUBNET_LABEL}"}}}}', + self.network, + ]) + if inspected.returncode == 0: + marker = inspected.stdout.strip() + if marker in {"", self._subnet}: + return + if inspected.returncode == 0: + # Migrate the stale auto-IPAM network created by older releases. + # Removing the fixed gateway is safe here: this launch recreates it. + run_docker(["docker", "rm", "--force", self.name]) + removed = run_docker(["docker", "network", "rm", self.network]) + if removed.returncode != 0: + raise GatewayError( + f"gateway network {self.network} needs explicit subnet " + f"{self._subnet} but could not be replaced: " + f"{removed.stderr.strip()}" + ) + proc = run_docker([ + "docker", "network", "create", + "--subnet", self._subnet, + "--label", f"{_GATEWAY_SUBNET_LABEL}={self._subnet}", + self.network, + ]) if proc.returncode != 0 and "already exists" not in proc.stderr: raise GatewayError( f"gateway network {self.network} failed to create: {proc.stderr.strip()}" @@ -143,9 +182,9 @@ class DockerGateway(Gateway): # Recreate when the running container's image is stale (a rebuild), # so source changes to the gateway's flat daemons take effect — not # just when the container is absent. + self._ensure_network() if self.is_running() and self._running_image_is_current(): return - self._ensure_network() # Clear any stale (stopped OR outdated-image) container holding the # fixed name, then start fresh. `rm --force` on an absent name is a # tolerated no-op. @@ -158,7 +197,7 @@ class DockerGateway(Gateway): # Persist the self-generated CA on the host so it survives both # container recreation AND docker volume pruning (agents trust it) # — see host_gateway_ca_dir / issue #450. - "--volume", f"{host_gateway_ca_dir()}:{MITMPROXY_HOME}", + "--volume", f"{self._ca_mount_source}:{MITMPROXY_HOME}", # No DB mount: the data plane (egress / supervise / git-gate) reaches # the supervise queue over the control-plane RPC and never opens # bot-bottle.db, so the gateway container gets no file handle on it @@ -253,4 +292,4 @@ class DockerGateway(Gateway): def provisioning_transport(self) -> GatewayTransport: """The exec/cp transport git-gate provisioning stages per-bottle repos + deploy keys through (over the docker socket).""" - return DockerGatewayTransport(self.name) \ No newline at end of file + return DockerGatewayTransport(self.name) diff --git a/bot_bottle/backend/docker/infra.py b/bot_bottle/backend/docker/infra.py index 66e26837..b0a74484 100644 --- a/bot_bottle/backend/docker/infra.py +++ b/bot_bottle/backend/docker/infra.py @@ -33,7 +33,6 @@ from .orchestrator import ( ORCHESTRATOR_NAME, ORCHESTRATOR_NETWORK, ) -from ...paths import bot_bottle_root from ... import resources from ...gateway import ( GATEWAY_IMAGE, @@ -69,6 +68,8 @@ class DockerInfraService(InfraService): gateway_image: str = GATEWAY_IMAGE, repo_root: Path | None = None, host_root: Path | None = None, + root_mount_source: str | Path | None = None, + gateway_ca_mount_source: str | Path | None = None, orchestrator_name: str = ORCHESTRATOR_NAME, orchestrator_label: str = ORCHESTRATOR_LABEL, gateway_name: str = GATEWAY_NAME, @@ -78,10 +79,14 @@ class DockerInfraService(InfraService): self.control_network = control_network self.orchestrator_image = orchestrator_image self.gateway_image = gateway_image - # Build context / bind-mount source: the repo root in a checkout, a - # staged copy from the installed wheel otherwise (bot_bottle.resources). + # Build context: the repo root in a checkout, a staged copy from the + # installed wheel otherwise (bot_bottle.resources). self._repo_root = repo_root if repo_root is not None else resources.build_root() - self._host_root = host_root or bot_bottle_root() + if host_root is not None and root_mount_source is not None: + raise ValueError("pass host_root or root_mount_source, not both") + self._host_root = host_root + self._root_mount_source = root_mount_source + self._gateway_ca_mount_source = gateway_ca_mount_source self._orchestrator_name = orchestrator_name self._orchestrator_label = orchestrator_label self._gateway_name = gateway_name @@ -98,6 +103,7 @@ class DockerInfraService(InfraService): control_network=self.control_network, repo_root=self._repo_root, host_root=self._host_root, + root_mount_source=self._root_mount_source, ) def gateway(self) -> DockerGateway: @@ -112,6 +118,7 @@ class DockerInfraService(InfraService): network=self.network, control_network=self.control_network, build_context=self._repo_root, + ca_mount_source=self._gateway_ca_mount_source, ) def ensure_running( diff --git a/bot_bottle/backend/docker/orchestrator.py b/bot_bottle/backend/docker/orchestrator.py index 6197fe87..b293c9c1 100644 --- a/bot_bottle/backend/docker/orchestrator.py +++ b/bot_bottle/backend/docker/orchestrator.py @@ -42,13 +42,9 @@ ORCHESTRATOR_IMAGE = os.environ.get( ) ORCHESTRATOR_DOCKERFILE = "Dockerfile.orchestrator" # Baked as a container label so `ensure_running` can detect whether the running -# orchestrator is executing the current bind-mounted source. +# orchestrator image was built from the current source. ORCHESTRATOR_SOURCE_HASH_LABEL = "bot-bottle-orchestrator-source-hash" -# The bind-mount path for the live control-plane source inside the container. -# PYTHONPATH points here so a code change takes effect on the next launch -# without an image rebuild. -_SRC_IN_CONTAINER = "/bot-bottle-src" # Bot-bottle host-root bind-mount (DB + state) inside the orchestrator. The # control plane opens bot-bottle.db under here (via BOT_BOTTLE_ROOT -> # host_db_path()); it is the ONLY container with a handle on it (issue #469). @@ -72,23 +68,51 @@ class DockerOrchestrator(Orchestrator): control_network: str = ORCHESTRATOR_NETWORK, repo_root: Path | None = None, host_root: Path | None = None, + root_mount_source: str | Path | None = None, + client_host: str | None = None, + client_network: str | None = None, + bind_host: str | None = None, dockerfile: str | None = ORCHESTRATOR_DOCKERFILE, ) -> None: + if host_root is not None and root_mount_source is not None: + raise ValueError("pass host_root or root_mount_source, not both") self.image_ref = image_ref self.name = name self.label = label self.port = port self.control_network = control_network - # Build context / bind-mount source: the repo root in a checkout, a - # staged copy from the installed wheel otherwise (bot_bottle.resources). + # Build context: the repo root in a checkout, a staged copy from the + # installed wheel otherwise (bot_bottle.resources). self._repo_root = repo_root if repo_root is not None else resources.build_root() - self._host_root = host_root or bot_bottle_root() + configured_root = os.environ.get("BOT_BOTTLE_DOCKER_ROOT_MOUNT", "").strip() + self._root_mount_source = str( + root_mount_source or configured_root or host_root or bot_bottle_root() + ) + configured_network = os.environ.get( + "BOT_BOTTLE_DOCKER_CLIENT_NETWORK", "" + ).strip() + self._client_network = client_network or configured_network or None + configured_host = os.environ.get( + "BOT_BOTTLE_DOCKER_HOST_ADDRESS", "" + ).strip() + self._client_host = ( + client_host or configured_host + or (self.name if self._client_network else "127.0.0.1") + ) + # A socket-shared CI runner reaches published ports through its Docker + # network rather than its own loopback. Production stays bound to host + # loopback unless a caller explicitly selects another client. + self._bind_host = bind_host or ( + "0.0.0.0" + if not self._client_network and self._client_host != "127.0.0.1" + else "127.0.0.1" + ) self._dockerfile = dockerfile def url(self) -> str: - """Host-side control-plane URL — the orchestrator's published loopback, - which the CLI reaches.""" - return f"http://127.0.0.1:{self.port}" + """Control-plane URL reachable by this Docker client.""" + port = DEFAULT_PORT if self._client_network else self.port + return f"http://{self._client_host}:{port}" def gateway_url(self) -> str: """The URL the gateway's data plane resolves policy against — the @@ -119,8 +143,7 @@ class DockerOrchestrator(Orchestrator): return self.name in proc.stdout.split() def _source_current(self, current_hash: str) -> bool: - """True iff the running orchestrator was started from the current - bind-mounted source.""" + """True iff the running orchestrator image matches current source.""" if not self.is_running(): return False proc = run_docker([ @@ -182,15 +205,19 @@ class DockerOrchestrator(Orchestrator): # Control network only — agents are never on it, so they have no # route to the control plane (the L3 block, not just the JWT). "--network", self.control_network, - # Host CLI reaches the control plane here (loopback only). The + # Host CLI reaches the control plane here (loopback by default). + # Socket-shared CI joins the container directly to the job network; + # the host-side mapping remains loopback-only in that topology. The # orchestrator listens on the fixed DEFAULT_PORT inside the # container; self.port is the host-side published port. - "--publish", f"127.0.0.1:{self.port}:{DEFAULT_PORT}", - # Live control-plane source (code changes without an image rebuild). - "--volume", f"{self._repo_root}:{_SRC_IN_CONTAINER}:ro", - "--env", f"PYTHONPATH={_SRC_IN_CONTAINER}", + "--publish", f"{self._bind_host}:{self.port}:{DEFAULT_PORT}", + # The image was rebuilt from `_repo_root` immediately before this + # launch. Running its baked package avoids a host-path bind mount, + # which is both more production-like and works with socket-shared + # CI where the daemon cannot see the job container's workspace. # Orchestrator registry DB on the host (sole writer: control plane). - "--volume", f"{self._host_root}:{_ROOT_IN_CONTAINER}", + # `root_mount_source` may be a host path or a named Docker volume. + "--volume", f"{self._root_mount_source}:{_ROOT_IN_CONTAINER}", "--env", f"BOT_BOTTLE_ROOT={_ROOT_IN_CONTAINER}", # The signing key — held ONLY by the orchestrator (it verifies # tokens); the gateway gets the pre-minted `gateway` JWT, never the @@ -205,6 +232,15 @@ class DockerOrchestrator(Orchestrator): raise OrchestratorStartError( f"orchestrator container failed to start: {proc.stderr.strip()}" ) + if self._client_network: + proc = run_docker([ + "docker", "network", "connect", self._client_network, self.name, + ]) + if proc.returncode != 0: + raise OrchestratorStartError( + f"orchestrator container failed to join client network " + f"{self._client_network}: {proc.stderr.strip()}" + ) def stop(self) -> None: """Remove the control-plane container (idempotent).""" diff --git a/docs/ci.md b/docs/ci.md index 64e96c5d..17c019f4 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -1,50 +1,53 @@ # CI -The test workflow lives at [`.gitea/workflows/test.yml`](../.gitea/workflows/test.yml). -It runs the unit suite plus one integration job per backend -(`integration-docker`, `integration-firecracker`, `integration-macos`) on: +## Required pull-request gate -- every push to a branch with an open pull request, and -- every push to `main`. +[`.gitea/workflows/test.yml`](../.gitea/workflows/test.yml) runs the unit +suite, Docker integration suite, combined coverage report, and diff-coverage +gate when tested package/build inputs change on a pull request or on `main`. -`integration-macos` is the exception: it is **advisory**, running only on -`workflow_dispatch` (manual dispatch), never on push or pull requests. It targets the -Apple Container backend on a self-hosted macOS runner (label `macos`, -registered in host mode — Apple Container can't run in a Linux container, so it -can't reuse the `kvm` runner). A single non-redundant laptop must not be able -to block a PR merge, so the job stays out of the `coverage` job's `needs` and -its coverage never feeds the diff-coverage gate. Because the infra container is -a singleton (`bot-bottle-mac-infra`), the job declares a `concurrency` group -and tears the container down on exit; keep runner concurrency at 1. See the -README "macOS Apple Container" CI note for runner provisioning. +The Docker job preflights the backend before discovery. Gitea's `act_runner` +runs the job in a container with the host Docker socket, so the test process +reaches control-plane siblings through the job's Docker network and uses named +Docker volumes for orchestrator/CA state the host daemon must mount. The +orchestrator runs the package baked into the image built from the checkout; it +does not bind the job container's invisible workspace into a sibling container. +Docker integration jobs share fixed singleton names, so required and manual +runs use one non-cancelling concurrency group. The shared agent/gateway network +has an explicit subnet, which Docker requires for the pinned source IPs used as +the isolation/attribution key. -Each integration job selects its backend via `BOT_BOTTLE_BACKEND` and -runs a **preflight** (`./cli.py backend status --backend=`) that -prints a clear per-check readiness summary and fails the job when the -backend is missing — so absent infrastructure is visible at the job level -rather than hidden among per-test `unittest.skip` lines. The skip guards in -[`tests/_backend.py`](../tests/_backend.py) gate on the same readiness -check (`bot_bottle.backend.has_backend`): backend-agnostic tests use -`skip_unless_selected_backend_available()` and run through whichever -backend is selected (checking, e.g., Linux + `/dev/kvm` for Firecracker -rather than unrelated Docker availability); Docker-implementation tests use -`skip_unless_backend("docker")` and no-op under a non-Docker run. +`scripts.unittest_gate` enforces the Docker job's contract: all 22 integration +tests must execute and none may skip. This includes the real gateway-image, +control-plane authentication, multitenant policy/token isolation, +sandbox-escape, and orphan-network tests. Backend skip decorators remain useful +for local runs, but the CI preflight plus execution-count gate prevents a +missing backend or runner-topology regression from becoming a green job. -A small subset of integration tests skip when running specifically -under Gitea Actions (`GITEA_ACTIONS=true`), because `act_runner` runs -the job inside a container with the host's `/var/run/docker.sock` -mounted in. That topology breaks two assumptions those tests make: +Combined unit + Docker coverage is informational globally. Two focused gates +are enforced: -- networks created via the host daemon aren't always visible to a - same-process `docker network ls` call from inside the job container, - and -- ports published by sibling containers land on the host's loopback, - not on the job container's `127.0.0.1` — so HTTP probes against - `http://127.0.0.1:` from inside the job time out. +- changed executable Python lines must be at least 90% covered; and +- the validated critical security/logic core must remain at least 90% covered. -The affected tests (`test_orphan_cleanup.test_create_and_remove`, -`test_gateway_image.TestGatewayImage`) still run -locally where the test process and Docker daemon share a host. -Making them work in CI is a follow-up: either re-write them to -discover container IPs via `docker inspect`, or reconfigure the -runner with host networking. +## Privileged pre-release matrix + +[`.gitea/workflows/pre-release-test.yml`](../.gitea/workflows/pre-release-test.yml) +is manually dispatched before a release. It repeats unit and Docker integration +coverage, then runs: + +- Firecracker integration on the self-hosted `kvm` runner; and +- advisory Apple Container integration on the self-hosted `macos` runner. + +These privileged host-mode runners never execute unreviewed pull-request code +automatically. Firecracker coverage is combined in the manual pre-release +report; macOS reports advisory coverage in its own job. The macOS infra +container is a singleton, so its job uses a concurrency group and always tears +the service down. + +## Scheduled canary + +[`.gitea/workflows/canaries.yml`](../.gitea/workflows/canaries.yml) runs weekly +and on manual dispatch. It verifies the pinned gitleaks release URL, checksum, +archive shape, and executable. The same unittest execution gate requires at +least one executed canary and rejects skips. diff --git a/docs/decisions/0004-coverage-policy.md b/docs/decisions/0004-coverage-policy.md index acaaa4e2..cb0ff858 100644 --- a/docs/decisions/0004-coverage-policy.md +++ b/docs/decisions/0004-coverage-policy.md @@ -34,12 +34,13 @@ a regression (Goodhart's law). Coverage is **risk-weighted**, measured over the **combined unit + integration** suites, with three rules: -1. **Critical modules target ≥ 90%.** The security/logic core — - `egress_addon{,_core}.py`, `dlp_detectors.py`, `egress.py`, - `manifest*.py`, `git_gate.py`, `git_http_backend.py`, `supervise.py`, - `yaml_subset.py`, `bottle_state.py` — is Docker-independent and - unit-testable, so it carries the high bar. We ratchet toward 90% as - these modules are touched; new gaps in them are not acceptable. +1. **Critical modules must remain ≥ 90%.** The curated security/logic core + covers the host and gateway egress policy, manifest trust boundary, + git-gate enforcement, supervise protocol/server, YAML parser, and bottle + state. The concrete module list lives in `scripts/critical-modules.txt`; + `scripts/critical_modules.py` rejects stale or ambiguous entries before + Coverage.py can silently ignore them. These modules are unit-testable, so + CI enforces the aggregate minimum independently of diff coverage. 2. **Subprocess/backend orchestration is covered by the integration suite, not omitted.** `scripts/coverage.sh` runs unit + integration @@ -82,6 +83,9 @@ omit list. (critical-module standard + diff coverage) are Docker-independent. - "We're at N%" is now a curated figure; outsiders should read the policy, not just the badge. +- A rename or removal in the curated list fails CI. Updating the list is an + explicit review of where the security-critical behavior moved, not a way to + improve the percentage by omission. ## Links diff --git a/scripts/coverage.sh b/scripts/coverage.sh index b202cc70..0dab9a84 100755 --- a/scripts/coverage.sh +++ b/scripts/coverage.sh @@ -20,10 +20,10 @@ cd "$(dirname "$0")/.." PY="${PYTHON:-python3}" -# Critical security/logic core held to the high bar by ADR 0004. The list -# lives in one place (scripts/critical-modules.txt) so this report and the -# README "core coverage" badge can't drift; comma-join it for --include. -CRITICAL=$(grep -vE '^[[:space:]]*(#|$)' scripts/critical-modules.txt | paste -sd, -) +# Critical security/logic core held to the high bar by ADR 0004. The helper +# fails before coverage when a curated path was renamed or removed; Coverage.py +# itself would silently ignore that stale include and inflate the score. +CRITICAL=$("$PY" scripts/critical_modules.py) if [ "${1:-}" = "aggregate" ]; then # Aggregate mode: combine .coverage.* artifacts already in the workspace. @@ -34,8 +34,8 @@ if [ "${1:-}" = "aggregate" ]; then "$PY" -m coverage report -m if [ "${2:-}" = "critical" ]; then - echo "== critical modules (ADR 0004 target: 90%) ==" >&2 - "$PY" -m coverage report --include="$CRITICAL" + echo "== critical modules (ADR 0004 minimum: 90%) ==" >&2 + "$PY" -m coverage report --include="$CRITICAL" --fail-under=90 fi exit 0 fi @@ -55,6 +55,6 @@ echo "== combined report ==" >&2 "$PY" -m coverage report -m if [ "${1:-}" = "critical" ]; then - echo "== critical modules (ADR 0004 target: 90%) ==" >&2 - "$PY" -m coverage report --include="$CRITICAL" + echo "== critical modules (ADR 0004 minimum: 90%) ==" >&2 + "$PY" -m coverage report --include="$CRITICAL" --fail-under=90 fi diff --git a/scripts/critical-modules.txt b/scripts/critical-modules.txt index 845e4154..1da2dbd0 100644 --- a/scripts/critical-modules.txt +++ b/scripts/critical-modules.txt @@ -7,19 +7,48 @@ # number that silently stops measuring a module is worse than no badge. # # One module path per line, relative to the repo root. Blank lines and -# `#` comments are ignored. +# `#` comments are ignored. scripts/critical_modules.py rejects missing, +# duplicate, non-Python, and out-of-repository entries before coverage runs. + +# Host-side egress planning and secret preparation. +bot_bottle/egress/plan.py +bot_bottle/egress/service.py + +# Gateway egress policy, matching, and DLP enforcement. bot_bottle/gateway/egress/addon.py bot_bottle/gateway/egress/addon_core.py +bot_bottle/gateway/egress/context.py +bot_bottle/gateway/egress/dlp.py +bot_bottle/gateway/egress/dlp_config.py bot_bottle/gateway/egress/dlp_detectors.py -bot_bottle/egress.py -bot_bottle/manifest.py -bot_bottle/manifest_egress.py -bot_bottle/manifest_agent.py -bot_bottle/manifest_schema.py -bot_bottle/git_gate.py +bot_bottle/gateway/egress/matching.py +bot_bottle/gateway/egress/schema.py +bot_bottle/gateway/egress/types.py + +# Manifest trust boundary and schema. +bot_bottle/manifest/agent.py +bot_bottle/manifest/bottle.py +bot_bottle/manifest/egress.py +bot_bottle/manifest/extends.py +bot_bottle/manifest/git.py +bot_bottle/manifest/index.py +bot_bottle/manifest/loader.py +bot_bottle/manifest/schema.py +bot_bottle/manifest/util.py + +# Host-side and gateway-side git policy enforcement. +bot_bottle/git_gate/host_key.py +bot_bottle/git_gate/plan.py +bot_bottle/git_gate/provision.py +bot_bottle/git_gate/service.py bot_bottle/gateway/git_gate/render.py -bot_bottle/git_gate_provision.py bot_bottle/gateway/git_gate/http_backend.py -bot_bottle/supervise.py + +# Supervise proposal protocol and data plane. +bot_bottle/supervisor/plan.py +bot_bottle/supervisor/types.py +bot_bottle/gateway/supervisor/server.py + +# Shared parsers and state validation. bot_bottle/yaml_subset.py bot_bottle/bottle_state.py diff --git a/scripts/critical_modules.py b/scripts/critical_modules.py new file mode 100644 index 00000000..c6ad3b4a --- /dev/null +++ b/scripts/critical_modules.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Validate and render the critical-module coverage manifest. + +Coverage.py silently ignores an ``--include`` path that does not exist. That +is useful for broad globs, but dangerous for bot-bottle's curated security +core: a rename could otherwise improve the reported percentage by removing a +module from the measurement. Keep the validation in one small stdlib helper +and make every coverage consumer call it. +""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +DEFAULT_MANIFEST = REPO_ROOT / "scripts" / "critical-modules.txt" + + +class CriticalModulesError(ValueError): + """The critical-module manifest is empty, ambiguous, or stale.""" + + +def load_critical_modules(manifest: Path, *, root: Path) -> list[str]: + """Return validated module paths relative to *root*. + + Entries must be unique, concrete Python files inside the repository. + Globs are deliberately rejected by the file check: each rename must update + this explicit security review surface. + """ + + root = root.resolve() + try: + lines = manifest.read_text(encoding="utf-8").splitlines() + except OSError as exc: + raise CriticalModulesError( + f"cannot read critical-module manifest {manifest}: {exc}" + ) from exc + + modules: list[str] = [] + seen: set[str] = set() + errors: list[str] = [] + for line_number, raw in enumerate(lines, start=1): + entry = raw.strip() + if not entry or entry.startswith("#"): + continue + path = Path(entry) + prefix = f"{manifest}:{line_number}: {entry!r}" + if path.is_absolute(): + errors.append(f"{prefix} must be relative to the repository root") + continue + try: + resolved = (root / path).resolve() + resolved.relative_to(root) + except ValueError: + errors.append(f"{prefix} escapes the repository root") + continue + if entry in seen: + errors.append(f"{prefix} is duplicated") + continue + seen.add(entry) + if path.suffix != ".py": + errors.append(f"{prefix} is not a Python module") + continue + if not resolved.is_file(): + errors.append(f"{prefix} does not exist") + continue + modules.append(path.as_posix()) + + if not modules and not errors: + errors.append(f"{manifest}: contains no critical modules") + if errors: + raise CriticalModulesError("\n".join(errors)) + return modules + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="validate and print the critical coverage include list" + ) + parser.add_argument("--manifest", type=Path, default=DEFAULT_MANIFEST) + parser.add_argument("--root", type=Path, default=REPO_ROOT) + parser.add_argument( + "--check", action="store_true", + help="validate only; do not print the comma-separated include list", + ) + args = parser.parse_args(argv) + try: + modules = load_critical_modules(args.manifest, root=args.root) + except CriticalModulesError as exc: + print(f"critical-modules: {exc}", file=sys.stderr) + return 1 + if not args.check: + print(",".join(modules)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/unittest_gate.py b/scripts/unittest_gate.py new file mode 100644 index 00000000..fa942452 --- /dev/null +++ b/scripts/unittest_gate.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python3 +"""Run unittest discovery with explicit execution-count assurances. + +The standard unittest CLI exits successfully when a suite contains skipped +tests. That is normally useful, but it let the Docker integration job stay +green while its security-boundary classes were all skipped under act_runner. +This wrapper keeps normal unittest output and adds opt-in minimum-executed and +no-skip gates for jobs that promise a concrete integration surface. +""" + +from __future__ import annotations + +import argparse +import sys +import unittest + + +def assurance_errors( + *, tests_run: int, skipped: int, minimum_executed: int, fail_on_skip: bool +) -> list[str]: + """Return human-readable assurance failures for a completed suite.""" + + executed = tests_run - skipped + errors: list[str] = [] + if executed < minimum_executed: + errors.append( + f"executed {executed} test(s), below required minimum " + f"{minimum_executed} (discovered {tests_run}, skipped {skipped})" + ) + if fail_on_skip and skipped: + errors.append(f"{skipped} test(s) skipped in a no-skip suite") + return errors + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="unittest discovery with execution-count assurance" + ) + parser.add_argument("-s", "--start-directory", default=".") + parser.add_argument("-t", "--top-level-directory", default=None) + parser.add_argument("-p", "--pattern", default="test*.py") + parser.add_argument("--minimum-executed", type=int, default=0) + parser.add_argument("--fail-on-skip", action="store_true") + parser.add_argument("-v", "--verbose", action="store_true") + args = parser.parse_args(argv) + + suite = unittest.defaultTestLoader.discover( + args.start_directory, + pattern=args.pattern, + top_level_dir=args.top_level_directory, + ) + result = unittest.TextTestRunner( + verbosity=2 if args.verbose else 1, + ).run(suite) + failures = assurance_errors( + tests_run=result.testsRun, + skipped=len(result.skipped), + minimum_executed=args.minimum_executed, + fail_on_skip=args.fail_on_skip, + ) + for failure in failures: + print(f"unittest-gate: {failure}", file=sys.stderr) + return 0 if result.wasSuccessful() and not failures else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/README.md b/tests/README.md index 28985cd9..988ffa86 100644 --- a/tests/README.md +++ b/tests/README.md @@ -20,10 +20,11 @@ tests/ ... # many others; see unit/ directory integration/ test_gateway_image.py - test_dry_run_plan.py + test_sandbox_escape.py test_orphan_cleanup.py ... - canaries/ # opt-in; see below (currently empty) + canaries/ + test_gitleaks_release.py # opt-in upstream artifact check ``` Classification falls out of the directory — no hand-maintained list to @@ -43,24 +44,27 @@ Discovery is invoked with `-t .` (top-level dir = repo root) so the ## What the integration tests cover -- `test_dry_run_plan.py` — `cli.py start --dry-run --format=json` emits - a structured plan that contains the resolved egress allowlist and - the bottle's runtime, and creates zero Docker resources. - `test_orphan_cleanup.py` — `network_remove` is idempotent against missing resources, so the EXIT trap can call it unconditionally. - `test_gateway_image.py` — builds Dockerfile.gateway and probes that gitleaks / mitmdump / supervise are all reachable inside the gateway image. +- `test_orchestrator_docker_auth.py` — drives the real control-plane + container and verifies role-scoped authentication. +- `test_multitenant_isolation.py` and `test_sandbox_escape.py` — exercise + token/allowlist separation and end-to-end escape attempts. ## Canaries `tests/canaries/` holds upstream-regression checks gated on `BOT_BOTTLE_RUN_CANARIES=1` and not part of the per-push suite. -They're invoked by the scheduled `canaries` workflow. Currently -no canaries are defined. +They're invoked by the scheduled `canaries` workflow. The gitleaks canary +downloads the exact release archive pinned by `Dockerfile.gateway`, verifies +its architecture-specific checksum, and executes the binary. ```bash -BOT_BOTTLE_RUN_CANARIES=1 python -m unittest discover -t . -s tests/canaries -v +BOT_BOTTLE_RUN_CANARIES=1 python -m scripts.unittest_gate \ + -t . -s tests/canaries -v --minimum-executed 1 --fail-on-skip ``` ## What's NOT covered diff --git a/tests/canaries/test_gitleaks_release.py b/tests/canaries/test_gitleaks_release.py new file mode 100644 index 00000000..31704af0 --- /dev/null +++ b/tests/canaries/test_gitleaks_release.py @@ -0,0 +1,85 @@ +"""Canary: the pinned gitleaks release remains downloadable and executable. + +The gateway Dockerfile verifies this archive during an image build. Repeating +the upstream check weekly keeps registry/release drift out of normal pull +requests while proving that the pinned URL, architecture checksum, archive +shape, and binary still agree. +""" + +from __future__ import annotations + +import hashlib +import os +import platform +import re +import subprocess +import tarfile +import tempfile +import unittest +import urllib.request +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[2] +DOCKERFILE = ROOT / "Dockerfile.gateway" + + +def _docker_arg(text: str, name: str) -> str: + match = re.search(rf"^ARG {re.escape(name)}=(\S+)$", text, re.MULTILINE) + if match is None: + raise AssertionError(f"Dockerfile.gateway has no concrete ARG {name}") + return match.group(1) + + +@unittest.skipUnless( + os.environ.get("BOT_BOTTLE_RUN_CANARIES") == "1", + "canary suite is opt-in; set BOT_BOTTLE_RUN_CANARIES=1 to run", +) +class TestGitleaksRelease(unittest.TestCase): + def test_pinned_archive_checksum_and_binary(self) -> None: + dockerfile = DOCKERFILE.read_text(encoding="utf-8") + version = _docker_arg(dockerfile, "GITLEAKS_VERSION") + machine = platform.machine().lower() + architectures = { + "x86_64": ("linux_x64", "GITLEAKS_SHA256_AMD64"), + "amd64": ("linux_x64", "GITLEAKS_SHA256_AMD64"), + "aarch64": ("linux_arm64", "GITLEAKS_SHA256_ARM64"), + "arm64": ("linux_arm64", "GITLEAKS_SHA256_ARM64"), + } + if machine not in architectures: + self.fail(f"unsupported canary runner architecture: {machine}") + asset, checksum_arg = architectures[machine] + expected_checksum = _docker_arg(dockerfile, checksum_arg) + url = ( + "https://github.com/gitleaks/gitleaks/releases/download/" + f"v{version}/gitleaks_{version}_{asset}.tar.gz" + ) + + with tempfile.TemporaryDirectory(prefix="bot-bottle-gitleaks-canary.") as tmp: + archive = Path(tmp) / "gitleaks.tar.gz" + urllib.request.urlretrieve(url, archive) + self.assertEqual( + expected_checksum, + hashlib.sha256(archive.read_bytes()).hexdigest(), + "the pinned upstream archive no longer matches Dockerfile.gateway", + ) + with tarfile.open(archive, "r:gz") as bundle: + member = bundle.getmember("gitleaks") + source = bundle.extractfile(member) + if source is None: + self.fail("gitleaks archive member is not a regular file") + binary = Path(tmp) / "gitleaks" + binary.write_bytes(source.read()) + binary.chmod(0o755) + result = subprocess.run( + [str(binary), "version"], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(0, result.returncode, result.stderr) + self.assertIn(version, result.stdout + result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/integration/test_gateway_image.py b/tests/integration/test_gateway_image.py index 9d27c8de..a0e7b878 100644 --- a/tests/integration/test_gateway_image.py +++ b/tests/integration/test_gateway_image.py @@ -14,9 +14,7 @@ the chunk-1 contract: expected "no daemons selected" line when the supervisor is pointed at an empty daemon set. -Skips cleanly when docker is unavailable, or under act_runner -where the host bind-mount topology breaks multi-stage builds -that pull large bases. +Skips cleanly only when the selected Docker backend is unavailable. """ from __future__ import annotations @@ -33,12 +31,6 @@ _DOCKERFILE = "Dockerfile.gateway" @skip_unless_backend("docker") -@unittest.skipIf( - os.environ.get("GITEA_ACTIONS") == "true", - "skipped under act_runner: multi-stage build pulls a 200+MB " - "mitmproxy base + two upstream gateway images; runner storage " - "+ time budget make this an interactive-only test", -) class TestGatewayImage(unittest.TestCase): """Builds the image once for the class, then runs a few `docker run` probes against it.""" @@ -51,10 +43,11 @@ class TestGatewayImage(unittest.TestCase): "-f", _DOCKERFILE, "."], cwd=repo_root, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + check=False, ) if proc.returncode != 0: - raise unittest.SkipTest( - f"docker build failed; skipping image probes.\n" + raise AssertionError( + f"docker build failed; image probes cannot run.\n" f"{proc.stdout.decode('utf-8', errors='replace')[-2000:]}" ) @@ -63,14 +56,16 @@ class TestGatewayImage(unittest.TestCase): subprocess.run( ["docker", "image", "rm", "-f", _IMAGE], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False, ) def _run_in_image(self, *cmd: str, timeout: float = 30.0) -> tuple[int, str]: proc = subprocess.run( ["docker", "run", "--rm", "--entrypoint", cmd[0], _IMAGE, - *cmd[1:]], + *cmd[1:]], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, timeout=timeout, + check=False, ) return proc.returncode, proc.stdout.decode("utf-8", errors="replace") @@ -91,7 +86,9 @@ class TestGatewayImage(unittest.TestCase): # Probe that the package imports resolve inside the image. rc, out = self._run_in_image( "python3", "-c", - "from bot_bottle.supervisor import types; from bot_bottle.gateway.supervisor import server as supervise_server; print('ok')", + "from bot_bottle.supervisor import types; " + "from bot_bottle.gateway.supervisor import server as supervise_server; " + "print('ok')", ) self.assertEqual(0, rc, msg=out) self.assertIn("ok", out) @@ -106,6 +103,7 @@ class TestGatewayImage(unittest.TestCase): _IMAGE], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, timeout=10.0, + check=False, ) out = proc.stdout.decode("utf-8", errors="replace") self.assertEqual(0, proc.returncode, msg=out) diff --git a/tests/integration/test_multitenant_isolation.py b/tests/integration/test_multitenant_isolation.py index 78082fdf..eb5ad609 100644 --- a/tests/integration/test_multitenant_isolation.py +++ b/tests/integration/test_multitenant_isolation.py @@ -16,11 +16,10 @@ throwaway BOT_BOTTLE_ROOT for a clean registry and tears everything down. from __future__ import annotations -import os +import secrets import subprocess -import tempfile +import time import unittest -from pathlib import Path from bot_bottle.backend.docker.consolidated_launch import ( _network_cidr, @@ -73,19 +72,12 @@ _PROBE_SRC = ( @skip_unless_backend("docker") -@unittest.skipIf( - os.environ.get("GITEA_ACTIONS") == "true", - "skipped under act_runner: the orchestrator container bind-mounts the repo " - "path into a container on the socket-shared host daemon, which can't see the " - "runner's /workspace — same host-bind-mount constraint as the other " - "bottle-bringup integration tests", -) class TestMultitenantIsolation(unittest.TestCase): def setUp(self) -> None: - self._tmp = tempfile.TemporaryDirectory() - self.addCleanup(self._tmp.cleanup) - # Throwaway root → a clean registry DB, independent of the host's. - self.svc = DockerInfraService(host_root=Path(self._tmp.name)) + # Named volume → a clean registry DB that is also visible to a + # socket-shared host daemon when the test process runs in act_runner. + self._root_volume = "bot-bottle-mtitest-root-" + secrets.token_hex(4) + self.svc = DockerInfraService(root_mount_source=self._root_volume) self.addCleanup(self._teardown_docker) # ensure_running builds the bundle image (slow on a cold cache) and # brings up the shared network + gateway + orchestrator. @@ -100,13 +92,8 @@ class TestMultitenantIsolation(unittest.TestCase): self.svc.stop() subprocess.run(["docker", "network", "rm", GATEWAY_NETWORK], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) - # The orchestrator container wrote the registry DB as root into the - # throwaway root; chown it back so the (non-root) tempdir cleanup can - # remove it. subprocess.run( - ["docker", "run", "--rm", "-v", f"{self._tmp.name}:/r", - "--entrypoint", "chown", GATEWAY_IMAGE, "-R", - f"{os.getuid()}:{os.getgid()}", "/r"], + ["docker", "volume", "rm", "--force", self._root_volume], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False) @staticmethod @@ -132,29 +119,61 @@ class TestMultitenantIsolation(unittest.TestCase): taken = _network_container_ips(GATEWAY_NETWORK) + extra_taken return next_free_ip(_network_cidr(GATEWAY_NETWORK), taken) - def _probe(self, source_ip: str, host: str) -> str: - proc = subprocess.run( - ["docker", "run", "--rm", "--network", GATEWAY_NETWORK, "--ip", source_ip, - "--entrypoint", "python3", GATEWAY_IMAGE, "-c", _PROBE_SRC, - f"http://{self.gw_ip}:{EGRESS_PORT}", host], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, check=False, timeout=90, + def _probe(self, source_ip: str, identity_token: str, host: str) -> str: + deadline = time.monotonic() + 30 + last = subprocess.CompletedProcess([], 1, "", "probe not attempted") + while time.monotonic() < deadline: + last = subprocess.run( + [ + "docker", "run", "--rm", + "--network", GATEWAY_NETWORK, "--ip", source_ip, + "--entrypoint", "python3", GATEWAY_IMAGE, "-c", _PROBE_SRC, + f"http://bottle:{identity_token}@{self.gw_ip}:{EGRESS_PORT}", + host, + ], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + check=False, timeout=90, + ) + output = last.stdout.strip() + if last.returncode == 0 and output: + return output + time.sleep(0.25) + self.fail( + f"gateway probe did not become ready: " + f"exit={last.returncode}, stderr={last.stderr.strip()!r}" ) - return proc.stdout.strip() def test_two_bottles_share_gateway_with_isolated_tokens_and_allowlists(self) -> None: ip_a = self._free_ip([]) ip_b = self._free_ip([ip_a]) - self.client.register_bottle(ip_a, policy=_POLICY_A, tokens={"EGRESS_TOKEN_0": _TOKEN_A}) - self.client.register_bottle(ip_b, policy=_POLICY_B, tokens={"EGRESS_TOKEN_0": _TOKEN_B}) + bottle_a = self.client.register_bottle( + ip_a, policy=_POLICY_A, tokens={"EGRESS_TOKEN_0": _TOKEN_A} + ) + bottle_b = self.client.register_bottle( + ip_b, policy=_POLICY_B, tokens={"EGRESS_TOKEN_0": _TOKEN_B} + ) # Each bottle gets its OWN token injected on the shared route — no bleed. - self.assertEqual(f"200 AUTH=Bearer {_TOKEN_A}", self._probe(ip_a, "echo-shared")) - self.assertEqual(f"200 AUTH=Bearer {_TOKEN_B}", self._probe(ip_b, "echo-shared")) + self.assertEqual( + f"200 AUTH=Bearer {_TOKEN_A}", + self._probe(ip_a, bottle_a.identity_token, "echo-shared"), + ) + self.assertEqual( + f"200 AUTH=Bearer {_TOKEN_B}", + self._probe(ip_b, bottle_b.identity_token, "echo-shared"), + ) # Allowlist is per-bottle: echo-bonly is only in B's policy. - self.assertTrue(self._probe(ip_a, "echo-bonly").startswith("403"), # fail-closed for A - "A reached a host outside its allowlist") - self.assertEqual("200 AUTH=NONE", self._probe(ip_b, "echo-bonly")) # allowed, unauthed for B + self.assertTrue( + self._probe( + ip_a, bottle_a.identity_token, "echo-bonly" + ).startswith("403"), # fail-closed for A + "A reached a host outside its allowlist", + ) + self.assertEqual( + "200 AUTH=NONE", + self._probe(ip_b, bottle_b.identity_token, "echo-bonly"), + ) # allowed, unauthed for B if __name__ == "__main__": diff --git a/tests/integration/test_orchestrator_docker_auth.py b/tests/integration/test_orchestrator_docker_auth.py index 23f4d8dc..24db3728 100644 --- a/tests/integration/test_orchestrator_docker_auth.py +++ b/tests/integration/test_orchestrator_docker_auth.py @@ -23,7 +23,6 @@ import secrets import subprocess import tempfile import unittest -from pathlib import Path from bot_bottle.orchestrator_auth import ROLE_CLI, ROLE_GATEWAY, mint from bot_bottle.orchestrator.client import OrchestratorClient @@ -38,13 +37,6 @@ _TEST_GATEWAY_IMAGE = "bot-bottle-gateway:itest" @skip_unless_backend("docker") -@unittest.skipIf( - os.environ.get("GITEA_ACTIONS") == "true", - "skipped under act_runner: the orchestrator container bind-mounts the repo " - "path into a container on the socket-shared host daemon, which can't see the " - "runner's /workspace — same host-bind-mount constraint as the other " - "bottle-bringup integration tests", -) class TestDockerOrchestratorAuthIntegration(unittest.TestCase): @classmethod def setUpClass(cls) -> None: @@ -74,10 +66,10 @@ class TestDockerOrchestratorAuthIntegration(unittest.TestCase): gateway_name = f"bot-bottle-gateway-itest-{suffix}" network = f"bot-bottle-net-itest-{suffix}" control_network = f"bot-bottle-ctrl-itest-{suffix}" - host_root = Path(cls._tmp.name) + root_volume = f"bot-bottle-root-itest-{suffix}" cls.addClassCleanup( cls._teardown_docker, - orchestrator_name, gateway_name, network, control_network, host_root, + orchestrator_name, gateway_name, network, control_network, root_volume, ) cls.svc = DockerInfraService( @@ -88,7 +80,7 @@ class TestDockerOrchestratorAuthIntegration(unittest.TestCase): orchestrator_image=_TEST_ORCHESTRATOR_IMAGE, gateway_image=_TEST_GATEWAY_IMAGE, port=20000 + secrets.randbelow(10000), - host_root=host_root, + root_mount_source=root_volume, ) cls.svc.ensure_running() # The control plane now verifies role-scoped signed tokens, not the raw @@ -100,7 +92,7 @@ class TestDockerOrchestratorAuthIntegration(unittest.TestCase): @staticmethod def _teardown_docker( orchestrator_name: str, gateway_name: str, - network: str, control_network: str, host_root: Path, + network: str, control_network: str, root_volume: str, ) -> None: for name in (gateway_name, orchestrator_name): subprocess.run( @@ -112,14 +104,8 @@ class TestDockerOrchestratorAuthIntegration(unittest.TestCase): ["docker", "network", "rm", net], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ) - # The orchestrator container (no USER directive) wrote the registry - # DB as root into the throwaway host_root; chown it back so the - # (non-root) tempdir cleanup can remove it. Same workaround - # test_multitenant_isolation.py uses for the identical bind mount. subprocess.run( - ["docker", "run", "--rm", "-v", f"{host_root}:/r", - "--entrypoint", "chown", _TEST_ORCHESTRATOR_IMAGE, "-R", - f"{os.getuid()}:{os.getgid()}", "/r"], + ["docker", "volume", "rm", "--force", root_volume], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, ) diff --git a/tests/integration/test_orphan_cleanup.py b/tests/integration/test_orphan_cleanup.py index d1702730..a6756948 100644 --- a/tests/integration/test_orphan_cleanup.py +++ b/tests/integration/test_orphan_cleanup.py @@ -42,11 +42,6 @@ class TestOrphanCleanup(unittest.TestCase): # Returning True == idempotent success. self.assertTrue(network_remove(f"bot-bottle-net-{self.slug}-does-not-exist")) - @unittest.skipIf( - os.environ.get("GITEA_ACTIONS") == "true", - "skipped under act_runner: docker socket mount topology breaks " - "in-process visibility of networks created on the host daemon", - ) def test_create_and_remove(self): self.internal_name = network_create_internal(self.slug) self.egress_name = network_create_egress(self.slug) diff --git a/tests/integration/test_sandbox_escape.py b/tests/integration/test_sandbox_escape.py index 69758bd1..c6ac8dae 100644 --- a/tests/integration/test_sandbox_escape.py +++ b/tests/integration/test_sandbox_escape.py @@ -67,26 +67,7 @@ _DUMMY_HOST_KEY = ( ) -# Backends whose CI runner is HOST-mode (self-hosted), so the test process -# and the backend share a host. The containerized act_runner (docker on -# ubuntu-latest) is the one that can't see the host bind mount egress_tls_init -# uses and hides sibling-gateway network topology; host-mode runners -# (firecracker/KVM, macos-container) don't have those constraints, so the test -# runs there. Keep this in sync with the `runs-on` labels in -# .gitea/workflows/test.yml. -_HOST_MODE_CI_BACKENDS = frozenset({"firecracker", "macos-container"}) - - @skip_unless_selected_backend_available() -@unittest.skipIf( - os.environ.get("GITEA_ACTIONS") == "true" - and os.environ.get("BOT_BOTTLE_BACKEND") not in _HOST_MODE_CI_BACKENDS, - "skipped under the containerized act_runner (docker on ubuntu-latest): " - "egress_tls_init uses a host bind mount the runner container can't " - "see, and the network topology hides sibling-gateway visibility — " - "these constraints don't apply on the self-hosted host-mode runners " - "(firecracker/KVM, macos-container)", -) class TestSandboxEscape(unittest.TestCase): """End-to-end attacks against a real bottle. The bottle stays up for the whole class — bringup is ~10-30s, so per-test @@ -189,7 +170,7 @@ class TestSandboxEscape(unittest.TestCase): missing.append(tool) if missing: cls._teardown_resources() - raise unittest.SkipTest( + raise AssertionError( f"agent missing required tools: {', '.join(missing)} — " f"add them to the backend's base image" ) diff --git a/tests/unit/test_critical_modules.py b/tests/unit/test_critical_modules.py new file mode 100644 index 00000000..fd4f3368 --- /dev/null +++ b/tests/unit/test_critical_modules.py @@ -0,0 +1,101 @@ +"""Tests for the fail-closed critical coverage manifest.""" + +from __future__ import annotations + +import tempfile +import unittest +from contextlib import redirect_stderr, redirect_stdout +from io import StringIO +from pathlib import Path + +from scripts.critical_modules import ( + DEFAULT_MANIFEST, + REPO_ROOT, + CriticalModulesError, + load_critical_modules, + main, +) + + +class TestCriticalModules(unittest.TestCase): + def test_repository_manifest_is_valid(self) -> None: + modules = load_critical_modules(DEFAULT_MANIFEST, root=REPO_ROOT) + self.assertGreater(len(modules), 20) + self.assertEqual(len(modules), len(set(modules))) + + def test_missing_module_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + manifest = root / "critical-modules.txt" + manifest.write_text("bot_bottle/renamed.py\n", encoding="utf-8") + with self.assertRaisesRegex(CriticalModulesError, "does not exist"): + load_critical_modules(manifest, root=root) + + def test_duplicate_module_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + module = root / "bot_bottle" / "core.py" + module.parent.mkdir() + module.write_text("", encoding="utf-8") + manifest = root / "critical-modules.txt" + manifest.write_text( + "bot_bottle/core.py\nbot_bottle/core.py\n", encoding="utf-8" + ) + with self.assertRaisesRegex(CriticalModulesError, "duplicated"): + load_critical_modules(manifest, root=root) + + def test_entry_cannot_escape_repository(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + manifest = root / "critical-modules.txt" + manifest.write_text("../outside.py\n", encoding="utf-8") + with self.assertRaisesRegex(CriticalModulesError, "escapes"): + load_critical_modules(manifest, root=root) + + def test_invalid_entry_forms_are_reported_together(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + manifest = root / "critical-modules.txt" + manifest.write_text( + f"{root / 'absolute.py'}\nREADME.md\n", + encoding="utf-8", + ) + with self.assertRaises(CriticalModulesError) as raised: + load_critical_modules(manifest, root=root) + self.assertIn("must be relative", str(raised.exception)) + self.assertIn("is not a Python module", str(raised.exception)) + + def test_empty_manifest_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + manifest = root / "critical-modules.txt" + manifest.write_text("# comments do not define modules\n", encoding="utf-8") + with self.assertRaisesRegex(CriticalModulesError, "contains no"): + load_critical_modules(manifest, root=root) + + def test_unreadable_manifest_fails(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + missing = Path(tmp) / "missing.txt" + with self.assertRaisesRegex(CriticalModulesError, "cannot read"): + load_critical_modules(missing, root=Path(tmp)) + + def test_main_prints_include_list_or_checks_silently(self) -> None: + output = StringIO() + with redirect_stdout(output): + self.assertEqual(0, main([])) + self.assertIn("bot_bottle/manifest/egress.py", output.getvalue()) + + output = StringIO() + with redirect_stdout(output): + self.assertEqual(0, main(["--check"])) + self.assertEqual("", output.getvalue()) + + def test_main_reports_manifest_error(self) -> None: + error = StringIO() + with redirect_stderr(error): + self.assertEqual(1, main(["--manifest", "/definitely/missing"])) + self.assertIn("critical-modules:", error.getvalue()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_docker_infra.py b/tests/unit/test_docker_infra.py index bbd41bfe..f7f3a7a7 100644 --- a/tests/unit/test_docker_infra.py +++ b/tests/unit/test_docker_infra.py @@ -9,6 +9,7 @@ a freshly minted token.""" from __future__ import annotations import unittest +from pathlib import Path from unittest.mock import MagicMock, patch from bot_bottle.backend.docker.infra import ( @@ -67,6 +68,21 @@ class TestDockerInfraService(unittest.TestCase): self.assertTrue(any(GATEWAY_NAME in a for a in rms)) self.assertTrue(any(ORCHESTRATOR_NAME in a for a in rms)) + def test_named_mounts_propagate_to_both_planes(self) -> None: + svc = DockerInfraService( + root_mount_source="registry-volume", + gateway_ca_mount_source="ca-volume", + ) + self.assertEqual("registry-volume", svc.orchestrator()._root_mount_source) + self.assertEqual("ca-volume", svc.gateway()._ca_mount_source) + + def test_host_path_and_named_root_mount_are_mutually_exclusive(self) -> None: + with self.assertRaisesRegex(ValueError, "host_root or root_mount_source"): + DockerInfraService( + host_root=Path("/host/path"), + root_mount_source="registry-volume", + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_docker_orchestrator.py b/tests/unit/test_docker_orchestrator.py index fea4e845..82fafed2 100644 --- a/tests/unit/test_docker_orchestrator.py +++ b/tests/unit/test_docker_orchestrator.py @@ -4,6 +4,7 @@ from __future__ import annotations import unittest import urllib.error +from pathlib import Path from unittest.mock import MagicMock, Mock, patch from bot_bottle.backend.docker.orchestrator import ( @@ -47,6 +48,55 @@ class TestDockerOrchestrator(unittest.TestCase): def test_url_is_host_loopback(self) -> None: self.assertEqual("http://127.0.0.1:8099", self.orch.url()) + def test_socket_shared_client_uses_explicit_host_and_open_bind(self) -> None: + orch = DockerOrchestrator( + port=8099, client_host="172.17.0.1", root_mount_source="state-volume" + ) + with patch(_TOKEN, return_value="k"), \ + patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \ + patch(_RUN, return_value=_proc()) as run, patch(_SLEEP): + orch.ensure_running() + self.assertEqual("http://172.17.0.1:8099", orch.url()) + argv = next(c.args[0] for c in run.call_args_list + if c.args[0][:2] == ["docker", "run"]) + self.assertEqual("0.0.0.0:8099:8099", argv[argv.index("--publish") + 1]) + self.assertIn("state-volume:/bot-bottle-root", argv) + + def test_host_path_and_named_root_mount_are_mutually_exclusive(self) -> None: + with self.assertRaisesRegex(ValueError, "host_root or root_mount_source"): + DockerOrchestrator( + host_root=Path("/host/path"), + root_mount_source="state-volume", + ) + + def test_socket_shared_job_network_uses_container_dns(self) -> None: + orch = DockerOrchestrator( + name="orchestrator-itest", + port=22001, + client_network="runner-job-network", + root_mount_source="state-volume", + ) + with patch(_TOKEN, return_value="k"), \ + patch( + _URLOPEN, + side_effect=[urllib.error.URLError("down"), _health(200)], + ), patch(_RUN, return_value=_proc()) as run, patch(_SLEEP): + orch.ensure_running() + self.assertEqual("http://orchestrator-itest:8099", orch.url()) + calls = [call.args[0] for call in run.call_args_list] + self.assertIn( + [ + "docker", "network", "connect", + "runner-job-network", "orchestrator-itest", + ], + calls, + ) + argv = next(call for call in calls if call[:2] == ["docker", "run"]) + self.assertEqual( + "127.0.0.1:22001:8099", + argv[argv.index("--publish") + 1], + ) + def test_gateway_url_is_the_container_dns_name(self) -> None: # The gateway reaches the orchestrator by name on the control network. self.assertEqual(f"http://{ORCHESTRATOR_NAME}:8099", self.orch.gateway_url()) @@ -110,6 +160,8 @@ class TestDockerOrchestrator(unittest.TestCase): # The lean control plane: no mitmproxy CA mount, no gateway daemons. self.assertFalse([a for a in argv if a.endswith(":/home/mitmproxy/.mitmproxy")]) self.assertNotIn("BOT_BOTTLE_GATEWAY_DAEMONS", " ".join(argv)) + self.assertNotIn("/bot-bottle-src", " ".join(argv)) + self.assertNotIn("PYTHONPATH", " ".join(argv)) # Orchestrator entrypoint args (image ENTRYPOINT is `-m bot_bottle.orchestrator`). self.assertIn("--broker", argv) self.assertIn("stub", argv) diff --git a/tests/unit/test_orchestrator_gateway.py b/tests/unit/test_orchestrator_gateway.py index a2aa1ddc..7c8cbd2e 100644 --- a/tests/unit/test_orchestrator_gateway.py +++ b/tests/unit/test_orchestrator_gateway.py @@ -7,7 +7,10 @@ import unittest from pathlib import Path from unittest.mock import Mock, patch -from bot_bottle.backend.docker.gateway import DockerGateway +from bot_bottle.backend.docker.gateway import ( + DEFAULT_GATEWAY_SUBNET, + DockerGateway, +) from bot_bottle.gateway import ( GATEWAY_CA_CERT, GATEWAY_NAME, @@ -142,6 +145,22 @@ class TestDockerGateway(unittest.TestCase): # Data plane resolves policy against the orchestrator control plane. self.assertIn(f"BOT_BOTTLE_ORCHESTRATOR_URL={_ORCH_URL}", runs[0]) + def test_named_ca_volume_supports_socket_shared_runner(self) -> None: + sc = DockerGateway( + "bot-bottle-gateway:latest", ca_mount_source="ci-ca-volume" + ) + + def fake(argv: list[str], **_kw: object) -> Mock: + return _proc(stdout="") if argv[:2] == ["docker", "ps"] else _proc() + + with patch(_RUN_DOCKER, side_effect=fake) as run: + sc.connect_to_orchestrator(_ORCH_URL, _TOKEN) + argv = next(c.args[0] for c in run.call_args_list + if c.args[0][:2] == ["docker", "run"]) + self.assertIn( + "ci-ca-volume:/home/mitmproxy/.mitmproxy", argv + ) + def test_connect_injects_the_pre_minted_gateway_token(self) -> None: # The gateway presents the token the orchestrator handed it — it never # mints (holds no signing key). The value rides the env (bare `--env @@ -193,7 +212,38 @@ class TestDockerGateway(unittest.TestCase): with patch(_RUN_DOCKER, side_effect=fake): self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN) creates = [c for c in calls if c[:3] == ["docker", "network", "create"]] - self.assertEqual([["docker", "network", "create", self.sc.network]], creates) + self.assertEqual( + [[ + "docker", "network", "create", + "--subnet", DEFAULT_GATEWAY_SUBNET, + "--label", + f"bot-bottle.gateway-subnet={DEFAULT_GATEWAY_SUBNET}", + self.sc.network, + ]], + creates, + ) + + def test_ensure_running_replaces_stale_auto_ipam_network(self) -> None: + calls: list[list[str]] = [] + + def fake(argv: list[str], **_kw: object) -> Mock: + calls.append(argv) + if argv[:2] == ["docker", "ps"]: + return _proc(stdout="") + if argv[:3] == ["docker", "network", "inspect"]: + return _proc(stdout="\n") + return _proc() + + with patch(_RUN_DOCKER, side_effect=fake): + self.sc.connect_to_orchestrator(_ORCH_URL, _TOKEN) + self.assertIn( + ["docker", "rm", "--force", self.sc.name], + calls, + ) + self.assertIn( + ["docker", "network", "rm", self.sc.network], + calls, + ) def test_ca_cert_pem_reads_from_container(self) -> None: with patch(_RUN_DOCKER, return_value=_proc(stdout=_CA_PEM)) as m: diff --git a/tests/unit/test_orchestrator_service.py b/tests/unit/test_orchestrator_service.py index 7f9b12f8..36931075 100644 --- a/tests/unit/test_orchestrator_service.py +++ b/tests/unit/test_orchestrator_service.py @@ -105,11 +105,19 @@ class TestOrchestrator(unittest.TestCase): def test_reprovision_rejects_missing_rows_and_wrong_key(self) -> None: self.assertFalse(self.orch.reprovision_from_secret("missing", new_env_var_secret())) - rec = self.orch.launch_bottle( - "10.243.0.13", tokens={"K": "value"}, - env_var_secret=new_env_var_secret(), - ) - self.assertFalse(self.orch.reprovision_from_secret(rec.bottle_id, new_env_var_secret())) + key = "AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE" + wrong_key = "FBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQ" + # Pin the nonce so this is a deterministic wrong-key/decryption vector + # instead of a probabilistic assertion over random bytes. + with patch( + "bot_bottle.orchestrator.store.secret_store.secrets.token_bytes", + return_value=b"\0" * 16, + ): + rec = self.orch.launch_bottle( + "10.243.0.13", tokens={"K": "value"}, + env_var_secret=key, + ) + self.assertFalse(self.orch.reprovision_from_secret(rec.bottle_id, wrong_key)) def test_set_policy_live_reload(self) -> None: rec = self.orch.launch_bottle("10.243.0.3") diff --git a/tests/unit/test_unittest_gate.py b/tests/unit/test_unittest_gate.py new file mode 100644 index 00000000..5d09a69a --- /dev/null +++ b/tests/unit/test_unittest_gate.py @@ -0,0 +1,91 @@ +"""Unit tests for CI's unittest execution-count gate.""" + +from __future__ import annotations + +import unittest +from contextlib import redirect_stderr +from io import StringIO +from unittest.mock import Mock, patch + +from scripts.unittest_gate import assurance_errors, main + + +class TestAssuranceErrors(unittest.TestCase): + def test_accepts_suite_that_meets_minimum_without_skips(self) -> None: + self.assertEqual( + [], + assurance_errors( + tests_run=22, skipped=0, minimum_executed=22, fail_on_skip=True + ), + ) + + def test_rejects_green_suite_below_execution_minimum(self) -> None: + errors = assurance_errors( + tests_run=22, skipped=18, minimum_executed=22, fail_on_skip=False + ) + self.assertEqual(1, len(errors)) + self.assertIn("executed 4", errors[0]) + + def test_rejects_any_skip_when_required(self) -> None: + errors = assurance_errors( + tests_run=23, skipped=1, minimum_executed=22, fail_on_skip=True + ) + self.assertEqual(["1 test(s) skipped in a no-skip suite"], errors) + + def test_main_accepts_successful_assured_suite(self) -> None: + result = Mock( + testsRun=22, + skipped=[], + wasSuccessful=Mock(return_value=True), + ) + runner = Mock() + runner.run.return_value = result + with patch( + "scripts.unittest_gate.unittest.defaultTestLoader.discover", + return_value=Mock(), + ) as discover, patch( + "scripts.unittest_gate.unittest.TextTestRunner", + return_value=runner, + ) as runner_type: + self.assertEqual( + 0, + main([ + "-s", "tests/integration", + "-t", ".", + "-p", "test_*.py", + "--minimum-executed", "22", + "--fail-on-skip", + "-v", + ]), + ) + discover.assert_called_once_with( + "tests/integration", pattern="test_*.py", top_level_dir="." + ) + runner_type.assert_called_once_with(verbosity=2) + + def test_main_rejects_unsuccessful_underfilled_suite(self) -> None: + result = Mock( + testsRun=1, + skipped=[(Mock(), "not available")], + wasSuccessful=Mock(return_value=False), + ) + runner = Mock() + runner.run.return_value = result + error = StringIO() + with patch( + "scripts.unittest_gate.unittest.defaultTestLoader.discover", + return_value=Mock(), + ), patch( + "scripts.unittest_gate.unittest.TextTestRunner", + return_value=runner, + ), redirect_stderr(error): + self.assertEqual( + 1, + main(["--minimum-executed", "2", "--fail-on-skip"]), + ) + self.assertIn("below required minimum", error.getvalue()) + self.assertIn("skipped in a no-skip suite", error.getvalue()) + + +if __name__ == "__main__": + unittest.main()