Hi Clawith team,
A handful of best-practice / hardening items from a security review at commit 30e8b774bc8f. None are critical, but a couple are worth a quick pass. Happy to open a PR for any of them.
1. Workflow shell-injection — .github/workflows/release.yml:483
- name: Extract Release Info
run: |
set -euo pipefail
branch_name="${{ github.event.pull_request.head.ref }}"
tag_name="${branch_name#release/}"
github.event.pull_request.head.ref is the PR's source branch name — attacker-controllable on a fork PR. A branch named with shell metacharacters (backticks, $(...), ;) is expanded directly into the run: shell. The fix is env:-indirection — bind it to an env var and reference the quoted variable so the runner passes it as data, not code:
env:
BRANCH_NAME: ${{ github.event.pull_request.head.ref }}
run: |
set -euo pipefail
tag_name="${BRANCH_NAME#release/}"
(The release_notes="$(cat …generated.md)" at line 443 is lower-risk but the same env: pattern applies.)
2. React Router — 6 advisories on the pinned version (frontend/package-lock.json)
Trivy flags six React Router CVEs on the deployed frontend: stored XSS (unescaped Location header in pre-render), XSS in unstable RSC redirect handling, same-origin redirect issue with //-prefixed paths, DoS via reflected user input, DoS via unbounded path expansion, and an arbitrary-construction advisory in vendored turbo-stream v2. A single bump clears them.
While here: there's no .github/dependabot.yml in the repo — wiring Dependabot for the npm + pip ecosystems would keep this tail from re-accumulating. Typically a ~10-line file.
3. Helm chart ships a default Postgres password — helm/clawith/values.yaml:123
postgresql:
auth:
password: clawith123456
The QUICKSTART (helm/QUICKSTART.md:70, helm/QUICKSTART_EN.md:70) repeats it with a "Strongly recommended to change to a strong password!" note — which is the failure mode: a default that ships in the chart + docs becomes the value a meaningful fraction of installs run unchanged. Two safer shapes:
- Leave
password: "" and {{ required "set postgresql.auth.password" .Values.postgresql.auth.password }} so the install fails closed if unset.
- Or generate a random password into a Secret on first install (
randAlphaNum + a lookup guard to keep it stable across upgrades).
4. nginx hardening — deploy/nginx/nginx.conf
Two standard reverse-proxy items: the config has the shape that permits HTTP/2 cleartext (h2c) request smuggling, and five location blocks proxy internal services without an internal; directive (so they're directly reachable rather than only via internal redirects). Worth an internal; on the internal-only locations and an explicit h2c guard.
5. Containers run as root — backend/Dockerfile, frontend/Dockerfile
Neither Dockerfile drops privileges before its entrypoint. Adding a non-root USER in the final stage is standard defense-in-depth for multi-tenant hosts.
Out of scope (mentioned for completeness, not asking for changes)
- ~25
text(f"…") SQL sites (mostly backend/app/api/tenants.py + alembic migrations). I looked at the runtime ones — the tenants.py cascade-delete interpolates a hardcoded SQL constant and binds the only variable (tid) as a parameter, so there's no injection vector today. It's a readability/lint nudge (a named CTE or SQLAlchemy construct over string-splicing), not a vulnerability.
- 3 SHA-1 uses: the WeCom one (
wecom.py:91) is required by the WeChat Work message-signature spec; the other two (webhooks.py, agent_tools.py) are non-crypto dedup/identifier hashing. All fine.
- The document-conversion
urllib calls are CDP wiring to a local headless Chrome (127.0.0.1, local file URI) — not an SSRF surface.
Full curated write-up: https://elfrost.github.io/ai-patchlab/scans/dataelement-clawith.html
Thanks for Clawith.
Hi Clawith team,
A handful of best-practice / hardening items from a security review at commit
30e8b774bc8f. None are critical, but a couple are worth a quick pass. Happy to open a PR for any of them.1. Workflow shell-injection —
.github/workflows/release.yml:483github.event.pull_request.head.refis the PR's source branch name — attacker-controllable on a fork PR. A branch named with shell metacharacters (backticks,$(...),;) is expanded directly into therun:shell. The fix isenv:-indirection — bind it to an env var and reference the quoted variable so the runner passes it as data, not code:(The
release_notes="$(cat …generated.md)"at line 443 is lower-risk but the sameenv:pattern applies.)2. React Router — 6 advisories on the pinned version (
frontend/package-lock.json)Trivy flags six React Router CVEs on the deployed frontend: stored XSS (unescaped
Locationheader in pre-render), XSS in unstable RSC redirect handling, same-origin redirect issue with//-prefixed paths, DoS via reflected user input, DoS via unbounded path expansion, and an arbitrary-construction advisory in vendoredturbo-streamv2. A single bump clears them.While here: there's no
.github/dependabot.ymlin the repo — wiring Dependabot for the npm + pip ecosystems would keep this tail from re-accumulating. Typically a ~10-line file.3. Helm chart ships a default Postgres password —
helm/clawith/values.yaml:123The QUICKSTART (
helm/QUICKSTART.md:70,helm/QUICKSTART_EN.md:70) repeats it with a "Strongly recommended to change to a strong password!" note — which is the failure mode: a default that ships in the chart + docs becomes the value a meaningful fraction of installs run unchanged. Two safer shapes:password: ""and{{ required "set postgresql.auth.password" .Values.postgresql.auth.password }}so the install fails closed if unset.randAlphaNum+ alookupguard to keep it stable across upgrades).4. nginx hardening —
deploy/nginx/nginx.confTwo standard reverse-proxy items: the config has the shape that permits HTTP/2 cleartext (h2c) request smuggling, and five
locationblocks proxy internal services without aninternal;directive (so they're directly reachable rather than only via internal redirects). Worth aninternal;on the internal-only locations and an explicit h2c guard.5. Containers run as root —
backend/Dockerfile,frontend/DockerfileNeither Dockerfile drops privileges before its entrypoint. Adding a non-root
USERin the final stage is standard defense-in-depth for multi-tenant hosts.Out of scope (mentioned for completeness, not asking for changes)
text(f"…")SQL sites (mostlybackend/app/api/tenants.py+ alembic migrations). I looked at the runtime ones — thetenants.pycascade-delete interpolates a hardcoded SQL constant and binds the only variable (tid) as a parameter, so there's no injection vector today. It's a readability/lint nudge (a named CTE or SQLAlchemy construct over string-splicing), not a vulnerability.wecom.py:91) is required by the WeChat Work message-signature spec; the other two (webhooks.py,agent_tools.py) are non-crypto dedup/identifier hashing. All fine.urllibcalls are CDP wiring to a local headless Chrome (127.0.0.1, local file URI) — not an SSRF surface.Full curated write-up: https://elfrost.github.io/ai-patchlab/scans/dataelement-clawith.html
Thanks for Clawith.