Skip to content

fix(dogfood 2026-06-18): guard unauth settings actions + retention input (ISSUE-006/007)#154

Merged
engels74 merged 2 commits into
mainfrom
fix/dogfood-2026-06-18-issues
Jun 18, 2026
Merged

fix(dogfood 2026-06-18): guard unauth settings actions + retention input (ISSUE-006/007)#154
engels74 merged 2 commits into
mainfrom
fix/dogfood-2026-06-18-issues

Conversation

@engels74

@engels74 engels74 commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

Verify-then-fix remediation of the 2026-06-18 dogfood findings (ISSUE-001…007). Per the plan (.omc/plans/fix-dogfood-2026-06-18-findings.md), every finding was first re-verified against current HEAD; only the two still-broken ones received code changes. The other five were already fixed by a concurrent remediation cycle or are third-party, and are documented here rather than re-patched.

Phase 0 verification table

Finding Disposition Evidence
ISSUE-001 (claim error silent) ALREADY-FIXED onboarding/claim/+page.svelte:51-53 renders role="alert" from the action's form.error; server returns fail(400,{error}) (claim/+page.server.ts:36).
ISSUE-002 (admin mobile nav) ALREADY-FIXED admin/+layout.svelte:50-148isMobileSidebar, .menu-button, overlay, Tab focus-trap.
ISSUE-003 (console authToken/JSON noise) THIRD-PARTY Plex's bundled OAuth JS; documented at auth/plex/+server.ts. No Obzorarr code emits it; console deliberately not monkey-patched.
ISSUE-004 (pause == stop) ALREADY-FIXED sync/scheduler.ts pausedByOperatorgetSchedulerStatus().isPaused; admin/sync/+page.svelte:546/586 distinct "Paused" badge + "Resume" button.
ISSUE-005 (CSRF origin unvalidated) ALREADY-FIXED admin/settings/security/+page.server.ts:179 runs CsrfOriginSchema.safeParsefail(400,'Invalid origin URL').
ISSUE-006 (dashboard/settings 500 on unauth POST) FIXED See below.
ISSUE-007 (retention float, minor) FIXED See below.

ISSUE-006 — unauthenticated settings actions return 401, not 500

SvelteKit form actions do not run the route's redirecting load, so a same-origin POST from an unauthenticated browser landed directly in the dashboard/settings handlers and dereferenced locals.user!.idTypeError → HTTP 500.

  • Added requireUserAction / requireUserActions to src/lib/server/auth/guards.ts (mirroring the existing requireAdminActions), returning fail(401, { error: 'Authentication required' }) when locals.user is absent.
  • Wrapped the three dashboard/settings actions (updateShareMode, regenerateToken, updateLogoPreference) with it.
  • Guarded the page load with an unauthenticated redirect mirroring the layout's isSafeReturnPath returnTo carrier, closing the parallel-load race.
  • Regression test (tests/unit/dashboard/settings-auth-guard.test.ts): each user-only action returns 401 (no TypeError) for an anonymous caller; the authenticated path still reaches the handler.

ISSUE-007 — whole-number retention-days input

admin/settings/system/+page.svelte retention-days input gains step="1" to match the sibling max-log-count input. The server LogSettingsSchema.retentionDays.int() remains the authoritative guard; this aligns the client with that contract.

Verification

  • bun run check — 0 errors / 0 warnings
  • bun run test — 2158 pass / 0 fail
  • bun run check:biome — clean
  • Architect review (read-only, Opus): APPROVED, approach OPTIMAL; confirmed no other user-only action route has the unguarded pattern (wrapped/[identifier] already inlines the 401 guard).

engels74 added 2 commits June 18, 2026 22:19
…E-006)

SvelteKit form actions do not run the route's redirecting `load`, so a
same-origin POST from an unauthenticated browser landed directly in the
dashboard/settings handlers and dereferenced `locals.user!.id`, throwing a
TypeError surfaced as HTTP 500.

Add `requireUserAction`/`requireUserActions` to the auth guards (mirroring the
existing `requireAdminActions`) returning `fail(401)` when `locals.user` is
absent, and wrap the dashboard/settings actions with it. Also guard the page
`load` with an unauthenticated redirect (mirroring the layout's isSafeReturnPath
returnTo carrier) so the parallel `load` race can no longer 500.

Regression test asserts each of the three user-only actions returns 401 (not a
TypeError/500) for an anonymous caller, and that the authenticated path still
reaches the handler.
…-007)

The retention-days number input lacked the `step="1"` attribute that its sibling
max-log-count input already carries, so the browser accepted fractional values
like 3.5. The server-side `LogSettingsSchema.retentionDays.int()` already rejects
non-integers; this aligns the client input with that contract for consistent
native validation and a clearer stepper UX.
@augmentcode

augmentcode Bot commented Jun 18, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR remediates the remaining 2026-06-18 dogfood findings by preventing unauthenticated POSTs to user-only settings actions from crashing (500) and aligning a retention input with the server’s integer contract.

Changes:

  • Added requireUserAction/requireUserActions guards to return fail(401) when locals.user is missing (mirrors the existing admin action guard pattern).
  • Wrapped /dashboard/settings form actions (updateShareMode, regenerateToken, updateLogoPreference) with the new user guard.
  • Guarded /dashboard/settings page load to redirect unauthenticated requests while preserving a safe returnTo path.
  • Removed unsafe non-null assertions in the dashboard settings load path after guarding.
  • Updated admin system settings retention-days numeric input to enforce whole-number steps (step="1").
  • Added a unit regression test ensuring anonymous callers get 401s (not TypeErrors/500s) and authenticated callers pass through to normal validation.

Technical Notes: The dashboard settings change addresses the SvelteKit nuance that form actions don’t run the route’s redirecting load, and that page+layout loads can execute in parallel.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@engels74
engels74 merged commit d3984dd into main Jun 18, 2026
3 checks passed
@engels74
engels74 deleted the fix/dogfood-2026-06-18-issues branch June 18, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant