fix: make daemon reset conflict safe
This commit is contained in:
+10
-33
@@ -12,7 +12,6 @@ import (
|
||||
"syscall"
|
||||
|
||||
tea "github.com/charmbracelet/bubbletea"
|
||||
"golang.org/x/sys/unix"
|
||||
|
||||
"proxyctl/internal/app"
|
||||
"proxyctl/internal/apt"
|
||||
@@ -244,44 +243,22 @@ func diagnosticChecks(configuration profile.Configuration) []target.Check {
|
||||
}
|
||||
|
||||
func storagePermissions(configuration profile.Configuration) (bool, string) {
|
||||
ok, evidence := writableLocation(configuration.Path)
|
||||
info, err := os.Stat(configuration.Path)
|
||||
authority := target.ReadOnlyWriteAuthority(configuration.Path)
|
||||
if !authority.OK {
|
||||
return false, authority.Evidence
|
||||
}
|
||||
info, err := os.Lstat(configuration.Path)
|
||||
if os.IsNotExist(err) {
|
||||
return ok, evidence + "; Profile configuration does not exist yet"
|
||||
return true, authority.Evidence + "; Profile configuration does not exist yet"
|
||||
}
|
||||
if err != nil {
|
||||
return false, evidence + "; Profile configuration cannot be inspected: " + err.Error()
|
||||
return false, authority.Evidence + "; Profile configuration cannot be inspected: " + err.Error()
|
||||
}
|
||||
if info.Mode().Perm() != 0o600 {
|
||||
return false, fmt.Sprintf("%s; Profile configuration mode is %o, want 600", evidence, info.Mode().Perm())
|
||||
return false, fmt.Sprintf("%s; Profile configuration mode is %o, want 600", authority.Evidence, info.Mode().Perm())
|
||||
}
|
||||
if stat, present := info.Sys().(*syscall.Stat_t); present && int(stat.Uid) != configuration.Owner.UID {
|
||||
return false, fmt.Sprintf("%s; Profile configuration owner uid is %d, want %d", evidence, stat.Uid, configuration.Owner.UID)
|
||||
}
|
||||
return ok, evidence + "; Profile configuration is owner-only"
|
||||
}
|
||||
|
||||
// writableLocation observes current authority without creating or changing files.
|
||||
func writableLocation(path string) (bool, string) {
|
||||
candidate := path
|
||||
for {
|
||||
info, err := os.Stat(candidate)
|
||||
if err == nil {
|
||||
if !info.IsDir() {
|
||||
candidate = filepath.Dir(candidate)
|
||||
}
|
||||
if err := unix.Access(candidate, unix.W_OK); err != nil {
|
||||
return false, candidate + " is not writable: " + err.Error()
|
||||
}
|
||||
return true, candidate + " is writable"
|
||||
}
|
||||
if !os.IsNotExist(err) {
|
||||
return false, candidate + " cannot be inspected: " + err.Error()
|
||||
}
|
||||
parent := filepath.Dir(candidate)
|
||||
if parent == candidate {
|
||||
return false, path + " has no writable existing ancestor"
|
||||
}
|
||||
candidate = parent
|
||||
return false, fmt.Sprintf("%s; Profile configuration owner uid is %d, want %d", authority.Evidence, stat.Uid, configuration.Owner.UID)
|
||||
}
|
||||
return true, authority.Evidence + "; Profile configuration is owner-only"
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -98,6 +99,69 @@ func TestDoctorReturnsFailureWhenAnyCheckFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoctorStorageAuthorityRejectsSymlinkedPathsWithoutTouchingVictim(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
path func(t *testing.T, root, victim string) string
|
||||
}{
|
||||
{
|
||||
name: "final path",
|
||||
path: func(t *testing.T, root, victim string) string {
|
||||
t.Helper()
|
||||
path := filepath.Join(root, "config", "profiles.json")
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(victim, path); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return path
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ancestor path",
|
||||
path: func(t *testing.T, root, victim string) string {
|
||||
t.Helper()
|
||||
link := filepath.Join(root, "config")
|
||||
if err := os.Symlink(filepath.Dir(victim), link); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return filepath.Join(link, filepath.Base(victim))
|
||||
},
|
||||
},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
victim := filepath.Join(t.TempDir(), "victim.json")
|
||||
const content = `{"profiles":[]}`
|
||||
if err := os.WriteFile(victim, []byte(content), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
before, err := os.Stat(victim)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
configuration := profile.Configuration{Path: test.path(t, root, victim), Owner: profile.UserIdentity{UID: os.Getuid(), GID: os.Getgid()}}
|
||||
|
||||
checks := diagnosticChecks(configuration)
|
||||
if len(checks) != 2 || checks[1].OK || !strings.Contains(checks[1].Evidence, "symbolic link") {
|
||||
t.Fatalf("Profile storage diagnostics accepted symlink: %#v", checks)
|
||||
}
|
||||
after, err := os.Stat(victim)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got, err := os.ReadFile(victim)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(got) != content || !reflect.DeepEqual(before.Sys(), after.Sys()) {
|
||||
t.Fatalf("storage diagnostics changed victim: before=%#v after=%#v content=%q", before, after, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoctorStoragePermissionCheckIsReadOnly(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json")
|
||||
writeConfig(t, path, `{"profiles":[]}`)
|
||||
|
||||
Reference in New Issue
Block a user