From d4b34e6130b60ef04617fc589a59a1add0eda034 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Sun, 2 Aug 2026 21:04:20 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20give=20the=20sandbox=20a=20real=20shell?= =?UTF-8?q?=20toolbelt=20=E2=80=94=20and=20make=20an=20image=20bump=20reac?= =?UTF-8?q?h=20existing=20users?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-user container shipped python+node and little else, so an agent asking for `unzip`, `ffprobe` or even `ps` found nothing and had to `sudo apt-get install` mid-task. That fallback works, but it re-runs on **every container recreate**, inside the task, where it costs latency and can fail — while the image is **one, shared by every container**, so preinstalling costs its size once for the whole box. Anything an agent reaches for repeatedly is therefore cheaper baked in. Added on that rule: jq, ripgrep, zip/unzip, xz-utils, sqlite3, wget, openssh-client, procps, less, file, tzdata, dnsutils, iputils-ping, ffmpeg (+ffprobe), imagemagick, poppler-utils and tesseract — with the ita/fra language packs, matching the app's supported UI locales (eng and osd arrive as hard deps). Deliberately left out: build-essential/python3-dev (~270 MB, only for a pip package with no wheel) and pandoc (~216 MB) are big *and* self-recoverable, so they stay on demand. 687 MB -> 1.3 GB, ffmpeg being most of it. The image tag goes v2 -> v3, which alone would have equipped nobody: a container pins the image it was created from, so `ensure()` would have rebuilt v3 and then happily reused every existing v2 container — the new tools would have reached only users created from here on. `reusable()` now compares `.Config.Image` too, turning a tag bump into a recreate, safe for the same reason the `--user`/`--init` self-heal already is: the container holds no durable state, everything lives in the bind mounts. An unreadable inspect answers true, so a docker hiccup never churns a working container. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 4 +-- crates/skald-core/src/container/Dockerfile | 38 ++++++++++++++++++-- crates/skald-core/src/container/mod.rs | 42 +++++++++++++++------- 3 files changed, 68 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 99aaaba..1c1bd91 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,7 +97,7 @@ Two rules keep the boundary real, and both are enforced by the compiler: | `crates/skald-core/src/chat_event_bus.rs` | Global async bus for cross-session events | | `crates/skald-core/src/agents.rs` | Discovers agents from `agents/*/`, loads meta + system prompt | | `crates/skald-core/src/tools/` | Built-in tools: `exec` (**runs inside the caller's per-user Docker container** via `docker exec`, as the non-root host uid — `sudo` for system installs — with a robust /stop that reaps the command's process-group; see `container/`; the only live path is `run_with` (needs `ToolContext`) — the context-free `Tool::execute`/`execute_async` now **error** (`HOST_PATH_ERROR`) instead of the old host `sh -c`, so nothing can run a command outside the sandbox), `list_agents`, `fs/*` (route `user-memory/`/`shared-memory/` to `memory_docs`, and every other **physical** path through `ctx.fs` to the caller's per-user host workspace — see DB tables + container), `notify`, `ast_outline`, `image_generate`, MCP tools, plugin tools, cron tools | -| `crates/skald-core/src/container/` | `ContainerManager` (§6): per-user Docker containers (the execution sandbox). Docker is a **hard requirement** — `check_docker()` fails `Skald::new` (→ shell exits) if the daemon is unreachable. Builds our own `skald-runtime` image (python+node+**sudo**; tag is **versioned** `skald-runtime:v2` so a `Dockerfile` change forces a rebuild) once from the embedded `Dockerfile`, then `reconcile_all()` at boot ensures one running container `skald-{userid}` per active user. Each container runs as the **host `uid:gid`** (`--user`, §6 UID coherence) with `--init` (tini reaps zombies); `ensure()` **self-heals** a container whose `--user` is stale (e.g. an old root one) by recreating it, and injects a passwd/shadow entry post-create so `sudo` (NOPASSWD, in the image) resolves the arbitrary uid. `build_user_fs()` assembles a user's `UserFs` (home `{WD}/homes/{userid}` → `/root`, plus each `shared/{name}` they belong to). Shells the `docker` CLI (no client crate) | +| `crates/skald-core/src/container/` | `ContainerManager` (§6): per-user Docker containers (the execution sandbox). Docker is a **hard requirement** — `check_docker()` fails `Skald::new` (→ shell exits) if the daemon is unreachable. Builds our own `skald-runtime` image (python+node+**sudo**, plus a shell-work toolbelt — `jq`/`ripgrep`/`unzip`/`ffmpeg`/`poppler-utils`/`tesseract`/`procps`…; tag is **versioned** `skald-runtime:v3` so a `Dockerfile` change forces a rebuild) once from the embedded `Dockerfile`, then `reconcile_all()` at boot ensures one running container `skald-{userid}` per active user. Each container runs as the **host `uid:gid`** (`--user`, §6 UID coherence) with `--init` (tini reaps zombies); `ensure()` **self-heals** a container that is stale on any of three axes — `--user` (e.g. an old root one), `--init`, or the **image tag** — by recreating it, and injects a passwd/shadow entry post-create so `sudo` (NOPASSWD, in the image) resolves the arbitrary uid. The image check is what makes a tag bump reach *existing* users: a container pins the image it was created from, so without it a rebuild would only ever equip new users. `build_user_fs()` assembles a user's `UserFs` (home `{WD}/homes/{userid}` → `/root`, plus each `shared/{name}` they belong to). Shells the `docker` CLI (no client crate) | | `crates/skald-core/src/tool_catalog.rs` | `ToolCatalog`: unified tool listing façade (wraps ToolRegistry + McpManager) | | `crates/skald-core/src/events.rs` | `ServerEvent` enum streamed over WebSocket to the frontend | | `crates/skald-core/src/db/` | sqlx SQLite — see below | @@ -152,7 +152,7 @@ Schema is greenfield (no migrations, §0), but a purely **additive** column land ## Filesystem & containers (blueprint §6) -Each user has one **permanent Docker container** (`skald-{userid}`, our own `skald-runtime` image with python+node), created on user creation and started at boot (`ContainerManager`, `crates/skald-core/src/container/`). Docker is **required**: a missing daemon fails `Skald::new` and the process exits. The container runs as the **host `uid:gid`** (not root) so files created in-container and by the host-side fs-tools share ownership on the bind mounts (matters on native Linux; masked on macOS Docker Desktop). Because that user isn't root, the image ships passwordless `sudo` (a passwd/shadow entry is injected at create) so an agent can still `sudo apt-get install …`; `--init` runs tini as pid 1 to reap zombies. +Each user has one **permanent Docker container** (`skald-{userid}`, our own `skald-runtime` image with python+node and a preinstalled shell toolbelt), created on user creation and started at boot (`ContainerManager`, `crates/skald-core/src/container/`). Docker is **required**: a missing daemon fails `Skald::new` and the process exits. **What goes in the image vs. what the agent installs on demand** is a real trade, and the Dockerfile states its rule: `sudo apt-get install` works in the sandbox but re-runs on **every container recreate**, inside a task, where it costs latency and can fail — while the image is **one, shared by every container**, so preinstalling costs its size once for the whole box. Anything an agent reaches for repeatedly is therefore baked in; `build-essential`/`python3-dev` and `pandoc` are deliberately left out as big *and* self-recoverable. The container runs as the **host `uid:gid`** (not root) so files created in-container and by the host-side fs-tools share ownership on the bind mounts (matters on native Linux; masked on macOS Docker Desktop). Because that user isn't root, the image ships passwordless `sudo` (a passwd/shadow entry is injected at create) so an agent can still `sudo apt-get install …`; `--init` runs tini as pid 1 to reap zombies. The agent sees **one namespace**, routed on the first path component. The choke point is `UserFs` (`core-api/src/user_fs.rs`, a pure value type carried in `ToolContext.fs`), plus `resolve_host_path()` in `tools/fs/mod.rs`: diff --git a/crates/skald-core/src/container/Dockerfile b/crates/skald-core/src/container/Dockerfile index beaa569..c8cfccc 100644 --- a/crates/skald-core/src/container/Dockerfile +++ b/crates/skald-core/src/container/Dockerfile @@ -5,24 +5,58 @@ # away. Built once at boot by `ContainerManager::ensure_image` (tag `skald-runtime`). # # Holds python + node so `execute_cmd` (and, later, per-user MCP servers) run -# inside the user's container against their bind-mounted home. Kept minimal; -# grow it here as needs arise. +# inside the user's container against their bind-mounted home. +# +# What belongs here vs. what an agent installs on demand: `sudo apt-get install` +# works inside the sandbox, but it re-downloads on **every** container recreate, +# inside a task, where it costs latency and can fail. Preinstalling costs image +# size **once for the whole box** — there is one image, shared by every user's +# container — so anything an agent reaches for repeatedly is cheaper baked in. +# What is deliberately left out is the converse: `build-essential`/`python3-dev` +# (~270 MB, only for `pip install` of a package with no wheel) and `pandoc` +# (~216 MB, niche) are big *and* self-recoverable, so they stay on demand. FROM debian:bookworm-slim ENV DEBIAN_FRONTEND=noninteractive RUN apt-get update && apt-get install -y --no-install-recommends \ + # Language runtimes. python3 \ python3-pip \ python3-venv \ nodejs \ npm \ + # Base plumbing. `util-linux` provides `setsid` (see the sudoers note below). ca-certificates \ curl \ + wget \ git \ + openssh-client \ sudo \ util-linux \ + procps \ + less \ + file \ + tzdata \ + dnsutils \ + iputils-ping \ + # Shell-work staples: JSON, fast search, archives, local data. + jq \ + ripgrep \ + unzip \ + zip \ + xz-utils \ + sqlite3 \ + # Media + documents. `ffmpeg` brings `ffprobe`; `poppler-utils` brings + # `pdftotext`. The tesseract language packs match the app's supported UI + # locales (`i18n::SUPPORTED_LOCALES`) — `eng` and `osd` arrive as hard deps. + ffmpeg \ + imagemagick \ + poppler-utils \ + tesseract-ocr \ + tesseract-ocr-ita \ + tesseract-ocr-fra \ && rm -rf /var/lib/apt/lists/* # The container runs as the host process's uid:gid (blueprint §6 UID coherence), so diff --git a/crates/skald-core/src/container/mod.rs b/crates/skald-core/src/container/mod.rs index e8e1f46..08351bd 100644 --- a/crates/skald-core/src/container/mod.rs +++ b/crates/skald-core/src/container/mod.rs @@ -30,10 +30,11 @@ use crate::db; /// Our runtime image tag. Built once from the embedded [`Dockerfile`]. The version /// suffix is the image cache-buster: [`ContainerManager::ensure_image`] rebuilds only -/// when the tag is absent, so **bump it whenever the [`Dockerfile`] changes** (e.g. -/// `v2` added `sudo` + a NOPASSWD sudoers for the non-root container user). Old tags -/// linger as orphaned images (harmless). -const IMAGE_TAG: &str = "skald-runtime:v2"; +/// when the tag is absent, so **bump it whenever the [`Dockerfile`] changes** (`v2` +/// added `sudo` + a NOPASSWD sudoers for the non-root container user; `v3` added +/// `unzip` + `ffmpeg`). Old tags linger as orphaned images (harmless), but existing +/// containers still *run* one — which is why [`reusable`] also compares the image. +const IMAGE_TAG: &str = "skald-runtime:v3"; /// The embedded Dockerfile — the source of truth, so the image can be built with /// no files shipped alongside the binary (binary-first). @@ -196,8 +197,9 @@ impl ContainerManager { /// Creates the host directories, the container (if missing) with the right bind /// mounts + `--user`, and starts it (if stopped). Self-healing: a container whose /// `--user` no longer matches the host uid:gid (e.g. an old root container from a - /// previous binary) is torn down and recreated. Idempotent — a no-op when a - /// matching container is already running. + /// previous binary), that predates `--init`, or that runs a superseded + /// [`IMAGE_TAG`], is torn down and recreated. Idempotent — a no-op when a matching + /// container is already running. pub async fn ensure(&self, user_id: &str) -> Result<()> { let fs = build_user_fs(&self.system, user_id).await?; @@ -221,11 +223,12 @@ impl ContainerManager { return Ok(()); } ContainerState::Absent => {} - // Present but stale — a mismatched `--user` (e.g. an old root container) or + // Present but stale — a mismatched `--user` (e.g. an old root container), // missing `--init` (an old container whose PID 1 is `sleep infinity`, which // ignores SIGTERM and hangs `docker stop` for the full grace, see - // `SHUTDOWN_STOP_GRACE`): tear it down. The container holds no durable state - // — everything is in the bind mounts — so a recreate is safe. + // `SHUTDOWN_STOP_GRACE`), or an outdated image: tear it down. The container + // holds no durable state — everything is in the bind mounts — so a recreate + // is safe. _ => { let _ = docker(&["rm", "-f", name]).await; } @@ -389,10 +392,25 @@ async fn init_matches(name: &str) -> bool { .unwrap_or(false) } -/// Whether an existing container can be reused as-is: right `--user` (§6 UID coherence) -/// **and** `--init` (fast, clean `docker stop`). A mismatch on either recreates it. +/// Whether a container runs the current [`IMAGE_TAG`]. A container pins the image it +/// was created from, so bumping the tag rebuilds the image but leaves every existing +/// container on the old one — the new tools would reach new users only. Comparing the +/// tag here turns the bump into a recreate, which is safe for the same reason the +/// `--user`/`--init` self-heal is: the container holds no durable state, everything +/// lives in the bind mounts. Unreadable inspect ⇒ `true`, so a docker hiccup never +/// churns a working container. +async fn image_matches(name: &str) -> bool { + docker(&["inspect", "-f", "{{.Config.Image}}", name]) + .await + .map(|s| s.trim() == IMAGE_TAG) + .unwrap_or(true) +} + +/// Whether an existing container can be reused as-is: right `--user` (§6 UID coherence), +/// `--init` (fast, clean `docker stop`) **and** the current image. A mismatch on any of +/// the three recreates it. async fn reusable(name: &str, want_user: &Option) -> bool { - user_matches(name, want_user).await && init_matches(name).await + user_matches(name, want_user).await && init_matches(name).await && image_matches(name).await } /// Gives the container's runtime `uid`/`gid` a passwd + shadow (+ group) entry, so