yolo/arr
3
0
Fork 0
Code Issues 12 Pull Requests Actions Packages Projects Releases Wiki Activity

A root already stored with a trailing slash cannot be edited #244

New Issue
Closed
opened 2026-08-25 11:14:56 +01:00 by naps62-yolo · 0 comments
naps62-yolo commented 2026-08-25 11:14:56 +01:00
Owner
Copy Link
Copy Source

#243 normalised the incoming path but compares it against current.path read
raw from the database, and nothing normalises the rows that are already stored.
So it fixed the half that cannot bite and left the half that can.

crates/arr-api/src/roots.rs:190 computes normalize_path(&input.path) and
:197 tests path == current.path.

Failure scenario for a root stored as /mnt/media/tv/main/ — creatable through
the API at any point before #243, when the code was input.path.trim():

  1. The operator opens settings and saves a change to the policy. The payload
    carries the path unchanged.
  2. normalize_path strips the slash, so the value no longer equals
    current.path and update takes the relocation branch.
  3. path_is_free passes — it excludes the row's own id.
  4. relocate_files plans /mnt/media/tv/main/Dune → /mnt/media/tv/main/Dune,
    the same directory. Source present, destination present, so the pre-check
    returns 409 the destination already has '/mnt/media/tv/main/Dune'.

No payload escapes it: sending the path back exactly as stored, slash included,
normalises to the same value and takes the same branch. That root can never be
edited again — not its policy, kind or audience — without hand-editing the
database.

Same root cause, smaller blast radius: path_is_free and the uniqueness check
compare a normalised value against unnormalised stored ones, so /mnt/media/x
and /mnt/media/x/ can coexist as two roots for one directory.

Scope:

  • Compare like with like — normalise current.path before the comparison, and
    use the normalised value when handing the old root to relocate_root, so a
    trailing slash cannot produce a doubled separator in a planned path.
  • A new migration (0031 or above — never edit an applied one) that
    normalises existing roots.path values. Guard it: if normalising one row
    would collide with another row that already holds the stripped path, leave it
    alone rather than failing the migration. A migration that cannot apply stops
    the daemon booting, which is worse than the bug it fixes.

Also fold in, same file, from the same review: undo() records one PathBuf
in created_root, but create_dir_all may materialise several levels. Moving
a root to /mnt/media-v2/tv/kids when /mnt/media-v2/tv does not exist creates
two directories and a failed move removes only the leaf. Cosmetic, but it is the
same "clean up what this request created" intent.

Depends on #241, which is rewriting roots::update for the policy change. Do
not start this until #241 has landed, or the two will conflict in that function.

#243 normalised the *incoming* path but compares it against `current.path` read raw from the database, and nothing normalises the rows that are already stored. So it fixed the half that cannot bite and left the half that can. `crates/arr-api/src/roots.rs:190` computes `normalize_path(&input.path)` and `:197` tests `path == current.path`. Failure scenario for a root stored as `/mnt/media/tv/main/` — creatable through the API at any point before #243, when the code was `input.path.trim()`: 1. The operator opens settings and saves a change to the *policy*. The payload carries the path unchanged. 2. `normalize_path` strips the slash, so the value no longer equals `current.path` and `update` takes the relocation branch. 3. `path_is_free` passes — it excludes the row's own id. 4. `relocate_files` plans `/mnt/media/tv/main/Dune` → `/mnt/media/tv/main/Dune`, the same directory. Source present, destination present, so the pre-check returns `409 the destination already has '/mnt/media/tv/main/Dune'`. No payload escapes it: sending the path back exactly as stored, slash included, normalises to the same value and takes the same branch. **That root can never be edited again** — not its policy, kind or audience — without hand-editing the database. Same root cause, smaller blast radius: `path_is_free` and the uniqueness check compare a normalised value against unnormalised stored ones, so `/mnt/media/x` and `/mnt/media/x/` can coexist as two roots for one directory. Scope: - Compare like with like — normalise `current.path` before the comparison, and use the normalised value when handing the old root to `relocate_root`, so a trailing slash cannot produce a doubled separator in a planned path. - A **new** migration (0031 or above — never edit an applied one) that normalises existing `roots.path` values. Guard it: if normalising one row would collide with another row that already holds the stripped path, leave it alone rather than failing the migration. A migration that cannot apply stops the daemon booting, which is worse than the bug it fixes. Also fold in, same file, from the same review: `undo()` records one `PathBuf` in `created_root`, but `create_dir_all` may materialise several levels. Moving a root to `/mnt/media-v2/tv/kids` when `/mnt/media-v2/tv` does not exist creates two directories and a failed move removes only the leaf. Cosmetic, but it is the same "clean up what this request created" intent. Depends on #241, which is rewriting `roots::update` for the policy change. Do not start this until #241 has landed, or the two will conflict in that function.
naps62-yolo added this to the Feedback pass 2 milestone 2026-08-25 11:14:56 +01:00
naps62-yolo added the area/apidifficulty/moderatetype/bug labels 2026-08-25 11:14:56 +01:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
arr-api
 area/ci
CI and test harness
 area/compat
arr-compat
 area/core
arr-core
 area/daemon
arr-daemon
 area/db
arr-db
 area/dl
arr-dl
 area/indexer
arr-indexer
 area/infra
workspace and tooling
 area/meta
arr-meta
 area/parse
arr-parse
 area/probe
arr-probe
 area/subs
arr-subs crate: providers, extraction, translation, sync
 area/web
web/
 difficulty/easy
bounded, obvious approach, few files
 difficulty/hard
subtle correctness or cross-cutting
 difficulty/moderate
multiple files, judgement, an interface to design
 difficulty/trivial
one file, mechanical, no design decisions
 phase/1-skeleton
workspace, CI, config, database, empty API and SPA
 phase/2-logic
parsing and policy engine, pure, no network
 phase/3-sourcing
TMDB and Prowlarr, read-only
 phase/4-movies
movies end to end
 phase/5-ui
search, buckets, library views, queues
 phase/6-tv
seasons, episodes, tracking, derived status
 phase/7-people
owner tags and notifications
 phase/8-compat
Jellyseerr shim
 phase/9-subtitles
build order: subtitles, DESIGN.md §15
 type/bug
type/chore
type/feature
type/test
No labels area/api difficulty/moderate type/bug
Milestone
No items
No Milestone Feedback pass 2
Projects
Clear projects
No projects
Assignees
Clear assignees
hermes (Hermes (agent)) naps62 (Miguel Palhas) naps62-yolo (naps62-yolo (agent))
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: yolo/arr#244