#009: html_fragment's content-shape check is substring-based, so it can false-positive on inert text inside <script>/<pre> (e.g. a JS string literal that happens to contain "<html>"). Documented as a known limitation in docs/api.md rather than implementing real HTML parsing - a proper fix is a much bigger change than the narrow trigger justifies, and substring matching is the same approach the client helper already uses. #011: added a curated CDN Snippets section to SKILL.md for html_full (Tailwind, Chart.js, with an offline/air-gapped caveat), scoped explicitly to that kind only. Fixed daybook_post.py's --theme help text, which named a non-existent "bootstrap" theme (written ahead of a vendored-Bootstrap feature that was planned but never built, not a typo). Also updated SKILL.md's kind cheat sheet, which never mentioned the json/csv kinds and still pointed at the old --as-markdown workaround for them - and filed issue #012 for a real gap that surfaced doing so: the posting script itself doesn't support --kind json/csv yet, only the raw API does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
09ea0a8b4f
commit
532b4bd4eb
6 changed files with 254 additions and 3 deletions
|
|
@ -0,0 +1,89 @@
|
|||
# 009 — `html_fragment` content-shape check false-positives on safe text inside `<script>` string literals
|
||||
|
||||
**Status:** Fixed (documented as a known limitation)
|
||||
|
||||
Follow-up to the #007 handover, which specifically asked to probe the
|
||||
`<html`/`<!doctype`/`<body`/`<section` substring checks for content
|
||||
containing those strings "inside a code block/string literal rather than
|
||||
as real markup."
|
||||
|
||||
## Description
|
||||
|
||||
`validate_content_shape` (added for #004) rejects `html_fragment` content
|
||||
containing the literal substrings `<html`, `<!doctype`, or `<body`
|
||||
anywhere in the text, case-insensitively — with no awareness of HTML
|
||||
structure/context. This correctly catches genuinely dangerous cases (see
|
||||
#004's original report and the escaped-vs-unescaped `<pre>` test below),
|
||||
but also rejects content where those substrings are inert — e.g. a JS
|
||||
string literal inside `<script>`, which can never become a real DOM
|
||||
element:
|
||||
|
||||
```json
|
||||
{
|
||||
"kind": "html_fragment",
|
||||
"content": "<p>Config example:</p><script>const tpl = \"<html><body></body></html>\";</script>"
|
||||
}
|
||||
```
|
||||
|
||||
→ `400 {"detail":"html_fragment must be body-only HTML; do not include doctype/html/body"}`
|
||||
|
||||
This content is legitimate, safe, body-only HTML (a `<script>` block
|
||||
whose string contents are never parsed as markup by the browser) — a
|
||||
completely reasonable pattern for an artifact demonstrating/generating
|
||||
HTML client-side. It's rejected purely because the substring check has
|
||||
no idea it's inside a JS string.
|
||||
|
||||
For contrast/completeness, confirmed correct behavior on two adjacent
|
||||
cases while investigating this:
|
||||
- Properly **escaped** literal HTML text in a `<pre>` block (`<html>`)
|
||||
is accepted, as it should be — the check only fires on unescaped `<`.
|
||||
- **Unescaped** `<html>` inside a `<pre>` block is correctly *rejected* —
|
||||
that's not actually a false positive, since unescaped tags inside
|
||||
`<pre>` still get parsed as real nested elements by the browser (same
|
||||
root cause as #004), so blocking it is correct.
|
||||
- Uppercase `<HTML>`/`<!DOCTYPE HTML>` is correctly still caught
|
||||
(case-insensitive check works both directions — no case-based bypass),
|
||||
and uppercase `<SECTION>` in a `deck` is correctly still accepted.
|
||||
|
||||
So the check's case-insensitivity and escaping-awareness are both solid;
|
||||
the gap is specifically "text inside script/string context is
|
||||
indistinguishable from real markup" to a substring check.
|
||||
|
||||
## Severity / Scope
|
||||
|
||||
Low-to-moderate. Narrow trigger (an artifact literally about generating
|
||||
or documenting HTML client-side), and there's a workaround (escape the
|
||||
inner `<` as `<` even inside `<script>`, since JS accepts HTML
|
||||
entities... actually it doesn't — entities are only decoded by the HTML
|
||||
parser, so escaping breaks the JS string value itself; the real
|
||||
workaround is string concatenation, e.g. `"<" + "html>"`, which is
|
||||
awkward). Worth a conscious call on whether this is worth fixing (real
|
||||
HTML parsing) or documenting as a known limitation, rather than treating
|
||||
it as a must-fix — a proper fix means parsing the content as HTML and
|
||||
checking node structure instead of substring matching, which is a bigger
|
||||
change than the substring-check approach #004 shipped.
|
||||
|
||||
## Suggested Fix (pick one)
|
||||
|
||||
- **Document as a known limitation** in `docs/api.md`'s kind table:
|
||||
"content-shape validation is substring-based, not a real HTML parse —
|
||||
avoid the literal strings `<html`, `<!doctype`, `<body` anywhere in
|
||||
`html_fragment` content, including inside `<script>`/`<pre>` text."
|
||||
Cheapest, matches the spirit of the #004 fix (mirrors the client
|
||||
helper's own substring approach).
|
||||
- **Real fix**: parse `content` with an HTML parser (e.g. Python's
|
||||
`html.parser` or `lxml`) and check for actual top-level `<html>`/
|
||||
`<body>` *elements* rather than substrings. More correct, more work,
|
||||
and adds a dependency if not using stdlib `html.parser`.
|
||||
|
||||
## Resolution
|
||||
|
||||
Took the cheap option, per the issue's own framing: documented as a known
|
||||
limitation in `docs/api.md` rather than implementing real HTML parsing. The
|
||||
narrow trigger (an artifact literally about generating/documenting HTML
|
||||
client-side) doesn't justify the bigger, riskier change a proper fix would
|
||||
need (parse `content` and check actual node structure instead of substrings)
|
||||
— substring matching is already the deliberate approach #004 shipped and
|
||||
that `daybook_post.py`'s own client-side check uses, so this keeps
|
||||
server/client behavior consistent rather than making the server stricter
|
||||
in a way the client can't warn about ahead of time.
|
||||
79
issues/011-vetted-cdn-snippets-for-html-full.md
Normal file
79
issues/011-vetted-cdn-snippets-for-html-full.md
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
# 011 — Document vetted CDN framework snippets for `html_full`, scoped to that kind only
|
||||
|
||||
**Status:** Fixed
|
||||
|
||||
## Description
|
||||
|
||||
`html_full` is documented (`SKILL.md`'s kind table) as the kind for
|
||||
"complete custom pages/frameworks" — Daybook injects nothing into it
|
||||
(`create_text_artifact`: `rendered = content` verbatim), so a poster
|
||||
wanting Tailwind, Chart.js, or similar has to already know a CDN URL and
|
||||
hand-write the `<link>`/`<script>` tags themselves. There's currently no
|
||||
guidance anywhere in `docs/` or `skills/daybook-posting/` pointing to
|
||||
known-good CDN URLs, so an agent has to search for one from scratch every
|
||||
time.
|
||||
|
||||
Separately (found while investigating this): `daybook_post.py`'s own
|
||||
`--theme` argparse help text reads `"Example: report, tasks, bootstrap"`
|
||||
— but `bootstrap` isn't a real theme (only `report`/`tasks`/`deck`/`base`
|
||||
exist as files under `static/themes/`). Requesting it silently produces a
|
||||
`<link>` to a 404'ing stylesheet, rendering unstyled with no error. Worth
|
||||
fixing alongside this, since it's the same "help the agent avoid a dead
|
||||
end" spirit.
|
||||
|
||||
Update: the `bootstrap` example wasn't a random typo — `planning/framework_artifact_rendering.md`
|
||||
already plans a vendored-Bootstrap-theme feature that just hasn't been built
|
||||
yet, and the help text was almost certainly written ahead of that. Worth
|
||||
linking that doc (and `planning/2026-09-02-theme_variants_and_user_palettes.md`,
|
||||
a related-but-distinct design conversation about palette variants within the
|
||||
existing theme families) when this is picked up, rather than treating the
|
||||
example as simply wrong.
|
||||
|
||||
**Explicitly out of scope / not being requested here**: injecting CDN
|
||||
references into the *themed* kinds (`markdown`/`html_fragment`/`deck`).
|
||||
Those exist specifically so Daybook — not the poster — owns styling
|
||||
(stated principle in Daybook's own `CLAUDE.md`: "Do not make agents
|
||||
responsible for styling"), and giving agents a default-reach-for CDN
|
||||
snippet for those kinds would undermine that. This is scoped to
|
||||
`html_full` only, where the poster has already opted out of managed
|
||||
styling by choosing that kind.
|
||||
|
||||
## Suggested Fix
|
||||
|
||||
- Add a short, curated list of CDN snippets (Tailwind, and whatever else
|
||||
comes up often — Chart.js is a reasonable second) to `SKILL.md`'s
|
||||
`html_full` row or a new subsection, so agents can copy-paste instead
|
||||
of searching. Note Tailwind's own CDN build
|
||||
(`cdn.tailwindcss.com`) is marked "not for production" by Tailwind
|
||||
themselves (compiles in-browser on every load) — worth linking a
|
||||
precompiled build instead, or explicitly caveating if using the Play
|
||||
CDN anyway for convenience.
|
||||
- Note the self-hosted/offline tension explicitly: Daybook runs locally
|
||||
and can be deployed air-gapped; any CDN-linked artifact won't render
|
||||
fully offline. A one-line caveat in the docs is enough — this is a
|
||||
tradeoff to flag, not something to solve.
|
||||
- Fix the `--theme` help text's `bootstrap` example to name a real theme
|
||||
(or drop the example value entirely and just say "see docs/api.md for
|
||||
available themes").
|
||||
|
||||
## Resolution
|
||||
|
||||
Added a "CDN Snippets For `html_full`" section to `SKILL.md` with Tailwind
|
||||
(Play CDN, with a one-line caveat that it's not a production-grade build)
|
||||
and Chart.js, an explicit offline/air-gapped caveat, and a reminder that
|
||||
this is scoped to `html_full` only — never the themed kinds. Fixed
|
||||
`daybook_post.py`'s `--theme` help text (`bootstrap` was never a real theme;
|
||||
it was written ahead of the vendored-Bootstrap-theme feature described in
|
||||
`planning/framework_artifact_rendering.md`, not a random typo) to name real
|
||||
themes instead.
|
||||
|
||||
While in `SKILL.md`'s kind cheat sheet, also found and fixed a second stale
|
||||
spot: it never mentioned the `json`/`csv` kinds added this session, and
|
||||
still told agents to use the `--as-markdown` fenced-code-block workaround
|
||||
for them even though they now have real first-class support. Updated the
|
||||
cheat sheet accordingly.
|
||||
|
||||
That surfaced a real, separate gap: the posting *script* itself
|
||||
(`daybook_post.py`) doesn't actually support `--kind json`/`csv` yet — filed
|
||||
as issue #012 rather than fixing inline here, since it needs real script
|
||||
changes (TEXT_KINDS, auto-detection, validation), not a doc tweak.
|
||||
46
issues/012-posting-script-missing-json-csv-support.md
Normal file
46
issues/012-posting-script-missing-json-csv-support.md
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
# 012 — `daybook_post.py` doesn't know about the `json`/`csv` kinds
|
||||
|
||||
**Status:** Open
|
||||
|
||||
## Description
|
||||
|
||||
Found while updating `SKILL.md`'s kind cheat sheet for issue #011.
|
||||
`artifacts/views.py` added first-class `json` and `csv` kinds, but the
|
||||
posting helper script wasn't updated alongside it:
|
||||
|
||||
- `skills/daybook-posting/scripts/daybook_post.py`'s `TEXT_KINDS = {"markdown",
|
||||
"html_fragment", "html_full", "deck"}` doesn't include `json`/`csv`, so
|
||||
`--kind json` is rejected by argparse before a request is ever sent.
|
||||
- `TEXT_SUFFIXES` (used by positional auto-mode to sniff a file's kind from
|
||||
its extension) doesn't include `.json`/`.csv` either — posting
|
||||
`data.json` via the simple `daybook_post.py data.json` auto-mode path
|
||||
uploads it as a generic downloadable `binary` artifact instead of using
|
||||
the new interactive kind.
|
||||
- `validate_payload()` has no shape-check branch for `json` (mirroring the
|
||||
server's `json.loads` validation, per how every other kind's client-side
|
||||
check mirrors its server-side one).
|
||||
|
||||
Net effect: the API itself supports `json`/`csv` fully (confirmed working
|
||||
via direct HTTP), but an agent following the documented, recommended
|
||||
workflow ("always use the helper script") currently cannot reach either
|
||||
kind at all.
|
||||
|
||||
## Suggested Fix
|
||||
|
||||
- Add `"json"`, `"csv"` to `TEXT_KINDS`.
|
||||
- Add `.json`/`.csv` to `TEXT_SUFFIXES` with matching `sniff_text_kind`
|
||||
logic (or a clear rule — e.g. `.csv` always maps to `kind=csv`; `.json`
|
||||
maps to `kind=json` unless some other signal suggests it's meant as
|
||||
metadata).
|
||||
- Add a `json` branch to `validate_payload()`: attempt to parse and raise a
|
||||
clear `ValueError` on invalid JSON, mirroring the server.
|
||||
- Add a usage example to `SKILL.md`/`docs/llm_posting_helper.md` for both
|
||||
kinds (e.g. `daybook_post.py --kind json --content-file data.json`).
|
||||
- No test suite currently exists for this script at all (it's exercised
|
||||
manually / via the API tests on the server side only) — worth deciding
|
||||
whether this is finally worth a small script-level test file, or whether
|
||||
server-side coverage is considered sufficient.
|
||||
|
||||
## Resolution
|
||||
|
||||
_(pending)_
|
||||
Loading…
Add table
Add a link
Reference in a new issue