Skip to content

fix(scan): stop the Linux descriptor path leaking into metadata and size - #849

Open
m4bard wants to merge 2 commits into
Listenarrs:canaryfrom
m4bard:fix/818-descriptor-path-leaks
Open

fix(scan): stop the Linux descriptor path leaking into metadata and size#849
m4bard wants to merge 2 commits into
Listenarrs:canaryfrom
m4bard:fix/818-descriptor-path-leaks

Conversation

@m4bard

@m4bard m4bard commented Aug 20, 2026

Copy link
Copy Markdown

Summary

On Linux a registration lease's metadata path is a /proc/{pid}/fd/{fd} descriptor link rather than a path to the file, and two things read it as though it were one. The probe's audio-extension guard rejects a candidate whose public half has no extension, so the embedded-metadata pass never runs; and FileInfo on the link reports the length of the link, a constant 64 bytes, rather than of the file.

Fixes #818 and #821. Rebased onto 03958c15 as promised on #818 once #819 landed.

The mechanism was worked out jointly with @kevinroberts on that thread rather than by me alone. He reproduced it independently on unpatched canary, withdrew his own suggested validation case once it turned out not to exercise the pass it was meant to test, and rewrote the issue around the root cause. The second of those is the reason the check below varies which agreements break instead of asserting one construction.

Changes

Fixed

  • AudiobookScanService.Metadata.cs passes new MetadataFileSource(pinnedMetadataFile.MetadataPath, candidate) instead of collapsing both halves onto the descriptor path. The read path stays the pinned generation; the public path keeps the real filename, so the guard sees an audio extension and ffprobe runs.
  • AudiobookFileService gains ResolveRegisteredLength, which takes the length from the pinned handle rather than stat'ing the visible path, and both size sites use it. Leases that do not expose generation-bound reads fall back to the metadata path, which for those callers is the public path.

The pinning guarantee is unchanged in both cases. The length comes from the handle the lease already holds, so it never consults the visible path, which is the property the lease exists to provide.

Testing

Two existing tests needed to follow the read onto the two-part source. Both mocked only the single-path overload, so a strict mock saw no matching setup once the scan routed through MetadataFileSource, the extractor returned nothing, and every file read as unreadable.

ScanAsync_MetadataReplacementAndRestore_ReadsPinnedFileGeneration needed a decision rather than a swap. Its callback reads ReadPath, because the point of the test is that the scan sees the original generation while the visible file is swapped underneath it. It now also asserts PublicPath is still the candidate with its extension intact, and that assertion is load-bearing: collapsing both halves onto the descriptor path fails it with the real path expected and /proc/<pid>/fd/<fd> observed. That is this defect, in a unit test, where before it was only reachable through a running container.

Full suite: 3,029 passed, 0 failed, 125 skipped, which is the baseline on 03958c15 unchanged, since this modifies two existing tests rather than adding any.

Reproduced before and after against ghcr.io/listenarrs/listenarr:canary and a build of this branch, with a public check that varies which agreements are broken rather than asserting a single case:

construction canary patched
claimed by path attribution claimed claimed
filename no longer matches the title claimed claimed
folder no longer matches the record's title claimed claimed
neither folder nor filename matches UNCLAIMED claimed
only the embedded tags identify it UNCLAIMED claimed

Both agreements have to break before anything reaches the pass, which is why the two single-mismatch rows are controls rather than filler: each rescues the other, so a check that only broke the filename would never exercise the path it was written to test.

Size, same book, 40 files:

on disk          223,520 bytes across 40 files
canary            2,560 bytes across 40 files     (40 x 64)
this branch     223,520 bytes across 40 files

What this does not fix

Worth being explicit, since the branch touches the neighbourhood of three other issues.

#542 is untouched. The book-level total still reads "not set" on both builds. This corrects the per-file rows; whatever should sum them is a separate defect and I have not looked at it here.

#822 is only partly addressed, and not by this. #819 added an early return recording MetadataEnrichmentSkippedLimitedStorage, which covers the limited-storage case. A probe refusal on storage that passes the generation check still commits Completed with nothing recorded and no diagnostic. That is what the matrix rows above show as UNCLAIMED with no diagnostic, and it stays true after this change for any other reason a probe might refuse.

I verified #819's early return does not shadow this: it returns at :25 and the patched call is at :73, so on storage with durable generation proof execution still reaches it.

One question

This fixes two filed issues in one diff because they share a cause and a call path, and the size sites were always part of the same patch. If you would rather have them as two PRs, say so and I will split it. I would rather ask than guess, since the split is cheap now and awkward after review has started.

m4bard and others added 2 commits August 20, 2026 12:46
The registration lease deliberately separates stable byte access (ReadPath)
from public media identity (PublicPath). On Linux the lease's metadata path
is a /proc/{pid}/fd/{fd} descriptor link, and two consumers treat it as if
it were the file.

The scan's embedded-metadata pass called the single-path overload of
ExtractFileMetadataAsync, which builds MetadataFileSource(path, path). The
probe guard tests the public half for an audio extension, a descriptor link
has none, and so the candidate was rejected before ffprobe ran. That pass is
the fallback for candidates path attribution could not claim, so on Linux a
correctly tagged file in an unrecognised folder shape could never be claimed
by any route.

The registered length was stat'ed from the same descriptor path. Stat on the
link reports the length of the link rather than of its target, a constant 64
bytes, so every registered file on Linux recorded Size = 64. Reading the
length from the pinned handle keeps the lease's generation guarantee, since
it never consults the visible path.

Refs Listenarrs#818
Both of these mocked only the single-path overload of ExtractFileMetadataAsync, so
once the scan routes through MetadataFileSource the strict mock saw no matching
setup, the extractor returned nothing, and every file read as unreadable. That
turned a passing suite into two failures that looked like behaviour regressions
and were not.

ScanAsync_CaseDistinctMetadataFolders_RemainConflicting just needed the overload.

ScanAsync_MetadataReplacementAndRestore_ReadsPinnedFileGeneration needed the
overload plus a decision about which half of the source its callback reads. It
reads ReadPath, because the point of the test is that the scan sees the original
generation even while the visible file is swapped underneath it.

It now also asserts the other half. PublicPath must still be the candidate as a
person sees it, extension included, because on Linux ReadPath is a /proc descriptor
link with no extension and anything deriving media identity from it loses the
extension entirely. Confirmed load-bearing by collapsing both halves onto the
descriptor path: the assertion fails with the real path expected and
/proc/<pid>/fd/<fd> observed, which is the defect this branch exists to fix,
previously only demonstrable against a running container.
@m4bard
m4bard requested a review from a team August 20, 2026 18:07
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.

Linux: metadata fallback passes a /proc fd path to ffprobe, so files not claimed by path attribution can never be indexed

1 participant