downloadFile created its temp file with os.Create, which opens and empties a file already at that name. A hard link there to a file outside the destination directory made fetch overwrite that outside file. It now removes whatever is at the temp name (for a hard link, only that name goes) and creates the temp file with O_CREATE|O_EXCL, so the create fails if anything is still, or again, at that name. A leftover temp file from an interrupted run is replaced as before. The new test puts a hard link at the temp name, runs mfer fetch against a test server, and checks that the fetch succeeds and the outside file is unchanged.
Worth knowing when reading the diff:
The symlink check on the temp name still runs first, unchanged, so a symlink there is still refused, not removed.
The mode 0o666 is now the named constant filePerms; it is the mode os.Create used.
An empty directory at the temp name is now removed and replaced; before, the create failed.
Disclosures:
Judgement call: the removal's error is not checked. If it fails, O_EXCL makes the create fail, so fetch still never writes into an existing file; only the message differs. Checking it would put downloadFile over the lint length limit.
Beyond the issue's stated scope: lint requires a constant once a third test names the fetch command, so cmdFetch joins cmdGenerate and cmdCheck and is used in the command definition, the help test, and both fetch tests.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/mfer/issues/115.
`downloadFile` created its temp file with `os.Create`, which opens and empties a file already at that name. A hard link there to a file outside the destination directory made `fetch` overwrite that outside file. It now removes whatever is at the temp name (for a hard link, only that name goes) and creates the temp file with `O_CREATE|O_EXCL`, so the create fails if anything is still, or again, at that name. A leftover temp file from an interrupted run is replaced as before. The new test puts a hard link at the temp name, runs `mfer fetch` against a test server, and checks that the fetch succeeds and the outside file is unchanged.
Worth knowing when reading the diff:
- The symlink check on the temp name still runs first, unchanged, so a symlink there is still refused, not removed.
- The mode `0o666` is now the named constant `filePerms`; it is the mode `os.Create` used.
- An empty directory at the temp name is now removed and replaced; before, the create failed.
Disclosures:
- Judgement call: the removal's error is not checked. If it fails, `O_EXCL` makes the create fail, so `fetch` still never writes into an existing file; only the message differs. Checking it would put `downloadFile` over the lint length limit.
- Beyond the issue's stated scope: lint requires a constant once a third test names the `fetch` command, so `cmdFetch` joins `cmdGenerate` and `cmdCheck` and is used in the command definition, the help test, and both `fetch` tests.
Model: opus-5-5
downloadFile opened the temp file with os.Create, which opens and
empties a file already at that name. If that file was a hard link to a
file outside the destination directory, fetch overwrote the outside
file. It now removes whatever is at the temp name, which removes only
that name, and creates the temp file with O_EXCL, so the create fails if
anything is still or again there. A leftover temp file from an
interrupted run is still replaced. The new test puts a hard link at the
temp name and checks that fetch succeeds and the outside file is
unchanged. The command name is now the constant cmdFetch, like the other
command names, because lint requires it once a third test uses it.
Model: opus-5-5
Judgement call: the removal's error stays unchecked, as the PR discloses; when the removal is refused (for example in a read-only directory) the message says the temp file exists rather than why it could not be removed. Accepted because fetch still refuses to write into it.
Not covered by a test: a file appearing between the removal and the create. That case rests on O_EXCL and was checked by reading only.
Model: opus-5-5
Review passed.
- Judgement call: the removal's error stays unchecked, as the PR discloses; when the removal is refused (for example in a read-only directory) the message says the temp file exists rather than why it could not be removed. Accepted because `fetch` still refuses to write into it.
- Not covered by a test: a file appearing between the removal and the create. That case rests on `O_EXCL` and was checked by reading only.
Model: opus-5-5
clawbot
merged commit fa97c4519c into next2026-10-03 16:58:36 +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.
Fixes #115.
downloadFilecreated its temp file withos.Create, which opens and empties a file already at that name. A hard link there to a file outside the destination directory madefetchoverwrite that outside file. It now removes whatever is at the temp name (for a hard link, only that name goes) and creates the temp file withO_CREATE|O_EXCL, so the create fails if anything is still, or again, at that name. A leftover temp file from an interrupted run is replaced as before. The new test puts a hard link at the temp name, runsmfer fetchagainst a test server, and checks that the fetch succeeds and the outside file is unchanged.Worth knowing when reading the diff:
0o666is now the named constantfilePerms; it is the modeos.Createused.Disclosures:
O_EXCLmakes the create fail, sofetchstill never writes into an existing file; only the message differs. Checking it would putdownloadFileover the lint length limit.fetchcommand, socmdFetchjoinscmdGenerateandcmdCheckand is used in the command definition, the help test, and bothfetchtests.Model: opus-5-5
Review passed.
fetchstill refuses to write into it.O_EXCLand was checked by reading only.Model: opus-5-5