Skip to content

fix(filesystem): say why a source file could not be pinned - #930

Open
m4bard wants to merge 4 commits into
Listenarrs:canaryfrom
m4bard:fix/source-capability-cause
Open

m4bard wants to merge 4 commits into
Listenarrs:canaryfrom
m4bard:fix/source-capability-cause

Conversation

@m4bard

@m4bard m4bard commented Sep 1, 2026

Copy link
Copy Markdown

Toward #890, and the same shape as #916.

FileMover.CheckAsync decides whether a downloaded file can be published. It runs before any destination is created and before FileMover attempts anything, so when it says no the import stops there. Its final catch (listenarr.infrastructure/FileSystem/FileMover.SourceCapability.cs:104-112) folds seven exception types into one fixed sentence:

catch (Exception exception) when (exception is
    IOException or UnauthorizedAccessException or Win32Exception
        or InvalidOperationException or NotSupportedException
        or PathTooLongException or System.Security.SecurityException)
{
    return FilePublicationSourceCapabilityResult.Unsupported(
        "The source file cannot be pinned to a durable physical generation and content proof.",
        FilePublicationSourceCapabilityFailureKind.Unavailable);
}

exception is bound and never read, and there is no logger call anywhere in that file.

Why nothing downstream recovers it

DownloadImportService.ResolvePublishableSourceProofAsync logs that reason at Warning and returns null (DownloadImportService.DirectoryOwnership.cs:277). The null becomes an ImportResult.ImportFailure reading "Unable to perform {action} on {source} to {destination}", the job records "Unable to import at least one file for the job (see the log entries)", and the download gets "See the log of job {id} for more information". All three point at text that never contains a cause.

That sentence is therefore the whole of what an operator has. #890 puts it plainly: "In logs there is nothing that could tell what is wrong."

The neighbours suggest an oversight rather than a policy

The catch immediately above, at :100-103, returns exception.Message. The sibling catch over almost the same exception family in FileMover.PathSafety.cs:82-92 logs the exception before returning. Both carry the cause; this one is the odd case out.

What changes

The catch logs the exception and puts its type and message into the reason.

Two details that turned out to matter. A Win32Exception constructed with a custom message keeps the errno in NativeErrorCode and out of Message, so the number is logged separately or it is lost. And when the cause is a symlinked ancestor the reason now names the segment, because the raw failure is an ENOTDIR from openat and that gives an operator nothing to act on.

The refusal itself is unchanged. CheckPublicationSource_LinkedAncestor_ReturnsUnsupported still passes, along with the rest of the suite.

How I know it is the right message

Running this build against an install whose imports had been failing for days produced, on the first attempt:

Source publication capability unavailable for <file>: Win32Exception:
Could not open a newly created pinned directory.

which was enough to find the cause in minutes. Before it, the same failure had been mistaken for a HardlinkCopy regression for a week.

There is a separate question about whether that particular cause should be refused at all. That is #929, not this pull request. This one is worth having either way: whatever is decided there, the other six exception types still need to say what they were.

Tests

Two, both [DirectoryLinkFact] as NativeTestWorkflowContractTests requires. They assert that the refusal still happens and only that the reason is actionable. Reverting the change fails one and leaves the other green.


Worked through with Claude Code at my direction. The claims above were checked by running them rather than by reading, and I reviewed this before posting.

CheckAsync catches seven exception types and returned one sentence naming none
of them, with the exception bound and never used and no logger call anywhere in
the file. A locked file, a permissions problem and a path reached through a
symlink were indistinguishable afterwards.

That gate refuses the import before any destination is created and before
FileMover attempts anything, so nothing further down produces a better message.
DownloadImportService logs only this reason string, which is why Listenarrs#890 reports
that the logs contain nothing explaining the failure.

Now logs the exception with its NativeErrorCode, which a Win32Exception keeps
out of Message when a custom message is supplied, and names the offending
segment when the cause is a symlinked ancestor.

The refusal itself is unchanged, and
CheckPublicationSource_LinkedAncestor_ReturnsUnsupported still passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
m4bard and others added 3 commits September 11, 2026 12:25
The new warning interpolated two attacker-influenced strings straight into
the log record. A download client picks the file name, and the file name is
what most of these exception messages quote, so a name containing a newline
could append a second, fabricated log line. Every other call site in this
directory already routes such text through LogRedaction.SanitizeText; these
two did not.

The native error code is now reported as absent rather than as zero for the
six exception types that carry no code. Zero is a real errno meaning success,
so logging it was a false reading rather than a missing one.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Both assertions in CheckAsync_PathThroughSymlinkedDirectory_NamesTheLink
matched literals from the format string, so a build that recorded an empty
cause passed them. The test now also requires the linked segment, which comes
from the ancestor walk, and an exception type name, which comes from the
caught exception.

Also drops two edits that changed nothing: a blank line added to
PinnedDirectoryCreation.Hierarchy.cs, which is otherwise untouched by this
branch, and a using directive the test project already declares globally and
which the build reported as IDE0005.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…formatter

Every consumer of the capability result renders Reason through
LogRedaction.SanitizeText, whose 200-character default cut the exception off the
end. With a realistic symlinked path the operator was left the fixed sentence
they already knew and lost both the cause and the instruction. Putting the cause
first means truncation costs the half that can be reconstructed from the code
rather than the half that cannot.

The cause is now formatted by ExceptionCause, the same helper the import result
uses to persist a failure into History, so the two records stop disagreeing
about what the cause is. It also gives this gate the inner chain it had no way
to reach while it formatted the outer exception alone.

The composition moves into ComposeUnsupportedReason so it can be tested without
provoking a real ENOTDIR, which is what let the ordering and the truncation be
pinned directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
m4bard added a commit to m4bard/Listenarr that referenced this pull request Sep 12, 2026
…reach it

ImportResult had grown a private DescribeWithCauses, and the source capability
gate on Listenarrs#930 had independently grown its own one-level version of the same
thing. The two disagreed, so a wrapped failure read as a chain in a History row
and as a single line in the capability log.

ExceptionCause lives in the domain, which references nothing, so the application
and infrastructure projects can both reach it. It formats and nothing else:
redaction and truncation stay with the caller, because LogRedaction is in the
application project. ImportResult now calls it, and Listenarrs#930 calls it in its own
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDr4ZwWYwhX8fAy26Vq5KD
m4bard added a commit to m4bard/Listenarr that referenced this pull request Sep 12, 2026
…reach it

ImportResult had grown a private DescribeWithCauses, and the source capability
gate on Listenarrs#930 had independently grown its own one-level version of the same
thing. The two disagreed, so a wrapped failure read as a chain in a History row
and as a single line in the capability log.

ExceptionCause lives in the domain, which references nothing, so the application
and infrastructure projects can both reach it. It formats and nothing else:
redaction and truncation stay with the caller, because LogRedaction is in the
application project. ImportResult now calls it, and Listenarrs#930 calls it in its own
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDr4ZwWYwhX8fAy26Vq5KD
(cherry picked from commit 7edf35f)
m4bard added a commit to m4bard/Listenarr that referenced this pull request Sep 14, 2026
…reach it

ImportResult had grown a private DescribeWithCauses, and the source capability
gate on Listenarrs#930 had independently grown its own one-level version of the same
thing. The two disagreed, so a wrapped failure read as a chain in a History row
and as a single line in the capability log.

ExceptionCause lives in the domain, which references nothing, so the application
and infrastructure projects can both reach it. It formats and nothing else:
redaction and truncation stay with the caller, because LogRedaction is in the
application project. ImportResult now calls it, and Listenarrs#930 calls it in its own
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDr4ZwWYwhX8fAy26Vq5KD
(cherry picked from commit 7edf35f)
m4bard added a commit to m4bard/Listenarr that referenced this pull request Sep 15, 2026
…reach it

ImportResult had grown a private DescribeWithCauses, and the source capability
gate on Listenarrs#930 had independently grown its own one-level version of the same
thing. The two disagreed, so a wrapped failure read as a chain in a History row
and as a single line in the capability log.

ExceptionCause lives in the domain, which references nothing, so the application
and infrastructure projects can both reach it. It formats and nothing else:
redaction and truncation stay with the caller, because LogRedaction is in the
application project. ImportResult now calls it, and Listenarrs#930 calls it in its own
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GDr4ZwWYwhX8fAy26Vq5KD
(cherry picked from commit 7edf35f)
m4bard added a commit to m4bard/Listenarr that referenced this pull request Sep 15, 2026
…reach it

ImportResult had grown a private DescribeWithCauses, and the source capability
gate on Listenarrs#930 had independently grown its own one-level version of the same
thing. The two disagreed, so a wrapped failure read as a chain in a History row
and as a single line in the capability log.

ExceptionCause lives in the domain, which references nothing, so the application
and infrastructure projects can both reach it. It formats and nothing else:
redaction and truncation stay with the caller, because LogRedaction is in the
application project. ImportResult now calls it, and Listenarrs#930 calls it in its own
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 7edf35f)
m4bard added a commit to m4bard/Listenarr that referenced this pull request Sep 15, 2026
…reach it

ImportResult had grown a private DescribeWithCauses, and the source capability
gate on Listenarrs#930 had independently grown its own one-level version of the same
thing. The two disagreed, so a wrapped failure read as a chain in a History row
and as a single line in the capability log.

ExceptionCause lives in the domain, which references nothing, so the application
and infrastructure projects can both reach it. It formats and nothing else:
redaction and truncation stay with the caller, because LogRedaction is in the
application project. ImportResult now calls it, and Listenarrs#930 calls it in its own
commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@m4bard
m4bard force-pushed the fix/source-capability-cause branch from 6154049 to 45af623 Compare September 15, 2026 18:35
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