This commit is contained in:
Your Name
153
bug-tracking.md
153
bug-tracking.md
@@ -1,148 +1,15 @@
|
||||
# Bug Tracking
|
||||
|
||||
## Status Legend
|
||||
- **Fixed** — resolved and verified
|
||||
- **Open** — not yet addressed
|
||||
- **Wontfix** — accepted design/known limitation
|
||||
No open bugs. All known issues have been fixed or accepted.
|
||||
|
||||
## Wontfix / Design Notes
|
||||
|
||||
### `_assert_safe_path` TOCTOU race
|
||||
`spectra/web/server.py` — Path check and `send_file()` are not atomic. An attacker needs write access to the uploads dir to exploit. Mitigation would add complexity with negligible security gain.
|
||||
|
||||
### No cascade delete on Rotation FK
|
||||
`spectra/web/models.py:31` — `Rotation.image_id` has no `ondelete="CASCADE"`. The single delete route handles cleanup explicitly. Add cascade if new deletion paths are added.
|
||||
|
||||
---
|
||||
|
||||
## Critical
|
||||
|
||||
### 1. ~~All settings forms broken (send URL-encoded, API expected JSON)~~
|
||||
|
||||
**Fixed:** Every htmx form on the settings page (saturation, orientation, MQTT, schedule, Unsplash) sent URL-encoded form data (`application/x-www-form-urlencoded`) but the `/api/config/<section>/<key>` PUT endpoint used `request.get_json(force=True, silent=True)`. URL-encoded data is not valid JSON, so `get_json` returned `None` → the endpoint aborted with 400. Result: **every single settings form returned 400 in the browser**. The MQTT checkbox bug (old #1) was a symptom of this larger issue.
|
||||
|
||||
**Fix** (`spectra/web/server.py`): `api_config_set()` now tries JSON first, then falls back to `request.form.get("value")`. A `_coerce_form_value(raw, current)` helper infers the correct Python type (bool/int/float/str) from the form string by comparing against the current config value's type.
|
||||
|
||||
### 2. ~~API config tests wrote to real `~/.config/spectra/config.yaml`~~
|
||||
|
||||
**Fixed:** `ConfigManager._write()` resolved the config path by searching `CONFIG_SEARCH_PATHS`, and when no file existed (test environment), fell back to `~/.config/spectra/config.yaml`. Every test that called `PATCH /api/config` or `PUT /api/config/<section>/<key>` wrote test data into the real user's config file — **potential data corruption**.
|
||||
|
||||
**Fix** (`tests/conftest.py`): The `app` fixture now writes a minimal `config.yaml` into the temp directory and passes its path to `create_app(config_path=...)`. The ConfigManager writes back to the same temp file.
|
||||
|
||||
---
|
||||
|
||||
## High
|
||||
|
||||
### 3. `_lookup_image_path` crashes if DB/table doesn't exist
|
||||
|
||||
**File:** `spectra/mqtt.py:19–25`
|
||||
|
||||
**Problem:** When the web server hasn't created the database yet (or the `image` table is missing), `_lookup_image_path` raises `sqlite3.OperationalError: no such table: image`. The MQTT message handler catches this in paho, but the error is unhandled at the application level.
|
||||
|
||||
**Fixed:** Wrapped the `conn.execute()` call in `try/except sqlite3.OperationalError`, logging a warning and returning `None`.
|
||||
|
||||
### 4. `_MIME_MAP` dict redefined on every file download request
|
||||
|
||||
**File:** `spectra/web/server.py:240`
|
||||
|
||||
**Problem:** The `api_image_file()` route handler defined a `_MIME_MAP` dict inside the function body on every request. This is a minor performance issue — dictionary creation is cheap, but the dict is constant data that should be a module-level constant.
|
||||
|
||||
**Fixed:** Moved `_MIME_MAP` to module level.
|
||||
|
||||
### 5. Dead link `/docs/mqtt.md` in settings page
|
||||
|
||||
**File:** `spectra/web/templates/settings.html:138`
|
||||
|
||||
**Problem:** The settings template has `<a href="/docs/mqtt.md">MQTT docs</a>`. No Flask route serves this path, so browsers get a 404.
|
||||
|
||||
**Fixed:** Replaced with text: `<code>docs/mqtt.md</code>` (no link).
|
||||
|
||||
### 6. `_generate_thumbnail` leaks file descriptor
|
||||
|
||||
**File:** `spectra/web/server.py:400–406`
|
||||
|
||||
**Problem:** `PILImage.open(src)` without a context manager leaves the file handle open until garbage collection. On long-running servers serving many thumbnails, this can exhaust file descriptors.
|
||||
|
||||
**Fixed:** Changed to `with PILImage.open(src) as img:`.
|
||||
|
||||
### 7. Dead code: `TRIGGER_PATH` and `_trigger_path()`
|
||||
|
||||
**File:** `spectra/web/server.py:69–70`
|
||||
|
||||
**Problem:** `app.config["TRIGGER_PATH"]` was set but never read by any code. The `_trigger_path()` function was only used for that config key.
|
||||
|
||||
**Fixed:** Removed both the config key and the function.
|
||||
|
||||
### 8. Redundant `section_data` variable in `api_config_set`
|
||||
|
||||
**File:** `spectra/web/server.py:122–124`
|
||||
|
||||
**Problem:** `section_data = cm.config.get(section, {})` was assigned but only used for `isinstance(section_data, dict)` check. The old redundant mutation `section_data[key] = data["value"]` was already removed in a previous fix, leaving the variable as dead code.
|
||||
|
||||
**Fixed:** Removed the `section_data` check entirely — `cm.set_nested()` handles missing sections.
|
||||
|
||||
---
|
||||
|
||||
## Medium
|
||||
|
||||
### 9. `api_display_status` always reports `simulate: false`
|
||||
|
||||
**File:** `spectra/web/server.py:148`
|
||||
|
||||
**Problem:** `cm.get("display", "simulate")` reads a config key that doesn't exist (`display.simulate` is set via CLI `--simulate`, not in the config file). The field always reports `false` even when the display loop runs with `--simulate`.
|
||||
|
||||
**Fixed:** Removed the `simulate` field from the API response and the dashboard template. The web server and CLI are separate processes; the web server cannot know the display loop's `--simulate` state.
|
||||
|
||||
### 10. `_assert_safe_path` TOCTOU race
|
||||
|
||||
**File:** `spectra/web/server.py:319–323`
|
||||
|
||||
**Problem:** The path safety check (`real_file.startswith(real_store + os.sep)`) and the subsequent `send_file()` are not atomic. A symlink could be swapped in between the check and the file read. An attacker needs write access to the uploads directory to exploit this.
|
||||
|
||||
**Status:** **Wontfix** — requires filesystem write access to exploit; mitigation would add complexity with minimal security gain.
|
||||
|
||||
### 11. No cascade delete on Rotation FK
|
||||
|
||||
**File:** `spectra/web/models.py:31`
|
||||
|
||||
**Problem:** `Rotation.image_id` has a foreign key to `image.id` but no `ondelete="CASCADE"`. Image deletion manually cleans up rotation entries (`Rotation.query.filter_by(image_id=...).delete()`). If a new deletion path is added without this cleanup, the FK constraint raises an error.
|
||||
|
||||
**Status:** **Wontfix** — explicit cleanup in the single delete route is sufficient for current code. Add cascade if new deletion paths are added.
|
||||
|
||||
---
|
||||
|
||||
## Low
|
||||
|
||||
### 12. Object URLs never revoked in upload page
|
||||
|
||||
**File:** `spectra/web/templates/upload.html:156`
|
||||
|
||||
**Problem:** `URL.createObjectURL(file)` creates blob URLs for file previews but never releases them with `URL.revokeObjectURL()`. For typical sessions (a few files) the memory impact is negligible.
|
||||
|
||||
**Status:** **Wontfix** — mitigated by the upload page: when a file's upload completes or fails, the `updateFileStatus` function checks `badgeClass === 'badge-ok' || badgeClass === 'badge-err'` and calls `URL.revokeObjectURL(blobUrl)` (lines 180–183). Blob URLs are already cleaned up per-file.
|
||||
|
||||
### 13. Unsplash filename collision on duplicate fetch
|
||||
|
||||
**File:** `spectra/library.py:56`
|
||||
|
||||
**Problem:** Original filename `f"unsplash_{unsplash_id}.png"` could collide if the same photo is fetched twice. **Already fixed** in previous session: filename now uses `f"unsplash_{unsplash_id}_{uuid.uuid4().hex[:8]}.png"` and a dedup check on `unsplash_id` prevents duplicate DB records.
|
||||
|
||||
---
|
||||
|
||||
## Previously Fixed
|
||||
|
||||
### Old #1 — MQTT checkbox can never be unchecked
|
||||
**Fixed:** Hidden `<input type="hidden" name="value" value="false">` added before the checkbox. Resolved as a side-effect of Bug #1 (form data now properly accepted with type coercion).
|
||||
|
||||
### Old #2 — Gallery delete shows "Saved"
|
||||
**Fixed:** Delete button uses `hx-target="closest article" hx-swap="delete"` with `hx-confirm`. The card is removed from the DOM directly.
|
||||
|
||||
### Old #3 — `MqttClient.reconfigure()` calls `__init__` directly
|
||||
**Fixed:** Extracted `_setup()` and `_create_client()` methods from `__init__`, called by both `__init__` and `reconfigure()`.
|
||||
|
||||
### Old #4 — Unsplash filename collision
|
||||
**Fixed:** See #13 above.
|
||||
|
||||
### Old #5 — Object URLs never revoked
|
||||
**Fixed:** See #12 above — already handled in per-file cleanup.
|
||||
|
||||
### Old #6 — Dead CSS `#upload-result:empty`
|
||||
**Fixed:** Selector removed from `app.css`.
|
||||
|
||||
### Old #8 — `api_config_set` double-mutates config
|
||||
**Fixed:** Redundant `section_data[key] = data["value"]` mutation removed.
|
||||
|
||||
### Old #9 — Gallery delete test missing
|
||||
**Fixed:** `tests/test_gallery.py` added with upload-delete flow E2E test.
|
||||
For a full history of fixes, see `git log`.
|
||||
|
||||
Reference in New Issue
Block a user