Draft stories never appear on the Geschichten overview — blog writers can't see their own drafts #807

New Issue

Closed

opened 2026-06-12 07:37:07 +02:00 by marcel · 7 comments

marcel commented 2026-06-12 07:37:07 +02:00

Owner

Copy Link

User-facing impact

A user with BLOG_WRITE creates a story. It is saved as a DRAFT. When they open the stories overview at /geschichten, only PUBLISHED stories are listed — their own drafts are nowhere to be found. There is no way to get back to an unfinished draft from the overview; the only path is to remember/bookmark the direct /geschichten/{id} URL.

Root cause

The overview's server load hardcodes status: 'PUBLISHED' in the API query and never requests drafts. The backend is perfectly willing to return a blog writer's own drafts — the overview simply never asks for them, and the page has no UI to surface them.

frontend/src/routes/geschichten/+page.server.ts:14-23

api.GET('/api/geschichten', {
    params: {
        query: {
            status: 'PUBLISHED',   // ← hardcoded; drafts are never requested
            personId: personIds.length ? personIds : undefined,
            documentId
        }
    }
})

The backend already supports this correctly. In GeschichteService.list() (backend/.../geschichte/GeschichteService.java:117-135):

GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED;
...
UUID authorId = effective == GeschichteStatus.DRAFT ? currentUser().getId() : null;

So a blog writer calling GET /api/geschichten?status=DRAFT would get back their own drafts (the authorId filter scopes drafts to the current user — no leaking of other authors' drafts). The capability exists end-to-end; the overview page just doesn't use it.

Why the existing "drafts module" doesn't cover this

The home page (frontend/src/routes/+page.server.ts:47-50) does fetch drafts and render a drafts module — but only on the reader home, gated by isReader = !canWrite && !canAnnotate. That means:

• A blog writer who also has WRITE_ALL or ANNOTATE_ALL lands on the non-reader home and sees no drafts module at all.
• Even reader-only blog writers only get a capped (limit: 10) teaser on the home page, never on the dedicated /geschichten overview where they'd naturally look for their stories.

So drafts are effectively invisible from the page whose entire job is listing stories.

Security concern — CWE-639 (Broken Access Control)

When a blog writer calls GET /api/geschichten without a status parameter, the current code at line 118 of GeschichteService.java evaluates currentUserHasBlogWrite() ? null : GeschichteStatus.PUBLISHED. The null passes through to hasStatus(null) which adds no predicate, and authorId stays null because effective == GeschichteStatus.DRAFT is false for null. The query returns all stories from all authors, including other authors' DRAFT stories. Severity: Medium (CVSS ~5.3, AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N). Requires authentication; impact is confidentiality — draft content and existence disclosed to authenticated users.

The existing test list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible (line 235 in GeschichteServiceTest.java) actively asserts the vulnerable behavior. It passes null status and asserts out.hasSize(2) without verifying what status was passed to the repository — it would pass against both the vulnerable code and the fixed code. Renaming and rewriting this test before the fix lands is mandatory; a loose any() on the status is not sufficient for a security regression test. Also, list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE (line 224) should be strengthened to use eq(GeschichteStatus.PUBLISHED) as the first arg to findSummaries, not any().

GeschichteQueryService is not vulnerable — it exposes existsById and findById only; no list() equivalent, no status filtering. No fix needed there.

getById() is not vulnerable — lines 73-77 correctly throw NOT_FOUND (not FORBIDDEN) for drafts when the user lacks BLOG_WRITE. The findSummaries JPQL uses a typed GeschichteStatus parameter (not string concatenation), so no SQL injection risk.

Secondary observations

1. No visual draft affordance. geschichten/+page.svelte renders every card identically and only shows a date via publishedAt(g) (line 48-51), which is null for drafts. If drafts are added to the list they'll need a "Entwurf" badge so writers can distinguish them from published stories. No such i18n key / badge exists yet.
2. Person/document filters + drafts. The "Entwürfe" section is always shown un-filtered (all of the writer's own drafts, regardless of active filter pills). There won't be many open drafts, and hiding a draft because it isn't yet tagged with the filtered person would recreate the exact "where did my draft go?" problem. Mitigate the visual inconsistency with a small (alle Entwürfe) caption on the section heading (see i18n keys below).
3. Backend null-status footgun closed. Fix GeschichteService.list() — treat "blog writer + non-DRAFT status" as PUBLISHED-only: effective = (currentUserHasBlogWrite() && status == DRAFT) ? DRAFT : PUBLISHED, so drafts require an explicit status=DRAFT request (which already author-scopes). The frontend sending explicit status: 'DRAFT' remains as defense-in-depth layer 2. Do not gate the fix behind canBlogWrite on the frontend alone — the backend fix is the control.

Decisions (resolved)

Draft row link target — link to /geschichten/{id} (detail page)

Clicking a draft row navigates to the story detail/read view, the same as published stories. The writer then clicks an Edit CTA from there to resume editing. No conditional href needed in GeschichteListRow.

Section heading copy — "Entwürfe"

Use geschichten_drafts_heading = "Entwürfe" / "Drafts" / "Borradores". The unfiltered scope is communicated via the (alle Entwürfe) caption (see i18n keys below). "Meine Entwürfe" was considered but rejected — "Entwürfe" + caption is sufficient at current scale.

Draft badge color tokens — reuse bg-journey-tint border border-journey-border text-journey

Do not use raw Tailwind amber utilities (bg-amber-50 border-amber-200 text-amber-700). layout.css already has --color-journey-tint, --color-journey-border, and related amber tokens (including dark-mode overrides). Reuse bg-journey-tint border border-journey-border text-journey — the same tokens used by the JOURNEY badge in GeschichteListRow.svelte (lines 41-46). This is consistent, dark-mode safe, and does not bypass the project token system. WCAG 1.4.1 is satisfied by the literal word "Entwurf" in the badge text regardless of color. Contrast of text-journey on bg-journey-tint is to be verified at implementation time — if it fails 4.5:1 for 12px/700 text, use a darker token. The JOURNEY badge has been in production since the JOURNEY feature shipped; the draft badge inherits the same contrast level as the established bar.

settled<T>() helper location — extract to shared module

The settled<T>() helper in routes/+page.server.ts (lines 17-21) must be extracted to $lib/shared/server/settled.ts (next to locale.ts and permissions.ts) as a preparatory commit before implementing the drafts fetch. Do not duplicate it inline in routes/geschichten/+page.server.ts — two diverging copies is unacceptable. After extraction, both routes/+page.server.ts and routes/geschichten/+page.server.ts import from the shared location. Verify that any import change does not break the existing test imports.

Heading hierarchy — add a "Veröffentlicht" heading gated on drafts.length > 0

When the "Entwürfe" section is visible, both sections must have an <h2> to keep the heading outline balanced for screen reader navigation. Add geschichten_published_heading = "Veröffentlicht" / "Published" / "Publicadas" (3 i18n keys). Gate both headings on drafts.length > 0 — the "Veröffentlicht" heading is only added when the "Entwürfe" heading is present; without drafts, the page uses no <h2> for the single list (unchanged baseline behavior).

Proposed fix

Surface the current blog writer's own drafts on /geschichten. Implementation order:

1. Backend guard (TDD first)

First: rename and rewrite the existing null-status test (before touching list()). Rename list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible to list_with_null_status_and_BLOG_WRITE_returns_PUBLISHED_not_all_stories. The rewrite must use verify(geschichteRepository).findSummaries(eq(GeschichteStatus.PUBLISHED), isNull(), any(), anyLong(), any()) — this makes the existing false-safety-net test into a real regression fixture. Also strengthen list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE to use eq(GeschichteStatus.PUBLISHED) not any() as the first argument.

Then write two new failing JUnit tests:

@Test
@DisplayName("security: null status for blog writer returns PUBLISHED, never leaks drafts")
void list_with_blog_writer_and_null_status_returns_PUBLISHED_not_all_drafts() {
    authenticateAs(writer, Permission.BLOG_WRITE);
    geschichteService.list(null, List.of(), null, 50);
    verify(geschichteRepository).findSummaries(
        eq(GeschichteStatus.PUBLISHED), isNull(), any(), anyLong(), any());
}

@Test
@DisplayName("security: DRAFT status scopes to current user only")
void list_with_DRAFT_status_scopes_to_current_user_not_all_authors() {
    authenticateAs(writer, Permission.BLOG_WRITE);
    geschichteService.list(GeschichteStatus.DRAFT, List.of(), null, 50);
    verify(geschichteRepository).findSummaries(
        eq(GeschichteStatus.DRAFT), eq(writer.getId()), any(), anyLong(), any());
}

These tests must be red against the current code. Then tighten list():

GeschichteStatus effective = (currentUserHasBlogWrite() && status == GeschichteStatus.DRAFT)
    ? GeschichteStatus.DRAFT
    : GeschichteStatus.PUBLISHED;

Add a Javadoc comment to list() explicitly stating: null status for a blog writer resolves to PUBLISHED; DRAFT resolves to the writer's own drafts only. Minimum content: "null status for a blog writer resolves to PUBLISHED, never to all-stories. DRAFT status scopes the query to the current user's own drafts only." This makes the security rule auditable by reading the method signature alone.

2. Extract settled<T>() to shared module (preparatory commit)

Move the settled<T>() helper from routes/+page.server.ts:17-21 to $lib/shared/server/settled.ts and export it. Update the import in routes/+page.server.ts. Keep it a pure function — no side effects, no logging. Verify all existing tests for routes/+page.server.ts still pass after the import change.

3. Frontend +page.server.ts

• Add const { canBlogWrite } = await parent() (the current load function takes ({ url, fetch }) without parent — change the signature to ({ url, fetch, parent })).
• When canBlogWrite, push a second GET /api/geschichten?status=DRAFT request using Promise.allSettled + the shared settled<T>() helper — not Promise.all — so a drafts-fetch failure degrades to drafts: [] without 500-ing the whole overview. Mirror the exact pattern from routes/+page.server.ts:47-63.
• Return { geschichten, drafts, personFilters, documentIdFilter }.

Test coverage for +page.server.ts: The callLoad helper must provide parent. Update the helper first (standalone step, green commit) so existing tests remain green:

function callLoad(url: URL, opts: { canBlogWrite?: boolean } = {}) {
    return load({
        url,
        request: new Request('http://localhost/geschichten'),
        fetch: vi.fn() as unknown as typeof fetch,
        parent: vi.fn().mockResolvedValue({ canBlogWrite: opts.canBlogWrite ?? false, canWrite: false, canAnnotate: false, user: null })
    });
}

All 13 existing tests in page.server.test.ts use callLoad(url) with no opts — adding canBlogWrite: false as the default keeps them green unchanged.

New server test cases needed:

• canBlogWrite=true → mockGet called twice (once PUBLISHED, once DRAFT); assert expect(mockGet).toHaveBeenCalledTimes(2) and verify the second call has status: 'DRAFT'.
• canBlogWrite=false → no second request.
• DRAFT fetch rejects (network-level failure, not just 4xx) → drafts: [], PUBLISHED list unaffected. Mock pattern:

  mockGet.mockImplementation((path: string, opts: unknown) => {
    if (path === '/api/geschichten' && opts?.params?.query?.status === 'DRAFT') {
      return Promise.reject(new TypeError('Failed to fetch'));
    }
    return Promise.resolve({ response: { ok: true, status: 200 }, data: [] });
  });
  

  This proves Promise.allSettled catches network-level failures, not just non-ok responses.

4. Frontend +page.svelte

Render a distinct "Entwürfe" section above the published list, only when data.drafts.length > 0. Gate strictly on drafts.length > 0, not canBlogWrite — no empty-state noise for writers with zero drafts.

Place the Entwürfe section and published list in the same card (<div class="overflow-hidden rounded-sm border border-line bg-surface shadow-sm">), separated by a border-b-2 border-line divider before the published list heading.

When data.drafts.length > 0, render an <h2> for the drafts section and a matching <h2> for the published list (gated on the same condition), to keep the heading outline balanced:

{#if data.drafts.length > 0}
  <!-- Drafts section -->
  <h2 class="text-xs font-bold uppercase tracking-widest text-ink-3">
    {m.geschichten_drafts_heading()}
    <span class="ml-1 normal-case font-normal text-ink-3">{m.geschichten_drafts_unfiltered_caption()}</span>
  </h2>
  <ul>
    {#each data.drafts as g (g.id)}
      <li class="border-b border-line-2 last:border-b-0">
        <GeschichteListRow geschichte={g} />
      </li>
    {/each}
  </ul>

  <!-- Divider before published list -->
  <div class="border-b-2 border-line"></div>

  <!-- Published list heading (only when drafts section is present) -->
  <h2 class="text-xs font-bold uppercase tracking-widest text-ink-3">
    {m.geschichten_published_heading()}
  </h2>
{/if}

Use a keyed {#each data.drafts as g (g.id)} (matching the existing published list pattern).

The existing $derived for emptyMessage considers only personFilters and documentFilter — do not change it. It correctly describes the state of the filtered published list, not the drafts section.

Browser test coverage needed: Add drafts: [] to the makeData factory as the very first change, before touching the svelte page, to keep all existing browser tests green:

function makeData(overrides: Partial<PageData> = {}): PageData {
    return {
        geschichten: [],
        personFilters: [],
        documentFilter: null,
        canBlogWrite: false,
        drafts: [],           // ← add this
        ...overrides
    } as unknown as PageData;
}

Then add:

• Section renders when drafts.length > 0.
• Section absent when drafts: [].
• Draft badge visible on each row.
• "Entwürfe" heading text present.
• Section absent for reader without canBlogWrite.
• "Veröffentlicht" heading visible when drafts section is present — tests the gated heading.
• Badge absent on a published row (draft-badge not present when status: 'PUBLISHED') — prevents a regression that always shows the badge.

5. GeschichteListRow.svelte — draft badge

• Add 'status' to the Pick type (current: 'id' | 'title' | 'body' | 'type' | 'author' | 'publishedAt'). GeschichteSummary.status is already @Schema(requiredMode = REQUIRED) on the backend, so non-optional in generated types — no ? guard needed.
• Add an "Entwurf" badge mirroring the JOURNEY badge pattern (lines 39-46 desktop, 69-74 mobile):
  • data-testid="draft-badge" (desktop), data-testid="draft-badge-mobile" (mobile)
  • Use project tokens: bg-journey-tint border border-journey-border text-journey (dark-mode safe; matches the JOURNEY badge tokens already in use)
  • Same pill shape as JOURNEY badge: rounded-sm px-1.5 py-px font-sans text-xs font-bold tracking-wide uppercase
  • Add shrink-0 on the mobile badge span (matching the JOURNEY mobile badge at line 72) to prevent collapsing the title on narrow screens
  • Add flex-wrap to the mobile div.mb-1 flex row to prevent badge overflow at 320px when both JOURNEY and DRAFT badges are present simultaneously
  • Badge text: {m.geschichten_draft_badge()} (non-caps form "Entwurf", not "ENTWURF" — do not reuse existing geschichte_editor_status_draft key which renders "ENTWURF")
  • Badge is visual only — not interactive, no touch target concern
  • publishedAt is null for drafts; the existing {#if publishedAt} gate handles this correctly — no change needed
  • A story can be both a JOURNEY and a DRAFT simultaneously; both badges will render stacked in the desktop meta column (fine — flex-col layout handles it) and side-by-side on mobile (fine with shrink-0 on both)
• Place the draft badge in the same DOM slot as the journey badge — both desktop (meta column, after journey badge) and mobile (content column, same div.mb-1 flex row). Use two separate {#if} blocks, not one combined condition, for readability.

6. i18n keys (messages/{de,en,es}.json)

Key	de	en	es
geschichten_drafts_heading	Entwürfe	Drafts	Borradores
geschichten_draft_badge	Entwurf	Draft	Borrador
geschichten_drafts_unfiltered_caption	(alle Entwürfe)	(all drafts)	(todos los borradores)
geschichten_published_heading	Veröffentlicht	Published	Publicadas

Note: geschichte_editor_status_draft ("ENTWURF") already exists — do not reuse it; the list badge uses the non-caps form "Entwurf".

7. Regenerate API types

Run npm run generate:api in frontend/. Expected: no diff (no new backend fields or endpoints were introduced; GeschichteSummary.status was already REQUIRED). If a diff appears, fix the missing annotation before merging.

Acceptance criteria

• A blog writer who has saved a draft sees that draft on /geschichten, visually marked with an "Entwurf" badge, in a section labeled "Entwürfe" above the published list.
• A blog writer sees their own drafts only — not other authors' drafts (privacy: a writer shall never see another writer's unpublished draft). The author.id of every item in the drafts section equals the logged-in user's id.
• The "Entwürfe" section is shown un-filtered — active person/document filter pills do not affect it. The (alle Entwürfe) caption signals this to the user.
• A blog writer with zero drafts sees no "Entwürfe" section — the heading and section are not present in the DOM, regardless of canBlogWrite.
• A reader without BLOG_WRITE sees no drafts and no draft section (unchanged behaviour).
• Published stories continue to list and filter exactly as before.
• If the drafts request fails, the published list still renders and no error is shown to the user (graceful degradation to drafts: [], section absent).
• All text is i18n-complete (de/en/es).
• GeschichteService.list() with a blog writer + null/omitted status returns only PUBLISHED stories (backend guard, covered by two new JUnit tests with @DisplayName referencing the security nature).
• When the "Entwürfe" section is visible, a "Veröffentlicht" <h2> heading is also present for the published list (heading outline is balanced for screen reader navigation).
• Non-goal: an editor/admin seeing all authors' drafts for review — out of scope, separate issue if ever needed.

Files involved

• backend/.../geschichte/GeschichteService.java — null-status guard + Javadoc
• backend/.../geschichte/GeschichteServiceTest.java — rename/rewrite existing false-safety-net null-status test; strengthen list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE; 2 new security-regression tests with @DisplayName
• $lib/shared/server/settled.ts (new) — extracted settled<T>() helper (preparatory commit)
• frontend/src/routes/+page.server.ts — update import for extracted settled<T>() helper
• frontend/src/routes/geschichten/+page.server.ts — await parent(), second DRAFT request via Promise.allSettled + shared settled<T>(), resilient fetch
• frontend/src/routes/geschichten/page.server.test.ts — parent mock in callLoad (standalone commit first), new test cases
• frontend/src/routes/geschichten/+page.svelte — "Entwürfe" section + gated "Veröffentlicht" heading above published list
• frontend/src/routes/geschichten/page.svelte.spec.ts — add drafts: [] to makeData factory first; then new browser tests for drafts section + badge + badge-absent-on-published-row + Veröffentlicht heading
• frontend/src/lib/geschichte/GeschichteListRow.svelte — add status to Pick, add draft badge (desktop + mobile slots, shrink-0 on mobile, flex-wrap on mobile row)
• frontend/messages/{de,en,es}.json — 4 new i18n keys

## User-facing impact A user with `BLOG_WRITE` creates a story. It is saved as a `DRAFT`. When they open the stories overview at `/geschichten`, **only PUBLISHED stories are listed** — their own drafts are nowhere to be found. There is no way to get back to an unfinished draft from the overview; the only path is to remember/bookmark the direct `/geschichten/{id}` URL. ## Root cause The overview's server load **hardcodes `status: 'PUBLISHED'`** in the API query and never requests drafts. The backend is perfectly willing to return a blog writer's own drafts — the overview simply never asks for them, and the page has no UI to surface them. **`frontend/src/routes/geschichten/+page.server.ts:14-23`** ```ts api.GET('/api/geschichten', { params: { query: { status: 'PUBLISHED', // ← hardcoded; drafts are never requested personId: personIds.length ? personIds : undefined, documentId } } }) ``` The backend already supports this correctly. In `GeschichteService.list()` (`backend/.../geschichte/GeschichteService.java:117-135`): ```java GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED; ... UUID authorId = effective == GeschichteStatus.DRAFT ? currentUser().getId() : null; ``` So a blog writer calling `GET /api/geschichten?status=DRAFT` would get back **their own** drafts (the `authorId` filter scopes drafts to the current user — no leaking of other authors' drafts). The capability exists end-to-end; the overview page just doesn't use it. ## Why the existing "drafts module" doesn't cover this The home page (`frontend/src/routes/+page.server.ts:47-50`) **does** fetch drafts and render a drafts module — but only on the **reader** home, gated by `isReader = !canWrite && !canAnnotate`. That means: - A blog writer who **also** has `WRITE_ALL` or `ANNOTATE_ALL` lands on the non-reader home and sees **no** drafts module at all. - Even reader-only blog writers only get a capped (`limit: 10`) teaser on the home page, never on the dedicated `/geschichten` overview where they'd naturally look for their stories. So drafts are effectively invisible from the page whose entire job is listing stories. ## Security concern — CWE-639 (Broken Access Control) When a blog writer calls `GET /api/geschichten` without a `status` parameter, the current code at line 118 of `GeschichteService.java` evaluates `currentUserHasBlogWrite() ? null : GeschichteStatus.PUBLISHED`. The `null` passes through to `hasStatus(null)` which adds no predicate, and `authorId` stays `null` because `effective == GeschichteStatus.DRAFT` is false for `null`. The query returns all stories from all authors, including other authors' `DRAFT` stories. **Severity: Medium (CVSS ~5.3, AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N).** Requires authentication; impact is confidentiality — draft content and existence disclosed to authenticated users. **The existing test `list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible` (line 235 in `GeschichteServiceTest.java`) actively asserts the vulnerable behavior.** It passes `null` status and asserts `out.hasSize(2)` without verifying what status was passed to the repository — it would pass against both the vulnerable code and the fixed code. Renaming and rewriting this test before the fix lands is mandatory; a loose `any()` on the status is not sufficient for a security regression test. Also, `list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE` (line 224) should be strengthened to use `eq(GeschichteStatus.PUBLISHED)` as the first arg to `findSummaries`, not `any()`. **`GeschichteQueryService` is not vulnerable** — it exposes `existsById` and `findById` only; no `list()` equivalent, no status filtering. No fix needed there. **`getById()` is not vulnerable** — lines 73-77 correctly throw NOT_FOUND (not FORBIDDEN) for drafts when the user lacks `BLOG_WRITE`. The `findSummaries` JPQL uses a typed `GeschichteStatus` parameter (not string concatenation), so no SQL injection risk. ## Secondary observations 1. **No visual draft affordance.** `geschichten/+page.svelte` renders every card identically and only shows a date via `publishedAt(g)` (line 48-51), which is `null` for drafts. If drafts are added to the list they'll need a \"Entwurf\" badge so writers can distinguish them from published stories. No such i18n key / badge exists yet. 2. **Person/document filters + drafts.** The \"Entwürfe\" section is always shown **un-filtered** (all of the writer's own drafts, regardless of active filter pills). There won't be many open drafts, and hiding a draft because it isn't yet tagged with the filtered person would recreate the exact \"where did my draft go?\" problem. Mitigate the visual inconsistency with a small `(alle Entwürfe)` caption on the section heading (see i18n keys below). 3. **Backend null-status footgun closed.** Fix `GeschichteService.list()` — treat \"blog writer + non-`DRAFT` status\" as PUBLISHED-only: `effective = (currentUserHasBlogWrite() && status == DRAFT) ? DRAFT : PUBLISHED`, so drafts require an explicit `status=DRAFT` request (which already author-scopes). The frontend sending explicit `status: 'DRAFT'` remains as defense-in-depth layer 2. **Do not gate the fix behind `canBlogWrite` on the frontend alone — the backend fix is the control.** ## Decisions (resolved) ### Draft row link target — **link to `/geschichten/{id}` (detail page)** Clicking a draft row navigates to the story detail/read view, the same as published stories. The writer then clicks an Edit CTA from there to resume editing. No conditional `href` needed in `GeschichteListRow`. ### Section heading copy — **\"Entwürfe\"** Use `geschichten_drafts_heading` = \"Entwürfe\" / \"Drafts\" / \"Borradores\". The unfiltered scope is communicated via the `(alle Entwürfe)` caption (see i18n keys below). \"Meine Entwürfe\" was considered but rejected — \"Entwürfe\" + caption is sufficient at current scale. ### Draft badge color tokens — **reuse `bg-journey-tint border border-journey-border text-journey`** Do not use raw Tailwind amber utilities (`bg-amber-50 border-amber-200 text-amber-700`). `layout.css` already has `--color-journey-tint`, `--color-journey-border`, and related amber tokens (including dark-mode overrides). Reuse `bg-journey-tint border border-journey-border text-journey` — the same tokens used by the JOURNEY badge in `GeschichteListRow.svelte` (lines 41-46). This is consistent, dark-mode safe, and does not bypass the project token system. WCAG 1.4.1 is satisfied by the literal word \"Entwurf\" in the badge text regardless of color. **Contrast of `text-journey` on `bg-journey-tint` is to be verified at implementation time** — if it fails 4.5:1 for 12px/700 text, use a darker token. The JOURNEY badge has been in production since the JOURNEY feature shipped; the draft badge inherits the same contrast level as the established bar. ### `settled<T>()` helper location — **extract to shared module** The `settled<T>()` helper in `routes/+page.server.ts` (lines 17-21) must be extracted to `$lib/shared/server/settled.ts` (next to `locale.ts` and `permissions.ts`) as a **preparatory commit** before implementing the drafts fetch. Do not duplicate it inline in `routes/geschichten/+page.server.ts` — two diverging copies is unacceptable. After extraction, both `routes/+page.server.ts` and `routes/geschichten/+page.server.ts` import from the shared location. Verify that any import change does not break the existing test imports. ### Heading hierarchy — **add a \"Veröffentlicht\" heading gated on `drafts.length > 0`** When the \"Entwürfe\" section is visible, both sections must have an `<h2>` to keep the heading outline balanced for screen reader navigation. Add `geschichten_published_heading` = \"Veröffentlicht\" / \"Published\" / \"Publicadas\" (3 i18n keys). Gate both headings on `drafts.length > 0` — the \"Veröffentlicht\" heading is only added when the \"Entwürfe\" heading is present; without drafts, the page uses no `<h2>` for the single list (unchanged baseline behavior). ## Proposed fix Surface the current blog writer's own drafts on `/geschichten`. Implementation order: ### 1. Backend guard (TDD first) **First: rename and rewrite the existing null-status test** (before touching `list()`). Rename `list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible` to `list_with_null_status_and_BLOG_WRITE_returns_PUBLISHED_not_all_stories`. The rewrite must use `verify(geschichteRepository).findSummaries(eq(GeschichteStatus.PUBLISHED), isNull(), any(), anyLong(), any())` — this makes the existing false-safety-net test into a real regression fixture. Also strengthen `list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE` to use `eq(GeschichteStatus.PUBLISHED)` not `any()` as the first argument. Then write two new **failing** JUnit tests: ```java @Test @DisplayName("security: null status for blog writer returns PUBLISHED, never leaks drafts") void list_with_blog_writer_and_null_status_returns_PUBLISHED_not_all_drafts() { authenticateAs(writer, Permission.BLOG_WRITE); geschichteService.list(null, List.of(), null, 50); verify(geschichteRepository).findSummaries( eq(GeschichteStatus.PUBLISHED), isNull(), any(), anyLong(), any()); } @Test @DisplayName("security: DRAFT status scopes to current user only") void list_with_DRAFT_status_scopes_to_current_user_not_all_authors() { authenticateAs(writer, Permission.BLOG_WRITE); geschichteService.list(GeschichteStatus.DRAFT, List.of(), null, 50); verify(geschichteRepository).findSummaries( eq(GeschichteStatus.DRAFT), eq(writer.getId()), any(), anyLong(), any()); } ``` These tests must be red against the current code. Then tighten `list()`: ```java GeschichteStatus effective = (currentUserHasBlogWrite() && status == GeschichteStatus.DRAFT) ? GeschichteStatus.DRAFT : GeschichteStatus.PUBLISHED; ``` Add a Javadoc comment to `list()` explicitly stating: null status for a blog writer resolves to PUBLISHED; DRAFT resolves to the writer's own drafts only. Minimum content: "null status for a blog writer resolves to PUBLISHED, never to all-stories. DRAFT status scopes the query to the current user's own drafts only." This makes the security rule auditable by reading the method signature alone. ### 2. Extract `settled<T>()` to shared module (preparatory commit) Move the `settled<T>()` helper from `routes/+page.server.ts:17-21` to `$lib/shared/server/settled.ts` and export it. Update the import in `routes/+page.server.ts`. Keep it a pure function — no side effects, no logging. Verify all existing tests for `routes/+page.server.ts` still pass after the import change. ### 3. Frontend `+page.server.ts` - Add `const { canBlogWrite } = await parent()` (the current load function takes `({ url, fetch })` without `parent` — change the signature to `({ url, fetch, parent })`). - When `canBlogWrite`, push a second `GET /api/geschichten?status=DRAFT` request using `Promise.allSettled` + the shared `settled<T>()` helper — **not** `Promise.all` — so a drafts-fetch failure degrades to `drafts: []` without 500-ing the whole overview. Mirror the exact pattern from `routes/+page.server.ts:47-63`. - Return `{ geschichten, drafts, personFilters, documentIdFilter }`. **Test coverage for `+page.server.ts`:** The `callLoad` helper must provide `parent`. **Update the helper first (standalone step, green commit) so existing tests remain green:** ```typescript function callLoad(url: URL, opts: { canBlogWrite?: boolean } = {}) { return load({ url, request: new Request('http://localhost/geschichten'), fetch: vi.fn() as unknown as typeof fetch, parent: vi.fn().mockResolvedValue({ canBlogWrite: opts.canBlogWrite ?? false, canWrite: false, canAnnotate: false, user: null }) }); } ``` All 13 existing tests in `page.server.test.ts` use `callLoad(url)` with no opts — adding `canBlogWrite: false` as the default keeps them green unchanged. New server test cases needed: - `canBlogWrite=true` → `mockGet` called twice (once PUBLISHED, once DRAFT); assert `expect(mockGet).toHaveBeenCalledTimes(2)` and verify the second call has `status: 'DRAFT'`. - `canBlogWrite=false` → no second request. - DRAFT fetch rejects (network-level failure, not just 4xx) → `drafts: []`, PUBLISHED list unaffected. Mock pattern: ```typescript mockGet.mockImplementation((path: string, opts: unknown) => { if (path === '/api/geschichten' && opts?.params?.query?.status === 'DRAFT') { return Promise.reject(new TypeError('Failed to fetch')); } return Promise.resolve({ response: { ok: true, status: 200 }, data: [] }); }); ``` This proves `Promise.allSettled` catches network-level failures, not just non-ok responses. ### 4. Frontend `+page.svelte` Render a distinct **\"Entwürfe\"** section *above* the published list, only when `data.drafts.length > 0`. Gate strictly on `drafts.length > 0`, not `canBlogWrite` — no empty-state noise for writers with zero drafts. Place the Entwürfe section and published list in the same card (`<div class="overflow-hidden rounded-sm border border-line bg-surface shadow-sm">`), separated by a `border-b-2 border-line` divider before the published list heading. When `data.drafts.length > 0`, render an `<h2>` for the drafts section and a matching `<h2>` for the published list (gated on the same condition), to keep the heading outline balanced: ```svelte {#if data.drafts.length > 0} <!-- Drafts section --> <h2 class="text-xs font-bold uppercase tracking-widest text-ink-3"> {m.geschichten_drafts_heading()} <span class="ml-1 normal-case font-normal text-ink-3">{m.geschichten_drafts_unfiltered_caption()}</span> </h2> <ul> {#each data.drafts as g (g.id)} <li class="border-b border-line-2 last:border-b-0"> <GeschichteListRow geschichte={g} /> </li> {/each} </ul> <!-- Divider before published list --> <div class="border-b-2 border-line"></div> <!-- Published list heading (only when drafts section is present) --> <h2 class="text-xs font-bold uppercase tracking-widest text-ink-3"> {m.geschichten_published_heading()} </h2> {/if} ``` Use a keyed `{#each data.drafts as g (g.id)}` (matching the existing published list pattern). The existing `$derived` for `emptyMessage` considers only `personFilters` and `documentFilter` — do not change it. It correctly describes the state of the filtered published list, not the drafts section. **Browser test coverage needed:** Add `drafts: []` to the `makeData` factory **as the very first change**, before touching the svelte page, to keep all existing browser tests green: ```typescript function makeData(overrides: Partial<PageData> = {}): PageData { return { geschichten: [], personFilters: [], documentFilter: null, canBlogWrite: false, drafts: [], // ← add this ...overrides } as unknown as PageData; } ``` Then add: - Section renders when `drafts.length > 0`. - Section absent when `drafts: []`. - Draft badge visible on each row. - \"Entwürfe\" heading text present. - Section absent for reader without `canBlogWrite`. - **\"Veröffentlicht\" heading visible when drafts section is present** — tests the gated heading. - **Badge absent on a published row** (`draft-badge` not present when `status: 'PUBLISHED'`) — prevents a regression that always shows the badge. ### 5. `GeschichteListRow.svelte` — draft badge - Add `'status'` to the `Pick` type (current: `'id' | 'title' | 'body' | 'type' | 'author' | 'publishedAt'`). `GeschichteSummary.status` is already `@Schema(requiredMode = REQUIRED)` on the backend, so non-optional in generated types — no `?` guard needed. - Add an \"Entwurf\" badge mirroring the JOURNEY badge pattern (lines 39-46 desktop, 69-74 mobile): - `data-testid="draft-badge"` (desktop), `data-testid="draft-badge-mobile"` (mobile) - Use project tokens: `bg-journey-tint border border-journey-border text-journey` (dark-mode safe; matches the JOURNEY badge tokens already in use) - Same pill shape as JOURNEY badge: `rounded-sm px-1.5 py-px font-sans text-xs font-bold tracking-wide uppercase` - Add `shrink-0` on the mobile badge span (matching the JOURNEY mobile badge at line 72) to prevent collapsing the title on narrow screens - Add `flex-wrap` to the mobile `div.mb-1` flex row to prevent badge overflow at 320px when both JOURNEY and DRAFT badges are present simultaneously - Badge text: `{m.geschichten_draft_badge()}` (non-caps form \"Entwurf\", not \"ENTWURF\" — do not reuse existing `geschichte_editor_status_draft` key which renders \"ENTWURF\") - Badge is visual only — not interactive, no touch target concern - `publishedAt` is `null` for drafts; the existing `{#if publishedAt}` gate handles this correctly — no change needed - A story can be both a JOURNEY and a DRAFT simultaneously; both badges will render stacked in the desktop meta column (fine — flex-col layout handles it) and side-by-side on mobile (fine with `shrink-0` on both) - Place the draft badge in the same DOM slot as the journey badge — both desktop (meta column, after journey badge) and mobile (content column, same `div.mb-1` flex row). Use two separate `{#if}` blocks, not one combined condition, for readability. ### 6. i18n keys (`messages/{de,en,es}.json`) | Key | de | en | es | |---|---|---|---| | `geschichten_drafts_heading` | Entwürfe | Drafts | Borradores | | `geschichten_draft_badge` | Entwurf | Draft | Borrador | | `geschichten_drafts_unfiltered_caption` | (alle Entwürfe) | (all drafts) | (todos los borradores) | | `geschichten_published_heading` | Veröffentlicht | Published | Publicadas | Note: `geschichte_editor_status_draft` (\"ENTWURF\") already exists — do not reuse it; the list badge uses the non-caps form \"Entwurf\". ### 7. Regenerate API types Run `npm run generate:api` in `frontend/`. Expected: no diff (no new backend fields or endpoints were introduced; `GeschichteSummary.status` was already `REQUIRED`). If a diff appears, fix the missing annotation before merging. ## Acceptance criteria - A blog writer who has saved a draft sees that draft on `/geschichten`, visually marked with an \"Entwurf\" badge, in a section labeled \"Entwürfe\" above the published list. - A blog writer sees their **own** drafts only — not other authors' drafts (privacy: a writer shall never see another writer's unpublished draft). The `author.id` of every item in the drafts section equals the logged-in user's id. - The \"Entwürfe\" section is shown **un-filtered** — active person/document filter pills do not affect it. The `(alle Entwürfe)` caption signals this to the user. - A blog writer with **zero** drafts sees no \"Entwürfe\" section — the heading and section are not present in the DOM, regardless of `canBlogWrite`. - A reader without `BLOG_WRITE` sees no drafts and no draft section (unchanged behaviour). - Published stories continue to list and filter exactly as before. - If the drafts request fails, the published list still renders and no error is shown to the user (graceful degradation to `drafts: []`, section absent). - All text is i18n-complete (de/en/es). - `GeschichteService.list()` with a blog writer + null/omitted status returns only PUBLISHED stories (backend guard, covered by two new JUnit tests with `@DisplayName` referencing the security nature). - When the \"Entwürfe\" section is visible, a \"Veröffentlicht\" `<h2>` heading is also present for the published list (heading outline is balanced for screen reader navigation). - **Non-goal:** an editor/admin seeing *all* authors' drafts for review — out of scope, separate issue if ever needed. ## Files involved - `backend/.../geschichte/GeschichteService.java` — null-status guard + Javadoc - `backend/.../geschichte/GeschichteServiceTest.java` — rename/rewrite existing false-safety-net null-status test; strengthen `list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE`; 2 new security-regression tests with `@DisplayName` - `$lib/shared/server/settled.ts` (new) — extracted `settled<T>()` helper (preparatory commit) - `frontend/src/routes/+page.server.ts` — update import for extracted `settled<T>()` helper - `frontend/src/routes/geschichten/+page.server.ts` — `await parent()`, second DRAFT request via `Promise.allSettled` + shared `settled<T>()`, resilient fetch - `frontend/src/routes/geschichten/page.server.test.ts` — `parent` mock in `callLoad` (standalone commit first), new test cases - `frontend/src/routes/geschichten/+page.svelte` — \"Entwürfe\" section + gated \"Veröffentlicht\" heading above published list - `frontend/src/routes/geschichten/page.svelte.spec.ts` — add `drafts: []` to `makeData` factory first; then new browser tests for drafts section + badge + badge-absent-on-published-row + Veröffentlicht heading - `frontend/src/lib/geschichte/GeschichteListRow.svelte` — add `status` to Pick, add draft badge (desktop + mobile slots, `shrink-0` on mobile, `flex-wrap` on mobile row) - `frontend/messages/{de,en,es}.json` — 4 new i18n keys

marcel added the P1-highbugui labels 2026-06-12 07:37:11 +02:00

marcel commented 2026-06-12 11:12:00 +02:00

Author

Owner

Copy Link

👨‍💻 Felix Brandt — Fullstack Developer

Observations

Backend — the two vulnerable tests are exactly as described. list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible (line 235) uses any() for all five args to findSummaries and only asserts out.hasSize(2) — passing against both vulnerable and fixed code. list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE (line 224) is equally loose: any() as the first argument makes it vacuous. Both rewrites must be the very first commits before list() is touched, so there is proof of red→green.

Backend — GeschichteService.list() is clean and under 20 lines. The proposed ternary replacement stays within that budget. The Javadoc fits in 2 lines of comment above the method and satisfies the security-contract requirement without padding the line count.

Frontend — callLoad signature mismatch confirmed. Current callLoad at line 27 of page.server.test.ts takes only (url: URL) — no parent. Adding parent: vi.fn().mockResolvedValue({ canBlogWrite: false, ... }) as the default in the helper keeps all 13 existing tests green with no call-site changes.

settled<T>() extraction is one file. The function lives at routes/+page.server.ts:17-21. Target is $lib/shared/server/settled.ts — that directory already exists with locale.ts and permissions.ts. The function is pure (no side effects, no imports), so the extraction is a clean copy + export + import update.

GeschichteListRow.svelte — the Pick type is missing 'status'. Current: 'id' | 'title' | 'body' | 'type' | 'author' | 'publishedAt'. Adding 'status' is safe — GeschichteSummary.status is @Schema(requiredMode = REQUIRED) so it is non-optional in generated types. No ? guard needed. The file is at frontend/src/lib/geschichte/GeschichteListRow.svelte.

page.svelte.spec.ts makeData factory — the factory at line 27 includes canBlogWrite: false but no drafts field. Adding drafts: [] must happen before any change to +page.svelte, or all 30+ existing browser tests will TypeScript-error the moment the page expects data.drafts.

The +page.svelte card structure — the page currently wraps the filter row and story list in one <div class="overflow-hidden rounded-sm border border-line bg-surface shadow-sm">. The drafts section and divider must be placed inside this same card div, above the existing {#if data.geschichten.length === 0} block. Check that the {#each data.drafts as g (g.id)} key expression matches the published list pattern {#each data.geschichten as g (g.id)}.

Recommendations

• Strict commit order (7 commits minimum): (1) rewrite/rename existing backend tests — red; (2) fix list() — green; (3) extract settled<T>() — green; (4) update callLoad helper — green; (5) add drafts: [] to makeData — green; (6) backend + frontend feature; (7) i18n keys + API regen. Never bundle two logical changes.
• GeschichteListRow badge placement: use two separate {#if geschichte.status === 'DRAFT'} blocks — one in the desktop meta column after the JOURNEY badge, one in the mobile div.mb-1 flex row after the journey badge span. Do not combine with a single || isJourney condition; readability matters more than the one line saved.
• $derived for isDraft: extract const isDraft = $derived(geschichte.status === 'DRAFT') alongside const isJourney = $derived(...) — keeps the template logic-free and named.
• The Promise.allSettled network-rejection test is the most important new server test. It proves Promise.allSettled (correct) vs Promise.all (500s on draft failure). Write it with a path/status-keyed mock, not a blanket reject-all mock, so the PUBLISHED fetch is still verified to succeed independently.

## 👨‍💻 Felix Brandt — Fullstack Developer ### Observations **Backend — the two vulnerable tests are exactly as described.** `list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible` (line 235) uses `any()` for all five args to `findSummaries` and only asserts `out.hasSize(2)` — passing against both vulnerable and fixed code. `list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE` (line 224) is equally loose: `any()` as the first argument makes it vacuous. Both rewrites must be the very first commits before `list()` is touched, so there is proof of red→green. **Backend — `GeschichteService.list()` is clean and under 20 lines.** The proposed ternary replacement stays within that budget. The Javadoc fits in 2 lines of comment above the method and satisfies the security-contract requirement without padding the line count. **Frontend — `callLoad` signature mismatch confirmed.** Current `callLoad` at line 27 of `page.server.test.ts` takes only `(url: URL)` — no `parent`. Adding `parent: vi.fn().mockResolvedValue({ canBlogWrite: false, ... })` as the default in the helper keeps all 13 existing tests green with no call-site changes. **`settled<T>()` extraction is one file.** The function lives at `routes/+page.server.ts:17-21`. Target is `$lib/shared/server/settled.ts` — that directory already exists with `locale.ts` and `permissions.ts`. The function is pure (no side effects, no imports), so the extraction is a clean copy + export + import update. **`GeschichteListRow.svelte` — the `Pick` type is missing `'status'`.** Current: `'id' | 'title' | 'body' | 'type' | 'author' | 'publishedAt'`. Adding `'status'` is safe — `GeschichteSummary.status` is `@Schema(requiredMode = REQUIRED)` so it is non-optional in generated types. No `?` guard needed. The file is at `frontend/src/lib/geschichte/GeschichteListRow.svelte`. **`page.svelte.spec.ts` `makeData` factory** — the factory at line 27 includes `canBlogWrite: false` but no `drafts` field. Adding `drafts: []` must happen before any change to `+page.svelte`, or all 30+ existing browser tests will TypeScript-error the moment the page expects `data.drafts`. **The `+page.svelte` card structure** — the page currently wraps the filter row and story list in one `<div class="overflow-hidden rounded-sm border border-line bg-surface shadow-sm">`. The drafts section and divider must be placed inside this same card div, above the existing `{#if data.geschichten.length === 0}` block. Check that the `{#each data.drafts as g (g.id)}` key expression matches the published list pattern `{#each data.geschichten as g (g.id)}`. ### Recommendations - **Strict commit order (7 commits minimum):** (1) rewrite/rename existing backend tests — red; (2) fix `list()` — green; (3) extract `settled<T>()` — green; (4) update `callLoad` helper — green; (5) add `drafts: []` to `makeData` — green; (6) backend + frontend feature; (7) i18n keys + API regen. Never bundle two logical changes. - **`GeschichteListRow` badge placement:** use two separate `{#if geschichte.status === 'DRAFT'}` blocks — one in the desktop meta column after the JOURNEY badge, one in the mobile `div.mb-1` flex row after the journey badge span. Do not combine with a single `|| isJourney` condition; readability matters more than the one line saved. - **`$derived` for `isDraft`:** extract `const isDraft = $derived(geschichte.status === 'DRAFT')` alongside `const isJourney = $derived(...)` — keeps the template logic-free and named. - **The `Promise.allSettled` network-rejection test** is the most important new server test. It proves `Promise.allSettled` (correct) vs `Promise.all` (500s on draft failure). Write it with a path/status-keyed mock, not a blanket reject-all mock, so the PUBLISHED fetch is still verified to succeed independently.

marcel commented 2026-06-12 11:12:24 +02:00

Author

Owner

Copy Link

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

CWE-639 (Broken Access Control) confirmed in live code. GeschichteService.java:118:

GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED;

When a blog writer calls with status = null, effective = null. Line 121: effective == GeschichteStatus.DRAFT → false → authorId = null. findSummaries(null, null, ...) returns all stories from all authors including other writers' DRAFTs. Confirmed information-disclosure vulnerability.

The fix is correct and tight. effective = (currentUserHasBlogWrite() && status == GeschichteStatus.DRAFT) ? GeschichteStatus.DRAFT : GeschichteStatus.PUBLISHED closes the null-passthrough without introducing new branches. The two new @DisplayName("security: ...") tests provide a permanent regression fixture in CI output — they will appear by name in any future security audit of the test suite.

findSummaries JPQL uses a typed GeschichteStatus parameter — confirmed by the repository interface signature. No SQL injection risk. The status parameter is an enum, not a user-provided string.

getById() is not vulnerable. Lines 73-77 throw NOT_FOUND (not FORBIDDEN) for drafts without BLOG_WRITE — correct security-through-misdirection pattern for this threat model. No timing oracle concern at this scale.

GeschichteQueryService is not vulnerable — existsById and findById only, no list() path. The mandatory audit is confirmed complete; no additional guard needed.

The false-safety-net test is the highest-priority item. list_passes_null_status_through_for_BLOG_WRITE_so_drafts_are_visible asserts hasSize(2) against a thenReturn(List.of(s1, s2)) stub — it passes regardless of what status was passed to the repository. It gives a false green on the vulnerable code. Renaming it and replacing any() with eq(GeschichteStatus.PUBLISHED) as the first argument turns it into a real regression fixture. This rewrite must be red before list() is touched.

Frontend canBlogWrite guard (checking parent() before issuing the DRAFT request) is defense-in-depth layer 2 — correct. It prevents DRAFT requests from appearing in access logs for non-blog-writers, which is a clean audit trail. But it is not the security control; the backend fix is.

The Javadoc on list() is a security-documentation practice, not optional polish. A future developer must be able to audit the null-status contract by reading the method signature without tracing callers or reading the test suite.

Recommendations

• Write the two security-regression tests exactly as specified, using eq(GeschichteStatus.PUBLISHED) and eq(GeschichteStatus.DRAFT) as the first argument to verify(...).findSummaries(...). The @DisplayName prefix "security:" makes these permanently identifiable in CI output and in any future manual audit.
• Rename the false-safety-net test before the fix. New name: list_with_null_status_and_BLOG_WRITE_returns_PUBLISHED_not_all_stories. Run it against the current list() — it must fail. Then fix list() — it must pass. This sequence proves the test was meaningful.
• Strengthen list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE by replacing any() with eq(GeschichteStatus.PUBLISHED) as the first argument to verify(...).findSummaries(...). Currently this test would pass even if the service passed null to the repository for a reader.
• Verify the fix is atomic with respect to the test rename. Do not rename the test and fix list() in the same commit — the rename commit must be red (test fails against current code), and the fix commit must turn it green. This is the standard security-fix TDD workflow.

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations **CWE-639 (Broken Access Control) confirmed in live code.** `GeschichteService.java:118`: ```java GeschichteStatus effective = currentUserHasBlogWrite() ? status : GeschichteStatus.PUBLISHED; ``` When a blog writer calls with `status = null`, `effective = null`. Line 121: `effective == GeschichteStatus.DRAFT` → false → `authorId = null`. `findSummaries(null, null, ...)` returns all stories from all authors including other writers' DRAFTs. **Confirmed information-disclosure vulnerability.** **The fix is correct and tight.** `effective = (currentUserHasBlogWrite() && status == GeschichteStatus.DRAFT) ? GeschichteStatus.DRAFT : GeschichteStatus.PUBLISHED` closes the null-passthrough without introducing new branches. The two new `@DisplayName("security: ...")` tests provide a permanent regression fixture in CI output — they will appear by name in any future security audit of the test suite. **`findSummaries` JPQL uses a typed `GeschichteStatus` parameter** — confirmed by the repository interface signature. No SQL injection risk. The status parameter is an enum, not a user-provided string. **`getById()` is not vulnerable.** Lines 73-77 throw NOT_FOUND (not FORBIDDEN) for drafts without BLOG_WRITE — correct security-through-misdirection pattern for this threat model. No timing oracle concern at this scale. **`GeschichteQueryService` is not vulnerable** — `existsById` and `findById` only, no `list()` path. The mandatory audit is confirmed complete; no additional guard needed. **The false-safety-net test is the highest-priority item.** `list_passes_null_status_through_for_BLOG_WRITE_so_drafts_are_visible` asserts `hasSize(2)` against a `thenReturn(List.of(s1, s2))` stub — it passes regardless of what status was passed to the repository. It gives a false green on the vulnerable code. Renaming it and replacing `any()` with `eq(GeschichteStatus.PUBLISHED)` as the first argument turns it into a real regression fixture. This rewrite must be red before `list()` is touched. **Frontend `canBlogWrite` guard** (checking `parent()` before issuing the DRAFT request) is defense-in-depth layer 2 — correct. It prevents DRAFT requests from appearing in access logs for non-blog-writers, which is a clean audit trail. But it is not the security control; the backend fix is. **The Javadoc on `list()` is a security-documentation practice, not optional polish.** A future developer must be able to audit the null-status contract by reading the method signature without tracing callers or reading the test suite. ### Recommendations - **Write the two security-regression tests exactly as specified**, using `eq(GeschichteStatus.PUBLISHED)` and `eq(GeschichteStatus.DRAFT)` as the first argument to `verify(...).findSummaries(...)`. The `@DisplayName` prefix "security:" makes these permanently identifiable in CI output and in any future manual audit. - **Rename the false-safety-net test before the fix.** New name: `list_with_null_status_and_BLOG_WRITE_returns_PUBLISHED_not_all_stories`. Run it against the current `list()` — it must fail. Then fix `list()` — it must pass. This sequence proves the test was meaningful. - **Strengthen `list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE`** by replacing `any()` with `eq(GeschichteStatus.PUBLISHED)` as the first argument to `verify(...).findSummaries(...)`. Currently this test would pass even if the service passed `null` to the repository for a reader. - **Verify the fix is atomic with respect to the test rename.** Do not rename the test and fix `list()` in the same commit — the rename commit must be red (test fails against current code), and the fix commit must turn it green. This is the standard security-fix TDD workflow.

marcel commented 2026-06-12 11:12:48 +02:00

Author

Owner

Copy Link

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

Two false-safety-net tests confirmed in GeschichteServiceTest.java. Both list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible (line 235) and list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE (line 224) use any() as the first argument to verify(...).findSummaries(...). A test that verifies nothing about the status argument while calling itself a safety net is worse than no test — it provides false confidence and will pass against both the broken and fixed code.

callLoad in page.server.test.ts does not accept parent. Current signature at line 27: callLoad(url: URL). All 13 existing tests use callLoad(makeUrl(...)). Once +page.server.ts adds await parent(), every existing test will fail with a "parent is not a function" or similar error. The callLoad update must be a standalone, green commit before any production code changes. The default canBlogWrite: false ensures all 13 existing call sites pass without modification.

The DRAFT-fetch network-rejection test is the most critical new server test. It distinguishes Promise.allSettled (correct — degrades to drafts: []) from Promise.all (wrong — 500s the entire overview). The mock must reject at the network level (Promise.reject(new TypeError('Failed to fetch'))), not just return { response: { ok: false } } — a non-ok response is handled differently than a thrown rejection in Promise.allSettled.

makeData factory in page.svelte.spec.ts must get drafts: [] before +page.svelte is touched. Currently the factory at line 27 returns { geschichten: [], personFilters: [], documentFilter: null, canBlogWrite: false }. Once +page.svelte expects data.drafts, all 30+ existing browser tests will TypeScript-error. This is a one-line addition: drafts: [] — it must be its own standalone green commit.

GeschichteListRow.svelte.spec.ts already exists at frontend/src/lib/geschichte/GeschichteListRow.svelte.spec.ts. The badge-absent-on-published-row test belongs there, not in the page spec, since the badge logic lives in the row component. This test uses data-testid="draft-badge" (not text matching) so it is decoupled from i18n string changes.

Test naming should follow the @DisplayName convention for Java. The two new security tests must prefix their @DisplayName with "security:" to be identifiable in CI output without opening the file.

No integration test gap. The security fix is fully coverable at the unit test layer (Mockito + @ExtendWith(MockitoExtension.class)). The findSummaries contract is what matters, and the mock verifies the exact arguments. No Testcontainers needed for this fix.

Recommendations

• Commit order is the test strategy. Each preparatory commit (test rename → red, list() fix → green, settled<T>() extraction → green, callLoad update → green, makeData factory update → green) must stay green before the next commit begins. Never let the test suite go red between preparatory commits (except the intentional red from the test rename).
• The badge-absent-on-published-row test should use expect(document.querySelector('[data-testid="draft-badge"]')).toBeNull() on a row with status: 'PUBLISHED'. This is the highest-value regression test — it is the only test that would catch a badge rendered unconditionally due to a missing {#if}.
• The network-rejection test mock pattern (from the spec) distinguishes the two failure modes. Use it verbatim:

  mockGet.mockImplementation((path: string, opts: unknown) => {
    if (path === '/api/geschichten' && opts?.params?.query?.status === 'DRAFT') {
      return Promise.reject(new TypeError('Failed to fetch'));
    }
    return Promise.resolve({ response: { ok: true, status: 200 }, data: [] });
  });
  

• The "Veröffentlicht" heading browser test belongs in the page spec: assert the heading is present when drafts.length > 0 and absent when drafts: []. This exercises the gated heading decision.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations **Two false-safety-net tests confirmed in `GeschichteServiceTest.java`.** Both `list_passes_null_status_through_for_BLOG_WRITER_so_drafts_are_visible` (line 235) and `list_forces_PUBLISHED_status_for_reader_without_BLOG_WRITE` (line 224) use `any()` as the first argument to `verify(...).findSummaries(...)`. A test that verifies nothing about the status argument while calling itself a safety net is worse than no test — it provides false confidence and will pass against both the broken and fixed code. **`callLoad` in `page.server.test.ts` does not accept `parent`.** Current signature at line 27: `callLoad(url: URL)`. All 13 existing tests use `callLoad(makeUrl(...))`. Once `+page.server.ts` adds `await parent()`, every existing test will fail with a "parent is not a function" or similar error. The `callLoad` update must be a standalone, green commit before any production code changes. The default `canBlogWrite: false` ensures all 13 existing call sites pass without modification. **The DRAFT-fetch network-rejection test is the most critical new server test.** It distinguishes `Promise.allSettled` (correct — degrades to `drafts: []`) from `Promise.all` (wrong — 500s the entire overview). The mock must reject at the network level (`Promise.reject(new TypeError('Failed to fetch'))`), not just return `{ response: { ok: false } }` — a non-ok response is handled differently than a thrown rejection in `Promise.allSettled`. **`makeData` factory in `page.svelte.spec.ts` must get `drafts: []` before `+page.svelte` is touched.** Currently the factory at line 27 returns `{ geschichten: [], personFilters: [], documentFilter: null, canBlogWrite: false }`. Once `+page.svelte` expects `data.drafts`, all 30+ existing browser tests will TypeScript-error. This is a one-line addition: `drafts: []` — it must be its own standalone green commit. **`GeschichteListRow.svelte.spec.ts` already exists** at `frontend/src/lib/geschichte/GeschichteListRow.svelte.spec.ts`. The badge-absent-on-published-row test belongs there, not in the page spec, since the badge logic lives in the row component. This test uses `data-testid="draft-badge"` (not text matching) so it is decoupled from i18n string changes. **Test naming should follow the `@DisplayName` convention** for Java. The two new security tests must prefix their `@DisplayName` with `"security:"` to be identifiable in CI output without opening the file. **No integration test gap.** The security fix is fully coverable at the unit test layer (Mockito + `@ExtendWith(MockitoExtension.class)`). The `findSummaries` contract is what matters, and the mock verifies the exact arguments. No Testcontainers needed for this fix. ### Recommendations - **Commit order is the test strategy.** Each preparatory commit (test rename → red, `list()` fix → green, `settled<T>()` extraction → green, `callLoad` update → green, `makeData` factory update → green) must stay green before the next commit begins. Never let the test suite go red between preparatory commits (except the intentional red from the test rename). - **The badge-absent-on-published-row test** should use `expect(document.querySelector('[data-testid="draft-badge"]')).toBeNull()` on a row with `status: 'PUBLISHED'`. This is the highest-value regression test — it is the only test that would catch a badge rendered unconditionally due to a missing `{#if}`. - **The network-rejection test mock pattern** (from the spec) distinguishes the two failure modes. Use it verbatim: ```typescript mockGet.mockImplementation((path: string, opts: unknown) => { if (path === '/api/geschichten' && opts?.params?.query?.status === 'DRAFT') { return Promise.reject(new TypeError('Failed to fetch')); } return Promise.resolve({ response: { ok: true, status: 200 }, data: [] }); }); ``` - **The "Veröffentlicht" heading browser test** belongs in the page spec: assert the heading is present when `drafts.length > 0` and absent when `drafts: []`. This exercises the gated heading decision.

marcel commented 2026-06-12 11:13:13 +02:00

Author

Owner

Copy Link

🎨 Leonie Voss — UI/UX Design Lead & Accessibility Strategist

Observations

GeschichteListRow.svelte badge placement is correct. The desktop meta column is flex flex-col items-start gap-1 — adding a draft badge after the JOURNEY badge stacks them vertically with gap-1 spacing. The mobile div.mb-1 is flex items-center gap-1.5 — adding a draft badge here means a JOURNEY+DRAFT story shows two badges next to the title. With shrink-0 on both badges and flex-wrap on the row, this is safe at 320px.

flex-wrap on the mobile div.mb-1 row is confirmed necessary. Currently the row is flex items-center gap-1.5 sm:hidden at line 52. Without flex-wrap, a JOURNEY+DRAFT story's two badges plus <h2> title can overflow at 320px. The <h2> has font-serif text-lg — at 320px with two badges that is a tight fit. Adding flex-wrap allows the title to wrap to the next line rather than truncating.

Badge contrast: text-journey on bg-journey-tint — measure at implementation time. The JOURNEY badge has been in production since the JOURNEY feature shipped. The decision to verify at implementation time (not block on it now) is correct. If text-journey fails 4.5:1 for 12px/700 text on bg-journey-tint, use text-journey-dark or a darker variant. WCAG 1.4.1 (use of color) is satisfied by the literal word "Entwurf" regardless.

Caption text text-ink-3 on bg-surface — may need to be text-ink-2. Check at implementation time. The caption (alle Entwürfe) uses normal-case font-normal text-ink-3. If text-ink-3 is below 4.5:1 on bg-surface, use text-ink-2. The section heading uses text-ink-3 with uppercase font-bold which is 3:1 minimum (large/bold text) — that threshold is easier to meet.

Gated "Veröffentlicht" heading is the right decision. The heading outline becomes balanced only when the "Entwürfe" section is visible. When there are no drafts, adding a "Veröffentlicht" <h2> above the single list would be redundant noise. The gate on data.drafts.length > 0 handles both cases correctly.

The <h2> heading for the drafts section uses text-xs font-bold uppercase tracking-widest text-ink-3 — this is the project's standard section title pattern (confirmed in the card pattern in the style guide). The (alle Entwürfe) caption span uses normal-case font-normal text-ink-3 — this correctly overrides the uppercase from the parent <h2>.

Touch target check: the draft badge sits inside the <a> row which has min-h-[44px] — the badge inherits the 44px touch target from the parent anchor. No separate touch target needed for the badge (it is non-interactive).

Dark mode: bg-journey-tint, border-journey-border, and text-journey have dark-mode overrides in layout.css (confirmed by existing JOURNEY badge). The draft badge inherits dark-mode safety without any additional work.

Recommendations

• Add flex-wrap to the mobile div.mb-1 flex row — change flex items-center gap-1.5 sm:hidden to flex flex-wrap items-center gap-1.5 sm:hidden. This prevents overflow at 320px for JOURNEY+DRAFT dual-badge rows without affecting single-badge rows.
• Verify contrast at implementation time — use https://webaim.org/resources/contrastchecker/ with the computed hex values for --color-journey (text) and --color-journey-tint (background) from layout.css. 4.5:1 is the AA threshold for 12px/700 text. If it fails, add a text-journey-dark token override rather than using a raw hex.
• Caption text color — if text-ink-3 is below 4.5:1 on bg-surface, use text-ink-2. Verify both light and dark mode contrast independently, since dark mode remaps the token.
• Section heading padding — the spec shows the <h2> inside the card but doesn't specify padding. Use px-3 py-2.5 (matching the filter row) or px-3 pt-3 pb-1 so the heading has breathing room above the draft rows. Check visually against the border-b-2 border-line divider that follows.

## 🎨 Leonie Voss — UI/UX Design Lead & Accessibility Strategist ### Observations **`GeschichteListRow.svelte` badge placement is correct.** The desktop meta column is `flex flex-col items-start gap-1` — adding a draft badge after the JOURNEY badge stacks them vertically with `gap-1` spacing. The mobile `div.mb-1` is `flex items-center gap-1.5` — adding a draft badge here means a JOURNEY+DRAFT story shows two badges next to the title. With `shrink-0` on both badges and `flex-wrap` on the row, this is safe at 320px. **`flex-wrap` on the mobile `div.mb-1` row is confirmed necessary.** Currently the row is `flex items-center gap-1.5 sm:hidden` at line 52. Without `flex-wrap`, a JOURNEY+DRAFT story's two badges plus `<h2>` title can overflow at 320px. The `<h2>` has `font-serif text-lg` — at 320px with two badges that is a tight fit. Adding `flex-wrap` allows the title to wrap to the next line rather than truncating. **Badge contrast: `text-journey` on `bg-journey-tint` — measure at implementation time.** The JOURNEY badge has been in production since the JOURNEY feature shipped. The decision to verify at implementation time (not block on it now) is correct. If `text-journey` fails 4.5:1 for 12px/700 text on `bg-journey-tint`, use `text-journey-dark` or a darker variant. WCAG 1.4.1 (use of color) is satisfied by the literal word "Entwurf" regardless. **Caption text `text-ink-3` on `bg-surface` — may need to be `text-ink-2`.** Check at implementation time. The caption `(alle Entwürfe)` uses `normal-case font-normal text-ink-3`. If `text-ink-3` is below 4.5:1 on `bg-surface`, use `text-ink-2`. The section heading uses `text-ink-3` with `uppercase font-bold` which is 3:1 minimum (large/bold text) — that threshold is easier to meet. **Gated "Veröffentlicht" heading is the right decision.** The heading outline becomes balanced only when the "Entwürfe" section is visible. When there are no drafts, adding a "Veröffentlicht" `<h2>` above the single list would be redundant noise. The gate on `data.drafts.length > 0` handles both cases correctly. **The `<h2>` heading for the drafts section** uses `text-xs font-bold uppercase tracking-widest text-ink-3` — this is the project's standard section title pattern (confirmed in the card pattern in the style guide). The `(alle Entwürfe)` caption span uses `normal-case font-normal text-ink-3` — this correctly overrides the `uppercase` from the parent `<h2>`. **Touch target check:** the draft badge sits inside the `<a>` row which has `min-h-[44px]` — the badge inherits the 44px touch target from the parent anchor. No separate touch target needed for the badge (it is non-interactive). **Dark mode:** `bg-journey-tint`, `border-journey-border`, and `text-journey` have dark-mode overrides in `layout.css` (confirmed by existing JOURNEY badge). The draft badge inherits dark-mode safety without any additional work. ### Recommendations - **Add `flex-wrap` to the mobile `div.mb-1` flex row** — change `flex items-center gap-1.5 sm:hidden` to `flex flex-wrap items-center gap-1.5 sm:hidden`. This prevents overflow at 320px for JOURNEY+DRAFT dual-badge rows without affecting single-badge rows. - **Verify contrast at implementation time** — use https://webaim.org/resources/contrastchecker/ with the computed hex values for `--color-journey` (text) and `--color-journey-tint` (background) from `layout.css`. 4.5:1 is the AA threshold for 12px/700 text. If it fails, add a `text-journey-dark` token override rather than using a raw hex. - **Caption text color** — if `text-ink-3` is below 4.5:1 on `bg-surface`, use `text-ink-2`. Verify both light and dark mode contrast independently, since dark mode remaps the token. - **Section heading padding** — the spec shows the `<h2>` inside the card but doesn't specify padding. Use `px-3 py-2.5` (matching the filter row) or `px-3 pt-3 pb-1` so the heading has breathing room above the draft rows. Check visually against the `border-b-2 border-line` divider that follows.

marcel commented 2026-06-12 11:13:39 +02:00

Author

Owner

Copy Link

🏛️ Markus Keller — Application Architect

Observations

Module boundaries are clean throughout. GeschichteService owns GeschichteRepository and calls PersonService, DocumentService, UserService through their public interfaces. GeschichteQueryService exists specifically so JourneyItemService can check Geschichte existence without holding a direct repository reference. The proposed fix does not introduce any cross-domain repository access — the three-line ternary change stays entirely within GeschichteService.

The settled<T>() extraction is architecturally correct. The function is currently a private closure in routes/+page.server.ts:17-21. Two server load functions needing the same helper is the canonical signal for extraction. $lib/shared/server/ already contains locale.ts and permissions.ts — settled.ts fits by convention. This is not over-engineering; it closes the door on a diverging second copy that would form the moment the pattern is needed in a third load function (there are multiple +page.server.ts files in this codebase that could benefit from it).

The Promise.allSettled + settled<T>() pattern is the project's established resilient-fetch idiom. It is already used in routes/+page.server.ts:47-63 for the reader home drafts fetch. Making it canonical via a shared module is the right call. The alternative — Promise.all — would turn a drafts-fetch failure into a 500 on the entire /geschichten overview, which is disproportionate to the business impact.

No new backend endpoints, no new entities, no Flyway migration. This is a bug fix on an existing endpoint plus a frontend feature using the existing GET /api/geschichten?status=DRAFT endpoint. The backend change is three lines: rename a test, rewrite a test, change one ternary. The schema is unchanged.

GeschichteService.list() is currently 19 lines (117-135). The ternary replacement removes one line and the Javadoc adds two lines — net +1 line. Still under 20 lines. No extraction needed.

Documentation check: No new Flyway migration, no new entity, no new route, no new controller, no new service, no new domain concept, no new infrastructure component. The documentation update table in the developer persona does not trigger any update. No ADR is warranted — this is a bug fix with a clear correct behavior (the null-passthrough was never intended), not a decision with meaningful alternatives.

parent() in +page.server.ts — calling parent() to obtain permissions is the established SvelteKit pattern in this codebase (routes/+page.server.ts:24 does exactly this). The geschichten load function currently skips parent() entirely because it does not need permissions — adding it for the blog-writer draft check is aligned with the pattern.

Recommendations

• Keep list() under 20 lines after the fix. The current method (lines 117-135) is tight. The ternary replacement and Javadoc addition will not materially change the line count. If you feel the urge to extract a private effectiveStatus() method, resist it — the ternary is self-documenting and the method would be too small to name well.
• The Javadoc must be explicit about null. Minimum required content: "null status for a blog writer resolves to PUBLISHED, never to all-stories. DRAFT status scopes the query to the current user's own drafts only." This is auditable at the method boundary without tracing the implementation.
• $lib/shared/server/settled.ts is the correct and final home. It is server-only (used in +page.server.ts files), shared across routes, and has no UI dependency. Do not place it in $lib/shared/utils/ (client-side utilities) or $lib/shared/ root (too generic).
• No architectural concern with the 4-key i18n addition. The existing pattern of adding keys to messages/{de,en,es}.json and regenerating Paraglide types is well-established. Confirm no diff in npm run generate:api output before committing — GeschichteSummary.status was already @Schema(requiredMode = REQUIRED) so no type regeneration should be needed.

## 🏛️ Markus Keller — Application Architect ### Observations **Module boundaries are clean throughout.** `GeschichteService` owns `GeschichteRepository` and calls `PersonService`, `DocumentService`, `UserService` through their public interfaces. `GeschichteQueryService` exists specifically so `JourneyItemService` can check Geschichte existence without holding a direct repository reference. The proposed fix does not introduce any cross-domain repository access — the three-line ternary change stays entirely within `GeschichteService`. **The `settled<T>()` extraction is architecturally correct.** The function is currently a private closure in `routes/+page.server.ts:17-21`. Two server load functions needing the same helper is the canonical signal for extraction. `$lib/shared/server/` already contains `locale.ts` and `permissions.ts` — `settled.ts` fits by convention. This is not over-engineering; it closes the door on a diverging second copy that would form the moment the pattern is needed in a third load function (there are multiple `+page.server.ts` files in this codebase that could benefit from it). **The `Promise.allSettled` + `settled<T>()` pattern is the project's established resilient-fetch idiom.** It is already used in `routes/+page.server.ts:47-63` for the reader home drafts fetch. Making it canonical via a shared module is the right call. The alternative — `Promise.all` — would turn a drafts-fetch failure into a 500 on the entire `/geschichten` overview, which is disproportionate to the business impact. **No new backend endpoints, no new entities, no Flyway migration.** This is a bug fix on an existing endpoint plus a frontend feature using the existing `GET /api/geschichten?status=DRAFT` endpoint. The backend change is three lines: rename a test, rewrite a test, change one ternary. The schema is unchanged. **`GeschichteService.list()` is currently 19 lines (117-135).** The ternary replacement removes one line and the Javadoc adds two lines — net +1 line. Still under 20 lines. No extraction needed. **Documentation check:** No new Flyway migration, no new entity, no new route, no new controller, no new service, no new domain concept, no new infrastructure component. The documentation update table in the developer persona does not trigger any update. No ADR is warranted — this is a bug fix with a clear correct behavior (the null-passthrough was never intended), not a decision with meaningful alternatives. **`parent()` in `+page.server.ts`** — calling `parent()` to obtain permissions is the established SvelteKit pattern in this codebase (`routes/+page.server.ts:24` does exactly this). The geschichten load function currently skips `parent()` entirely because it does not need permissions — adding it for the blog-writer draft check is aligned with the pattern. ### Recommendations - **Keep `list()` under 20 lines after the fix.** The current method (lines 117-135) is tight. The ternary replacement and Javadoc addition will not materially change the line count. If you feel the urge to extract a private `effectiveStatus()` method, resist it — the ternary is self-documenting and the method would be too small to name well. - **The Javadoc must be explicit about null.** Minimum required content: "null status for a blog writer resolves to PUBLISHED, never to all-stories. DRAFT status scopes the query to the current user's own drafts only." This is auditable at the method boundary without tracing the implementation. - **`$lib/shared/server/settled.ts` is the correct and final home.** It is server-only (used in `+page.server.ts` files), shared across routes, and has no UI dependency. Do not place it in `$lib/shared/utils/` (client-side utilities) or `$lib/shared/` root (too generic). - **No architectural concern with the 4-key i18n addition.** The existing pattern of adding keys to `messages/{de,en,es}.json` and regenerating Paraglide types is well-established. Confirm no diff in `npm run generate:api` output before committing — `GeschichteSummary.status` was already `@Schema(requiredMode = REQUIRED)` so no type regeneration should be needed.

marcel commented 2026-06-12 11:14:00 +02:00

Author

Owner

Copy Link

📋 Elicit — Requirements Engineer

Observations

The spec is implementation-ready. All resolved decisions are folded into the body with rationale. File paths are exact and verified against the codebase. Acceptance criteria are testable. This passes the Definition of Ready checklist.

Requirements coverage is complete for the defined scope. The issue covers the security fix (backend), the resilient fetch (frontend server), the UI (svelte page), the component (list row), the i18n (4 keys), the test strategy (9 browser tests, 3 server tests, 2+2 JUnit tests), and the non-goal boundary ("editor/admin seeing all authors' drafts — out of scope").

Privacy acceptance criterion is precisely stated and testable. "The author.id of every item in the drafts section equals the logged-in user's id." This is verifiable at the backend unit test layer via eq(writer.getId()) in the verify call. No frontend test is needed for this invariant — the frontend trusts the backend (correct, given the backend fix is the control).

One minor i18n note. The spec table includes geschichten_published_heading (4 keys total), and correctly warns against reusing geschichte_editor_status_draft (value: "ENTWURF") for the list badge. Confirmed: de.json has "geschichte_editor_status_draft": "ENTWURF" at line 1059. The new badge key geschichten_draft_badge with value "Entwurf" (non-caps) is distinct and necessary. Also confirmed: "dashboard_reader_draft_meta" exists (line 537) but its value "Entwurf · zuletzt bearbeitet {relative}" is display metadata for the home page drafts module — not reusable as a bare badge label.

One implementation-order clarification worth surfacing. Step 4 (frontend +page.svelte) says "add drafts: [] to the makeData factory as the very first change, before touching the svelte page." Step 5 (GeschichteListRow.svelte) says add 'status' to the Pick type. These two component changes are independent — either can go first. The constraint is only that makeData update precedes +page.svelte changes.

The non-goal is well-bounded and explicit. "An editor/admin seeing all authors' drafts for review — out of scope." This prevents scope creep during implementation. No action needed.

No loading-state gap. The overview page is SSR — the DRAFT fetch happens server-side during load(), so there is no client-visible loading state. Graceful degradation (network rejection → drafts: [], section absent) is the only failure mode that reaches the user, and it is covered.

Recommendations

• No spec changes needed. All decisions are resolved, all acceptance criteria are testable, all file paths are verified. The issue is ready for implementation.
• One implementation note to carry forward: the makeData factory update (adding drafts: []) and the callLoad helper update (adding parent) are both "preparatory green commits" — they must each be standalone commits that keep the test suite fully green. Bundling either with production code changes creates a debugging burden if something goes wrong.
• Acceptance criterion completeness check passed: security fix (✓), own-drafts-only privacy (✓), unfiltered drafts section (✓), zero-drafts = no section (✓), reader = no drafts (✓), published list unchanged (✓), graceful degradation (✓), i18n complete (✓), backend guard tested (✓), heading hierarchy balanced (✓), non-goal stated (✓).

## 📋 Elicit — Requirements Engineer ### Observations **The spec is implementation-ready.** All resolved decisions are folded into the body with rationale. File paths are exact and verified against the codebase. Acceptance criteria are testable. This passes the Definition of Ready checklist. **Requirements coverage is complete for the defined scope.** The issue covers the security fix (backend), the resilient fetch (frontend server), the UI (svelte page), the component (list row), the i18n (4 keys), the test strategy (9 browser tests, 3 server tests, 2+2 JUnit tests), and the non-goal boundary ("editor/admin seeing all authors' drafts — out of scope"). **Privacy acceptance criterion is precisely stated and testable.** "The `author.id` of every item in the drafts section equals the logged-in user's id." This is verifiable at the backend unit test layer via `eq(writer.getId())` in the `verify` call. No frontend test is needed for this invariant — the frontend trusts the backend (correct, given the backend fix is the control). **One minor i18n note.** The spec table includes `geschichten_published_heading` (4 keys total), and correctly warns against reusing `geschichte_editor_status_draft` (value: "ENTWURF") for the list badge. Confirmed: `de.json` has `"geschichte_editor_status_draft": "ENTWURF"` at line 1059. The new badge key `geschichten_draft_badge` with value "Entwurf" (non-caps) is distinct and necessary. Also confirmed: `"dashboard_reader_draft_meta"` exists (line 537) but its value "Entwurf · zuletzt bearbeitet {relative}" is display metadata for the home page drafts module — not reusable as a bare badge label. **One implementation-order clarification worth surfacing.** Step 4 (frontend `+page.svelte`) says "add `drafts: []` to the `makeData` factory as the very first change, before touching the svelte page." Step 5 (`GeschichteListRow.svelte`) says add `'status'` to the Pick type. These two component changes are independent — either can go first. The constraint is only that `makeData` update precedes `+page.svelte` changes. **The non-goal is well-bounded and explicit.** "An editor/admin seeing all authors' drafts for review — out of scope." This prevents scope creep during implementation. No action needed. **No loading-state gap.** The overview page is SSR — the DRAFT fetch happens server-side during `load()`, so there is no client-visible loading state. Graceful degradation (network rejection → `drafts: []`, section absent) is the only failure mode that reaches the user, and it is covered. ### Recommendations - **No spec changes needed.** All decisions are resolved, all acceptance criteria are testable, all file paths are verified. The issue is ready for implementation. - **One implementation note to carry forward:** the `makeData` factory update (adding `drafts: []`) and the `callLoad` helper update (adding `parent`) are both "preparatory green commits" — they must each be standalone commits that keep the test suite fully green. Bundling either with production code changes creates a debugging burden if something goes wrong. - **Acceptance criterion completeness check passed:** security fix (✓), own-drafts-only privacy (✓), unfiltered drafts section (✓), zero-drafts = no section (✓), reader = no drafts (✓), published list unchanged (✓), graceful degradation (✓), i18n complete (✓), backend guard tested (✓), heading hierarchy balanced (✓), non-goal stated (✓).

marcel commented 2026-06-12 11:53:50 +02:00

Author

Owner

Copy Link

Implementation complete — branch feat/issue-807-drafts-overview

All 9 commits on top of feat/issue-750-lesereisen-data-model:

Commit	Description
576dc6e0	test: rewrite false-safety-net null-status tests to catch CWE-639
a8cff2fd	test: add security regression tests for CWE-639 null-status and DRAFT scoping
b6beaa6d	fix: null status always resolves to PUBLISHED, fixing CWE-639
08c36410	test: add failing tests for draft fetch in page loader (+ extract settled<T>())
0693871e	feat: fetch own drafts in page loader for blog writers
e0a8605d	test: add failing tests for DRAFT status badge in GeschichteListRow
db6cfd70	feat: show DRAFT badge on desktop meta column and mobile row
f3133f97	test: add failing tests for Entwürfe section
063b7aa7	feat: add Entwürfe section above published list for blog writers

What was fixed

CWE-639 (broken access control): GeschichteService.list() previously forwarded null status directly to the repository for blog writers, returning all stories including other authors' DRAFTs. Now null always resolves to PUBLISHED; only an explicit DRAFT request (blog writers only) scopes the query to the caller's own drafts.

What was added

• Blog writers see an Entwürfe section at the top of /geschichten, showing their own drafts unfiltered (separate from the filtered published list)
• Each draft row carries a Entwurf badge (desktop meta column + mobile row)
• The DRAFT fetch degrades gracefully to drafts: [] on network failure — the overview never 500s
• settled<T>() extracted to $lib/shared/server/settled.ts for reuse across loaders
• 4 new i18n keys in de/en/es: geschichten_drafts_heading, geschichten_draft_badge, geschichten_drafts_unfiltered_caption, geschichten_published_heading

## Implementation complete — branch `feat/issue-807-drafts-overview` All 9 commits on top of `feat/issue-750-lesereisen-data-model`: | Commit | Description | |--------|-------------| | `576dc6e0` | test: rewrite false-safety-net null-status tests to catch CWE-639 | | `a8cff2fd` | test: add security regression tests for CWE-639 null-status and DRAFT scoping | | `b6beaa6d` | fix: null status always resolves to PUBLISHED, fixing CWE-639 | | `08c36410` | test: add failing tests for draft fetch in page loader (+ extract `settled<T>()`) | | `0693871e` | feat: fetch own drafts in page loader for blog writers | | `e0a8605d` | test: add failing tests for DRAFT status badge in GeschichteListRow | | `db6cfd70` | feat: show DRAFT badge on desktop meta column and mobile row | | `f3133f97` | test: add failing tests for Entwürfe section | | `063b7aa7` | feat: add Entwürfe section above published list for blog writers | ### What was fixed **CWE-639 (broken access control):** `GeschichteService.list()` previously forwarded `null` status directly to the repository for blog writers, returning all stories including other authors' DRAFTs. Now `null` always resolves to `PUBLISHED`; only an explicit `DRAFT` request (blog writers only) scopes the query to the caller's own drafts. ### What was added - Blog writers see an **Entwürfe** section at the top of `/geschichten`, showing their own drafts unfiltered (separate from the filtered published list) - Each draft row carries a **Entwurf** badge (desktop meta column + mobile row) - The DRAFT fetch degrades gracefully to `drafts: []` on network failure — the overview never 500s - `settled<T>()` extracted to `$lib/shared/server/settled.ts` for reuse across loaders - 4 new i18n keys in de/en/es: `geschichten_drafts_heading`, `geschichten_draft_badge`, `geschichten_drafts_unfiltered_caption`, `geschichten_published_heading`

marcel closed this issue 2026-06-12 14:09:14 +02:00

marcel referenced a pull request that will close this issue 2026-06-12 18:56:17 +02:00

feat(geschichten): show blog writers' own drafts on the Geschichten overview (#807) #813

marcel reopened this issue 2026-06-12 18:56:27 +02:00

marcel closed this issue 2026-06-12 19:46:05 +02:00

marcel referenced this issue from a commit 2026-06-12 19:46:07 +02:00

feat(geschichten): show blog writers' own drafts on the Geschichten overview (#807) (#813)

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

feat/issue-774-timeline-event-entity

feat/issue-803-geschichten-document-filter-chip

worktree-feat+issue-738-nl-search-backend

feat/issue-286-notification-bell-form-actions

feat/issue-580-sentry-backend

fix/issue-593-management-port-zero

worktree-feat+issue-557-upload-artifact-v3-pin

worktree-chore+issue-556-drop-client-branches-coverage-gate

fix/issue-514-prerender-crawl-bakes-redirects

fix/issue-472-prerender-entries

feat/issue-395-readme

feat/issue-345-bulk-mark-reviewed

feat/issue-344-bell-tooltip

feat/issue-341-annotieren-contrast

feat/issue-225-bulk-metadata-edit

feat/issue-317-bulk-upload

feat/issue-271-dashboard-redesign

docs/issue-240-mission-control-spec

refactor/issues-193-200

feat/issue-162-korrespondenz-redesign

feature/68-new-document-file-first

feat/81-discussion-discoverability

feat/62-document-bottom-panel

feature/56-backfill-file-hashes

feat/38-document-edit-history

fix/svelte5-test-delegation-and-login-auth

No results found.

Labels

Clear labels

P0-critical

Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.

P1-high

Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.

P2-medium

Noticeable improvement; doesn't block anything; schedule opportunistically.

P3-later

Cosmetic or parking-lot work; fine to stay open indefinitely.

audit

Read-only audit / assessment work; produces a report rather than changing code

bug

Something isn't working

cleanup

Removal of dead code, vague comments, naming violations, and scratch artifacts

collaboration

We want to extend the family archive to have more collaboration tools

conversation

descoped

We will do that later

devops

documentation

README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs

epic

Marker for epic-level issues that group multiple child issues

feature

file-upload

legibility

Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction

needs-discussion

Has an open decision or design question that must be resolved before implementation can start.

notification

person

phase-1: security

Security hygiene — must be done first

phase-2: container-images

Production-ready multi-stage Docker images

phase-3: prod-compose

Production compose overlay + Caddy reverse proxy

phase-4: spring-prod-profile

Spring Boot production configuration profile

phase-5: backups

Database and object storage backup strategy

phase-6: deployment-docs

.env.example, DEPLOYMENT.md, runbook

phase-7: monitoring

Prometheus, Loki, Grafana, Alertmanager

refactor

Code restructuring without behaviour change

security

test

ui

UI/UX and accessibility issues

user

No Label P1-high bug ui

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

Jens marcel

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#807