Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion internal/localruntime/socket.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path/filepath"
"syscall"
)

func DefaultSocketPath() string {
Expand All @@ -14,5 +15,29 @@ func DefaultSocketPath() string {
}

func EnsureSocketDir(socketPath string) error {
return os.MkdirAll(filepath.Dir(socketPath), 0o700)
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)
Comment on lines +18 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Avoid chmodding cwd

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.

Comment on lines +23 to +42

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Close validation race

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.

}
84 changes: 84 additions & 0 deletions internal/localruntime/socket_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package localruntime

import (
"os"
"path/filepath"
"strings"
"syscall"
"testing"
)

func TestEnsureSocketDirCreatesPrivateDirectory(t *testing.T) {
t.Parallel()

root := t.TempDir()
socketPath := filepath.Join(root, "guard", "kontext.sock")
if err := EnsureSocketDir(socketPath); err != nil {
t.Fatalf("EnsureSocketDir() error = %v", err)
}

info, err := os.Stat(filepath.Dir(socketPath))
if err != nil {
t.Fatalf("Stat() error = %v", err)
}
if got := info.Mode().Perm(); got != 0o700 {
t.Fatalf("socket dir mode = %o, want 700", got)
}

stat, ok := info.Sys().(*syscall.Stat_t)
if !ok {
t.Fatal("socket dir stat missing uid")
}
if got := int(stat.Uid); got != os.Getuid() {
t.Fatalf("socket dir owner uid = %d, want %d", got, os.Getuid())
}
}

func TestEnsureSocketDirTightensExistingDirectoryPermissions(t *testing.T) {
t.Parallel()

root := t.TempDir()
dir := filepath.Join(root, "guard")
if err := os.MkdirAll(dir, 0o777); err != nil {
t.Fatalf("MkdirAll() error = %v", err)
}
if err := os.Chmod(dir, 0o777); err != nil {
t.Fatalf("Chmod() setup error = %v", err)
}

socketPath := filepath.Join(dir, "kontext.sock")
if err := EnsureSocketDir(socketPath); err != nil {
t.Fatalf("EnsureSocketDir() error = %v", err)
}

info, err := os.Stat(dir)
if err != nil {
t.Fatalf("Stat() error = %v", err)
}
if got := info.Mode().Perm(); got != 0o700 {
t.Fatalf("socket dir mode = %o, want 700", got)
}
}

func TestEnsureSocketDirRejectsSymlinkDirectory(t *testing.T) {
t.Parallel()

root := t.TempDir()
target := filepath.Join(root, "target")
if err := os.MkdirAll(target, 0o700); err != nil {
t.Fatalf("MkdirAll() error = %v", err)
}

link := filepath.Join(root, "guard")
if err := os.Symlink(target, link); err != nil {
t.Fatalf("Symlink() error = %v", err)
}

err := EnsureSocketDir(filepath.Join(link, "kontext.sock"))
if err == nil {
t.Fatal("EnsureSocketDir() error = nil, want symlink rejection")
}
if !strings.Contains(err.Error(), "must not be a symlink") {
t.Fatalf("EnsureSocketDir() error = %v, want symlink rejection", err)
}
}
Loading