feat(dashboard-api): route logs through read-only service endpoint#808
Conversation
Update extension_logs() to call /v1/service/logs instead of /v1/extension/logs, enabling log viewing for core services. Add container_name to SERVICES dict for accurate name resolution. Replace bare except with specific urllib error types for proper error propagation. Add Docker Compose label spoofing prevention. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lightheartdevs
left a comment
There was a problem hiding this comment.
Audit: APPROVE — good security hardening
The error handling rewrite is a significant improvement — forwarding actual HTTP status codes instead of always returning 503. The Docker Compose label spoofing prevention is a valuable addition, correctly handling both dict and list label formats in YAML.
Adding container_name to SERVICES dict enables core service log viewing through the same endpoint as extensions.
Minor notes (non-blocking):
- The f-string in URLError fallback uses
dream-{service_id}which may not match the actual container name (e.g.,dream-webuivs the real name). Minor UX issue in error messages.
Depends on #806. Should merge after it.
Lightheartdevs
left a comment
There was a problem hiding this comment.
Revised Audit: REQUEST CHANGES — status code leakage + XSS risk
Withdrawing my earlier approval after deeper review.
Bug 1 (MEDIUM): Raw host-agent status codes forwarded to dashboard client
The HTTPError handler forwards exc.code verbatim. A 500 from the host agent (containing internal paths, Docker errors, stderr snippets) is sent directly to the client. A 401/403 (agent-to-agent auth failure) confuses the dashboard client into thinking its own auth is bad.
Fix: Clamp forwarded status codes — map 4xx from host agent to 502 (bad gateway), map 5xx to 502. Only forward sanitized error strings:
except HTTPError as exc:
status = 502 if exc.code >= 400 else exc.code
raise HTTPException(status_code=status, detail=f"Host agent error: {exc.code}")Security (LOW-MEDIUM): XSS via Docker log output
Docker log content is proxied from host agent to dashboard client. If the frontend renders logs via innerHTML or dangerouslySetInnerHTML, a malicious container could inject <script> tags in its own logs. Verify the frontend uses text rendering (not HTML) for log display.
Info: container_name in SERVICES dict is dead code for this PR
The field is added to the dict but never referenced by the log fetching logic — the host agent resolves container names independently via Docker label lookup. Not a bug, but unused.
Info: Strict merge ordering required
PR #806 must merge first or /v1/service/logs won't exist and ALL log requests return 404.
What's good: Label spoofing prevention is solid (handles both dict and list formats, covers all com.docker.compose.* labels). SSRF risk is zero (URL from server config, not user input). HTML error page fallback works correctly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addressing review feedbackBug 1 (Raw status codes forwarded) — Fixed:
Error detail string is still forwarded (sanitized via Re: XSS via Docker log output — The dashboard frontend renders logs using text content (React's default JSX escaping), not Re: |
Lightheartdevs
left a comment
There was a problem hiding this comment.
Re-audit: APPROVE — fix verified
HTTPError handler now returns 502 (Bad Gateway) instead of forwarding raw host-agent status codes. Prevents internal error leakage and auth confusion. Label spoofing prevention is solid.
CI all green. Depends on #806.
|
Note: The Rust dashboard-api rewrite (#821) merged and deleted the Python files this PR modifies. This PR's changes were entirely to Python files ( Please rebase or rewrite against the current |
What
extension_logs()to call/v1/service/logsinstead of/v1/extension/logs, enabling log viewing for core servicescontainer_nameto SERVICES dict for accurate container name resolutionexcept Exception:with specificurllib.error.HTTPErrorandurllib.error.URLErrorcatches_scan_compose_content()Why
validate_service_id()blocks themdream-{service_id}) fails for open-webui (container isdream-webui)How
/v1/extension/logs→/v1/service/logs(the new read-only endpoint that allows all services)HTTPErrorhandler forwards the actual status code from host agent (not always 503).URLError/OSErrorreturns 503 with f-string fallback: "Host agent unavailable. Use 'docker logs dream-{service_id}' in terminal."com.docker.compose.*labels (handles both dict and list label formats)dream-{service_id}Three Pillars Impact
/v1/extension/logsunchanged; label spoofing prevention strengthens extension securityModified Files
dashboard-api/config.py—container_namefield in SERVICES dictdashboard-api/routers/extensions.py—extension_logs()rewrite + label spoofing checkTesting
Automated
Manual
com.docker.compose.servicelabel → rejected at install timeReview
Platform Impact
Sequence
PR 5 of 7 (Phase 3). Depends on PR 4 (#806 — host agent logs endpoint)