Harden Docker infra, add credential passthrough and SSH agent forwarding#29226
Harden Docker infra, add credential passthrough and SSH agent forwarding#29226pnlpleasr wants to merge 9 commits intoopenclaw:mainfrom
Conversation
Design for a pre-execution verification gateway that supports HTTP webhooks and built-in Telegram approval for tool calls. Layers on top of existing tool policy and exec allowlist systems. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9-task TDD plan covering config types, zod validation, scope matcher, webhook client, Telegram verifier, orchestrator, pipeline wiring, documentation, and integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oc and HTML briefs Incorporates P0/P1 fixes from tech lead review: - Telegram: direct API polling instead of long-polling bot lifecycle - Telegram: sender validation via allowedUserIds - Webhook: HTTPS enforcement, param redaction, response size limits - Orchestrator: most-restrictive-wins failMode resolution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SVG component architecture diagram showing existing pipeline, verifier subsystem, and external systems. Webhook verification sequence diagram showing the 9-step allow path and alt deny path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enable sandbox containers to git push/pull via SSH without raw private keys entering the container. Docker Desktop's SSH agent socket is bind- mounted and proxied, so only cryptographic signatures cross the boundary. - Add openssh-client to sandbox image - Add sandbox user to group root for agent socket access (root:root 0660) - Allowlist /run/host-services/ssh-auth.sock in security validator - Add 4 test cases for allowlist (sibling, parent, traversal all blocked) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…view findings Docker infrastructure: - Install Docker CLI in gateway image for sandbox management - Parameterize UID/GID for macOS compatibility (UID 501) - Bind ports to loopback only (127.0.0.1) - Mount vault, docker.sock, and duplicate host-path volumes - Fix chmod 1777 to run as root (was running as node) - Warn when docker-setup.sh runs as root (UID 0) - Redact token from setup script output Credential passthrough: - Pass TELEGRAM_BOT_TOKEN, GOOGLE_PLACES_API_KEY, NOTION_API_KEY, OPENAI_WHISPER_API_KEY through docker-compose.yml to gateway/CLI - Load ~/.openclaw/.env secrets in docker-setup.sh with robust parser (handles quotes, export prefix, key validation, malformed line warnings) - Persist secrets to project .env via upsert_env; chmod 600 the file - Default OPENCLAW_VAULT_DIR to workspace/vault with validation Security hardening: - tryRealpathAbsolute now fails closed (returns null + logs) instead of silently falling back to unresolved path on realpath errors - validateBindMounts throws on unresolvable paths (closes symlink bypass) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add architecture comments, design rationale, and operational notes to: - Dockerfile: image purpose, design decisions, build/run references - Dockerfile.sandbox: security constraints, SSH agent forwarding design - docker-compose.yml: service roles, mount rationale, security notes - docker-setup.sh: script overview, section headers, env var docs - validate-sandbox-security.ts: defense layer summary, allowlist docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…agents P0 fixes: - Add .env to .dockerignore to prevent secrets baking into image layers - Fix chmod 600 race in upsert_env: set permissions on temp file before writing secrets, add trap cleanup on failure - Add :- defaults to CLAUDE_* env vars so compose doesn't fail when unset - Make tryRealpathAbsolute fail closed for non-absolute paths (return null) P1 fixes: - Allow spaces in OPENCLAW_VAULT_DIR for iCloud paths (Mobile Documents) - Add SESSION_KEY, SESSION_TOKEN, COOKIE to env var sanitizer blocklist - Add trap to clean up temp file containing secrets on script failure - Warn on unmatched quotes in .env parser to prevent silent credential corruption Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a67a3faf3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # GID 0 (root group) grants access to /var/run/docker.sock inside the | ||
| # container, which is needed to spawn sibling sandbox containers. | ||
| group_add: | ||
| - "0" |
There was a problem hiding this comment.
Use docker.sock group GID instead of hardcoded root group
This hardcodes supplementary group 0, but on standard Linux Docker installs /var/run/docker.sock is typically owned by the docker group (non-zero GID). With user: "${OPENCLAW_UID}:${OPENCLAW_GID}", the gateway process remains non-root and will get permission denied when trying to create sandbox containers unless the socket happens to be group root; this breaks sandbox execution on those hosts.
Useful? React with 👍 / 👎.
| # Obsidian vault (or any knowledge base directory). Uses long-form | ||
| # syntax because iCloud paths contain spaces. | ||
| - type: bind | ||
| source: "${OPENCLAW_VAULT_DIR}" |
There was a problem hiding this comment.
Provide fallback for new OPENCLAW_VAULT_DIR bind source
The new bind mount source uses ${OPENCLAW_VAULT_DIR} with no default, so upgrades from older setups (whose existing project .env lacks this key) expand to an empty source and make docker compose up fail mount validation. Since docker-setup.sh is documented as one-time setup, this introduces a startup regression unless users rerun setup or manually patch .env.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR implements SSH agent forwarding and credential passthrough for the Docker infrastructure while adding comprehensive security hardening. The implementation is well-documented with inline architecture comments throughout. Key improvements:
Security architecture: Test coverage: Confidence Score: 4/5
Last reviewed commit: 8a67a3f |
Summary
git pushvia SSH without exposing private keys. Exact-match allowlist in the security validator with 4 regression tests.TELEGRAM_BOT_TOKEN,GOOGLE_PLACES_API_KEY,NOTION_API_KEY,OPENAI_WHISPER_API_KEY, andCLAUDE_*session keys throughdocker-compose.ymlto gateway/CLI containers..envparser (quote stripping,exportprefix, key validation, unmatched quote warnings),tryRealpathAbsolutefail-closed,chmod 600on temp files before writing secrets, trap cleanup, env var sanitizer coverage forSESSION_KEY/COOKIEpatterns.Changes
Dockerfilechmod 1777runs asUSER root, architecture headerDockerfile.sandbox--groups rootfor socket accessdocker-compose.yml${VAR:-}defaults, vault mount (long-form for spaces), loopback port bindingdocker-setup.sh.envloader, UID/GID detection + root warning, vault dir allows spaces,chmod 600+ trap inupsert_env, unmatched quote warningvalidate-sandbox-security.tsALLOWED_HOST_PATHSallowlist,tryRealpathAbsolutefail-closed for non-absolute pathsvalidate-sandbox-security.test.tssanitize-env-vars.tsSESSION_KEY,SESSION_TOKEN,COOKIEadded to blocklist.dockerignore.envexcluded from build contextTest plan
npx vitest run src/agents/sandbox/validate-sandbox-security.test.ts— 19 tests passnpx vitest run src/agents/sandbox/sanitize-env-vars.test.ts— 4 tests pass🤖 Generated with Claude Code