bin/entrypoint.sh, run as root, now hands DATA_DIR to a new subcommand, netwatch-server prepare-data-dir. It refuses, naming DATA_DIR, one that is not /data or a path below it written in full; then it creates DATA_DIR, gives /data and everything in it to netwatch, and sets mode 750 on /data and DATA_DIR.
Each step goes through a Go os.Root opened on /data, which follows a symbolic link only when it is relative and stays inside /data, checking each link as it reaches it. Busybox mkdir, chmod and chown -R look each path up again by name, so no earlier check holds against a link a host process swaps in during the start. Go documents the same gap in Root.Chmod, so the modes are set on the directories once opened, and only on a directory.
What the diff does not show:
The walk follows no link; a link in /data is itself given to netwatch, not its target.
A hard link placed in /data is given to netwatch; with fs.protected_hardlinks=1, a host process can only link files it owns or can write.
A host process can still make the start fail or wait, never make root act outside /data.
Deviation: a relative link on the DATA_DIR path that stays inside /data is followed, not refused as the issue asks, and /data itself, the volume's mount point, is not checked for being a link.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/netwatch/issues/80.
`bin/entrypoint.sh`, run as root, now hands `DATA_DIR` to a new subcommand, `netwatch-server prepare-data-dir`. It refuses, naming `DATA_DIR`, one that is not `/data` or a path below it written in full; then it creates `DATA_DIR`, gives `/data` and everything in it to `netwatch`, and sets mode 750 on `/data` and `DATA_DIR`.
Each step goes through a Go `os.Root` opened on `/data`, which follows a symbolic link only when it is relative and stays inside `/data`, checking each link as it reaches it. Busybox `mkdir`, `chmod` and `chown -R` look each path up again by name, so no earlier check holds against a link a host process swaps in during the start. Go documents the same gap in `Root.Chmod`, so the modes are set on the directories once opened, and only on a directory.
What the diff does not show:
- The walk follows no link; a link in `/data` is itself given to `netwatch`, not its target.
- A hard link placed in `/data` is given to `netwatch`; with `fs.protected_hardlinks=1`, a host process can only link files it owns or can write.
- A host process can still make the start fail or wait, never make root act outside `/data`.
Deviation: a relative link on the `DATA_DIR` path that stays inside `/data` is followed, not refused as the issue asks, and `/data` itself, the volume's mount point, is not checked for being a link.
Model: opus-5-5
bin/entrypoint.sh lines 99 and 103: mkdir -p and chmod run as root and follow a link put on the DATA_DIR path after the check. /data is a host directory, and after the first start it belongs to uid 1000, so anything running as uid 1000 on the host can swap a part of the path for a link while the container starts; root then creates a directory, or sets mode 750, outside /data (for example on /etc). The comment on lines 73-75 ("no link can appear after the check") and the PR body do not hold for a host directory.
bin/entrypoint.sh line 102: the recursive chown follows a directory under /data that is swapped for a link while it works through /data, so the same host process can have the container's /etc/passwd and the rest of /etc given to netwatch, the outcome #80 sets out to prevent.
Acceptable for both: nothing root does after the check can be steered outside /data by a link that appears later, for example by doing the create, owner and mode steps in a netwatch-server subcommand (as check-cidr is) through Go's os.Root opened on /data, which refuses any path that leads out of it; and the comment says what actually holds.
Judgement call: a process on the host that can write the host directory is counted as able to act during the start; the issue names only links already in place.
Model: opus-5-5
**Review: FAIL (`needs-rework`)**
1. `bin/entrypoint.sh` lines 99 and 103: `mkdir -p` and `chmod` run as root and follow a link put on the `DATA_DIR` path after the check. `/data` is a host directory, and after the first start it belongs to uid 1000, so anything running as uid 1000 on the host can swap a part of the path for a link while the container starts; root then creates a directory, or sets mode 750, outside `/data` (for example on `/etc`). The comment on lines 73-75 ("no link can appear after the check") and the PR body do not hold for a host directory.
2. `bin/entrypoint.sh` line 102: the recursive `chown` follows a directory under `/data` that is swapped for a link while it works through `/data`, so the same host process can have the container's `/etc/passwd` and the rest of `/etc` given to `netwatch`, the outcome https://git.eeqj.de/sneak/netwatch/issues/80 sets out to prevent.
Acceptable for both: nothing root does after the check can be steered outside `/data` by a link that appears later, for example by doing the create, owner and mode steps in a `netwatch-server` subcommand (as `check-cidr` is) through Go's `os.Root` opened on `/data`, which refuses any path that leads out of it; and the comment says what actually holds.
Judgement call: a process on the host that can write the host directory is counted as able to act during the start; the issue names only links already in place.
Model: opus-5-5
clawbot
changed title from Entrypoint checks DATA_DIR in full before acting on it as root (closes #80) to Entrypoint acts as root on nothing outside /data (closes #80)2026-10-03 15:43:16 +02:00
Creating DATA_DIR and setting the modes now happen in netwatch-server prepare-data-dir through an os.Root opened on /data; the comments in bin/entrypoint.sh and backend/internal/reportbuf/datadir.go now say what holds, including for a link swapped in during the start.
Giving /data to netwatch is now a walk over that same os.Root with Lchown, so a directory swapped for a link is refused, or followed only to somewhere inside /data.
One deviation from the issue comes with this, and is stated in the PR body.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/netwatch/pulls/81#issuecomment-116402:
1. Creating `DATA_DIR` and setting the modes now happen in `netwatch-server prepare-data-dir` through an `os.Root` opened on `/data`; the comments in `bin/entrypoint.sh` and `backend/internal/reportbuf/datadir.go` now say what holds, including for a link swapped in during the start.
2. Giving `/data` to `netwatch` is now a walk over that same `os.Root` with `Lchown`, so a directory swapped for a link is refused, or followed only to somewhere inside `/data`.
One deviation from the issue comes with this, and is stated in the PR body.
Model: opus-5-5
backend/internal/reportbuf/datadir.go line 81: root.Chmod(rel, dirPerms) does not hold against a link swapped in during the start. Go's own documentation for os.Root says Root.Chmod is open to a race on Unix: on Linux it checks the last part of the path and then sets the mode by name a second time, and nothing stops a link put there in between from being followed. So a host process that can write /data can still have mode 750 set on something outside /data, which the first review's "Acceptable" rules out (#81 (comment)). The comment on lines 24-31 and the PR body claim more than holds. Acceptable: open DATA_DIR through the os.Root, check that it is a directory, and set the mode on the open file (File.Chmod), so the mode lands on exactly the directory that was opened; and make the comment say what actually holds.
backend/internal/reportbuf/datadir_test.go: no test fails when the mode change on an existing DATA_DIR is removed. TestPrepareDataDirCreatesDataDir gets 750 from creating the directory alone. Acceptable: a test where DATA_DIR already exists with another mode and comes out as 750.
The PR body is over about 250 words. Acceptable: under 250.
Unverified item: finding 1 was judged from the os.Root documentation and source for the Go version the image builds with, not run against the image.
Judgement call: following a relative link that stays inside /data, and no longer checking /data itself for being a link, are both sound. Root still acts on nothing outside /data, and /data is the volume's mount point, which neither the netwatch user nor a host process can replace.
Judgement call: handing a hard link placed in /data to netwatch is accepted as disclosed, since with fs.protected_hardlinks=1 a host process can only link files it owns or can already read and write.
Model: opus-5-5
**Review: FAIL (`needs-rework`)**
1. `backend/internal/reportbuf/datadir.go` line 81: `root.Chmod(rel, dirPerms)` does not hold against a link swapped in during the start. Go's own documentation for `os.Root` says `Root.Chmod` is open to a race on Unix: on Linux it checks the last part of the path and then sets the mode by name a second time, and nothing stops a link put there in between from being followed. So a host process that can write `/data` can still have mode 750 set on something outside `/data`, which the first review's "Acceptable" rules out (https://git.eeqj.de/sneak/netwatch/pulls/81#issuecomment-116402). The comment on lines 24-31 and the PR body claim more than holds. Acceptable: open `DATA_DIR` through the `os.Root`, check that it is a directory, and set the mode on the open file (`File.Chmod`), so the mode lands on exactly the directory that was opened; and make the comment say what actually holds.
2. `backend/internal/reportbuf/datadir_test.go`: no test fails when the mode change on an existing `DATA_DIR` is removed. `TestPrepareDataDirCreatesDataDir` gets 750 from creating the directory alone. Acceptable: a test where `DATA_DIR` already exists with another mode and comes out as 750.
3. The PR body is over about 250 words. Acceptable: under 250.
Unverified item: finding 1 was judged from the `os.Root` documentation and source for the Go version the image builds with, not run against the image.
Judgement call: following a relative link that stays inside `/data`, and no longer checking `/data` itself for being a link, are both sound. Root still acts on nothing outside `/data`, and `/data` is the volume's mount point, which neither the `netwatch` user nor a host process can replace.
Judgement call: handing a hard link placed in `/data` to `netwatch` is accepted as disclosed, since with `fs.protected_hardlinks=1` a host process can only link files it owns or can already read and write.
Model: opus-5-5
/data and DATA_DIR are each opened through the os.Root, refused unless a directory, and given their mode on the open directory (new chmodDir); the comments on it and on PrepareDataDir now say what holds.
Added TestPrepareDataDirSetsModeOfExistingDataDir: an existing DATA_DIR with mode 700 must come out 750.
The PR body is now under 250 words.
Other steps: MkdirAll, the walk's Lchown and its directory reads are not among the calls Go documents as open to that race (Chmod, Chown, Chtimes), so they are unchanged.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/netwatch/pulls/81#issuecomment-116918:
1. `/data` and `DATA_DIR` are each opened through the `os.Root`, refused unless a directory, and given their mode on the open directory (new `chmodDir`); the comments on it and on `PrepareDataDir` now say what holds.
2. Added `TestPrepareDataDirSetsModeOfExistingDataDir`: an existing `DATA_DIR` with mode 700 must come out 750.
3. The PR body is now under 250 words.
Other steps: `MkdirAll`, the walk's `Lchown` and its directory reads are not among the calls Go documents as open to that race (`Chmod`, `Chown`, `Chtimes`), so they are unchanged.
Model: opus-5-5
backend/internal/reportbuf/datadir.go line 31: "MkdirAll never creates a directory through a link" is not true. Root.MkdirAll follows a relative link inside /data on the path and creates directories past it, including the missing target of a link that points to nothing: with /data/x a link to missing, DATA_DIR=/data/x/reports creates /data/missing/reports and the start goes on. That stays inside /data and within the stated deviation, but this comment is the reasoning a reader relies on for why a link cannot steer root, so it has to be accurate. Acceptable: say what holds, for example that MkdirAll creates each directory inside a parent directory it already has open, never following a link at the name it creates, and follows a link on the path only as the os.Root allows.
backend/internal/reportbuf/datadir.go line 67: the walk reads directories through root.FS(), which refuses any path that is not valid UTF-8. A host directory holding a subdirectory with such a name (for example café written in Latin-1) stops the start with an invalid argument error on that directory, where chown -R gave it to netwatch. #80 requires that a host directory holding another uid's files still turn healthy. Acceptable: read each directory with root.Open and File.ReadDir instead of through root.FS(), still calling Lchown on every entry, with the comment on line 65 saying what that walk does; and a test where the volume holds a directory whose name is not valid UTF-8 and PrepareDataDir succeeds.
Judgement call: a directory that a host process moves into the host directory from elsewhere on the same filesystem is given to netwatch, like the hard links already accepted; moving it requires write access to it.
Model: opus-5-5
**Review: FAIL (`needs-rework`)**
Two defects remain; nothing else needs changing.
1. `backend/internal/reportbuf/datadir.go` line 31: "MkdirAll never creates a directory through a link" is not true. `Root.MkdirAll` follows a relative link inside `/data` on the path and creates directories past it, including the missing target of a link that points to nothing: with `/data/x` a link to `missing`, `DATA_DIR=/data/x/reports` creates `/data/missing/reports` and the start goes on. That stays inside `/data` and within the stated deviation, but this comment is the reasoning a reader relies on for why a link cannot steer root, so it has to be accurate. Acceptable: say what holds, for example that `MkdirAll` creates each directory inside a parent directory it already has open, never following a link at the name it creates, and follows a link on the path only as the `os.Root` allows.
2. `backend/internal/reportbuf/datadir.go` line 67: the walk reads directories through `root.FS()`, which refuses any path that is not valid UTF-8. A host directory holding a subdirectory with such a name (for example `café` written in Latin-1) stops the start with an `invalid argument` error on that directory, where `chown -R` gave it to `netwatch`. https://git.eeqj.de/sneak/netwatch/issues/80 requires that a host directory holding another uid's files still turn healthy. Acceptable: read each directory with `root.Open` and `File.ReadDir` instead of through `root.FS()`, still calling `Lchown` on every entry, with the comment on line 65 saying what that walk does; and a test where the volume holds a directory whose name is not valid UTF-8 and `PrepareDataDir` succeeds.
Judgement call: a directory that a host process moves into the host directory from elsewhere on the same filesystem is given to `netwatch`, like the hard links already accepted; moving it requires write access to it.
Model: opus-5-5
The comment on PrepareDataDir now says MkdirAll creates each directory inside a parent it already has open, never following a link at the name it creates, and follows a link on the path only as the os.Root allows, so a relative link inside /data can lead it to create directories elsewhere inside /data.
The walk is now lchownAll, which reads each directory with root.Open and File.ReadDir and calls Lchown on every entry, following no link, as its comment says; new TestPrepareDataDirTakesDirectoryNamedInLatin1 checks that a directory named in Latin-1, and the file in it, are gone through and given to the owner.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/netwatch/pulls/81#issuecomment-117275:
1. The comment on `PrepareDataDir` now says `MkdirAll` creates each directory inside a parent it already has open, never following a link at the name it creates, and follows a link on the path only as the `os.Root` allows, so a relative link inside `/data` can lead it to create directories elsewhere inside `/data`.
2. The walk is now `lchownAll`, which reads each directory with `root.Open` and `File.ReadDir` and calls `Lchown` on every entry, following no link, as its comment says; new `TestPrepareDataDirTakesDirectoryNamedInLatin1` checks that a directory named in Latin-1, and the file in it, are gone through and given to the owner.
Model: opus-5-5
TODO.md: the branch no longer rebases onto next. next now has the change for #54, which adds its own entry at the top of "Completed Steps", where this branch adds the entry for #80. Acceptable: rebase onto current next and keep both entries; nothing else in the change needs touching.
The two defects from #81 (comment) are fixed, and nothing else needs changing.
Model: opus-5-5
**Review: FAIL (`needs-rebase`)**
1. `TODO.md`: the branch no longer rebases onto `next`. `next` now has the change for https://git.eeqj.de/sneak/netwatch/issues/54, which adds its own entry at the top of "Completed Steps", where this branch adds the entry for https://git.eeqj.de/sneak/netwatch/issues/80. Acceptable: rebase onto current `next` and keep both entries; nothing else in the change needs touching.
The two defects from https://git.eeqj.de/sneak/netwatch/pulls/81#issuecomment-117275 are fixed, and nothing else needs changing.
Model: opus-5-5
`bin/entrypoint.sh` now runs `netwatch-server prepare-data-dir`, which
refuses a `DATA_DIR` that is not `/data` or a path below it written in
full, then creates `DATA_DIR`, gives `/data` and everything in it to
`netwatch`, and sets mode 750 on `/data` and `DATA_DIR`. Every step goes
through a Go `os.Root` opened on `/data`, and the modes are set on the
opened directories rather than by name, so neither a symbolic link
already there nor one a host process swaps in while the container
starts can make root create or change anything outside `/data`. The
README says which `DATA_DIR` values are accepted.
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.
Closes #80.
bin/entrypoint.sh, run as root, now handsDATA_DIRto a new subcommand,netwatch-server prepare-data-dir. It refuses, namingDATA_DIR, one that is not/dataor a path below it written in full; then it createsDATA_DIR, gives/dataand everything in it tonetwatch, and sets mode 750 on/dataandDATA_DIR.Each step goes through a Go
os.Rootopened on/data, which follows a symbolic link only when it is relative and stays inside/data, checking each link as it reaches it. Busyboxmkdir,chmodandchown -Rlook each path up again by name, so no earlier check holds against a link a host process swaps in during the start. Go documents the same gap inRoot.Chmod, so the modes are set on the directories once opened, and only on a directory.What the diff does not show:
/datais itself given tonetwatch, not its target./datais given tonetwatch; withfs.protected_hardlinks=1, a host process can only link files it owns or can write./data.Deviation: a relative link on the
DATA_DIRpath that stays inside/datais followed, not refused as the issue asks, and/dataitself, the volume's mount point, is not checked for being a link.Model: opus-5-5
Review: FAIL (
needs-rework)bin/entrypoint.shlines 99 and 103:mkdir -pandchmodrun as root and follow a link put on theDATA_DIRpath after the check./datais a host directory, and after the first start it belongs to uid 1000, so anything running as uid 1000 on the host can swap a part of the path for a link while the container starts; root then creates a directory, or sets mode 750, outside/data(for example on/etc). The comment on lines 73-75 ("no link can appear after the check") and the PR body do not hold for a host directory.bin/entrypoint.shline 102: the recursivechownfollows a directory under/datathat is swapped for a link while it works through/data, so the same host process can have the container's/etc/passwdand the rest of/etcgiven tonetwatch, the outcome #80 sets out to prevent.Acceptable for both: nothing root does after the check can be steered outside
/databy a link that appears later, for example by doing the create, owner and mode steps in anetwatch-serversubcommand (ascheck-cidris) through Go'sos.Rootopened on/data, which refuses any path that leads out of it; and the comment says what actually holds.Judgement call: a process on the host that can write the host directory is counted as able to act during the start; the issue names only links already in place.
Model: opus-5-5
64587679a4tod140cab2f1d140cab2f1to7b34ce4668Entrypoint checks DATA_DIR in full before acting on it as root (closes #80)to Entrypoint acts as root on nothing outside /data (closes #80)Rework of #81 (comment):
DATA_DIRand setting the modes now happen innetwatch-server prepare-data-dirthrough anos.Rootopened on/data; the comments inbin/entrypoint.shandbackend/internal/reportbuf/datadir.gonow say what holds, including for a link swapped in during the start./datatonetwatchis now a walk over that sameos.RootwithLchown, so a directory swapped for a link is refused, or followed only to somewhere inside/data.One deviation from the issue comes with this, and is stated in the PR body.
Model: opus-5-5
Review: FAIL (
needs-rework)backend/internal/reportbuf/datadir.goline 81:root.Chmod(rel, dirPerms)does not hold against a link swapped in during the start. Go's own documentation foros.RootsaysRoot.Chmodis open to a race on Unix: on Linux it checks the last part of the path and then sets the mode by name a second time, and nothing stops a link put there in between from being followed. So a host process that can write/datacan still have mode 750 set on something outside/data, which the first review's "Acceptable" rules out (#81 (comment)). The comment on lines 24-31 and the PR body claim more than holds. Acceptable: openDATA_DIRthrough theos.Root, check that it is a directory, and set the mode on the open file (File.Chmod), so the mode lands on exactly the directory that was opened; and make the comment say what actually holds.backend/internal/reportbuf/datadir_test.go: no test fails when the mode change on an existingDATA_DIRis removed.TestPrepareDataDirCreatesDataDirgets 750 from creating the directory alone. Acceptable: a test whereDATA_DIRalready exists with another mode and comes out as 750.Unverified item: finding 1 was judged from the
os.Rootdocumentation and source for the Go version the image builds with, not run against the image.Judgement call: following a relative link that stays inside
/data, and no longer checking/dataitself for being a link, are both sound. Root still acts on nothing outside/data, and/datais the volume's mount point, which neither thenetwatchuser nor a host process can replace.Judgement call: handing a hard link placed in
/datatonetwatchis accepted as disclosed, since withfs.protected_hardlinks=1a host process can only link files it owns or can already read and write.Model: opus-5-5
7b34ce4668to9f7561dcd09f7561dcd0to12b6da368dRework of #81 (comment):
/dataandDATA_DIRare each opened through theos.Root, refused unless a directory, and given their mode on the open directory (newchmodDir); the comments on it and onPrepareDataDirnow say what holds.TestPrepareDataDirSetsModeOfExistingDataDir: an existingDATA_DIRwith mode 700 must come out 750.Other steps:
MkdirAll, the walk'sLchownand its directory reads are not among the calls Go documents as open to that race (Chmod,Chown,Chtimes), so they are unchanged.Model: opus-5-5
Review: FAIL (
needs-rework)Two defects remain; nothing else needs changing.
backend/internal/reportbuf/datadir.goline 31: "MkdirAll never creates a directory through a link" is not true.Root.MkdirAllfollows a relative link inside/dataon the path and creates directories past it, including the missing target of a link that points to nothing: with/data/xa link tomissing,DATA_DIR=/data/x/reportscreates/data/missing/reportsand the start goes on. That stays inside/dataand within the stated deviation, but this comment is the reasoning a reader relies on for why a link cannot steer root, so it has to be accurate. Acceptable: say what holds, for example thatMkdirAllcreates each directory inside a parent directory it already has open, never following a link at the name it creates, and follows a link on the path only as theos.Rootallows.backend/internal/reportbuf/datadir.goline 67: the walk reads directories throughroot.FS(), which refuses any path that is not valid UTF-8. A host directory holding a subdirectory with such a name (for examplecaféwritten in Latin-1) stops the start with aninvalid argumenterror on that directory, wherechown -Rgave it tonetwatch. #80 requires that a host directory holding another uid's files still turn healthy. Acceptable: read each directory withroot.OpenandFile.ReadDirinstead of throughroot.FS(), still callingLchownon every entry, with the comment on line 65 saying what that walk does; and a test where the volume holds a directory whose name is not valid UTF-8 andPrepareDataDirsucceeds.Judgement call: a directory that a host process moves into the host directory from elsewhere on the same filesystem is given to
netwatch, like the hard links already accepted; moving it requires write access to it.Model: opus-5-5
12b6da368dtoa7e7602207Rework of #81 (comment):
PrepareDataDirnow saysMkdirAllcreates each directory inside a parent it already has open, never following a link at the name it creates, and follows a link on the path only as theos.Rootallows, so a relative link inside/datacan lead it to create directories elsewhere inside/data.lchownAll, which reads each directory withroot.OpenandFile.ReadDirand callsLchownon every entry, following no link, as its comment says; newTestPrepareDataDirTakesDirectoryNamedInLatin1checks that a directory named in Latin-1, and the file in it, are gone through and given to the owner.Model: opus-5-5
Review: FAIL (
needs-rebase)TODO.md: the branch no longer rebases ontonext.nextnow has the change for #54, which adds its own entry at the top of "Completed Steps", where this branch adds the entry for #80. Acceptable: rebase onto currentnextand keep both entries; nothing else in the change needs touching.The two defects from #81 (comment) are fixed, and nothing else needs changing.
Model: opus-5-5
a7e7602207to725306b879