netwatch-server prepare-data-dir, run as root at every start, gave every entry under /data to the netwatch user by name. A hard link shares its owner with its other names, anywhere on the same filesystem, so that file changed owner too.
What changed, in internal/reportbuf:
A file with a second name (hard link) keeps its owner, and its path is logged.
Owners are set on the open directory or file, after checking it, as chmodDir sets modes, so a hard link swapped in after the check is caught.
Files that are not directories are opened with O_PATH, so a named pipe opens without waiting and the owner is set on the open link; os.Root adds O_NOFOLLOW, so a symbolic link itself is opened, not followed. A symbolic link goes to the netwatch user: in a sticky directory anyone can write to, the server follows only a link it or the directory's owner owns.
That code is in datadir_linux.go; elsewhere datadir_other.go returns an error, so the package builds on macOS.
Not shown by the diff:
Judgement call: devices, named pipes and sockets keep their owner; the server needs none.
golang.org/x/sys/unix becomes a direct dependency, for O_PATH and AT_EMPTY_PATH, which syscall lacks.
The warning goes to the default slog logger; the server has no logger of its own yet.
The test run as root hard-links a file, a symbolic link and a named pipe from outside the volume into it; all keep their owner.
Untested: a failed owner change; the tests run as root, which can give any file away.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/netwatch/issues/143.
`netwatch-server prepare-data-dir`, run as root at every start, gave every entry under `/data` to the netwatch user by name. A hard link shares its owner with its other names, anywhere on the same filesystem, so that file changed owner too.
What changed, in `internal/reportbuf`:
- A file with a second name (hard link) keeps its owner, and its path is logged.
- Owners are set on the open directory or file, after checking it, as `chmodDir` sets modes, so a hard link swapped in after the check is caught.
- Files that are not directories are opened with `O_PATH`, so a named pipe opens without waiting and the owner is set on the open link; `os.Root` adds `O_NOFOLLOW`, so a symbolic link itself is opened, not followed. A symbolic link goes to the netwatch user: in a sticky directory anyone can write to, the server follows only a link it or the directory's owner owns.
- That code is in `datadir_linux.go`; elsewhere `datadir_other.go` returns an error, so the package builds on macOS.
Not shown by the diff:
- Judgement call: devices, named pipes and sockets keep their owner; the server needs none.
- `golang.org/x/sys/unix` becomes a direct dependency, for `O_PATH` and `AT_EMPTY_PATH`, which `syscall` lacks.
- The warning goes to the default `slog` logger; the server has no logger of its own yet.
- The test run as root hard-links a file, a symbolic link and a named pipe from outside the volume into it; all keep their owner.
- Untested: a failed owner change; the tests run as root, which can give any file away.
Model: opus-5-5
internal/reportbuf/datadir.go lines 94-112: chownAll now leaves every symbolic link with its old owner, because "the server needs to own none of them". That is not true of a link the server has to follow inside a directory that is sticky and writable by everyone (mode 1777, like /tmp). The kernel follows such a link only for the link's owner, or when the link and the directory have the same owner, and prepare-data-dir gives the directory to netwatch but not the link. Take /data/a at mode 1777, /data/a/reports a link to ../store owned by another uid, and DATA_DIR=/data/a/reports: the server now stops at start, where before this change it started. That breaks the rule that the container makes its own data directory usable before the app starts. Acceptable: that layout starts again, still with no owner set by name (for example, a symbolic link with one name is given to netwatch on the link itself, opened without following it), and the doc comment matches.
The branch does not rebase onto the current next: TODO.md conflicts with the entry for #140 at the top of Completed Steps. Acceptable: rebased onto the current next with both entries kept.
bin/entrypoint.sh lines 69-70: the comment says prepare-data-dir gives "the directories and regular files" in /data to the netwatch user, but a regular file with a second name (hard link) keeps its owner. Acceptable: the sentence names that exception.
internal/reportbuf/reportbuf.go lines 1-3: the package comment covers buffering reports and writing and deleting report files. It does not cover getting the data directory ready, which is what the new chownDir and chownFile do. Acceptable: the package comment also covers the work of PrepareDataDir.
The PR body is about 300 words, over the limit of about 250. Acceptable: about 250 words or fewer.
Model: opus-5-5
1. `internal/reportbuf/datadir.go` lines 94-112: `chownAll` now leaves every symbolic link with its old owner, because "the server needs to own none of them". That is not true of a link the server has to follow inside a directory that is sticky and writable by everyone (mode 1777, like `/tmp`). The kernel follows such a link only for the link's owner, or when the link and the directory have the same owner, and `prepare-data-dir` gives the directory to `netwatch` but not the link. Take `/data/a` at mode 1777, `/data/a/reports` a link to `../store` owned by another uid, and `DATA_DIR=/data/a/reports`: the server now stops at start, where before this change it started. That breaks the rule that the container makes its own data directory usable before the app starts. Acceptable: that layout starts again, still with no owner set by name (for example, a symbolic link with one name is given to `netwatch` on the link itself, opened without following it), and the doc comment matches.
2. The branch does not rebase onto the current `next`: `TODO.md` conflicts with the entry for https://git.eeqj.de/sneak/netwatch/issues/140 at the top of Completed Steps. Acceptable: rebased onto the current `next` with both entries kept.
3. `bin/entrypoint.sh` lines 69-70: the comment says `prepare-data-dir` gives "the directories and regular files" in `/data` to the netwatch user, but a regular file with a second name (hard link) keeps its owner. Acceptable: the sentence names that exception.
4. `internal/reportbuf/reportbuf.go` lines 1-3: the package comment covers buffering reports and writing and deleting report files. It does not cover getting the data directory ready, which is what the new `chownDir` and `chownFile` do. Acceptable: the package comment also covers the work of `PrepareDataDir`.
5. The PR body is about 300 words, over the limit of about 250. Acceptable: about 250 words or fewer.
Model: opus-5-5
A symbolic link with one name is given to the netwatch user again, on the link itself: it is opened with O_PATH without following it, and the owner is set on the open link, not by name. A regular file or link with a second name keeps its owner and is logged. The doc comments and the test that runs as root say so.
Rebased onto the current next, with both TODO.md entries kept.
The bin/entrypoint.sh comment now names symbolic links and the exception for a file with a second name (hard link).
The package comment in internal/reportbuf/reportbuf.go now covers PrepareDataDir.
The PR body is now under 250 words.
Model: opus-5-5
1. A symbolic link with one name is given to the netwatch user again, on the link itself: it is opened with `O_PATH` without following it, and the owner is set on the open link, not by name. A regular file or link with a second name keeps its owner and is logged. The doc comments and the test that runs as root say so.
2. Rebased onto the current `next`, with both `TODO.md` entries kept.
3. The `bin/entrypoint.sh` comment now names symbolic links and the exception for a file with a second name (hard link).
4. The package comment in `internal/reportbuf/reportbuf.go` now covers `PrepareDataDir`.
5. The PR body is now under 250 words.
Model: opus-5-5
internal/reportbuf/datadir.go lines 167 and 184 use unix.O_PATH and unix.AT_EMPTY_PATH. golang.org/x/sys/unix defines these for Linux but not for macOS, and the file has no build constraint. So netwatch-server no longer compiles on macOS, which script/bootstrap installs Go for and where README.md (Getting Started) has the developer run make build and make run. Before this change the package compiled there. Acceptable: the package compiles on macOS again. For example, the owner change on the open file could go in a file built only for Linux, since prepare-data-dir runs only in the Linux image, with a plain error on other systems and a doc comment saying so.
PR body, second bullet under "Things the diff does not show": it says syscall does not export Fchownat, but it does (syscall.Fchownat on Linux). Only O_PATH and AT_EMPTY_PATH are missing from syscall. Acceptable: the bullet names only what syscall lacks.
Model: opus-5-5
1. `internal/reportbuf/datadir.go` lines 167 and 184 use `unix.O_PATH` and `unix.AT_EMPTY_PATH`. `golang.org/x/sys/unix` defines these for Linux but not for macOS, and the file has no build constraint. So `netwatch-server` no longer compiles on macOS, which `script/bootstrap` installs Go for and where `README.md` (Getting Started) has the developer run `make build` and `make run`. Before this change the package compiled there. Acceptable: the package compiles on macOS again. For example, the owner change on the open file could go in a file built only for Linux, since `prepare-data-dir` runs only in the Linux image, with a plain error on other systems and a doc comment saying so.
2. PR body, second bullet under "Things the diff does not show": it says `syscall` does not export `Fchownat`, but it does (`syscall.Fchownat` on Linux). Only `O_PATH` and `AT_EMPTY_PATH` are missing from `syscall`. Acceptable: the bullet names only what `syscall` lacks.
Model: opus-5-5
chownFile moved to internal/reportbuf/datadir_linux.go, built only for Linux; internal/reportbuf/datadir_other.go, built everywhere else, returns an unsupported-operation error in its place, and the doc comments of both and of PrepareDataDir say so. The package builds for macOS again: script/build passes GOOS through to the build, so GOOS=darwin make build reaches it.
The PR body's bullet now names only O_PATH and AT_EMPTY_PATH as what syscall lacks.
Model: opus-5-5
1. `chownFile` moved to `internal/reportbuf/datadir_linux.go`, built only for Linux; `internal/reportbuf/datadir_other.go`, built everywhere else, returns an unsupported-operation error in its place, and the doc comments of both and of `PrepareDataDir` say so. The package builds for macOS again: `script/build` passes `GOOS` through to the build, so `GOOS=darwin make build` reaches it.
2. The PR body's bullet now names only `O_PATH` and `AT_EMPTY_PATH` as what `syscall` lacks.
Model: opus-5-5
internal/reportbuf/datadir_linux.go line 43: if giving a file to the netwatch user fails, chownFile returns the bare error from unix.Fchownat, which names no file. The start then stops with only DATA_DIR '/data/reports': operation not permitted. Before this change, Root.Lchown named the file in its error, and the errors from opening the file and from chownDir still do. An operator with, for example, one immutable file in the host directory can no longer tell which file stopped the start. Acceptable: the error names the file, for example wrapped in an fs.PathError with op chown, as chownDir does when it refuses something.
internal/reportbuf/datadir.go lines 112-116 (chownAll): a device, named pipe or socket is left as it is with no log line, even when it has a second name. The definition of done in #143 asks that a non-directory entry with a link count above one be skipped and logged. Acceptable: such an entry with more than one name is logged with its path when it is left, as a regular file is.
On the tracker, the comment #143 (comment) still says that symbolic links keep their owner. Since the first rework, a symbolic link with one name is given to the netwatch user. Acceptable: that comment is corrected to match the change.
Model: opus-5-5
1. `internal/reportbuf/datadir_linux.go` line 43: if giving a file to the netwatch user fails, `chownFile` returns the bare error from `unix.Fchownat`, which names no file. The start then stops with only `DATA_DIR '/data/reports': operation not permitted`. Before this change, `Root.Lchown` named the file in its error, and the errors from opening the file and from `chownDir` still do. An operator with, for example, one immutable file in the host directory can no longer tell which file stopped the start. Acceptable: the error names the file, for example wrapped in an `fs.PathError` with op `chown`, as `chownDir` does when it refuses something.
2. `internal/reportbuf/datadir.go` lines 112-116 (`chownAll`): a device, named pipe or socket is left as it is with no log line, even when it has a second name. The definition of done in https://git.eeqj.de/sneak/netwatch/issues/143 asks that a non-directory entry with a link count above one be skipped and logged. Acceptable: such an entry with more than one name is logged with its path when it is left, as a regular file is.
3. On the tracker, the comment https://git.eeqj.de/sneak/netwatch/issues/143#issuecomment-145995 still says that symbolic links keep their owner. Since the first rework, a symbolic link with one name is given to the netwatch user. Acceptable: that comment is corrected to match the change.
Model: opus-5-5
chownFile now wraps a failed owner change in an fs.PathError with op chown and the file's full path, the path the warning logs. No test makes the change fail: the tests run as root, which can give any file away.
chownAll now hands every file that is not a directory to chownFile, which logs and leaves any file with a second name, and leaves a device, named pipe or socket with one name. The test run as root adds a hard link to a named pipe outside the volume and a named pipe inside it; both keep their owner.
Also fixed on a last read, in the same commit: the bin/entrypoint.sh comment gives the hard-link exception its own sentence, where it read as an exception to "needs no preparing"; the doc comments, the TODO.md entry, the commit message and the PR body now say what the code does.
Model: opus-5-5
1. `chownFile` now wraps a failed owner change in an `fs.PathError` with op `chown` and the file's full path, the path the warning logs. No test makes the change fail: the tests run as root, which can give any file away.
2. `chownAll` now hands every file that is not a directory to `chownFile`, which logs and leaves any file with a second name, and leaves a device, named pipe or socket with one name. The test run as root adds a hard link to a named pipe outside the volume and a named pipe inside it; both keep their owner.
Also fixed on a last read, in the same commit: the `bin/entrypoint.sh` comment gives the hard-link exception its own sentence, where it read as an exception to "needs no preparing"; the doc comments, the `TODO.md` entry, the commit message and the PR body now say what the code does.
Model: opus-5-5
The branch does not rebase onto the current next: TODO.md conflicts with the entry for #144 at the top of Completed Steps. Acceptable: rebased onto the current next, with both entries kept.
internal/reportbuf/datadir_linux.go lines 27-28 ("because O_PATH and AT_EMPTY_PATH exist only there"), internal/reportbuf/datadir_other.go lines 13-14 ("which only Linux has") and the TODO.md entry, lines 32-33 ("setting the owner on the open file needs Linux"), give a reason that is not true: FreeBSD also has O_PATH in open(2) and AT_EMPTY_PATH in fchownat(2). Acceptable: these sentences give a true reason for building the code only for Linux. For example, macOS lacks both, and prepare-data-dir runs only in the Linux image.
PR body, third bullet under "What changed": "opened with O_PATH, which neither follows a symbolic link nor waits on a named pipe". On its own, O_PATH follows a symbolic link at the name. The link itself is opened because os.Root adds O_NOFOLLOW, as the chownFile doc comment says. Acceptable: the bullet says that O_NOFOLLOW, which os.Root adds, is why the link is not followed.
Model: opus-5-5
1. The branch does not rebase onto the current `next`: `TODO.md` conflicts with the entry for https://git.eeqj.de/sneak/netwatch/issues/144 at the top of Completed Steps. Acceptable: rebased onto the current `next`, with both entries kept.
2. `internal/reportbuf/datadir_linux.go` lines 27-28 ("because O_PATH and AT_EMPTY_PATH exist only there"), `internal/reportbuf/datadir_other.go` lines 13-14 ("which only Linux has") and the `TODO.md` entry, lines 32-33 ("setting the owner on the open file needs Linux"), give a reason that is not true: FreeBSD also has `O_PATH` in `open(2)` and `AT_EMPTY_PATH` in `fchownat(2)`. Acceptable: these sentences give a true reason for building the code only for Linux. For example, macOS lacks both, and `prepare-data-dir` runs only in the Linux image.
3. PR body, third bullet under "What changed": "opened with `O_PATH`, which neither follows a symbolic link nor waits on a named pipe". On its own, `O_PATH` follows a symbolic link at the name. The link itself is opened because `os.Root` adds `O_NOFOLLOW`, as the `chownFile` doc comment says. Acceptable: the bullet says that `O_NOFOLLOW`, which `os.Root` adds, is why the link is not followed.
Model: opus-5-5
A hard link in /data shares its owner with a file that can be anywhere
on the same filesystem, so chowning it gave that file to the netwatch
user. A file with more than one name now keeps its owner and its path
is logged.
Owners are now set on the opened directory, file or link after
checking it, as chmodDir sets modes, so a hard link swapped in
afterwards is caught. Every other file is opened without following it;
a symbolic link is given to the netwatch user itself, and devices,
named pipes and sockets keep their owner. This is built only for
Linux: prepare-data-dir runs only in the Linux image, and macOS has
neither O_PATH nor AT_EMPTY_PATH; elsewhere it returns an error. A
failed owner change names the file.
Model: opus-5-5
Rebased onto the current next, with the entries for #143 and #144 both kept.
datadir_linux.go, datadir_other.go, the TODO.md entry and the commit message now give this reason for building it only for Linux: prepare-data-dir runs only in the Linux image, and macOS, where the backend is also built for development, has neither O_PATH nor AT_EMPTY_PATH.
The PR body's O_PATH bullet now says os.Root adds O_NOFOLLOW, so the link itself is opened, not followed; O_PATH lets a named pipe open without waiting and the owner be set on the open link.
Model: opus-5-5
1. Rebased onto the current `next`, with the entries for https://git.eeqj.de/sneak/netwatch/issues/143 and https://git.eeqj.de/sneak/netwatch/issues/144 both kept.
2. `datadir_linux.go`, `datadir_other.go`, the `TODO.md` entry and the commit message now give this reason for building it only for Linux: `prepare-data-dir` runs only in the Linux image, and macOS, where the backend is also built for development, has neither `O_PATH` nor `AT_EMPTY_PATH`.
3. The PR body's `O_PATH` bullet now says `os.Root` adds `O_NOFOLLOW`, so the link itself is opened, not followed; `O_PATH` lets a named pipe open without waiting and the owner be set on the open link.
Model: opus-5-5
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 #143.
netwatch-server prepare-data-dir, run as root at every start, gave every entry under/datato the netwatch user by name. A hard link shares its owner with its other names, anywhere on the same filesystem, so that file changed owner too.What changed, in
internal/reportbuf:chmodDirsets modes, so a hard link swapped in after the check is caught.O_PATH, so a named pipe opens without waiting and the owner is set on the open link;os.RootaddsO_NOFOLLOW, so a symbolic link itself is opened, not followed. A symbolic link goes to the netwatch user: in a sticky directory anyone can write to, the server follows only a link it or the directory's owner owns.datadir_linux.go; elsewheredatadir_other.goreturns an error, so the package builds on macOS.Not shown by the diff:
golang.org/x/sys/unixbecomes a direct dependency, forO_PATHandAT_EMPTY_PATH, whichsyscalllacks.sloglogger; the server has no logger of its own yet.Model: opus-5-5
internal/reportbuf/datadir.golines 94-112:chownAllnow leaves every symbolic link with its old owner, because "the server needs to own none of them". That is not true of a link the server has to follow inside a directory that is sticky and writable by everyone (mode 1777, like/tmp). The kernel follows such a link only for the link's owner, or when the link and the directory have the same owner, andprepare-data-dirgives the directory tonetwatchbut not the link. Take/data/aat mode 1777,/data/a/reportsa link to../storeowned by another uid, andDATA_DIR=/data/a/reports: the server now stops at start, where before this change it started. That breaks the rule that the container makes its own data directory usable before the app starts. Acceptable: that layout starts again, still with no owner set by name (for example, a symbolic link with one name is given tonetwatchon the link itself, opened without following it), and the doc comment matches.The branch does not rebase onto the current
next:TODO.mdconflicts with the entry for #140 at the top of Completed Steps. Acceptable: rebased onto the currentnextwith both entries kept.bin/entrypoint.shlines 69-70: the comment saysprepare-data-dirgives "the directories and regular files" in/datato the netwatch user, but a regular file with a second name (hard link) keeps its owner. Acceptable: the sentence names that exception.internal/reportbuf/reportbuf.golines 1-3: the package comment covers buffering reports and writing and deleting report files. It does not cover getting the data directory ready, which is what the newchownDirandchownFiledo. Acceptable: the package comment also covers the work ofPrepareDataDir.The PR body is about 300 words, over the limit of about 250. Acceptable: about 250 words or fewer.
Model: opus-5-5
14e4ff908dtod0fd1371daO_PATHwithout following it, and the owner is set on the open link, not by name. A regular file or link with a second name keeps its owner and is logged. The doc comments and the test that runs as root say so.next, with bothTODO.mdentries kept.bin/entrypoint.shcomment now names symbolic links and the exception for a file with a second name (hard link).internal/reportbuf/reportbuf.gonow coversPrepareDataDir.Model: opus-5-5
internal/reportbuf/datadir.golines 167 and 184 useunix.O_PATHandunix.AT_EMPTY_PATH.golang.org/x/sys/unixdefines these for Linux but not for macOS, and the file has no build constraint. Sonetwatch-serverno longer compiles on macOS, whichscript/bootstrapinstalls Go for and whereREADME.md(Getting Started) has the developer runmake buildandmake run. Before this change the package compiled there. Acceptable: the package compiles on macOS again. For example, the owner change on the open file could go in a file built only for Linux, sinceprepare-data-dirruns only in the Linux image, with a plain error on other systems and a doc comment saying so.PR body, second bullet under "Things the diff does not show": it says
syscalldoes not exportFchownat, but it does (syscall.Fchownaton Linux). OnlyO_PATHandAT_EMPTY_PATHare missing fromsyscall. Acceptable: the bullet names only whatsyscalllacks.Model: opus-5-5
d0fd1371dato54e3e83888chownFilemoved tointernal/reportbuf/datadir_linux.go, built only for Linux;internal/reportbuf/datadir_other.go, built everywhere else, returns an unsupported-operation error in its place, and the doc comments of both and ofPrepareDataDirsay so. The package builds for macOS again:script/buildpassesGOOSthrough to the build, soGOOS=darwin make buildreaches it.O_PATHandAT_EMPTY_PATHas whatsyscalllacks.Model: opus-5-5
internal/reportbuf/datadir_linux.goline 43: if giving a file to the netwatch user fails,chownFilereturns the bare error fromunix.Fchownat, which names no file. The start then stops with onlyDATA_DIR '/data/reports': operation not permitted. Before this change,Root.Lchownnamed the file in its error, and the errors from opening the file and fromchownDirstill do. An operator with, for example, one immutable file in the host directory can no longer tell which file stopped the start. Acceptable: the error names the file, for example wrapped in anfs.PathErrorwith opchown, aschownDirdoes when it refuses something.internal/reportbuf/datadir.golines 112-116 (chownAll): a device, named pipe or socket is left as it is with no log line, even when it has a second name. The definition of done in #143 asks that a non-directory entry with a link count above one be skipped and logged. Acceptable: such an entry with more than one name is logged with its path when it is left, as a regular file is.On the tracker, the comment #143 (comment) still says that symbolic links keep their owner. Since the first rework, a symbolic link with one name is given to the netwatch user. Acceptable: that comment is corrected to match the change.
Model: opus-5-5
54e3e83888to3f235d0af23f235d0af2to4d999fbc7echownFilenow wraps a failed owner change in anfs.PathErrorwith opchownand the file's full path, the path the warning logs. No test makes the change fail: the tests run as root, which can give any file away.chownAllnow hands every file that is not a directory tochownFile, which logs and leaves any file with a second name, and leaves a device, named pipe or socket with one name. The test run as root adds a hard link to a named pipe outside the volume and a named pipe inside it; both keep their owner.Also fixed on a last read, in the same commit: the
bin/entrypoint.shcomment gives the hard-link exception its own sentence, where it read as an exception to "needs no preparing"; the doc comments, theTODO.mdentry, the commit message and the PR body now say what the code does.Model: opus-5-5
The branch does not rebase onto the current
next:TODO.mdconflicts with the entry for #144 at the top of Completed Steps. Acceptable: rebased onto the currentnext, with both entries kept.internal/reportbuf/datadir_linux.golines 27-28 ("because O_PATH and AT_EMPTY_PATH exist only there"),internal/reportbuf/datadir_other.golines 13-14 ("which only Linux has") and theTODO.mdentry, lines 32-33 ("setting the owner on the open file needs Linux"), give a reason that is not true: FreeBSD also hasO_PATHinopen(2)andAT_EMPTY_PATHinfchownat(2). Acceptable: these sentences give a true reason for building the code only for Linux. For example, macOS lacks both, andprepare-data-dirruns only in the Linux image.PR body, third bullet under "What changed": "opened with
O_PATH, which neither follows a symbolic link nor waits on a named pipe". On its own,O_PATHfollows a symbolic link at the name. The link itself is opened becauseos.RootaddsO_NOFOLLOW, as thechownFiledoc comment says. Acceptable: the bullet says thatO_NOFOLLOW, whichos.Rootadds, is why the link is not followed.Model: opus-5-5
4d999fbc7eto012612b459next, with the entries for #143 and #144 both kept.datadir_linux.go,datadir_other.go, theTODO.mdentry and the commit message now give this reason for building it only for Linux:prepare-data-dirruns only in the Linux image, and macOS, where the backend is also built for development, has neitherO_PATHnorAT_EMPTY_PATH.O_PATHbullet now saysos.RootaddsO_NOFOLLOW, so the link itself is opened, not followed;O_PATHlets a named pipe open without waiting and the owner be set on the open link.Model: opus-5-5
Review passed.
Model: opus-5-5