ValidatePath ran only on the write side (Builder). Untrusted .mf
files were deserialized with no path checks, so an entry like ../../etc/passwd flowed into filepath.Join against the checker's base
path, letting mfer check stat and read outside basePath.
This validates every entry path as the manifest loads, in deserializeInner, so all consumers benefit — Checker today and any
future restore/extract path — instead of each call site re-checking. The
manifest is rejected on the first offending entry (never silently
dropped, which could hide files from a check) with a new package-level errInvalidManifestPath sentinel wrapped via %w; the error names the
path. A comment at the checker join site records that the invariant is
enforced upstream.
Tests: a table-driven load-time test rejecting parent/interior
traversal, absolute, backslash, double-slash, and empty paths, plus a
valid round-trip. A non-UTF-8 path is refused at load by the proto3
string decoder before validation runs; the test asserts that, and a
direct ValidatePath test pins the UTF-8 rule.
make lint/make fmt panic locally on the known older-golangci-lint/Go
toolchain quirk; the Docker lint stage is the cited authority, and
markdown was formatted via the repo's prettier step directly.
A concurrent process intermittently rewrote unrelated internal/cli
files during host make runs; restored to HEAD before staging, so the
commit holds only the four intended files.
Model: opus-4-8
Closes https://git.eeqj.de/sneak/mfer/issues/61
`ValidatePath` ran only on the write side (`Builder`). Untrusted `.mf`
files were deserialized with no path checks, so an entry like
`../../etc/passwd` flowed into `filepath.Join` against the checker's base
path, letting `mfer check` stat and read outside `basePath`.
This validates every entry path as the manifest loads, in
`deserializeInner`, so all consumers benefit — `Checker` today and any
future restore/extract path — instead of each call site re-checking. The
manifest is rejected on the first offending entry (never silently
dropped, which could hide files from a check) with a new package-level
`errInvalidManifestPath` sentinel wrapped via `%w`; the error names the
path. A comment at the checker join site records that the invariant is
enforced upstream.
Tests: a table-driven load-time test rejecting parent/interior
traversal, absolute, backslash, double-slash, and empty paths, plus a
valid round-trip. A non-UTF-8 path is refused at load by the proto3
string decoder before validation runs; the test asserts that, and a
direct `ValidatePath` test pins the UTF-8 rule.
Verification: `docker build .` with `--no-cache` (lint, fmt-check, test,
build) passed.
Disclosures:
- `make lint`/`make fmt` panic locally on the known older-golangci-lint/Go
toolchain quirk; the Docker lint stage is the cited authority, and
markdown was formatted via the repo's prettier step directly.
- A concurrent process intermittently rewrote unrelated `internal/cli`
files during host `make` runs; restored to HEAD before staging, so the
commit holds only the four intended files.
Model: opus-4-8
Accepted judgement call: an invalid-UTF-8 entry path is refused at load by the proto3 string decoder before ValidatePath runs, so that one case is rejected loudly at load time but its error does not name the offending path; the UTF-8 rule itself is pinned directly by a ValidatePath unit test, which is adequate.
Disclosure: the branch is one commit behind next and conflicts only in TODO.md (both units add a Completed Steps entry at the top), resolved by keeping both — not a review finding; the rebase push is handled at merge.
Model: opus-4-8
Review passed.
Accepted judgement call: an invalid-UTF-8 entry path is refused at load by the proto3 string decoder before `ValidatePath` runs, so that one case is rejected loudly at load time but its error does not name the offending path; the UTF-8 rule itself is pinned directly by a `ValidatePath` unit test, which is adequate.
Disclosure: the branch is one commit behind `next` and conflicts only in `TODO.md` (both units add a Completed Steps entry at the top), resolved by keeping both — not a review finding; the rebase push is handled at merge.
Model: opus-4-8
Untrusted .mf files were parsed with no path validation, so an entry
like ../../etc/passwd flowed into filepath.Join against the checker's
base path. ValidatePath ran only on the write side (Builder). Enforce it
on every entry in deserializeInner, so all consumers — Checker now, any
restore/extract path later — are protected. Reject the whole manifest on
the first offender rather than dropping entries, which could hide files
from a check; failures wrap a new errInvalidManifestPath sentinel and
name the path.
A path that is not valid UTF-8 is refused at load time by the proto3
string decoder before validation runs; the test covers that plus the
traversal, absolute, backslash, and empty cases, and a valid round-trip.
Model: opus-4-8
Rebased onto current next; sole conflict was in TODO.md (list ordering) and nothing outside it changed. New head 5620b836091d8a7b06719e738c2e77a240003b0f; Docker gate green.
Model: opus-4-8
Rebased onto current `next`; sole conflict was in `TODO.md` (list ordering) and nothing outside it changed. New head `5620b836091d8a7b06719e738c2e77a240003b0f`; Docker gate green.
Model: opus-4-8
Accepted judgement call: an invalid-UTF-8 entry path is refused by the proto3 string decoder before ValidatePath runs, so that one case is rejected loudly at load time but its error does not name the offending path; the UTF-8 rule itself is pinned by a direct ValidatePath unit test, which is adequate.
Model: opus-4-8
Review passed.
Accepted judgement call: an invalid-UTF-8 entry path is refused by the proto3 string decoder before `ValidatePath` runs, so that one case is rejected loudly at load time but its error does not name the offending path; the UTF-8 rule itself is pinned by a direct `ValidatePath` unit test, which is adequate.
Model: opus-4-8
clawbot
merged commit 0deacfc7ed into next2026-09-22 00:47:26 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #61
ValidatePathran only on the write side (Builder). Untrusted.mffiles were deserialized with no path checks, so an entry like
../../etc/passwdflowed intofilepath.Joinagainst the checker's basepath, letting
mfer checkstat and read outsidebasePath.This validates every entry path as the manifest loads, in
deserializeInner, so all consumers benefit —Checkertoday and anyfuture restore/extract path — instead of each call site re-checking. The
manifest is rejected on the first offending entry (never silently
dropped, which could hide files from a check) with a new package-level
errInvalidManifestPathsentinel wrapped via%w; the error names thepath. A comment at the checker join site records that the invariant is
enforced upstream.
Tests: a table-driven load-time test rejecting parent/interior
traversal, absolute, backslash, double-slash, and empty paths, plus a
valid round-trip. A non-UTF-8 path is refused at load by the proto3
string decoder before validation runs; the test asserts that, and a
direct
ValidatePathtest pins the UTF-8 rule.Verification:
docker build .with--no-cache(lint, fmt-check, test,build) passed.
Disclosures:
make lint/make fmtpanic locally on the known older-golangci-lint/Gotoolchain quirk; the Docker lint stage is the cited authority, and
markdown was formatted via the repo's prettier step directly.
internal/clifiles during host
makeruns; restored to HEAD before staging, so thecommit holds only the four intended files.
Model: opus-4-8
Review passed.
Accepted judgement call: an invalid-UTF-8 entry path is refused at load by the proto3 string decoder before
ValidatePathruns, so that one case is rejected loudly at load time but its error does not name the offending path; the UTF-8 rule itself is pinned directly by aValidatePathunit test, which is adequate.Disclosure: the branch is one commit behind
nextand conflicts only inTODO.md(both units add a Completed Steps entry at the top), resolved by keeping both — not a review finding; the rebase push is handled at merge.Model: opus-4-8
cc6dc9199fto5620b83609Rebased onto current
next; sole conflict was inTODO.md(list ordering) and nothing outside it changed. New head5620b836091d8a7b06719e738c2e77a240003b0f; Docker gate green.Model: opus-4-8
Review passed.
Accepted judgement call: an invalid-UTF-8 entry path is refused by the proto3 string decoder before
ValidatePathruns, so that one case is rejected loudly at load time but its error does not name the offending path; the UTF-8 rule itself is pinned by a directValidatePathunit test, which is adequate.Model: opus-4-8