Skip to content

Auth Fixes - #32

Merged
umeraamir09 merged 2 commits into
mainfrom
auth/Auth-fixes
Aug 9, 2026
Merged

umeraamir09 merged 2 commits into
mainfrom
auth/Auth-fixes

Conversation

@umeraamir09

Copy link
Copy Markdown
Owner

This pull request updates the image proxy and authentication logic for the Jellyfin API and adjusts the proxy middleware to allow unauthenticated access to certain image endpoints. The main goal is to permit public image requests for specific routes, likely to improve compatibility with proxies or public deployments.

API authentication changes:

Proxy middleware updates:

  • Updated the proxy logic in src/proxy.ts to allow unauthenticated access to image proxy routes, including /api/jellyfin/image, /api/jellyfin/chapter-image, /api/jellyfin/trickplay, /api/radarr/MediaCover, and /api/sonarr/MediaCover.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request adjusts authentication and proxy middleware behavior to allow unauthenticated access to specific “image proxy” API routes (primarily Jellyfin image/artwork endpoints), likely to support public/proxied deployments without requiring a user session for artwork retrieval.

Changes:

  • Allowed unauthenticated access to multiple image-related API routes in the proxy middleware (src/proxy.ts).
  • Removed the session requirement from the Jellyfin image route handler so it can serve images without a logged-in session (src/app/api/jellyfin/image/[id]/route.ts).

Reviewed changes

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

File Description
src/proxy.ts Expands the unauthenticated allowlist to include Jellyfin/Radarr/Sonarr image proxy routes.
src/app/api/jellyfin/image/[id]/route.ts Removes session gating for Jellyfin item image fetches so artwork can be retrieved publicly.
Suppressed comments (1)

src/app/api/jellyfin/image/[id]/route.ts:24

  • Since this endpoint is now reachable without a session, unbounded width/height query params can be used to force expensive upstream resizes (or very large responses). Consider clamping to sane max values and requiring positive finite numbers before forwarding them to Jellyfin.
    const { searchParams } = new URL(req.url)
    const rawType = searchParams.get("type") || "Primary"
    const type = ALLOWED_TYPES.has(rawType) ? rawType : "Primary"
    const width = searchParams.get("width")
    const height = searchParams.get("height")

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/proxy.ts
Comment on lines 48 to 57
if (
pathname.startsWith("/_next") ||
pathname.startsWith("/api/auth/login") ||
pathname.startsWith("/api/jellyfin/image") ||
pathname.startsWith("/api/jellyfin/chapter-image") ||
pathname.startsWith("/api/jellyfin/trickplay") ||
pathname.startsWith("/api/radarr/MediaCover") ||
pathname.startsWith("/api/sonarr/MediaCover") ||
PUBLIC_PATHS.some((path) => pathname === path || pathname.startsWith("/public"))
) {
Comment on lines 3 to 5
import { authenticate } from "@/lib/jellyfin"
import { getSession } from "@/lib/auth"
import { env } from "@/lib/env"

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review complete. Full diff analyzed (2 files, +6/-9) against the codebase and AGENTS.md conventions.

OpenCode Severity Report - PR #32

Summary

FULL REVIEW MODE: CRITICAL: 1 | HIGH: 1 | MEDIUM: 1 | LOW: 2 | INFO: 1

Issues

CRITICAL

  • [src/proxy.ts:54-55 + src/app/api/radarr/MediaCover/[...path]/route.ts:18 + src/app/api/sonarr/MediaCover/[...path]/route.ts:18] - Unauthenticated path traversal to arbitrary Radarr/Sonarr API endpoints. This PR adds /api/radarr/MediaCover and /api/sonarr/MediaCover to the middleware's unauthenticated allowlist. Those route handlers build targetUrl = ${BASE}/api/v3/MediaCover/${path.join("/")} with no validation of path segments (they accept any [...path] catch-all), then forward the request with the server's X-Api-Key header. A request like /api/radarr/MediaCover/../../../api/v3/system/status (segments arrive URL-decoded) normalizes to ${RADARR_BASE}/api/v3/system/status and is served with the API key attached, giving any unauthenticated client access to the full Radarr/Sonarr API (system status, config, download clients, etc.). The traversal was latent before this PR but was previously blocked by the auth-gated middleware (401 for anonymous); the allowlist entry removes that gate. Suggested fix: reject any segment containing . (or ..), validate each segment against ^[A-Za-z0-9_-]+$, cap segment count, and/or keep these routes behind auth.

HIGH

  • [src/app/api/jellyfin/image/[id]/route.ts:10 + src/proxy.ts:51-53] - Jellyfin media library catalog is now unconditionally public. The getSession() requirement (and the ALLOW_PUBLIC_IMAGES opt-in escape hatch) is removed entirely, and /api/jellyfin/image, /api/jellyfin/chapter-image, and /api/jellyfin/trickplay are whitelisted in the middleware. Any anonymous visitor can now enumerate valid Jellyfin item IDs and retrieve posters/backdrops/chapter frames/trickplay tiles, revealing the entire library catalog (title art, content availability) — including for deployments that previously relied on production auth being on by default. This is likely intended for the public /login poster wall (/api/login-covers → LoginPosterWall), but it also permanently removes the ability to keep the library private. Suggested fix: at minimum keep an env-controlled toggle (e.g., re-instate ALLOW_PUBLIC_IMAGES-style opt-in) so public exposure is a deliberate per-deployment choice, and document it in README/.env.example.

MEDIUM

  • [src/proxy.ts:51-55] - Dead/undocumented configuration: the ALLOW_PUBLIC_IMAGES env var is now referenced nowhere (grep returns zero matches), so any deployment that set it has silently lost its privacy control with no migration path or doc update. README/.env.example never mention these image routes being public. Suggested fix: either re-add the toggle or explicitly document that image endpoints are now always public.

LOW

  • [src/app/api/jellyfin/image/[id]/route.ts:16] - Item-ID regex ^[a-zA-Z0-9._-]+$ permits ..; since the ID is interpolated into /Items/${id}/Images/${type}, an ID of .. normalizes the upstream URL to ${baseUrl}/Images/Primary. Impact is minimal (fixed prefix), but restricting to ^[a-zA-Z0-9]+$ (or rejecting ..) would be safer now that the route is public.
  • [src/proxy.ts:51-55] - pathname.startsWith("/api/jellyfin/image") and /api/radarr/MediaCover prefix matches are broad; a future route sharing these prefixes (e.g., a /api/jellyfin/image-preview or /api/radarr/MediaCoverExport endpoint) would silently become unauthenticated too. Prefer exact path patterns or a dedicated allowlist constant with comments.

INFO

  • [src/app/api/jellyfin/chapter-image/[...slug]/route.ts:14 + src/app/api/jellyfin/trickplay/[...slug]/route.ts:16] - Making chapter-image and trickplay public exposes video frame thumbnails/tiles of library items to anonymous users. Acceptable under the "public images" goal, but be aware it is content disclosure, not just poster art — consider keeping trickplay behind auth if frame privacy matters.

github run

- Added @base-ui/react, ogl, shadcn, and tw-animate-css to dependencies.
- Updated globals.css to include new styles and imports.
- Implemented Scanner component for a full-screen WebGL shader background in the login page.
- Enhanced API routes for jellyfin, radarr, and sonarr to include session validation.
- Updated proxy logic to allow public image access based on environment variables.
- Created components.json for shadcn configuration.
- Added validation for media cover paths in API routes.
@umeraamir09
umeraamir09 merged commit 0e8fbd1 into main Aug 9, 2026
1 check failed
@umeraamir09
umeraamir09 deleted the auth/Auth-fixes branch August 9, 2026 16:27
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.

2 participants