SECURITY_ASSESSMENT.md

Security Assessment

This is a technical risk assessment of Questarr's most likely and impactful potential security problems. It is distinct from two other documents that cover adjacent ground and are linked rather than duplicated here:

• docs/SECURITY.md — vulnerability reporting process, deployment hardening checklist, and the collaborator access/ escalation policy.
• docs/SECRETS.md — a full inventory of every secret/ credential in the system, how each is stored, and how it's rotated.
• docs/VEX.md — exploitability assessments for known vulnerabilities (CVE/GHSA) reported against third-party components Questarr ships. This document covers first-party design risk; VEX covers third-party component vulnerabilities.

Read those first for the complete credential, access-control, and component-vulnerability picture; this document analyzes risk, not inventory.

Update policy: revisit this register whenever a PR touches authentication, SSRF protections, credential storage, rate limiting, or adds a new external integration/actor (see docs/ARCHITECTURE.md for the actor list).

Methodology

Each row below is a specific, source-verified risk area rated by likelihood and impact given Questarr's threat model — a self-hosted, typically single-or-few-user application, often run behind a home network or reverse proxy rather than exposed as a multi-tenant public service. Full STRIDE-style modeling was judged to be more process than a small project can keep current; this lighter table format matches the pragmatic tone of the rest of docs/.

Risk register

Risk area	Description	Likelihood	Impact	Mitigation / status	Reference
Session revocation	JWTs are valid for 7 days (server/auth.ts:79-82) with no server-side revocation list or logout endpoint. A stolen token remains usable for up to a week.	Low–Medium	Medium	Accepted risk for the self-hosted, single-tenant use case; no fix planned. Operators concerned about this can shorten the effective window by rotating JWT_SECRET, which invalidates all sessions immediately.	server/auth.ts:77-82
Integration API key blast radius	The /api/integration surface (added for the Playnite extension and other machine clients) accepts a long-lived API key instead of a JWT. A key never expires on its own and, unlike the 7-day JWT above, is revoked only by explicit user action.	Low	Low–Medium	Scope is deliberately narrow: requireAuthenticationForApi accepts a key only under /api/integration, never on key management (/api/api-keys) or any other route, so a leaked key cannot mint/revoke keys or reach the rest of the app. Only a SHA-256 hash is persisted (api_keys.key_hash), the raw key is shown once at creation, and a user can revoke a key at any time from Settings → Integrations. getApiKeyByHash is a single indexed lookup, not a scan, so this doesn't reintroduce the timing/scan concerns a per-key bcrypt comparison would.	server/auth.ts (authenticateApiKeyOrToken, generateApiKey), server/routes.ts (isIntegrationApiRequest), server/routes/api-keys.ts
CodeQL js/insufficient-password-hash on the integration API key hash	CodeQL's dataflow treats extractApiKey's output (the X-Api-Key/Bearer value) as a "password" and flags hashApiKey's plain crypto.createHash("sha256") as insufficient computational effort — the same class of advice that's correct for a human-chosen password, applied here to a 256-bit random token.	Low	Low	False positive — the query can't see that its input is crypto.randomBytes(32) output, not attacker-guessable. A slow KDF (bcrypt/scrypt/argon2/PBKDF2) defends against brute-forcing a low-entropy secret; brute-forcing a 256-bit key is infeasible regardless of hash speed, so it buys nothing here. It would also actively regress the design: getApiKeyByHash is a single indexed equality lookup on every authenticated integration request, which requires a deterministic, unsalted digest — a salted slow hash can't be looked up by index at all (each row's salt differs), forcing a full-table scan-and-compare per request instead. Same disposition as the js/request-forgery row below: dismissed as a false positive, not fixed, per docs/VULNERABILITY_MANAGEMENT.md §2.2 item 3.	server/auth.ts (hashApiKey, generateApiKey)
Integration API key sent over plaintext HTTP	authenticateApiKeyOrToken accepts a key over any connection the server itself accepts — there is no req.secure/trusted-proxy check requiring TLS specifically for API-key auth. On an HTTP deployment, a network observer on that segment could capture the key from the X-Api-Key/Authorization header and reuse it.	Low	Low–Medium	Consistent with the JWT row above and the CORS/CSP/HSTS row below, not a new gap: this project has never code-enforced TLS for any credential — that's an explicit, documented operator responsibility (docs/REVERSE_PROXY.md, docs/SECRETS.md), because plenty of legitimate self-hosted deployments run entirely on a trusted home LAN with no reverse proxy. Hard-blocking HTTP specifically for API keys (and not the JWT the browser UI already sends the same way) would be a new, inconsistent, surprising restriction rather than a real hardening step. The client (extensions/playnite-questarr/Questarr.psm1) is deliberately consent-based rather than a hard block: connecting over a plain http:// address always shows an explicit warning that the key travels in cleartext on every sync or request, worded more strongly for an address outside the caller's own local network (Test-QuestarrIsPrivateHost) since that traffic can cross the open internet — but the operator's own server is theirs to configure, so the choice to proceed is theirs too. A same-host redirect (a reverse proxy enforcing https://, or normalizing a trailing slash) is followed with the key reattached by hand; a redirect to any other host, or a same-host redirect that downgrades https:// to http://, is refused outright, closing the vector that could otherwise silently move the key to a different host or a weaker scheme than the one configured.	server/auth.ts (authenticateApiKeyOrToken), extensions/playnite-questarr/Questarr.psm1 (Invoke-QuestarrConnect, Invoke-QuestarrHttpRequest)
Concurrent duplicate game creation	POST /api/games/match-and-add, POST /api/integration/games/request, and other IGDB-match-and-insert call sites (shared via quickAddGameByTitle for the first two) check getUserGames for an existing igdbId/title match, then call storage.addGame — two concurrent requests for the same title from the same user can both pass the check and each insert a row, since there is no unique constraint on (userId, igdbId).	Low	Low	Pre-existing pattern, not introduced by the integration API: /api/games/match-and-add has worked this way since before this PR, and quickAddGameByTitle extends the exact same logic to a second caller rather than adding a new race. Worst case is a duplicate library entry a user can delete, not a security boundary crossed. A proper fix (a partial unique index on games(user_id, igdb_id) WHERE igdb_id IS NOT NULL plus ON CONFLICT handling in storage.addGame) would need to cover every insert path that can race the same way, not just the two quick-add routes — tracked as a follow-up rather than done piecemeal here.	server/game-quick-add.ts, server/routes.ts (POST /api/games/match-and-add), server/storage.ts (addGame)
JWT secret storage	The signing secret resolves env var → DB → auto-generated via crypto.randomBytes(64) (server/auth.ts:23-67). When auto-generated, it is persisted in the same SQLite database as application data — a DB compromise yields both data and the means to forge sessions.	Low	Medium	Mitigated by operator action: set JWT_SECRET explicitly in production (documented in docs/SECRETS.md §2 and docs/SECURITY.md).	server/auth.ts:23-67, server/config.ts:18-24
SSRF / DNS rebinding window	safeFetch resolves a hostname once and validates every returned IP, then pins HTTP requests to that IP. HTTPS requests cannot be IP-pinned (TLS SNI/certificate validation requires the original hostname), leaving a narrow window where a DNS record could change between the safety check and the actual connection.	Low	Medium	Partially mitigated — the window is narrow (single resolution immediately before the request) and the attack requires the operator to point an indexer/downloader/service URL at an attacker-controlled domain in the first place. Documented as a known limitation, not fixed further.	server/ssrf.ts:172-249
Private-network access allowed by design	isSafeUrl/isSafeIp/safeFetch default allowPrivate: true, so RFC1918 and loopback targets are reachable.	N/A (by design)	N/A	Accepted, intentional: indexers and download clients commonly run on the same LAN or even the same host as Questarr in a self-hosted deployment. Cloud-metadata and link-local ranges remain blocked unconditionally regardless of this setting.	server/ssrf.ts:4-18,19-22,86-170
CodeQL js/request-forgery findings in server/downloaders/*.ts	CodeQL flags every downloader call site where a fetch URL traces back to a user/indexer-supplied value (e.g. deluge.ts:284, qbittorrent.ts:91, rtorrent.ts:89, nzbget.ts:277, sabnzbd.ts:106,243, synology.ts:214,715, transmission.ts:178, utils.ts:62).	Low	Low	False positive — every listed call site already gates the URL through isSafeUrl() and/or routes the request through safeFetch() (same mechanism as the SSRF/DNS-rebinding row above) before any network call is made. CodeQL's default js/request-forgery query does not model this project-specific validator as a taint sanitizer, so it can't see the guard and keeps reporting the path as live. Dismissed in Code Scanning as "false positive" per docs/VULNERABILITY_MANAGEMENT.md §2.2 item 3.	server/ssrf.ts, server/downloaders/*.ts
Inconsistent input validation on auth endpoints	/api/auth/setup and /api/auth/login validate username/password with manual typeof checks instead of the express-validator/Zod pattern used consistently elsewhere in routes.ts.	Low	Low–Medium	Functionally adequate today (rejects non-string input, and login sits behind authRateLimiter), but inconsistent with the rest of the codebase and easier to get wrong on future edits. Tracked as a follow-up to migrate to the standard validator pattern.	server/routes.ts:282-299,361-368
Legacy plaintext credentials	Indexer apiKey and downloader username/password values written before AES-256-GCM encryption was introduced remain plaintext indefinitely — decryptCredential() detects the missing enc:v1: prefix and returns them unchanged, with no forced migration.	Low	Medium	Re-saving a credential (e.g. via PATCH /api/indexers/:id) re-encrypts it going forward. No background migration exists; operators with credentials configured before this feature shipped should re-save them once to force encryption.	server/credential-crypto.ts:98-108
Rate-limiting gaps	RSS feed creation has no limiter beyond the general 100 req/min-per-IP fallback. (POST /api/auth/setup was previously unlimited too — fixed: it now sits behind authRateLimiter, the same limiter as /api/auth/login.)	Low	Low	RSS creation is a low-value target for abuse; monitor for abuse reports rather than pre-emptively hardening further. /api/auth/setup no longer needs a mitigation note — it's rate-limited like /api/auth/login.	server/routes.ts:282-359, server/middleware.ts:33-57
Rate-limit bypass switch	DISABLE_RATE_LIMITS=true makes every limiter in server/middleware.ts skip, so the e2e harness (npm run dev:test, the E2E workflow) isn't throttled into 429s. It is honoured only when NODE_ENV is explicitly development or test; an unset NODE_ENV (which the server config treats as production) or production ignores it.	Low	Medium	Allowlist on NODE_ENV rather than a production denylist; unit tests pin that production and an unset value never disable the limits. The Docker image sets NODE_ENV=production (Dockerfile), so the switch is inert there even if set.	server/middleware.ts (rateLimitsDisabled), server/__tests__/middleware.test.ts
Download path handling	downloadPath fields reject values containing .. (server/middleware.ts:293-301,366-374,420-428) but do not otherwise normalize or allow-list paths — absolute paths, symlink traversal, null bytes, and Windows-style separators are not explicitly handled.	Low–Medium	Medium	Partial mitigation via the .. substring check. Full path normalization/allow-listing is tracked as a hardening follow-up.	server/middleware.ts:293-301,366-374,420-428
Supply-chain patch lag	package-lock.json is committed and Dependabot runs weekly for both npm and GitHub Actions. Major-version bumps were previously excluded from auto-PRs, which meant a security patch shipping only in a new major version wouldn't be proposed automatically. Fixed: the exclusion has been removed — Dependabot now opens PRs for major-version bumps too (as individual, ungrouped PRs so they still get dedicated review), see docs/DEPENDENCIES.md.	Low	Low	Resolved by removing the ignore rule in .github/dependabot.yml. Residual: major-version PRs still require manual review/merge, so a patch can sit open until reviewed — recommend periodic triage of open Dependabot PRs rather than letting them accumulate.	.github/dependabot.yml
CORS/CSP/HSTS posture	CORS is restricted to config.server.allowedOrigins with credentials: true (server/index.ts:24-29). Helmet's CSP allows 'unsafe-inline'/'unsafe-eval' in script-src only outside production (server/routes.ts:326-337); font-src/style-src are narrowed off helmet's https:-wildcard defaults to 'self' (plus data:/'unsafe-inline' respectively) since fonts/styles are all self-hosted. HSTS is enabled only when SSL is configured (server/routes.ts:365).	Low	Medium	Standard hardening already in place. Ensure ALLOWED_ORIGINS is set correctly and SSL is enabled in any production deployment so HSTS actually applies.	server/index.ts:24-29, server/routes.ts:323-376
Cross-Origin-Embedder-Policy not set	Helmet's crossOriginEmbedderPolicy (COEP) is left disabled (only crossOriginOpenerPolicy is set, server/routes.ts:361-366) -- a first DAST scan (dast.yml, 2026-07-14) flagged this as a WARN-level finding (ZAP rule 90004).	N/A (by design)	Low	Accepted risk, deliberately not enabled: COEP's require-corp mode blocks loading any cross-origin subresource that doesn't send a matching Cross-Origin-Resource-Policy/CORS header, which would break the IGDB and NexusMods game-cover/mod-image CDNs Questarr embeds and doesn't control. Suppressed with a documented reason in .zap/rules.tsv rather than left as a recurring unexplained scan finding.	server/routes.ts:361-366, .zap/rules.tsv
CSP style-src unsafe-inline	style-src keeps 'unsafe-inline' (server/routes.ts:361) -- several components (GameGrid.tsx, ShareDiscordDialog.tsx, GameDetailsModal.tsx, chart.tsx, progress.tsx) render real inline style="..." attributes. A DAST scan (dast.yml, 2026-07-14/15) flags this as a WARN-level finding (ZAP rule 10055, same plugin as the wildcard-directive check already fixed).	N/A (by design)	Low	Accepted risk, deliberately not removed: dropping 'unsafe-inline' from style-src would break rendering wherever those components set inline styles, not just tighten policy -- a real functional regression, not a hardening step. Matches Helmet's own CSP defaults, which don't gate style-src's 'unsafe-inline' on environment either. Suppressed with a documented reason in .zap/rules.tsv rather than left as a recurring unexplained scan finding.	server/routes.ts:361, .zap/rules.tsv
TLS certificate verification bypass scope (sabnzbd)	fetchInsecure()'s self-signed-cert fallback sets rejectUnauthorized: false whenever it detects any SSL error code, not just self-signed-specific ones — an expired cert or hostname mismatch on the configured sabnzbd URL would also fall through to a full verification bypass, not only the self-signed case it's meant for.	Low	Medium	Intentional, tested fallback for self-hosted sabnzbd instances with self-signed certs (downloaders_ssl_fallback.test.ts), but broader than necessary. Follow-up: narrow the trigger to DEPTH_ZERO_SELF_SIGNED_CERT/SELF_SIGNED_CERT_IN_CHAIN specifically, and/or make the insecure fallback opt-in per downloader rather than automatic.	server/downloaders/sabnzbd.ts:139-144
Container runs as root	The Dockerfile has no USER directive in either build stage, so the containerized process runs as root inside the container.	Low	Medium	No fix yet. Follow-up: add a non-root USER directive before ENTRYPOINT/CMD in the production stage. Doesn't affect host security on its own (still requires a separate container-escape vulnerability), but is unnecessary privilege for a Node process with no need for root.	Dockerfile
CI/supply-chain pinning gaps	.github/dependabot.yml has no cooldown period configured, and one GitHub Actions workflow step references an action by a mutable tag rather than a pinned commit SHA.	Low	Low	Hygiene gaps, not exploitable app-level issues on their own. Follow-up: add a cooldown window to the Dependabot config, and pin the flagged Action to a full commit SHA.	.github/dependabot.yml, flagged workflow
Search-engine indexing of exposed instances	Questarr instances are self-hosted and often carry personal library/collection data; an operator who exposes one to the public internet without an auth wall risked it being crawled and indexed by Google and other search engines, since no anti-indexing signal was sent. Fixed: every response now sends X-Robots-Tag: noindex, nofollow, /robots.txt disallows all crawlers, and client/index.html carries a <meta name="robots"> tag.	Low	Low–Medium	These signals only stop well-behaved crawlers going forward; they don't retroactively de-index an already-crawled instance (use Google Search Console's removal tool for that) or replace putting the instance behind authentication/a VPN, which remains the operator's responsibility.	server/app.ts, client/index.html
Root-folder delete scope expansion	Root folders let the library scanner discover games living outside the configured library root. The DELETE /api/games/:id?deleteFiles=true flow can now remove a discovered game's files even though they sit outside that root, if the root folder they live in has allowDelete: true (server/root-folders.ts isWithinDeletableRootFolder). This widens the set of on-disk paths Questarr's normal delete flow can remove files from, driven entirely by config rather than a fixed root.	Low	Medium	Opt-in and off by default (allow_delete defaults to false); toggled per-folder in Settings, not globally. Any JWT holder able to reach root-folder config could already register/repoint a root folder (same trust level as the existing indexer/downloader/library-root config, which is unscoped admin-level config in this single-or-few-user threat model — see docs/API.md Root Folders section), so this doesn't introduce a new privilege boundary, only a config-gated extension of an existing capability. Discovery/scanning itself remains read-only regardless of this setting. Deletion only ever targets a path strictly inside the library root or an opted-in root folder, never the root itself, so a game whose libraryPath points at a root cannot take every other game's files with it.	server/root-folders.ts, server/routes.ts (DELETE /api/games/:id)
zizmor cache-poisoning on actions/setup-node (ci.yml)	zizmor's static analysis flags every job in .github/workflows/ci.yml that sets cache: "npm" on actions/setup-node (secrets-scan, check-overrides, sca-scan, test, merge-test-reports, build) — code executing before the cache is saved could in principle poison it for a later, more-trusted run.	Low	Low	Accepted risk, suppressed inline (# zizmor: ignore[cache-poisoning]) rather than splitting cache restore/save by trust: GitHub Actions caches are scoped per-branch, so a pull_request run cannot write into the cache a push/main run restores from, and every npm ci in this workflow already runs --ignore-scripts, so a compromised dependency can't run install/postinstall code to tamper with the cache before it's saved. Repo also has a single maintainer with push access. Follow-up considered and declined during PR #806 review.	.github/workflows/ci.yml
Unauthenticated Socket.IO channel	The Socket.IO server used to accept any connection and broadcast every event to it, including the live server log stream (logLine), notifications, and download/import progress, so anyone who could reach the port could read them without logging in.	Medium	Medium	Fixed for 1.5.0: io.use in server/socket.ts verifies the same JWT the REST API accepts (the httpOnly auth cookie, or a bearer token passed as auth.token for legacy sessions) and rejects the handshake otherwise. A cookie-authenticated handshake must also come from this server's own origin (its Host, or X-Forwarded-Host behind a reverse proxy) or one listed in ALLOWED_ORIGINS / APP_URL, since WebSockets bypass CORS and a same-site page could otherwise ride the cookie. Events are still broadcast to every authenticated socket, which matches the single-user design; per-user rooms remain tracked in #1081.	server/socket.ts, client/src/lib/socket.ts

Out of scope / already covered elsewhere

• Full credential inventory, rotation procedures, and per-secret storage detail: docs/SECRETS.md.
• Vulnerability disclosure process and repository/infrastructure access escalation policy: docs/SECURITY.md.
• Exploitability assessments for known vulnerabilities in third-party dependencies and the container base image (satisfies OSPS-VM-04.02): docs/VEX.md and the feed itself at security/vex/questarr.openvex.json.
• System actors and data-flow diagrams referenced throughout this register: docs/ARCHITECTURE.md.
• Formal attack-surface analysis (trust boundaries, high-risk data flows, per-integration trust table, unauthenticated-route inventory): docs/THREAT_MODEL.md, which satisfies the related OSPS-SA-03.02 requirement. This document (SECURITY_ASSESSMENT.md) satisfies OSPS-SA-03.01 and takes a risk-register view rather than duplicating that analysis.