Skip to content

Audit packages 1–4: /trace identity guard, admin-write + CF-Access tests, public copy fixes, accessibility, slim reads#202

Merged
MarkusNeusinger merged 6 commits into
mainfrom
claude/website-codebase-audit-d6d3mk
Jul 15, 2026
Merged

Audit packages 1–4: /trace identity guard, admin-write + CF-Access tests, public copy fixes, accessibility, slim reads#202
MarkusNeusinger merged 6 commits into
mainfrom
claude/website-codebase-audit-d6d3mk

Conversation

@MarkusNeusinger

@MarkusNeusinger MarkusNeusinger commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Packages 1–4 from the July full audit (backend · frontend · tests/CI · docs · content, ~90 findings): security/data-integrity fixes with the tests that pin them, the public copy corrections, the accessibility package, and the slim-read performance package.

Security & data integrity

  • POST /trace can no longer cross-link template rows. The template upsert conflicts on (style, glyph, position, variant) while reads go by glyph_key — a client bug pairing a wrong URL key with a payload identity could conflict-update another row and rewrite its glyph_key, after which reads silently 404 on the shared prod DB. Three layers now guard it: the expected key is derived from the shared registry (core.shaping.expected_glyph_key, the Python twin of glyphs.ts) with 422 on mismatch; an out-of-registry glyph may never claim a registry-owned key (422); and a stored (style, glyph_key, variant) row refuses being re-keyed to a different (glyph, position) (409).
  • DB engine init race closed. The lazy asyncio.Lock getter in core/database/connection.py was itself check-then-set; the lock is created at import now, the dead _sync_init_lock is removed.

Tests (closes the two untested prod-critical paths)

  • tests/test_api_admin_writes.py — the authorized side of the admin gate (templates.py sat at 26 % coverage): bbox PUT/GET roundtrip incl. the coalesce contract, the full /trace pipeline against the on-disk synthetic chart, 423 lock + force, the key-identity backstops (incl. the historical s/ſ allograph keys), DELETE semantics, and the new /bboxes/status read.
  • tests/test_api_auth.py — the Cloudflare Access JWT branch that actually gates prod (api/auth.py was at 47 %): listed email authorizes, unlisted email is a hard 403 with no token fallback, an unverifiable JWT falls through to the break-glass token path, plus unit tests of _verify_cf_access_jwt.
  • The shared ASGI harness moved into tests/api_harness.py + a conftest api fixture.

Public copy (content-audit fixes)

  • /quiz and /tafel no longer show raw English exception text as the boot-error detail — a fixed German sentence renders instead.
  • /schriftkunde intro switched to du-form; typographic German quotes everywhere; untrue feature claims removed (worksheet "Wörter zum Nachfahren", Tafel overclaim, "Kurrent-Buchstaben" in the Sütterlin-only quiz); "Super Übereinstimmung" → "Richtig gelesen."; terminology unified (Übungsblatt, Ausgangsschrift, freigegeben); fact fixes against docs/schriftkunde/; static <title>/meta aligned with the SEO catalogue, ampersands escaped, twitter:description updated.

Accessibility package (audit Paket 3)

  • WCAG AA contrast: new paper.viridianText (#2e6152, derived for contrast — 5.15:1 on the paper ground vs 3.28:1 for the accent) for all body-size viridian text; quiz.resolvedText darkened to #6e5c42 (5.5:1). The accent #40826d stays for large display, initials, borders, fills, focus rings.
  • Contiguous heading outline: explicit heading components on card titles (hub h2, landing/Schriftkunde/Tafel h3) and a theme-level subtitle→<p> variantMapping (no more phantom <h6> sections).
  • Nav current-area state: aria-current + visible active underline, covering the standalone tool routes via an AREA_ROUTES map (/quiz, /tafel → Lesen; /federprobe → Schreiben).
  • PaperCardLink/PaperCardCta replaces the card styling copy-pasted across LandingView, HubView and the Schriftkunde try-cards; jsx-a11y (recommended) wired into the lint gate.

Slim reads (audit Paket 4)

  • TemplateRepository.list_summaries(): the template list selects only the six summary columns instead of materialising full rows with multi-MB raw_path/anchors/trace_meta JSONB per sidebar load.
  • GET /sources/{id}/bboxes/status (flags only) + a new useQuizSource hook: /quiz left the pinned AdminProvider and stops downloading every bbox's mask/ink/patch payload just to read the locked/split gating flags.
  • fetchRenderWord error eviction now guards entry identity like the glyph cache (a late rejection could evict a fresh entry after FIFO reuse).
  • CHANGELOG [Unreleased] regrouped to one heading per Keep-a-Changelog category.

Verification

  • uv run --extra test pytest: 352 passed, ruff check + format clean.
  • npm run lint (0 errors incl. jsx-a11y) · npm run test (35 passed) · npm run build green.
  • Browser smoke (Chromium against the dev server): heading levels contiguous on landing//lesen//schriftkunde, exactly one aria-current nav link, darkened viridian CTA text rendered, card CTA underline spans present; the copy smoke (fixed German error detail on /tafel + /quiz with the API down) still holds.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE

…public copy fixes

Security/data integrity:
- POST /trace now validates the URL glyph_key against the payload's
  (glyph, position) via the shared registry (core.shaping.expected_glyph_key)
  and rejects mismatches with 422 — a mismatched pair could conflict-update
  another template row and rewrite its glyph_key (silent 404s on prod data)
- DB engine init lock is created at import; the lazy check-then-set lock
  getter could double-initialise the engine and leak a connector

Tests:
- tests/test_api_admin_writes.py: authorized bbox PUT/GET roundtrip incl.
  coalesce semantics, full /trace pipeline against the synthetic chart,
  423 lock + force, key-identity backstop cases, DELETE semantics
- tests/test_api_auth.py: the Cloudflare Access JWT branch (listed email,
  unlisted 403 without token fallback, garbage-JWT break-glass path) plus
  _verify_cf_access_jwt unit tests
- shared ASGI harness extracted to tests/api_harness.py + conftest fixture

Public copy (content audit):
- fixed German error detail on /quiz and /tafel instead of raw String(e)
- du-form intro on /schriftkunde, typographic German quotes everywhere
- removed untrue worksheet feature claim, honest Tafel description,
  quiz card no longer claims Kurrent letters, terminology unified
  (Übungsblatt, Ausgangsschrift, freigegeben), fact fixes per
  docs/schriftkunde (Sütterlin 1915/~1930, Swiss 1890–1930, ß note,
  1915/1918 caveat), grammar fixes, static title/description in index.html

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE
Copilot AI review requested due to automatic review settings July 15, 2026 12:06
@codecov-commenter

codecov-commenter commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
api/routers/templates.py 85.71% 2 Missing ⚠️
core/database/connection.py 33.33% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a second-round audit pack focused on hardening admin write integrity/auth paths (notably /trace identity safety and DB init locking), adds HTTP-level test coverage for those prod-critical branches, and applies a broad set of public German copy/SEO corrections across the SPA.

Changes:

  • Add an identity backstop for POST /trace by deriving the canonical glyph_key from the shared shaping registry and rejecting mismatches with 422.
  • Close a DB engine initialization race by creating the async init lock at import time, preventing double-init leaks under concurrent first requests.
  • Add/reshape API test suites (admin writes + Cloudflare Access JWT) and update multiple public locale/SEO strings + HTML metadata to match the content audit.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_api_http.py Refactors to use the shared ASGI/Harness stack and narrows this suite to the “reject/fail-closed + public reads + write read-path” coverage.
tests/api_harness.py Introduces shared in-process ASGI client + DB seeding harness used across API HTTP test suites.
tests/conftest.py Adds a shared api fixture wiring the FastAPI app to in-memory SQLite for HTTP-level tests.
tests/test_api_admin_writes.py Adds authorized (correct-token) admin write-path coverage: bbox PUT/GET semantics, /trace persistence, lock/force behavior, and DELETE semantics.
tests/test_api_auth.py Adds Cloudflare Access JWT gate tests (HTTP-level via monkeypatch + unit-level JWKS/decode plumbing).
core/shaping.py Adds expected_glyph_key() helper to derive canonical keys from the Python registry.
api/routers/templates.py Enforces URL-key vs payload-identity consistency for /trace to prevent cross-row template updates.
core/database/connection.py Fixes engine init race by replacing a lazy lock getter with an import-time asyncio.Lock().
CHANGELOG.md Adds detailed [Unreleased] entries documenting the security fixes, new test suites, and public copy changes.
app/src/sections/tafel/useGrundtafeln.ts Replaces raw exception text shown to users with fixed German copy (logs the exception to console).
app/src/context/AdminContext.tsx Same user-facing error-detail fix for boot failures affecting public routes (e.g. quiz boot).
app/src/locales/de/common.ts Adds the fixed German “source unreachable” detail string used by public pages.
app/src/locales/de/{landing,hub,quiz,schriftkunde,worksheet,seo}.ts Applies content-audit fixes: terminology, quotes, factual corrections, and clarified feature claims.
app/index.html Aligns static <title> and <meta description> (and OG description) with the SEO catalogue for no-JS crawlers.

Comment thread tests/api_harness.py
…hart

Copilot review: seed_style_and_source() hard-coded chart_size to 100x100
even for the 800x800 synthetic chart, making GET /sources return wrong
metadata. The harness now reads the actual pixel size from disk (keeping
the placeholder only for the deliberately-nonexistent default path) and
accepts an explicit chart_size override.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE
Copilot AI review requested due to automatic review settings July 15, 2026 12:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Comment thread api/routers/templates.py
Copilot review round 2: the bare position-suffix fallback still let an
unknown glyph claim a registry-owned key (glyph="X" posting to
n-medial passes the suffix check, then the upsert stamps n-medial onto
its own row — duplicating the key against the real n row). Two new
layers:

- is_registry_glyph_key(): a registry-owned key may never be claimed by
  an out-of-registry glyph (422)
- stored-row backstop: whatever row (style, glyph_key, variant) already
  names must match the payload's (glyph, position), else 409 — covers
  collisions between two custom glyphs sharing a key

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE
Copilot AI review requested due to automatic review settings July 15, 2026 12:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Comment thread app/index.html Outdated
Comment thread app/index.html Outdated
Copilot review round 3: the static <title>/og:title/twitter:title carried
a bare & (valid HTML5 here, but escaped for validators/crawlers), and
twitter:description still had the pre-audit copy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE
Copilot AI review requested due to automatic review settings July 15, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

…, shared card link, jsx-a11y gate

- paper.viridianText (#2e6152, derived for contrast) for all body-size
  viridian text (card CTAs, quiz score/verdict, scribe confirmation,
  tafel chip/links, prose-link hovers); quiz.resolvedText darkened to
  #6e5c42 — both clear WCAG AA where the old values sat at 3.3-4.0:1
- contiguous heading outline: explicit heading components on card
  titles (hub h2, landing/schriftkunde/tafel h3) and a theme-level
  subtitle->p variantMapping so definition terms and timeline years
  stop rendering as phantom h6 sections
- PublicHeader marks the current area (aria-current=page + ink color
  + full underline)
- new PaperCardLink/PaperCardCta shared component replaces the card
  styling copy-pasted across LandingView, HubView and the Schriftkunde
  try-cards
- eslint-plugin-jsx-a11y (recommended) wired into the lint gate

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE
Copilot AI review requested due to automatic review settings July 15, 2026 16:10
@MarkusNeusinger MarkusNeusinger changed the title Audit round 2: /trace identity guard, admin-write + CF-Access tests, public copy fixes Audit rounds 2+3: /trace identity guard, admin-write + CF-Access tests, public copy fixes, accessibility package Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 36 out of 37 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • app/package-lock.json: Generated file

Comment thread CHANGELOG.md Outdated
Comment thread app/src/components/PublicHeader/PublicHeader.tsx
…dmin context, nav area coverage, changelog regroup

- TemplateRepository.list_summaries(): the template list selects only the
  six summary columns instead of materialising full rows with raw_path/
  anchors/trace_meta JSONB
- new GET /sources/{id}/bboxes/status (flags only) + useQuizSource hook:
  /quiz no longer mounts the AdminProvider and stops downloading every
  bbox's mask/ink/patch payload just to read locked/split
- fetchRenderWord error eviction guards entry identity like the glyph
  cache (late rejection could evict a fresh entry after FIFO reuse)
- Copilot round 4: nav current-area detection now covers the standalone
  tool routes (/quiz,/tafel -> Lesen; /federprobe -> Schreiben) with
  aria-current page/true split; CHANGELOG [Unreleased] regrouped to one
  heading per Keep-a-Changelog category (bullets were misfiled by
  successive top-insertions)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RjxGFmMybaLQ4e88qg6jE
Copilot AI review requested due to automatic review settings July 15, 2026 16:23
@MarkusNeusinger MarkusNeusinger changed the title Audit rounds 2+3: /trace identity guard, admin-write + CF-Access tests, public copy fixes, accessibility package Audit packages 1–4: /trace identity guard, admin-write + CF-Access tests, public copy fixes, accessibility, slim reads Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 47 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • app/package-lock.json: Generated file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants