[AUTOMATION] fix(localruntime): harden guard socket dir#292
Conversation
michiosw
changed the title
Greptile SummaryThis PR hardens local runtime socket directory setup. The main changes are:
Confidence Score: 3/5This should be fixed before merging.
|
| Filename | Overview |
|---|---|
| internal/localruntime/socket.go | Adds socket directory validation and permission tightening, but introduces a relative-path chmod bug and leaves a validation race. |
| internal/localruntime/socket_test.go | Adds coverage for basic creation, permission tightening, and direct symlink rejection. |
Reviews (1): Last reviewed commit: "fix(localruntime): harden guard socket d..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| dir := filepath.Dir(socketPath) | ||
| if err := os.MkdirAll(dir, 0o700); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| info, err := os.Lstat(dir) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if info.Mode()&os.ModeSymlink != 0 { | ||
| return fmt.Errorf("socket directory %q must not be a symlink", dir) | ||
| } | ||
| if !info.IsDir() { | ||
| return fmt.Errorf("socket directory %q is not a directory", dir) | ||
| } | ||
|
|
||
| stat, ok := info.Sys().(*syscall.Stat_t) | ||
| if !ok { | ||
| return fmt.Errorf("socket directory %q owner could not be verified", dir) | ||
| } | ||
| if int(stat.Uid) != os.Getuid() { | ||
| return fmt.Errorf("socket directory %q must be owned by uid %d", dir, os.Getuid()) | ||
| } | ||
|
|
||
| return os.Chmod(dir, 0o700) |
There was a problem hiding this comment.
When socketPath is a basename such as kontext.sock, filepath.Dir(socketPath) returns .. The new unconditional os.Chmod(dir, 0o700) then changes the current working directory permissions, so a user setting KONTEXT_GUARD_SOCKET=kontext.sock from a shared project directory can unexpectedly remove group/world access from that directory. The previous MkdirAll(".", 0700) path did not tighten existing cwd permissions.
| info, err := os.Lstat(dir) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if info.Mode()&os.ModeSymlink != 0 { | ||
| return fmt.Errorf("socket directory %q must not be a symlink", dir) | ||
| } | ||
| if !info.IsDir() { | ||
| return fmt.Errorf("socket directory %q is not a directory", dir) | ||
| } | ||
|
|
||
| stat, ok := info.Sys().(*syscall.Stat_t) | ||
| if !ok { | ||
| return fmt.Errorf("socket directory %q owner could not be verified", dir) | ||
| } | ||
| if int(stat.Uid) != os.Getuid() { | ||
| return fmt.Errorf("socket directory %q must be owned by uid %d", dir, os.Getuid()) | ||
| } | ||
|
|
||
| return os.Chmod(dir, 0o700) |
There was a problem hiding this comment.
The directory is checked with Lstat, but Chmod re-resolves the path later and follows symlinks. In a writable parent, another process can replace the checked directory after the ownership/symlink checks and before Chmod or the later socket bind, causing the hardening to apply to a different target than the one that was validated. This leaves the symlink protection incomplete for the path this PR is trying to secure.
1 participant
Where We Are
The guard daemon trusted any existing parent directory for its Unix socket under /tmp. Another local user could pre-create that directory with unsafe permissions or redirect it through a symlink.
Where We Want To Go
The guard daemon should only start when the socket directory is a real directory owned by the current user and locked down to mode 0700.
How do we get there
We harden internal/localruntime.EnsureSocketDir to create the directory, reject symlinks and non-directories, verify the owner UID, and chmod safe same-user directories to 0700 before the daemon listens. We added focused socket directory tests for create, chmod, and symlink rejection. Verification: go test ./..., go vet ./..., npm exec --yes --package pnpm@10.0.0 -- pnpm install --frozen-lockfile, npm exec --yes --package pnpm@10.0.0 -- pnpm --dir web/guard-dashboard typecheck, git diff --check.