peter
07328a3412
- #006 (high, data loss): make_room_for() no longer deletes unkept artifacts on a doomed request - checks whether eviction can possibly succeed before deleting anything. Also fixed a related latent bug found while testing: the function returned evicted Artifact objects after calling .delete() on them, so their .id was always None (Django clears pk on delete); now returns ids captured beforehand. - #003: added server-side field-length validation (source/title/ source_name/source_id/theme) on POST and PATCH, matching the model's own max_length limits, which were previously unenforced on the ingest path entirely. - #004: ported daybook_post.py's content-shape validation (html_fragment/html_full/deck/markdown) to the server, closing the gap for any client that doesn't go through the helper script. - #005: PATCH now rejects wrong-typed `tags` (previously silently wiped existing tags) and rejects immutable fields (content/source/kind/day) with 400 instead of silently no-op'ing them. - #002: deleted the misleading skills/daybook-posting/.env.example (a zero-dependency script that never reads .env files) and clarified SKILL.md/docs/llm_posting_helper.md that these must be real process/shell env vars. Adds tests/unit/ for storage-layer tests. 46 tests passing (was 25). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
71 lines
3 KiB
Markdown
71 lines
3 KiB
Markdown
# 004 — `html_fragment`/`deck`/`html_full` content-shape rules exist only in the client helper, not the server
|
|
|
|
**Status:** Fixed
|
|
|
|
## Description
|
|
|
|
`docs/api.md` and `skills/daybook-posting/SKILL.md` both describe a
|
|
content-shape contract per text `kind`:
|
|
|
|
- `html_fragment` — "Wrapped with a Daybook theme" (body-only HTML, no
|
|
`doctype`/`html`/`body`).
|
|
- `html_full` — complete document, must contain `<html>`.
|
|
- `deck` — top-level `<section>` blocks.
|
|
|
|
`skills/daybook-posting/scripts/daybook_post.py`'s `validate_payload()`
|
|
enforces all three of these client-side before ever sending the request.
|
|
**`artifacts/views.py`'s `ArtifactListCreateView.post` / `create_text_artifact`
|
|
enforce none of them** — the only server-side checks are "kind is one of
|
|
the 4 text kinds" and "required fields present." Any client that talks to
|
|
`POST /api/artifacts` directly (curl, a different language, a
|
|
misconfigured agent) bypasses the contract entirely, and the server
|
|
accepts and stores whatever is sent.
|
|
|
|
Confirmed: posting `kind: "html_fragment"` with a full
|
|
`<!doctype html><html><head>...</head><body>...</body></html>` as
|
|
`content` returns `201`, and the stored page (`GET /a/{id}`) is:
|
|
|
|
```html
|
|
<!doctype html>
|
|
<html>
|
|
<head>
|
|
...
|
|
<title>nested html fragment</title>
|
|
<link rel="stylesheet" href=".../report.css">
|
|
</head>
|
|
<body class="artifact">
|
|
<!doctype html><html><head><title>Injected</title></head><body><h1>Nested doc</h1></body></html>
|
|
</body>
|
|
</html>
|
|
```
|
|
|
|
i.e. a doctype and a full `<html><head><body>` nested inside the theme
|
|
wrapper's own `<html><head><body>` — invalid markup that browsers will
|
|
silently "fix up" in unpredictable ways (duplicate/conflicting `<title>`,
|
|
nested `<head>`, etc.), rather than the clean, themed fragment the kind
|
|
is supposed to guarantee.
|
|
|
|
This is the same class of gap as #003: validation the docs promise is
|
|
implemented only in the optional client helper, not the server that's
|
|
supposed to be the actual contract enforcement point. Related to (but
|
|
distinct from) issue #003, which is about unenforced *length* limits
|
|
rather than unenforced *content shape*.
|
|
|
|
## Suggested Fix
|
|
|
|
Port `daybook_post.py`'s `validate_payload()` checks (or equivalent) into
|
|
`create_text_artifact`/`ArtifactListCreateView.post` server-side, returning
|
|
`400` with the same messages the helper already uses. This also closes the
|
|
gap for any future non-Python client.
|
|
|
|
## Resolution
|
|
|
|
Added `validate_content_shape(kind, content)` in `artifacts/views.py`,
|
|
ported directly from `daybook_post.py`'s `validate_payload()` (same rules,
|
|
same case-insensitive substring checks) so client and server agree: `deck`
|
|
requires `<section`, `html_fragment` rejects `<html`/`<!doctype`/`<body`,
|
|
`html_full` requires `<html`, and `markdown` rejects whitespace-only content
|
|
(which the old required-field check missed, since a non-empty
|
|
whitespace-only string is truthy). Called from `ArtifactListCreateView.post`
|
|
after the existing field/source validation. Tests added covering each
|
|
rejection case plus a well-formed `html_fragment` still succeeding.
|