phase: 181_svg_document_preview
**Phase 181 — final verification pass (all tasks already complete; verified + docs pass)**
- Verified: `app/rag/svg_text.py` (100 % cov), vector branch in `_index_image_file`, `_row_kind_mismatch` text→image migration, `IMAGE_MIMES[".svg"]`, sandboxed CSP on `GET /api/documents/{id}/image`, `.doc-image-img--vector` in `document.js`/`styles.css`, figure/thumbnail deliberate non-changes.
- Tests/lint (exact commands): `uv run pytest --cov=app --cov-report=term-missing` → **4265 passed, TOTAL 99 %** (exit 0) · `uv run pytest tests/e2e/test_svg_document_preview.py -v --no-cov` → **5 passed in isolation** · `uv run ruff check .` → clean · `uv run pyright` → **0 errors**.
- Regression (each run in isolation, `--no-cov`, all rc=0): `test_image_documents`, `test_chat_image_questions`, `test_answer_images`, `test_chat_answer_clusters`, `test_image_anchor_context`, `test_image_upload_feedback`, `test_markdown_inline_links_images`, `test_prose_word_wrap`, `test_pdf_support`, `test_pictorial_pdf`, `test_garbage_pdf`, `test_pdf_image_extraction`, `test_pdf_image_context`, `test_pdf_pages_in_sources`, `test_pdf_strip_additive_selfheal`, `test_sync_image_progress`.
- Completion criteria: ①SVG→`is_image`+`image_path`, no vision call, own `<title>/<desc>/<text>` — **met**; ②`image/svg+xml` + `default-src 'none'; sandbox` + `private, max-age=3600`, raster headers byte-unchanged — **met**; ③chat figure → modal `<img>`, no XML — **met**; ④`/document.html` full-width vector in `.doc-image` + text below — **met**; ⑤`` paints — **met**; ⑥raster/PDF unchanged — **met**; ⑦all gates green — **met**; ⑧commit + phase-dir move — harness-owned.
- Documentation: found and fixed README inconsistencies — stale forward reference "left to phase 181 / is phase 181's job" → completed-tense sizing note; "Updating the documents" standalone-image list missing `svg`; new **Raster only** bullet for question uploads (`.svg` → 422) documenting the `chat_images` guard; missing `svg_indexed=N` / `import: svg indexed` log contract; env-table heading `122/123/124` → `122/123/124/181`; new troubleshooting entry for an `.svg` showing raw XML. Full audit: every `Settings` field and every `.env.example` var is documented; pages/commands/scripts all exist. AGENTS.md verified accurate (phase-181 §5 clause present) — no changes needed beyond what the phase wrote.
- Notable deviation (kept from task 02): `app/api/chat_images.py` subtracts `_VECTOR_IMAGE_EXTENSIONS` from the question-image accept set — a required consequence of LOCKED A7 (`svg` in the default image set) since `_CHAT_IMAGE_FILENAME_RE` refuses to *serve* `.svg`; the regex itself is untouched, as the phase specified.
- Note for the commit: changes include `README.md`, `AGENTS.md` and `.env.example` — the phase file's `git add app/ frontend/ tests/ .env.example .agents/phases/` line omits the two markdown files; stage them too.
- Next pending phase: `182_verbatim_show_text`.
This commit is contained in:
+26
-7
@@ -22,7 +22,9 @@ unguessable ``uuid4().hex`` — no enumeration value).
|
||||
The extension is the source of truth (the Content-Type header is a
|
||||
hint — the archive-uploader precedent): it must be in the phase-122
|
||||
image set (``settings.image_extension_set`` — the same frozenset the
|
||||
image-document walk uses), lowercased; total bytes are streamed with a
|
||||
image-document walk uses) minus the vector members
|
||||
(:data:`_VECTOR_IMAGE_EXTENSIONS` — a question image is RASTER-only,
|
||||
phase 181), lowercased; total bytes are streamed with a
|
||||
``chat_image_max_mb`` cap (413, fixed detail naming the cap — never
|
||||
echoing the filename).
|
||||
"""
|
||||
@@ -55,6 +57,20 @@ _STREAM_CHUNK = 1 << 20
|
||||
#: upload endpoint's own naming).
|
||||
_CHAT_IMAGE_FILENAME_RE = re.compile(r"^[0-9a-fA-F]{32}\.(png|jpe?g|webp|gif|bmp)$")
|
||||
|
||||
#: The VECTOR members of ``settings.image_extension_set`` — excluded
|
||||
#: from the question-image accept set (phase 181: the vector family
|
||||
#: belongs to INDEXING, not to question attachments). Two reasons, both
|
||||
#: hard: an OpenAI-compatible vision endpoint rejects
|
||||
#: ``image/svg+xml`` (the phase-181 LOCKED A1 reason — the vector file
|
||||
#: never reaches the describer at all), and
|
||||
#: :data:`_CHAT_IMAGE_FILENAME_RE` above deliberately refuses to SERVE a
|
||||
#: ``.svg``, so an accepted upload would be stored, unanswerable and
|
||||
#: unservable (a broken figure in the user's own bubble). Subtracting
|
||||
#: instead of touching the serve regex keeps question attachments
|
||||
#: raster-only by design while ``svg`` rides the image set for the
|
||||
#: document walk.
|
||||
_VECTOR_IMAGE_EXTENSIONS: frozenset[str] = frozenset({".svg"})
|
||||
|
||||
|
||||
@router.post("/chat-images")
|
||||
async def upload_chat_image(
|
||||
@@ -66,10 +82,12 @@ async def upload_chat_image(
|
||||
Gates, in order:
|
||||
|
||||
1. **extension** — the file name's extension (lowercased) must be in
|
||||
the phase-122 image set (``settings.image_extension_set``); the
|
||||
Content-Type header is a hint, never a source of truth (the
|
||||
archive-uploader precedent). A missing/unknown extension is a
|
||||
422 naming the accepted set — a fixed detail, no filename echo.
|
||||
the phase-122 image set (``settings.image_extension_set``)
|
||||
MINUS the vector members (:data:`_VECTOR_IMAGE_EXTENSIONS` —
|
||||
question images are RASTER-only, phase 181); the Content-Type
|
||||
header is a hint, never a source of truth (the archive-uploader
|
||||
precedent). A missing/unknown/vector extension is a 422 naming
|
||||
the accepted set — a fixed detail, no filename echo.
|
||||
2. **size** — the bytes are streamed (1 MiB chunks) with the
|
||||
``chat_image_max_mb`` cap; over-cap is a 413 naming the cap
|
||||
(fixed detail — never echoing the filename), the temp file is
|
||||
@@ -85,8 +103,9 @@ async def upload_chat_image(
|
||||
settings = get_settings()
|
||||
filename = file.filename or ""
|
||||
ext = Path(filename).suffix.lower()
|
||||
if ext not in settings.image_extension_set:
|
||||
accepted = ", ".join(sorted(settings.image_extension_set))
|
||||
accepted_extensions = settings.image_extension_set - _VECTOR_IMAGE_EXTENSIONS
|
||||
if ext not in accepted_extensions:
|
||||
accepted = ", ".join(sorted(accepted_extensions))
|
||||
raise HTTPException(
|
||||
status_code=422, detail=f"only {accepted} images are accepted"
|
||||
)
|
||||
|
||||
+59
-5
@@ -52,10 +52,15 @@ BYTES route: the persistent copy behind an ``is_image`` document's
|
||||
``image_path``, served with the extension's ``Content-Type`` (the
|
||||
``app.rag.summarizer.IMAGE_MIMES`` map — one map, one truth) and a
|
||||
``Cache-Control: private, max-age=3600`` header (the bytes are
|
||||
content-hashed — long enough, bustable by re-upload). PUBLIC (phase
|
||||
124 — the phase-123 chat-image precedent: the image is part of a
|
||||
shared chat's rendered content, and the shared chat — the app's ONLY
|
||||
anonymous surface — must render the answer figure faithfully for
|
||||
content-hashed — long enough, bustable by re-upload), plus — for the
|
||||
vector ``image/svg+xml`` entry only, phase 181 LOCKED A3 — a
|
||||
``Content-Security-Policy: default-src 'none'; sandbox`` so a DIRECT
|
||||
navigation to the URL cannot run script from stored SVG markup (the
|
||||
``SecurityHeadersMiddleware`` preserves that inner CSP; raster
|
||||
responses gain nothing). PUBLIC (phase 124 — the phase-123 chat-image
|
||||
precedent: the image is part of a shared chat's rendered content, and
|
||||
the shared chat — the app's ONLY anonymous surface — must render the
|
||||
answer figure faithfully for
|
||||
guests; the unguessable ``uuid4`` document id IS the credential, no
|
||||
enumeration value): a missing doc, a non-image doc, a doc whose
|
||||
``image_path`` is NULL, or a row whose copy was lost all map to 404
|
||||
@@ -132,6 +137,23 @@ from app.schemas import (
|
||||
|
||||
router = APIRouter(tags=["kb"])
|
||||
|
||||
#: The vector media type (phase 181, task 03) — read out of
|
||||
#: :data:`app.rag.summarizer.IMAGE_MIMES` rather than restated, so the
|
||||
#: guard below can never drift from the ``Content-Type`` the route
|
||||
#: sends for an ``.svg`` (one map, one truth).
|
||||
SVG_MEDIA_TYPE = IMAGE_MIMES[".svg"]
|
||||
|
||||
#: The CSP an SVG bytes response carries (phase 181, task 03, LOCKED
|
||||
#: A3). ``sandbox`` — no scripting, and the document gets an OPAQUE
|
||||
#: origin (no same-origin fetch/cookie/form capability);
|
||||
#: ``default-src 'none'`` — no subresource of any kind. Scoped to the
|
||||
#: vector type ON PURPOSE: an ``<img>`` never runs SVG script, but this
|
||||
#: URL is directly navigable, and the bytes come from an untrusted
|
||||
#: source tree, so a direct visit must not become a script-capable
|
||||
#: same-origin document. Raster types are untouched (their header set
|
||||
#: stays byte-identical). See :func:`get_document_image`.
|
||||
SVG_SANDBOX_CSP = "default-src 'none'; sandbox"
|
||||
|
||||
|
||||
def doc_format(path: str, extensions: frozenset[str] = frozenset()) -> str:
|
||||
"""Lowercased path suffix without its dot (``kubernetes.md`` → ``md``,
|
||||
@@ -353,6 +375,33 @@ def get_document_image(
|
||||
``application/octet-stream``) and ``Cache-Control: private,
|
||||
max-age=3600`` (the bytes are content-hashed — long enough to be
|
||||
useful, bustable by re-upload).
|
||||
|
||||
Phase 181 (task 03, LOCKED A3): an ``image/svg+xml`` response — the
|
||||
vector entry of that same map, derived from the ROW's extension and
|
||||
NEVER from a client value — carries ONE extra header,
|
||||
:data:`SVG_SANDBOX_CSP` (``default-src 'none'; sandbox``). An
|
||||
``<img>`` can never execute an SVG's script, which is why the
|
||||
figure surfaces are safe as they stand; but this URL is also
|
||||
DIRECTLY NAVIGABLE (a guest who pastes it into the address bar),
|
||||
and without the policy a stored SVG would render as a
|
||||
script-capable SAME-ORIGIN document — the ingestion path takes
|
||||
untrusted files from a source tree, so that is a stored-XSS seam
|
||||
scoped to the app's origin. ``sandbox`` blocks scripting AND makes
|
||||
the response an opaque origin (no same-origin fetches, cookies or
|
||||
form submission from the document); ``default-src 'none'`` blocks
|
||||
every subresource load, so the SVG cannot pull a tracker or a
|
||||
remote image either. Deliberately NOT applied to the raster types:
|
||||
their header set stays byte-identical (a PNG needs no policy
|
||||
change, and adding one would risk the phase-123/124 image
|
||||
surfaces), and no ``Content-Disposition: attachment`` is set — the
|
||||
graphic must render inline, and ``sandbox`` is the stronger, less
|
||||
intrusive control. The route sets the value on the response
|
||||
itself; :class:`app.core.security_headers.SecurityHeadersMiddleware`
|
||||
PRESERVES a CSP an inner layer already set, so this is the policy
|
||||
the browser sees (the app-wide ``CSP`` string would be wrong here —
|
||||
``default-src 'self'`` would grant the SVG document the app's own
|
||||
origins), while the middleware's global
|
||||
``X-Content-Type-Options: nosniff`` still applies.
|
||||
"""
|
||||
try:
|
||||
uid = uuid.UUID(doc_id)
|
||||
@@ -368,10 +417,15 @@ def get_document_image(
|
||||
# onerror fallback renders the "image unavailable" note.
|
||||
raise HTTPException(status_code=404, detail="document not found")
|
||||
media_type = IMAGE_MIMES.get(image_file.suffix.lower(), IMAGE_FALLBACK_MIME)
|
||||
headers = {"Cache-Control": "private, max-age=3600"}
|
||||
if media_type == SVG_MEDIA_TYPE:
|
||||
# Phase 181 (LOCKED A3): vector bytes only — see the route
|
||||
# docstring. Raster responses keep the header set they had.
|
||||
headers["Content-Security-Policy"] = SVG_SANDBOX_CSP
|
||||
return FileResponse(
|
||||
image_file,
|
||||
media_type=media_type,
|
||||
headers={"Cache-Control": "private, max-age=3600"},
|
||||
headers=headers,
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user