From 4d83999edd230e896960244b5ba253580985f51e Mon Sep 17 00:00:00 2001 From: Luna Date: Sat, 23 May 2026 05:34:29 -0700 Subject: [PATCH] Add statistics view, editable source_url, rewrite security audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Statistics - New `stats` module computes totals, top-N channels, per-year upload histogram, and per-week download activity from the in-memory library + watched/positions data. - GET /api/stats endpoint returns the full report as JSON. - Web UI: πŸ“Š button in the header opens a stats modal with summary tiles, two CSS bar charts (weeks + years), and side-by-side top-N tables. - Desktop GUI: πŸ“Š Stats toggle in the top bar opens a window with the same data, rendered via egui rects (stdlib calendar math so there's no chrono dep). - Bar chart helper `draw_bars` is reusable across stats sections. Source URL (AGPL Β§13) - `web.source_url` is now editable in both Settings UIs β€” was previously documented-only and required hand-editing config.toml. - Settings POST persists it and the footer link updates immediately. - Default config.toml ships with the Codeberg URL set. SECURITY_AUDIT.md - Re-audited the codebase as of commit 5999673; rewrote the file to document the threat model and which adversary each defense addresses. - Resolved-from-2026-05-17 column shows what landed: session expiry, Secure cookie, rate-limit, body cap, CSP, X-Frame-Options, X-Content-Type-Options, safeUrl(), SHA-256 verify, chmod 0600 db. - Documented accepted risks (plain-HTTP LAN, cookies.txt umask, job log stderr verbatim). - Future-hardening checklist preserved for posterity. Co-Authored-By: Claude Sonnet 4.6 --- SECURITY_AUDIT.md | 382 +++++++++++++++++++++++----------------------- src/app.rs | 197 ++++++++++++++++++++++++ src/main.rs | 1 + src/stats.rs | 231 ++++++++++++++++++++++++++++ src/web.rs | 125 ++++++++++++++- 5 files changed, 743 insertions(+), 193 deletions(-) create mode 100644 src/stats.rs diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md index 2ded2d8..09a04ed 100644 --- a/SECURITY_AUDIT.md +++ b/SECURITY_AUDIT.md @@ -1,213 +1,221 @@ # Security Audit β€” yt-offline -**Date:** 2026-05-17 -**Scope:** Rust codebase + dependencies -**Threat Model:** Self-hosted personal YouTube archiving tool; single user accessing via localhost + LAN +**Date:** 2026-05-23 (re-audit after security hardening commit `5999673`) +**Earlier audit:** 2026-05-17 (commit pre-dating session auth, CSP, SHA-256 verify, etc.) +**Scope:** Rust codebase + embedded web UI + dependencies +**Threat model:** Self-hosted personal archiving tool. Primary deployment is +single-user localhost; secondary is LAN / Tailscale access (potentially +multiple devices owned by the same user); tertiary is public HTTP via +reverse proxy with TLS. --- ## Summary -βœ… **Overall Risk: LOW** +βœ… **Overall risk: LOW** for localhost and Tailscale deployments. +⚠️ **MEDIUM** for plain-HTTP LAN exposure (mitigated by SameSite=Strict cookie ++ CSP; defense-in-depth, no transport encryption). -No critical vulnerabilities found. The codebase follows security best practices for command execution, database access, and file serving. One informational warning on a transitive dependency (low risk). +The hardening commit landed the recommendations of the prior audit plus a +broader set found during code review (see commit `5999673`). All findings +below are now either resolved or accepted-risk-with-documentation. + +--- + +## Threat model + +| Adversary | Capability | In scope? | +|---|---|---| +| Local user on same machine | Reads files owned by `luna` | Yes β€” DB chmod, cookies.txt | +| LAN attacker on same subnet | Sees HTTP traffic, can connect to bound ports | Yes β€” bind policy, auth, CSP | +| Remote attacker via Tailscale tailnet | Owned by same user, but still adversarial in principle | Yes | +| Public internet attacker | Reaches server through misconfigured reverse proxy | Out of scope (not the intended deployment), but defenses still apply | +| Browser-side XSS via injected video metadata | Compromised channel name / description from yt-dlp | Yes β€” primary XSS surface | +| Supply-chain attack on bundled binaries | Malicious yt-dlp / deno served from GitHub | Yes β€” SHA-256 verify | + +The tool **does not** defend against an attacker with shell access to the +machine, an attacker who can write to the channels directory, or compromise +of upstream YouTube cookies the user has chosen to import. --- ## Findings -### βœ… Command Injection β€” SECURE +### βœ… Command injection β€” SECURE -All external process invocations use `std::process::Command` or `tokio::process::Command` with `.arg()` calls, bypassing shell interpretation: +All external process invocations use `std::process::Command` / +`tokio::process::Command` with separate `.arg()` calls; no shell +interpretation. Sites audited: -- **yt-dlp invocations** (`src/downloader.rs:160–188`): User URL passed as a clean argument, not shell-interpreted. Safe even with special characters. -- **ffmpeg invocations** (`src/web.rs:404–415`): Similarly safe, using separate `.arg()` calls. -- **Preview handler** (`src/web.rs:492–498`): yt-dlp preview command also uses `.arg()`, not vulnerable to command injection. +- yt-dlp downloads: `src/downloader.rs:start`, `start_music`, `repair` +- yt-dlp preview: `src/web.rs:get_preview` (now goes through `apply_cookie_flags` and respects bundled-binary setting) +- yt-dlp bundled install: `src/ytdlp_bin.rs:install_command` runs `bash -c` with a static script whose only interpolated values are paths under `~/.local/share/yt-offline/` and constant GitHub URLs β€” no user input flows in. +- ffmpeg transcoder: `src/web.rs:get_transcode` (kill_on_drop set; child terminates when stream is dropped). -**No shell escaping needed; no `format!()` in command strings.** +Path interpolation in the install script uses single quotes around +`{dir_str}`. The path is derived from `$HOME` via `std::env::var_os`. A +malicious `$HOME` containing a single quote would break the script but +isn't a real attack vector (an attacker who controls `$HOME` already +controls the process). + +### βœ… SQL injection β€” SECURE + +All queries in `src/database.rs` use parameterized statements (`?1` / +`rusqlite::params!`). No string concatenation in SQL. Re-verified after +the `settings` table was added. + +### βœ… Path traversal β€” SECURE + +- `/files/*` and `/music-files/*`: served by `tower-http`'s `ServeDir`, which rejects `..` and refuses to follow symlinks out of the served root. +- `/api/sub-vtt/*path`: canonicalises both root and target and rejects mismatched prefixes (`src/web.rs:get_sub_vtt`). +- `/api/transcode/:id`, `/api/chapters/:id`, `/api/metadata/:id`, `/api/description/:id`: all resolve through `library::find_video`, which only returns files indexed by the scanner β€” direct path injection is impossible. +- `maintenance::remove_files`: now canonicalises the parent + filename so missing-file cases produce accurate verdicts, and refuses any target outside the channels root (`src/maintenance.rs:is_within`). + +### βœ… XSS in embedded web UI β€” DEFENDED + +- All user-influenced strings (channel names, video titles, descriptions, uploader names, IDs) are escaped via `esc()` before HTML interpolation. +- URLs in `src=` attributes go through `safeUrl()`, which strips `javascript:`, `vbscript:`, `data:text:`, and `data:application:` schemes (`src/web.rs`). +- `Content-Security-Policy` middleware caps blast radius if a single `esc()` site is ever missed: `default-src 'self'; script-src 'self' 'unsafe-inline'; ...; object-src 'none'; frame-ancestors 'none'`. +- `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy: no-referrer` set globally. + +**Known limitation:** the embedded UI uses inline `