Skip to content

fix(manual-import): authorize companion files against the root folder, not the book folder - #832

Open
m4bard wants to merge 2 commits into
Listenarrs:canaryfrom
m4bard:fix/bug12-companion-import-boundary
Open

fix(manual-import): authorize companion files against the root folder, not the book folder#832
m4bard wants to merge 2 commits into
Listenarrs:canaryfrom
m4bard:fix/bug12-companion-import-boundary

Conversation

@m4bard

@m4bard m4bard commented Aug 19, 2026

Copy link
Copy Markdown

Summary

A manual import with includeCompanionFiles: true currently drops every companion file and still reports success. The companion pass hands the ownership store the book folder as its managed boundary, and the authorizer requires the boundary to be a configured root folder rather than to be inside one, so every companion is refused. This selects the boundary the same way the primary audio file's own import already does.

Full write-up, measurements and the alternative I did not take are in #831.

Changes

Fixed

  • ManualImportCompanionImporter.ImportAsync selects the managed boundary with LibraryDirectoryOwnershipPlanning.SelectMostSpecificBoundary over the configured root folders, falling back to destinationResolution.BoundaryPath, instead of passing destinationRoot. destinationRoot is DetermineScanPath over the batch's destination paths, so on a single-book import it is the book folder, which is never a root.
  • A companion whose destination yields no managed boundary is skipped with a warning rather than throwing, so a companion can still be lost but it cannot take the import with it.
  • ImportAsync takes a rootFolders argument, supplied at its single call site in ManualImportController where rootFolders is already in scope.

The authorizer is untouched and keeps its current strictness. The companion is now authorized against the same root folder that the audio file it sits beside is already authorized against.

Testing

ManualImportCompanionImporterTests gains ImportAsync_ManagedBoundaryIsTheConfiguredRootFolderRatherThanTheBookFolder, which captures the boundary handed to EnsureCreatedHierarchyAsync and asserts it is the configured root. With destinationRoot restored it fails on the boundary (library/Author/Book against the expected library); with the change it passes.

It asserts the argument rather than the outcome deliberately. The four existing companion tests all mock ILibraryDirectoryOwnershipStore and match the boundary with It.IsAny<string>(), so the real authorizer never runs and none of them can tell the book folder from the root. All four pass unchanged either way, which is a fair explanation for how this shipped. They gain the new argument and nothing else.

Full suite on this branch: 3,010 passed, 0 failed, 125 skipped. The skips are environment-gated and sit in areas this does not touch.

Reproduced end to end before and after against ghcr.io/listenarrs/listenarr:canary. On canary the audio arrives alone, with four boundary is not a configured root folder refusals and completed with 0 imported companion file(s). On a build of this branch the same run reports four imported, zero refusals, and the blacklisted decoy still filtered. The check is public, in the test-data repo linked from the issue.

Notes

The alternative was to relax LibraryDirectoryOwnershipBoundaryAuthorizer so a boundary inside a root is accepted. I left it out because it widens an authorization check, and because it would change the rename and download-import paths too, neither of which is failing. If you would rather converge that way, say so and I will redo it.

One thing the reproduction ruled out, since it is the natural first guess: where the source folder sits makes no difference. The same import with the source outside every configured root folder and again inside one gives an identical result, before and after.

m4bard and others added 2 commits August 19, 2026 15:03
… the book folder

ManualImportCompanionImporter passed destinationRoot to EnsureCreatedHierarchyAsync as the
managed boundary. destinationRoot is DetermineScanPath over the batch's destination paths,
so for a single-book import it is the book folder. LibraryDirectoryOwnershipBoundaryAuthorizer
matches the boundary against configured root folders by equivalence rather than containment,
so it refused every companion with 'The requested directory boundary is not a configured root
folder'. The exception was caught and logged as a warning, so the import reported success and
silently dropped the companions.

Select the boundary the way the primary audio file's import already does, via
LibraryDirectoryOwnershipPlanning.SelectMostSpecificBoundary over the configured roots, falling
back to the destination resolution's boundary path. The authorizer is unchanged and keeps its
existing strictness; the companion is now authorized against the same root as the audio file it
sits beside.
…p store

The four existing companion tests all mock ILibraryDirectoryOwnershipStore with
It.IsAny<string>() for the boundary, so the real authorizer never runs and nothing in the
suite could tell the book folder from the root folder. All five pass unchanged either way,
which is why this shipped.

ImportAsync_ManagedBoundaryIsTheConfiguredRootFolderRatherThanTheBookFolder captures the
boundary argument instead of the outcome and asserts it is the configured root. It fails
with destinationRoot restored ("library/Author/Book" against the expected "library") and
passes with the fix. The four existing call sites gain the new rootFolders argument.
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.

1 participant