fix: don't scan or serve ebook symlinks that lead outside the library #40

Merged

jfro merged 3 commits from jtk/ebook-symlinks into main

2026-10-18 17:51:24 +00:00

jfro commented

2026-10-18 17:26:10 +00:00

Owner

Closes #27.

Problem: the scanner uses Path.wildcard and File.regular?, which follow symlinks. A link inside a library pointing at e.g. /etc/passwd (named *.epub) became an ebook record, and /ebooks/:id/download served it to any signed-in user. A directory link to somewhere else on the server worked the same way.

Fix:

New FuzzyCatalog.Ebooks.SafePath: realpath/1 resolves every symlink in a path (relative and absolute targets, directory links too, gives up after 40 links like Linux), and inside?/2 checks the resolved path is the resolved root or under it (so library2/ isn't inside library/).
ScanWorker drops discovered files that resolve outside the library dir and logs a warning with the count and one example path. Links that stay inside the library (and a library root that is itself a symlink) keep working.
EbookController.download checks again before sending, so a file swapped for a link after the scan isn't served. It refuses with a flash and logs a warning with the ebook id. Ebooks with no library (older ad-hoc scans) must not contain any symlink at all.

Tests: safe_path_test.exs (8: plain, file link out, relative .. escape, dir link out, inside link, symlinked root, prefix sibling, link loop), a scan test (outside link skipped, inside alias kept, warning logged), and 3 download tests (outside link refused, inside link served, no-library link refused). mix precommit: 499 passed.

Not covered: TOCTOU between the check and send_download (someone with write access to the library could race it). Libraries are admin-configured, so that's left as is.

Closes #27. **Problem:** the scanner uses `Path.wildcard` and `File.regular?`, which follow symlinks. A link inside a library pointing at e.g. `/etc/passwd` (named `*.epub`) became an ebook record, and `/ebooks/:id/download` served it to any signed-in user. A directory link to somewhere else on the server worked the same way. **Fix:** - New `FuzzyCatalog.Ebooks.SafePath`: `realpath/1` resolves every symlink in a path (relative and absolute targets, directory links too, gives up after 40 links like Linux), and `inside?/2` checks the resolved path is the resolved root or under it (so `library2/` isn't inside `library/`). - ScanWorker drops discovered files that resolve outside the library dir and logs a warning with the count and one example path. Links that stay inside the library (and a library root that is itself a symlink) keep working. - EbookController.download checks again before sending, so a file swapped for a link after the scan isn't served. It refuses with a flash and logs a warning with the ebook id. Ebooks with no library (older ad-hoc scans) must not contain any symlink at all. **Tests:** `safe_path_test.exs` (8: plain, file link out, relative `..` escape, dir link out, inside link, symlinked root, prefix sibling, link loop), a scan test (outside link skipped, inside alias kept, warning logged), and 3 download tests (outside link refused, inside link served, no-library link refused). `mix precommit`: 499 passed. Not covered: TOCTOU between the check and `send_download` (someone with write access to the library could race it). Libraries are admin-configured, so that's left as is.

jfro added 1 commit

2026-10-18 17:26:10 +00:00

fix: don't scan or serve ebook symlinks that lead outside the library

CI / test (pull_request) Successful in 43s

Details

d9ee8fde4e

New Ebooks.SafePath resolves symlinks (realpath). The scanner skips files
that resolve outside the library dir and logs how many; the download route
checks again so a file swapped for a link after the scan isn't served.

Closes #27

jfro reviewed

2026-10-18 17:45:36 +00:00

jfro left a comment

Review from Nye: APPROVE at d9ee8fd (posted by Upper Wolfjaw; Nye's token is read-only)

CI green. realpath resolves one component at a time from an already-resolved prefix; relative targets expand against the link's own dir; dir links and a symlinked root work; loops stop after 40 links and return false. The prefix check adds a trailing '/', so a sibling like /lib2 doesn't match /lib (tested), and a root of '/' works. Library deletes nilify ebooks.library_id, so get_library! can't raise for a missing library.

Non-blocking lows (filed separately): (1) Path.expand(target, current) expands a leading '~' in a link target to $HOME, while POSIX treats it literally; (2) check-then-send TOCTOU in EbookController.download (needs write access to the library). FYI: after a library is deleted, its ebooks under a symlinked mount won't download because of the no-library "no symlinks" rule; acceptable.

**Review from Nye: APPROVE** at d9ee8fd (posted by Upper Wolfjaw; Nye's token is read-only) CI green. realpath resolves one component at a time from an already-resolved prefix; relative targets expand against the link's own dir; dir links and a symlinked root work; loops stop after 40 links and return false. The prefix check adds a trailing '/', so a sibling like /lib2 doesn't match /lib (tested), and a root of '/' works. Library deletes nilify ebooks.library_id, so get_library! can't raise for a missing library. Non-blocking lows (filed separately): (1) Path.expand(target, current) expands a leading '~' in a link target to $HOME, while POSIX treats it literally; (2) check-then-send TOCTOU in EbookController.download (needs write access to the library). FYI: after a library is deleted, its ebooks under a symlinked mount won't download because of the no-library "no symlinks" rule; acceptable.

jfro added 1 commit

2026-10-18 17:46:37 +00:00

Merge branch 'main' into jtk/ebook-symlinks

CI / test (pull_request) Successful in 43s

Details

b014fe4933

jfro referenced this pull request

2026-10-18 17:46:55 +00:00

[low] Ebook symlinks: '~' in link targets, download TOCTOU #43

jfro added 1 commit

2026-10-18 17:49:06 +00:00

Merge branch 'main' into jtk/ebook-symlinks