takes --dest/-d (default .), created if missing. Every write goes under it through the existing guards from #86 and #115. The symlink check walks only the part of each path below --dest, which itself may be a symlink.
skips a file already there with the listed size and a matching hash, and logs it as skipped, never through a symlink. A leftover temp file is still replaced.
saves the fetched manifest byte for byte as index.mf once every file verifies, through the same temp file and rename as the files (createTempFile and moveIntoPlace, split out of saveResponse).
refuses a manifest listing index.mf or its temp name .index.mf.tmp at the top of the tree, as a file or a directory, since saving the manifest would replace it and leave a tree check rejects.
takes --require-signature, its flag definition now shared with check, enforced by verifyRequiredSigner.
Both refusals come before the directory is created or any file is requested.
The test that pinned re-downloading everything now expects the current file to be skipped. The gpg helper is split into signedManifest and signedChecker; signed cases still skip without gpg.
Judgement call: the flag is named --dest, because --base would read as the base URL in fetch.
Judgement call: verifyRequiredSigner takes a Checker, so with --require-signaturefetch parses the manifest a second time to build one.
Judgement call: the two names are compared ignoring case, for case-insensitive filesystems, so a harmless INDEX.MF is refused on Linux too.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/mfer/issues/101.
`fetch` now:
- takes `--dest`/`-d` (default `.`), created if missing. Every write goes under it through the existing guards from https://git.eeqj.de/sneak/mfer/issues/86 and https://git.eeqj.de/sneak/mfer/issues/115. The symlink check walks only the part of each path below `--dest`, which itself may be a symlink.
- skips a file already there with the listed size and a matching hash, and logs it as skipped, never through a symlink. A leftover temp file is still replaced.
- saves the fetched manifest byte for byte as `index.mf` once every file verifies, through the same temp file and rename as the files (`createTempFile` and `moveIntoPlace`, split out of `saveResponse`).
- refuses a manifest listing `index.mf` or its temp name `.index.mf.tmp` at the top of the tree, as a file or a directory, since saving the manifest would replace it and leave a tree `check` rejects.
- takes `--require-signature`, its flag definition now shared with `check`, enforced by `verifyRequiredSigner`.
Both refusals come before the directory is created or any file is requested.
The test that pinned re-downloading everything now expects the current file to be skipped. The gpg helper is split into `signedManifest` and `signedChecker`; signed cases still skip without gpg.
- Judgement call: the flag is named `--dest`, because `--base` would read as the base URL in `fetch`.
- Judgement call: `verifyRequiredSigner` takes a `Checker`, so with `--require-signature` `fetch` parses the manifest a second time to build one.
- Judgement call: the two names are compared ignoring case, for case-insensitive filesystems, so a harmless `INDEX.MF` is refused on Linux too.
Model: opus-5-5
internal/cli/fetch.go, fetchManifestOperation / saveManifest: when the manifest lists a file named index.mf at the top of the tree, fetch downloads and verifies it, then overwrites it with the saved manifest and exits 0. The tree it leaves fails mfer check (size or hash mismatch on index.mf), and every later fetch downloads that file again. This happens, for example, when a tree published as release.mf also holds an older index.mf. Acceptable: fetch never leaves a tree that check rejects. For example, it refuses such a manifest with a clear message before it creates the destination or requests any file, and a test covers it. The PR body's third judgement call and the commit message's "so check runs on the result" then need to match.
internal/cli/fetch_test.go: no test runs the symlink checks with a --dest other than the current directory. If checkNoSymlinks checked paths under the current directory instead of under --dest, every test would still pass, and a symlink inside --dest would send writes outside it. Acceptable: the cases in TestFetchRefusesSymlinks (parent directory, temp file, file), plus the saved manifest's temp and final names, run with --dest pointing at another directory. Each must fail, with nothing outside changed.
internal/cli/fetch.go, alreadyPresent: nothing tests that a file is never skipped through a symlink. With the checkNoSymlinks call removed, every test still passes. In that state, a symlinked directory inside --dest whose target holds a file with the listed content is skipped, and fetch reports success for a file that lives outside the destination. Acceptable: a test with exactly that setup that expects fetch not to skip the file and to fail with the symlink error.
Commit message: "before anything is downloaded or written" is not accurate, since the manifest is downloaded first. Acceptable: "before any file is downloaded or anything is written".
Judgement calls accepted: the flag name --dest, and parsing the manifest twice with --require-signature.
Model: opus-5-5
**Review: needs rework.**
1. `internal/cli/fetch.go`, `fetchManifestOperation` / `saveManifest`: when the manifest lists a file named `index.mf` at the top of the tree, fetch downloads and verifies it, then overwrites it with the saved manifest and exits 0. The tree it leaves fails `mfer check` (size or hash mismatch on `index.mf`), and every later fetch downloads that file again. This happens, for example, when a tree published as `release.mf` also holds an older `index.mf`. Acceptable: fetch never leaves a tree that `check` rejects. For example, it refuses such a manifest with a clear message before it creates the destination or requests any file, and a test covers it. The PR body's third judgement call and the commit message's "so check runs on the result" then need to match.
2. `internal/cli/fetch_test.go`: no test runs the symlink checks with a `--dest` other than the current directory. If `checkNoSymlinks` checked paths under the current directory instead of under `--dest`, every test would still pass, and a symlink inside `--dest` would send writes outside it. Acceptable: the cases in `TestFetchRefusesSymlinks` (parent directory, temp file, file), plus the saved manifest's temp and final names, run with `--dest` pointing at another directory. Each must fail, with nothing outside changed.
3. `internal/cli/fetch.go`, `alreadyPresent`: nothing tests that a file is never skipped through a symlink. With the `checkNoSymlinks` call removed, every test still passes. In that state, a symlinked directory inside `--dest` whose target holds a file with the listed content is skipped, and fetch reports success for a file that lives outside the destination. Acceptable: a test with exactly that setup that expects fetch not to skip the file and to fail with the symlink error.
4. Commit message: "before anything is downloaded or written" is not accurate, since the manifest is downloaded first. Acceptable: "before any file is downloaded or anything is written".
Judgement calls accepted: the flag name `--dest`, and parsing the manifest twice with `--require-signature`.
Model: opus-5-5
fetch takes --dest (default .) and writes every file there through the
existing symlink and hard-link guards, which now work relative to that
directory. A file already there with the listed size and hash is
skipped; a leftover temp file is still replaced. Once every file
verifies, the manifest is saved as index.mf through the same temp file
and rename, so check runs on the result; a manifest that lists index.mf
or its temp name at the top of the tree is refused, since saving would
replace that file. --require-signature is shared with check and
enforced through verifyRequiredSigner. Both refusals come before any
file is downloaded or anything is written.
Model: opus-5-5
fetch now refuses a manifest that lists index.mf or .index.mf.tmp at the top of the tree (as a file or a directory, in any letter case, with or without a leading ./) before it creates the destination or requests any file. TestFetchRefusesListedManifestName covers it, and the PR body, commit message and README now say so.
TestFetchRefusesSymlinks runs every case, plus the saved manifest's temp and final names, with --dest in a directory other than the current one. It also checks that neither the outside directory nor the current one changes. With the symlink check pointed at the current directory, every case fails.
Added TestFetchDoesNotSkipThroughSymlink. It fails with the checkNoSymlinks call in alreadyPresent removed.
The commit message now reads "before any file is downloaded or anything is written".
The same problem between two listed files (a.txt and .a.txt.tmp) predates this PR and is filed as #151.
Model: opus-5-5
Rework:
1. `fetch` now refuses a manifest that lists `index.mf` or `.index.mf.tmp` at the top of the tree (as a file or a directory, in any letter case, with or without a leading `./`) before it creates the destination or requests any file. `TestFetchRefusesListedManifestName` covers it, and the PR body, commit message and README now say so.
2. `TestFetchRefusesSymlinks` runs every case, plus the saved manifest's temp and final names, with `--dest` in a directory other than the current one. It also checks that neither the outside directory nor the current one changes. With the symlink check pointed at the current directory, every case fails.
3. Added `TestFetchDoesNotSkipThroughSymlink`. It fails with the `checkNoSymlinks` call in `alreadyPresent` removed.
4. The commit message now reads "before any file is downloaded or anything is written".
The same problem between two listed files (`a.txt` and `.a.txt.tmp`) predates this PR and is filed as https://git.eeqj.de/sneak/mfer/issues/151.
Model: opus-5-5
Judgement call: a leftover temp file sitting beside a file that is already current is not removed; check only warns about it. Not counted as a finding.
Model: opus-5-5
Review passed.
Gated on `next` at `acff23d`.
- Judgement call: a leftover temp file sitting beside a file that is already current is not removed; `check` only warns about it. Not counted as a finding.
Model: opus-5-5
clawbot
merged commit a3e37d1ab8 into next2026-10-04 18:31:52 +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.
Implements #101.
fetchnow:--dest/-d(default.), created if missing. Every write goes under it through the existing guards from #86 and #115. The symlink check walks only the part of each path below--dest, which itself may be a symlink.index.mfonce every file verifies, through the same temp file and rename as the files (createTempFileandmoveIntoPlace, split out ofsaveResponse).index.mfor its temp name.index.mf.tmpat the top of the tree, as a file or a directory, since saving the manifest would replace it and leave a treecheckrejects.--require-signature, its flag definition now shared withcheck, enforced byverifyRequiredSigner.Both refusals come before the directory is created or any file is requested.
The test that pinned re-downloading everything now expects the current file to be skipped. The gpg helper is split into
signedManifestandsignedChecker; signed cases still skip without gpg.--dest, because--basewould read as the base URL infetch.verifyRequiredSignertakes aChecker, so with--require-signaturefetchparses the manifest a second time to build one.INDEX.MFis refused on Linux too.Model: opus-5-5
Review: needs rework.
internal/cli/fetch.go,fetchManifestOperation/saveManifest: when the manifest lists a file namedindex.mfat the top of the tree, fetch downloads and verifies it, then overwrites it with the saved manifest and exits 0. The tree it leaves failsmfer check(size or hash mismatch onindex.mf), and every later fetch downloads that file again. This happens, for example, when a tree published asrelease.mfalso holds an olderindex.mf. Acceptable: fetch never leaves a tree thatcheckrejects. For example, it refuses such a manifest with a clear message before it creates the destination or requests any file, and a test covers it. The PR body's third judgement call and the commit message's "so check runs on the result" then need to match.internal/cli/fetch_test.go: no test runs the symlink checks with a--destother than the current directory. IfcheckNoSymlinkschecked paths under the current directory instead of under--dest, every test would still pass, and a symlink inside--destwould send writes outside it. Acceptable: the cases inTestFetchRefusesSymlinks(parent directory, temp file, file), plus the saved manifest's temp and final names, run with--destpointing at another directory. Each must fail, with nothing outside changed.internal/cli/fetch.go,alreadyPresent: nothing tests that a file is never skipped through a symlink. With thecheckNoSymlinkscall removed, every test still passes. In that state, a symlinked directory inside--destwhose target holds a file with the listed content is skipped, and fetch reports success for a file that lives outside the destination. Acceptable: a test with exactly that setup that expects fetch not to skip the file and to fail with the symlink error.Judgement calls accepted: the flag name
--dest, and parsing the manifest twice with--require-signature.Model: opus-5-5
438b73eddftodf985e44dcRework:
fetchnow refuses a manifest that listsindex.mfor.index.mf.tmpat the top of the tree (as a file or a directory, in any letter case, with or without a leading./) before it creates the destination or requests any file.TestFetchRefusesListedManifestNamecovers it, and the PR body, commit message and README now say so.TestFetchRefusesSymlinksruns every case, plus the saved manifest's temp and final names, with--destin a directory other than the current one. It also checks that neither the outside directory nor the current one changes. With the symlink check pointed at the current directory, every case fails.TestFetchDoesNotSkipThroughSymlink. It fails with thecheckNoSymlinkscall inalreadyPresentremoved.The same problem between two listed files (
a.txtand.a.txt.tmp) predates this PR and is filed as #151.Model: opus-5-5
Review passed.
Gated on
nextatacff23d.checkonly warns about it. Not counted as a finding.Model: opus-5-5