Fix v0.2 audit findings across security, data safety, and UX - #4
Merged
Conversation
A six-dimension audit of v0.2.0 (architecture, code quality, security, frontend, tests, docs) produced 31 findings; every one acted on here was independently verified against the code before being fixed. Security (the only high-severity finding) - Binding a non-loopback host now enables token auth: the access URL is printed at startup, and cookie/header/query credentials are accepted. Previously the app had no authentication at all and relied on a spoofable Host header, so a published Docker port exposed every dream to the local network and let anyone repoint the AI provider. - Header-less writes are refused from non-loopback clients unless they authenticate. Explicit --no-auth / DREAMLOOP_NO_AUTH=1 keeps them open: that mode has no token to offer a scripted client, so rejecting them would leave the JSON API unusable with no way to recover. - Compare tokens as bytes; compare_digest raises TypeError on non-ASCII input, which turned a bad token into a 500 at the auth boundary. - Hand ?token= to a cookie and redirect it out of page URLs, and redact it from uvicorn's access log, which records full query strings. - Return JSON rather than HTML for 401s on /api/ paths. Data safety - migrate_analysis_table dropped the renamed old table unconditionally, so an analysis table missing a required column lost every stored analysis silently. Unmigratable data is now preserved in a backup table, and the previously untested migration paths have coverage. - Recover from a corrupt config.json instead of returning 500 from every page, including the settings page needed to fix it. Both config writers now replace the file atomically through one shared helper. - Wrap invalid model JSON as AnalysisIncomplete; a truncated or fenced response from a local model surfaced as a 500. Diagnosability - Log the three swallowed exceptions in web.py (the project had no logging at all), stop reporting a missing dream as a provider error, and surface image-generation failures to the user. - Map import errors to 400/404 instead of 500. Analysis flow - POST /drafts/analyze now stores the draft and redirects, so a refresh no longer re-runs a slow LLM call and the URL survives navigation. Frontend - Clamp dream text in cards, which long entries turned into walls of text. - Fix heat-cell contrast to meet WCAG AA. Dark ink cannot reach 4.5:1 on the mid ramp steps, so only the brightest step inverts. - Add a skip link, reset loading buttons on bfcache restore, and preload the bundled fonts. Performance - init() is idempotent; every DreamLoop method re-ran the full migration. - feedback_summary uses one join instead of a query per feedback row. Tests and docs - Replace byte-exact source-string assertions with format-insensitive checks, and cover the image generator, the endpoints that had none, and the CLI auth wiring. 212 tests pass; coverage 87% -> 91%. - Refresh PROJECT_CONTEXT.md and the superseded frontend plan, document that .dreamloop/ is created relative to the working directory, and add SECURITY.md with issue and PR templates.
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
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.
Summary
A six-dimension audit of v0.2.0 (architecture, code quality, security, frontend, tests, docs) produced 31 findings. Every finding acted on here was independently re-verified against the code first, which corrected three overstatements and reversed one recommendation outright (see Review notes).
Highest-severity issue: the web app had no authentication and leaned on a spoofable
Hostheader, while the shippedDockerfileand compose file hardcode--host 0.0.0.0. A published port therefore let anyone on the local network read every dream and repoint the AI provider to exfiltrate future analyses.Changes
Security
X-DreamLoop-Token/?token=); the access URL is printed at startup and persisted in.dreamloop/web_token, or pinned viaDREAMLOOP_TOKEN.--no-auth/DREAMLOOP_NO_AUTH=1deliberately keeps them open — that mode has no token to offer a scripted client, so rejecting them would leave the JSON API unusable with no way to recover.compare_digestraisesTypeErroron non-ASCII input, turning a bad token into a 500 at the auth boundary.?token=is handed to a cookie and redirected out of page URLs, and redacted from uvicorn's access log (which records full query strings). API paths keep?token=working — curl has no cookie jar./api/paths return JSON, not HTML.Data safety
migrate_analysis_tabledropped the renamed old table unconditionally: an analysis table missing a required column lost every stored analysis silently. Unmigratable data is now preserved in a backup table, and the migration paths (previously 0% covered) have tests.config.jsonreturned 500 from every page, including the settings page needed to fix it. Both config writers now replace the file atomically through one shared helper.AnalysisIncomplete; a truncated or fenced local-model response used to surface as a 500.Diagnosability
web.pyare now logged (the project had no logging at all), a missing dream no longer reports as a provider error, and image-generation failures are visible to the user.Analysis flow
POST /drafts/analyzestores the draft and redirects, so refreshing no longer re-runs a slow LLM call and the result survives navigation.Frontend
Performance
init()is idempotent — everyDreamLoopmethod re-ran the full migration.feedback_summaryuses one join instead of a query per feedback row (measured 1757 ms at 800 rows before).Tests and docs
PROJECT_CONTEXT.mdand the superseded frontend plan, documented that.dreamloop/is created relative to the working directory, addedSECURITY.mdplus issue and PR templates.Verification
pytest— 212 passed, 0 failed (was 157); coverage 87% → 91%git diff --check— passedReview notes
web.pycarries token auth, the PRG rework, and error handling;database.pycarries the migration fix and the drafts table. Splitting along those seams would produce commits that do not pass tests.SECURITY.mdintentionally lists only GitHub Security Advisories — no maintainer email is published. Add one if you want that channel.CONTRIBUTING.mdadvertises "Good First Issues" but the repository has zero open issues and nothing under that label; that needs work on GitHub, not in code.unicode-rangesharding left for later), andweb.py/core.pyremaining large modules.