Skip to content

core: an unreadable listing silently turns the whole naming layer off, and raw names are written #89

Description

@otsobide

What happens

plan_for swallows a listing error and carries on with the identity plan:

let Ok(names) = list_entries(archive, algorithm) else {
    return Ok(NamePlan::identity());
};

The comment justifies it well: "the extractor is about to open the same archive and fail on it in its own vocabulary", and replacing that message with a worse one would be unhelpful. What the reasoning misses is that the extractor fails while streaming, after it has already written every entry that precedes the fault. So an archive whose listing pass dies part way gets no rewriting, no NoReplacement refusal, no collision check and no device or trailing adjustment. One unreadable listing turns the entire feature off from inside the archive.

Both front ends widen it: apps/cli/src/lib.rs and apps/desktop/src-tauri/src/commands.rs call unwritable_names_with(...).unwrap_or_default(), so the user is told the archive has nothing to answer for.

Reproduction

The same two entries, twice, under NameRules::windows() with no replacements. The second archive is identical except that its end-of-archive marker is replaced with garbage.

-- intact listing
   report        : Ok(1)
   extract_with  : Err("the archive entry \"notes.txt:hidden\" contains ':', which thi...")
   ON DISK       : []

-- listing that cannot be read
   report        : Err("numeric field did not have utf-8 text ...")
   extract_with  : Err("numeric field did not have utf-8 text ...")
   ON DISK       : ["notes.txt:hidden", "second.txt"]

Why it matters

On Windows, notes.txt:hidden is not a file name, it is the hidden alternate data stream of notes.txt. The write succeeds, the bytes land where no listing shows them, and the user was told there was nothing to answer for. That is issue #63's harm performed without consent, and the guarantee in extract_with's own doc comment (the unrecoverable answers "leave the output directory as they found it") is void.

It is easy to reach for tar specifically, because list_tar_entries walks all headers before extraction starts while extract_tar_planned writes as it walks: appending one bad 512 byte header after the good entries is enough. zip and 7z are not exposed the same way, since their listing and their extractor both fail on the same central directory or header parse and so fail together.

Suggested direction

The pre-flight pass should not be the only thing standing between a raw name and the disk. Validating each entry name against the rules at the write site, whatever the plan says, makes the guarantee hold even when the plan degraded. That is defence in depth rather than a redesign, and it keeps the good reason the listing error is swallowed in the first place.

Found while auditing the Windows CI failure that led to #88.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions