Stop a Bot's shell reading the deployment's keys from a neighbouring process (#552)

Co-authored-by: David McKay <david@copilotkit.ai>

Author: zopeVaibhav

.github/workflows/ci.yml

@@ -418,6 +418,21 @@ jobs:
             docker logs openbot-ci
             exit 1
           }
+      - name: A Bot's shell cannot reach the deployment secrets
+        run: |
+          set -uo pipefail
+          if docker exec -u pwuser openbot-ci env -i PATH=/usr/bin:/bin sh -c '
+                for e in /proc/[0-9]*/environ; do tr "\0" "\n" < "$e" 2>/dev/null; done
+              ' | grep -q "^KEY_ENCRYPTION_KEY="; then
+            echo "::error::A pwuser shell reads KEY_ENCRYPTION_KEY from a sibling process /proc/<pid>/environ."
+            exit 1
+          fi
+          if docker exec -u pwuser openbot-ci env -i PATH=/usr/bin:/bin sh -c \
+                'cat /run/s6/container_environment/KEY_ENCRYPTION_KEY 2>/dev/null' | grep -q .; then
+            echo "::error::A pwuser shell reads KEY_ENCRYPTION_KEY from /run/s6/container_environment."
+            exit 1
+          fi
+          echo "pwuser cannot reach KEY_ENCRYPTION_KEY"
       - if: always()
         run: docker rm -f openbot-ci >/dev/null 2>&1 || true
 

CHANGELOG.md

@@ -8,6 +8,17 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A Bot's shell can no longer read the deployment's keys from a neighbouring process
+
+In the all-in-one image the API and the browser ran under one account, and a Bot's shell — a child
+of the browser — could read a same-account process's environment through `/proc`, whatever its own
+environment had been scrubbed to. One allowed `computer_run_command` returned `KEY_ENCRYPTION_KEY`,
+the session-signing secret and the database password, none of it on the audit trail. The API and its
+migrations now run as their own account, so the kernel refuses that read; the browser is handed only
+the variables it needs, so its own environment carries none of those keys; and the files under
+`/run/s6/container_environment` are closed to the shell. A deployment that runs each Bot in its own
+sandboxed computer, as the documentation asks for, was never exposed to this.
+
 ## 0.0.11
 
 ### The LlamaIndex Bot answers with the model the setup screen chose

Dockerfile

@@ -36,7 +36,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
   && ln -s bun /usr/local/bin/bunx \
   && bunx --bun "playwright@${PLAYWRIGHT_VERSION}" install --with-deps chromium \
   && rm -rf /root/.cache /tmp/* /var/lib/apt/lists/* \
-  && useradd --create-home --shell /bin/bash pwuser
+  && useradd --create-home --shell /bin/bash pwuser \
+  && useradd --create-home --shell /usr/sbin/nologin apiuser
 
 
 FROM base AS deps
@@ -215,9 +216,9 @@ ENV AGENT_COMPUTER_URL=http://127.0.0.1:4100
 # NOTHING THAT MATTERS RUNS AS ROOT.
 #
 # s6 stays root because that is the only way it can drop each service to a different user, and they
-# genuinely differ: the browser and API run as `pwuser`, the database as `postgres`. One shared
-# account would put the process that renders the open internet in the same skin as the one holding
-# the audit trail.
+# genuinely differ: the browser and the Bot's shell run as `pwuser`, the API and migrations as
+# `apiuser`, the database as `postgres`. One shared account would put the process that renders the
+# open internet in the same skin as the one holding the audit trail.
 #
 # This matters more than usual here. Chromium is launched with `--no-sandbox` unless the host can
 # support its sandbox, and with that flag the process user IS the boundary, so root would mean a

docker/s6/s6-rc.d/api/run

@@ -2,4 +2,5 @@
 # The API, and the app it serves. Started after the browser so a Bot's first action does not race a
 # computer that is still coming up.
 cd /app/server
-exec s6-setuidgid pwuser /usr/local/bin/bun src/index.ts
+export HOME=/home/apiuser
+exec s6-setuidgid apiuser /usr/local/bin/bun src/index.ts

docker/s6/s6-rc.d/computer/run

@@ -17,6 +17,39 @@ if [ "${EMBEDDED_COMPUTER:-on}" != "on" ]; then
   exec /bin/true
 fi
 cd /app/agent-computer
-export PORT=4100
-export WORKSPACE_DIR=/workspace
-exec s6-setuidgid pwuser /usr/local/bin/bun src/index.ts
+
+set --
+for name in \
+  PATH LANG LANGUAGE TERM TERMINFO COLORTERM DISPLAY \
+  HTTP_PROXY HTTPS_PROXY NO_PROXY ALL_PROXY FTP_PROXY \
+  http_proxy https_proxy no_proxy all_proxy ftp_proxy \
+  BUN_INSTALL PLAYWRIGHT_BROWSERS_PATH SPIFFE_ENDPOINT_SOCKET \
+  COMPUTER_TOKEN COMPUTER_BOT_ID COMPUTER_BROWSER_MODE COMPUTER_MAX_BROWSERS \
+  COMPUTER_BROWSER_IDLE_MS COMPUTER_SANDBOX COMPUTER_SHELL_ENV COMPUTER_STOPPED \
+  ACTION_TIMEOUT_MS NAVIGATION_TIMEOUT_MS AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS
+do
+  eval "isset=\${$name+x}"
+  [ -n "${isset:-}" ] && eval "set -- \"\$@\" \"$name=\${$name}\""
+done
+for name in $(printenv | sed -n 's/=.*//p' | grep -E '^(LC_[A-Za-z0-9_]+|EGRESS_PROXY_[A-Za-z0-9_]+)$' || true); do
+  eval "set -- \"\$@\" \"$name=\${$name}\""
+done
+if [ -n "${COMPUTER_SHELL_ENV:-}" ]; then
+  oldifs=$IFS
+  IFS=,
+  for name in $COMPUTER_SHELL_ENV; do
+    IFS=$oldifs
+    name=$(printf %s "$name" | tr -d '[:space:]')
+    case "$name" in
+      '' | *[!A-Za-z0-9_]*) IFS=, ; continue ;;
+    esac
+    eval "isset=\${$name+x}"
+    [ -n "${isset:-}" ] && eval "set -- \"\$@\" \"$name=\${$name}\""
+    IFS=,
+  done
+  IFS=$oldifs
+fi
+
+exec s6-setuidgid pwuser env -i "$@" \
+  HOME=/home/pwuser PORT=4100 WORKSPACE_DIR=/workspace PROFILES_DIR="${PROFILES_DIR:-/profiles}" \
+  /usr/local/bin/bun src/index.ts

docker/s6/scripts/computer-token.sh

@@ -12,3 +12,5 @@ if [ -z "${COMPUTER_TOKEN:-}" ]; then
   head -c 32 /dev/urandom | od -An -tx1 | tr -d ' \n' \
     > /run/s6/container_environment/COMPUTER_TOKEN
 fi
+
+chmod 0700 /run/s6/container_environment

docker/s6/scripts/migrate.sh

@@ -8,7 +8,8 @@
 set -eu
 [ "${EMBEDDED_POSTGRES:-off}" = "on" ] || exit 0
 cd /app/server
+export HOME=/home/apiuser
 # `scripts/migrate.ts`, not `drizzle-kit`. The CLI is a development dependency and needs esbuild to
 # read its TypeScript config, which `bun install --production` leaves out of this image: asked to
 # migrate here it exits 1 without printing why, and the container comes up against an empty database.
-exec s6-setuidgid pwuser /usr/local/bin/bun scripts/migrate.ts
+exec s6-setuidgid apiuser /usr/local/bin/bun scripts/migrate.ts