feat/improve-dockerfile-and-run.sh#5
Conversation
|
@WolframRavenwolf thank you for the PR!
Also need to cleanup the Dockerfile a bit from commented commands. |
|
Thanks for taking a look! I have addressed all the issues:
The updates are pushed and ready for your review. |
Review findings\n\n- High: Python CLI dependency conflict — installing then downgrades shared deps (, ), likely breaking earlier-installed tools. and .\n- Medium: Non-reproducible builds — many installs use or (Go/NPM/Pip), making builds brittle and hard to roll back. and throughout .\n- Medium: Startup/build time regression — the expanded toolchain causes long ERR_PNPM_NO_PKG_MANIFEST No package.json found in /Users/guti/projects/clawdbot-ha-addon; the container didn’t reach gateway start within 120s in my run. This could be worse on HA hardware. overall.\n\nIf you want, I can propose fixes (pin versions, isolate Python CLIs, or split “full tools” into an optional build). |
Review findings
|
|
Good point regarding the nano-pdf and whisper-cli PIP installs. Since uv is available, I prefer using a uv environment, though I must adjust the container paths so Clawdbot locates it correctly. I will look into that. As for using "latest": we already run "apt-get update && apt-get upgrade -y", which inherently results in a non-reproducible build. Pinning package versions mandates constant Dockerfile maintenance. I prefer defaulting to "latest" and letting users pin versions individually if they require stability over currency. Ultimately, these changes are just intended to bring your Home Assistant add-on to feature parity with a full local Clawdbot installation regarding skill support. Fully local setups allow Clawdbot to auto-install requirements for base skills, but the container lacks full persistence and extra tools like Homebrew. Pre-installing the prerequisites directly into the container solves this and makes the add-on more valuable for Clawd users - it may make it slower to start the first time, but will save users a lot more time due to ease of use. |
|
There are a lot of changes in this pr. I would really like the ability to define a bunch of docker packages to install maybe as a string and maybe go / pnpm packages to install as a string and the docker container could handle this and move more of these deps / versions into ha configuration values. Then less changes in the docker file. Right now I cannot use this image because I'm installing a bunch of apt dependencies that are not included here. As is this feels unmaintantable with the rapid release. Feels like moving image tags out to configuration and passing more of your deps in that you want to install is the way to go. |
136baaf to
7767ebf
Compare
Installing nano-pdf then whisper-cli with pip3 caused dependency downgrades (typer, rich), potentially breaking earlier-installed tools. Solution: Use 'uv tool install' for isolated virtual environments per tool, preventing dependency conflicts while maintaining tool availability. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Enhance git checkout logic to support: - Remote branches (via origin/branch) - Tags (fetched with --tags) - Commit SHAs Maintains Upstream openclaw-only changes while adding flexible ref support. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
7767ebf to
37f2ede
Compare
Remove fork-specific URLs to ensure PR contains only technical changes. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update WORKDIR from /opt/clawdbot to /opt/openclaw to match the repository rename. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add BUN_INSTALL environment variable for bun installation path - Add migrate.sh COPY and chmod to ensure migration script is available - Clean up ENV section organization Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update both README files to document all 28 included CLI tools: - Main README: detailed list with descriptions for each tool - Add-on README: categorized summary by tool type Tools cover: Home Assistant, productivity, AI & media processing, smart home control, communication, development, and utilities. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add clawhub for searching, installing, updating, and publishing agent skills from clawhub.com. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add clawhub to both README files: - Main README: detailed description - Add-on README: added to Development category clawhub enables searching, installing, updating, and publishing agent skills from clawhub.com. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove the old clawdhub entry as it has been replaced by clawhub. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove clawdhub from both READMEs as it has been replaced by clawhub. Tool count remains at 28. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@ngutman I updated the PR and resolved the Python CLI dependency conflict. IMHO, installing the latest tool versions by default is a feature, not an issue. This ensures users receive the most current versions immediately. Users requiring older or pinned versions can set these themselves. Since the I consider the first-time startup delay a fair trade-off. User time is more valuable than build time, and having all tools immediately available is a huge plus. If you want to disable specific tools by default to save build time, feel free to comment them out; users can then enable them in their forks (though that requires more effort than simply waiting a bit longer during the initial run). @niemyjski Customizing installed tools via the Home Assistant UI to build images dynamically is a great idea. tried implementing it, but Home Assistant apparently only interprets options at runtime, not build time. Consequently, I reverted to the current implementation. By default, users get all requirements for the built-in tools. For anything else, they can create custom forks (and open PRs if their additions are universally useful enhancements). I upgraded my Clawdbot to OpenClaw using this version. Everything looks good on my end, so I consider this ready to be merged. |
|
I also feel like we need to install a bunch of deps to make sure the default image can use agent-browser and jq (lots of things use jq for terminal parsing)
|
There was a problem hiding this comment.
I agree with latest vs pinned with how frequently this gets updated, which makes me feel like it should be args passed into it from the ha addon for verions. However, I had codex 5.3 do an audit (take it with a grain of salt) this is what it found. (please note I also included deps in this audit from the previous comment).
Security audit summary for this PR (#5):
High risk
- Supply-chain trust: many installs use floating
latesttags (go install ...@latest,releases/latest, npm/pnpm globals) with no deterministic pinning. - Download integrity: external binaries fetched via
curl | tar/ zip downloads without checksum/signature verification.
Medium risk
3) Credential exposure: runtime currently injects github_token into clone URL, which can leak via process args/logging/remote URL storage.
4) Expanded attack surface: large browser/media/system dependency footprint (GTK/WebKit/GStreamer/X11/Wayland/fonts) significantly increases CVE surface and patching burden.
5) Reproducibility: apt-get upgrade -y + floating tool versions make builds non-reproducible and harder to rollback.
Low/Medium
6) Ref input handling: broader branch/tag/commit checkout support increases input surface; ref validation should be strict.
Recommended remediations:
- Pin all third-party tool versions (explicit semver/tag/SHA) and avoid
latestdefaults. - Add checksum or signature verification for downloaded artifacts.
- Avoid embedding tokens in repo URLs; prefer authenticated headers/credential helpers.
- Keep only required runtime packages, split optional tools, and run regular image scanning/SBOM.
- Validate
branch/ref values and reject unsafe patterns.
I checked both main and pr-5, and they both contain the same line in the PR snapshot:
TOKEN_OPT="$(jq -r .github_token /data/options.json)"
REPO_URL="https://${TOKEN_OPT}@${REPO_URL#https://}"
It exists at line 87 in both refs (main and pr-5), so my audit point should be read as a pre-existing security issue still present in that PR, not a new diff introduced by PR #5.
|
@ngutman this pertains to the main branch as well (most of it) Here's a comprehensive security analysis of this add-on: Security Risk Assessment: OpenClaw Gateway HA Add-onCRITICAL: Full Read-Write Access to HA ConfigThe most significant risk is in config.json: "map": ["config:rw"]This maps the entire
Can it read HA data? Yes — essentially all of it. The HIGH: Host Network Modeconfig.json: "host_network": true
HIGH: SSH Root Shellrun.sh starts an OpenSSH server:
HIGH: Arbitrary Code Execution by Designrun.sh does:
This means whoever controls the MEDIUM: GitHub Token Exposurerun.sh: The GitHub PAT is read from MEDIUM: Unverified Binary DownloadsThe Dockerfile downloads binaries from the internet without checksum or signature verification:
A compromised CDN or DNS hijack during the Docker build could inject a trojanized binary. MEDIUM: Supply Chain — 28+ Third-Party ToolsThe image installs tools from many different authors/orgs:
MEDIUM:
|
Hey Nimrod, thanks for creating the Clawdbot add-on for Home Assistant! I'd like to share the improvements I made for my own setup, as they will hopefully benefit you and other users.
Dockerfile improvements:
run.sh improvement: