Fix static path traversal, improve first-run UX, add Docker/Hub docs#20
Merged
Conversation
Deep-test findings on the published ci-local-bench container: Security: the static file handler resolved request paths with `'.' + req.url` and no containment check, so `GET /../../../etc/passwd` returned 200 with the file contents. Resolve requests against a fixed STATIC_ROOT (process.cwd()), normalize + decode first, and reject anything that escapes the root. Adds a regression test proving the real /etc/passwd is never read. First-run UX: a fresh Hub install (no benchmark data) showed a red "⚠️ Error: No benchmark results found" banner — alarming for the expected empty state that every new user sees. Replace it with a neutral, welcoming empty-state message that points the user at the Run Benchmark flow. Docs: README had no container section. Add a Docker / Companion Intelligence Hub section documenting the public image, port 3000, the OLLAMA_API_URL env var (the real var the code reads) with host.docker.internal/LAN examples, the graceful no-Ollama / first-run behavior, and result persistence. All 111 jest tests pass; tsc build clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deep-test pass on the published
ghcr.io/companionintelligence/ci-local-bench:latestcontainer. Three fixes, all verified against a rebuilt image.Security (high): static-file path traversal
The static handler resolved requests as
'.' + req.urlwith no containment check. Against the live container,GET /../../../etc/passwd(raw,--path-as-is) returned HTTP 200 with the contents of/etc/passwd.Fix: resolve each request against a fixed
STATIC_ROOT(=process.cwd()), URL-decode andpath.normalizefirst, and reject anything that escapes the root. After the fix the same request returns 404 and the real file is never read. Adds a regression test asserting the resolved path stays inside the root.First-run UX
A fresh Hub install (no benchmark data, no Ollama) rendered a red
⚠️ Error: No benchmark results foundbanner at the top of the page — the first thing every new user sees, for an entirely expected empty state. Replaced with a neutral, welcoming info message that points to the Run Benchmark flow. The model picker / specs / chart already degraded gracefully; only the error banner was alarming.Docs
README had no container section. Added a Docker / Companion Intelligence Hub section: public image, internal port 3000, the real
OLLAMA_API_URLenv var (withhost.docker.internaland LAN examples), the graceful no-Ollama / first-run behavior, and where results are written for persistence.Verification
npm ci && npm test— 111/111 jest tests pass (110 original + 1 new traversal test);tscbuild clean.tests/container,APP_URL=...) passes; playwright console sweep shows no JS errors (only the expected/api/models503 when Ollama is absent).GET /200, all GET endpoints behave, traversal closed, logs clean, restart OK.No redesigns; minimal, on-brand changes only.
🤖 Generated with Claude Code