Skip to content

fix(scanner): resolve scan roots to absolute paths - #105

Merged
Muawiya-contact merged 6 commits into
mainfrom
fix/absolute-scan-roots
Sep 7, 2026
Merged

fix(scanner): resolve scan roots to absolute paths#105
Muawiya-contact merged 6 commits into
mainfrom
fix/absolute-scan-roots

Conversation

@Muawiya-contact

@Muawiya-contact Muawiya-contact commented Sep 7, 2026

Copy link
Copy Markdown
Member

Closes #103.

What & why

Rules anchored at the filesystem root — /tmp/**, /var/log/** — are
anchored deliberately: that is what keeps /tmp out of
~/tmp/tax-return.pdf (#41). But jwalk builds every entry's path from
the root as it was handed in, so a relative root produced entries no
anchored pattern could match:

$ diskern scan /var/tmp --top 2
Reclaimable: 3.0 MB across 1 finding and 0 duplicate sets.
       3.0 MB  /var/tmp/build-9a2f/out.o
               matched rule temp-dirs: Temporary files...

$ cd /var && diskern scan tmp --top 2
Scanned 1 files.
Reclaimable: 0 B across 0 findings and 0 duplicate sets.

The walk found the file; the rules could not tell where it was. That is
a plausible-looking empty result rather than an error — the #82 failure
arriving by a different route, and worse under --json, where nothing
says the paths were relative.

Roots are now resolved in scanner::scan, so the CLI and the desktop
app both get it.

absolute(), not canonicalize(). No filesystem access, works on a
path that does not exist, and leaves symlinks alone — canonicalize
would rewrite /var/tmp to /private/var/tmp on macOS and scan
somewhere other than what was asked for. It leaves .. in place,
because a/../b is only b when a is not a symlink; that limit is
documented beside the code rather than papered over.

Side effect worth naming: every path the scanner yields is now
absolute, which settles the open question in #103 about
actions::quarantine recording a file's original location. A relative
original could not have been restored from a different working
directory.

Tests

  • scanner.rs: absolute_root resolves a relative root and leaves an
    absolute one untouched, including /var/tmp.
  • crates/diskern-cli/tests/relative_root.rs: drives the real binary
    with current_dir and . as the root, using a root-anchored rule.
    Changing the process working directory inside a test would race the
    other tests in the same binary, so this uses a child process instead.
    Unix only — a /-anchored pattern cannot match c:/... on Windows,
    so neither can the bug.

Verified against the unfixed scanner: the relative case fails, the
absolute control passes. A regression test that passes without the fix
is not one.

Validation

  • cargo fmt --all
  • cargo clippy -p diskern-core -p diskern-cli --all-targets -- -D warnings
  • cargo test -p diskern-core -p diskern-cli — 70 pass, 0 fail

Checklist

  • cargo fmt --all and the required core/CLI clippy validation are clean
  • The required core/CLI test suite passes
  • Commits are small and focused (one logical change each)
  • Doesn't weaken a safety principle (read-only scans, quarantine over deletion, deterministic verdicts)

Update. Two follow-ups from a self-review, each its own commit:

  • The new test file gated every item with #[cfg(unix)] but not its
    imports, so on Windows all four were unused. CI missed it — clippy
    runs on ubuntu, and the Windows test job treats them as warnings —
    but a Windows contributor running the clippy line CONTRIBUTING asks
    for would have got four errors from a file they had not touched. The
    file is gated at module level now.
  • Making roots absolute brought them into range of the exclude list,
    which is correct, but the walk then dropped every entry and returned
    an empty report. $XDG_RUNTIME_DIR lives under the /run exclude
    and holds real caches, so a scan there came back looking like a clean
    disk. That is the same wrong answer as diskern scan reports a clean, empty result for a path that doesn't exist #82 and Anchored rules never match when the scan root is relative #103, so the scan now
    names the exclude instead of reporting nothing.

The rules are written against absolute paths, and the ones anchored at
the filesystem root are anchored deliberately: `/tmp/**` stops there so
it cannot reach `~/tmp/tax-return.pdf`, which is what #41 was about.

jwalk builds every entry's path from the root as it was handed in, so
`diskern scan tmp` from `/var` produced `tmp/systemd-private/x` and no
anchored pattern could match it. The walk found the files and the rules
could not tell where they were, so the scan printed "Scanned 1 files"
and then nothing to clean — a plausible empty result for a directory
full of matches, which is the #82 failure arriving by another route.

absolute() rather than canonicalize(): no filesystem access, works on a
path that does not exist, and leaves symlinks alone. canonicalize would
rewrite /var/tmp to /private/var/tmp on macOS and scan somewhere other
than what was asked for. It leaves `..` in place, because a/../b is only
b when a is not a symlink; that limit is written down beside the code.

Every path the scanner yields is now absolute, which also settles what
`actions::quarantine` records as a file's original location — a
relative original could not be restored from a different directory.
Changing the process working directory inside a test would race every
other test in the same binary, so this drives the installed binary with
`current_dir` instead and asks for `.` as the root.

The rule it uses is anchored at the filesystem root, like the shipped
`/tmp/**`, because an unanchored pattern matches either way and would
pass with the bug still present. Verified against the unfixed scanner:
the relative case fails, the absolute control passes.

Unix only. Windows paths normalize to `c:/...`, so a `/`-anchored
pattern cannot match there and neither can the bug.
First entry under Unreleased since v0.2.0 shipped.
@Muawiya-contact Muawiya-contact added bug Something isn't working engine diskern-core: scanner, rules, risk, graph, report cli diskern-cli: the terminal frontend labels Sep 7, 2026
`/var/tmp` is not an absolute path on Windows. It is rooted but carries
no drive, so `absolute` resolves it against the current one and returns
`D:\var\tmp` — correct behaviour, and what the Windows CI job caught.

The guarantee being tested is that a root the rules already understand
survives untouched, so each platform states it with a path that is
actually absolute there.
Every item carried `#[cfg(unix)]` but the imports did not, so on Windows
all four were unused. CI never saw it: the lint job runs clippy on
ubuntu, and the Windows test job treats unused imports as warnings.

A Windows contributor running the clippy line CONTRIBUTING asks for
would have got four errors out of a file they had not touched.
Making roots absolute brought them into range of the exclude list, which
is right — `is_within` matches on whole components, so `/run` covers
`/run/user/1000/cache`, and that is what the list is for. But the walk
then dropped every entry and the report came back empty, which reads as
a clean disk. `$XDG_RUNTIME_DIR` lives under `/run` and does hold caches,
so this is reachable rather than theoretical.

An empty report that means "I refused to look here" is the same wrong
answer as #82 and #103: plausible, silent, and indistinguishable from
the real thing. It names the exclude instead.

The exclude list is normalized once for the whole scan now rather than
per root, since the root check and the walk both need it.

@Muawiya-contact Muawiya-contact left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@Muawiya-contact
Muawiya-contact merged commit 0eeb2cf into main Sep 7, 2026
12 checks passed
@Muawiya-contact
Muawiya-contact deleted the fix/absolute-scan-roots branch September 7, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cli diskern-cli: the terminal frontend engine diskern-core: scanner, rules, risk, graph, report

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Anchored rules never match when the scan root is relative

1 participant