From a60e60c8993d8d53ef5f99ea0825f0d3594eb16c Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sun, 19 Jul 2026 14:54:55 +0200 Subject: [PATCH] fix(sandbox): add platform-specific write lock around grant state file (#752) Add grants_lock_unix.go and grants_lock_windows.go with a lockStateFile method that serializes access to the persisted grant state across processes. Wire the lock into Grant, GrantCommandPrefix, Revoke, RevokePath, and Clear. Also add .gitattributes to force LF line endings for Go files so gofmt checks pass on Windows. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .gitattributes | 3 ++ internal/sandbox/grants.go | 66 ++++++++++++++++++++++++- internal/sandbox/grants_lock_unix.go | 24 +++++++++ internal/sandbox/grants_lock_windows.go | 60 ++++++++++++++++++++++ internal/sandbox/grants_test.go | 42 ++++++++++++++++ 5 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 .gitattributes create mode 100644 internal/sandbox/grants_lock_unix.go create mode 100644 internal/sandbox/grants_lock_windows.go diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 000000000..a6231632d --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +*.go text eol=lf +*.mod text eol=lf +*.sum text eol=lf diff --git a/internal/sandbox/grants.go b/internal/sandbox/grants.go index ad2170c87..591588526 100644 --- a/internal/sandbox/grants.go +++ b/internal/sandbox/grants.go @@ -15,7 +15,11 @@ import ( "github.com/Gitlawb/zero/internal/redaction" ) -const grantSchemaVersion = 2 +const ( + grantSchemaVersion = 2 + grantLockTimeout = 5 * time.Second + grantLockRetry = 10 * time.Millisecond +) type Grant struct { ToolName string `json:"toolName"` @@ -125,6 +129,12 @@ func (store *GrantStore) Grant(input GrantInput) (Grant, error) { } store.mu.Lock() defer store.mu.Unlock() + unlock, err := store.lockStateFile() + if err != nil { + return Grant{}, err + } + defer unlock() + state, err := store.readState() if err != nil { return Grant{}, err @@ -157,6 +167,12 @@ func (store *GrantStore) GrantCommandPrefix(input CommandPrefixInput) (CommandPr } store.mu.Lock() defer store.mu.Unlock() + unlock, err := store.lockStateFile() + if err != nil { + return CommandPrefixGrant{}, err + } + defer unlock() + state, err := store.readState() if err != nil { return CommandPrefixGrant{}, err @@ -301,6 +317,12 @@ func (store *GrantStore) Revoke(toolName string) (int, error) { } store.mu.Lock() defer store.mu.Unlock() + unlock, err := store.lockStateFile() + if err != nil { + return 0, err + } + defer unlock() + state, err := store.readState() if err != nil { return 0, err @@ -336,6 +358,12 @@ func (store *GrantStore) RevokePath(toolName string, scopePath string) (int, err } store.mu.Lock() defer store.mu.Unlock() + unlock, err := store.lockStateFile() + if err != nil { + return 0, err + } + defer unlock() + state, err := store.readState() if err != nil { return 0, err @@ -371,6 +399,12 @@ func (store *GrantStore) RevokePath(toolName string, scopePath string) (int, err func (store *GrantStore) Clear() (int, error) { store.mu.Lock() defer store.mu.Unlock() + unlock, err := store.lockStateFile() + if err != nil { + return 0, err + } + defer unlock() + state, err := store.readState() if err != nil { return 0, err @@ -607,6 +641,36 @@ func (store *GrantStore) writeState(state grantFile) error { return nil } +func (store *GrantStore) lockStateFile() (func(), error) { + lockPath := store.filePath + ".lockfile" + if err := os.MkdirAll(filepath.Dir(store.filePath), 0o700); err != nil { + return nil, err + } + file, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, err + } + deadline := time.Now().Add(grantLockTimeout) + for { + locked, err := tryLockGrantFile(file) + if err != nil { + _ = file.Close() + return nil, err + } + if locked { + return func() { + _ = unlockGrantFile(file) + _ = file.Close() + }, nil + } + if time.Now().After(deadline) { + _ = file.Close() + return nil, fmt.Errorf("timed out waiting for sandbox grants lock at %s", lockPath) + } + time.Sleep(grantLockRetry) + } +} + func normalizeStoredGrant(name string, grant Grant) (Grant, error) { if strings.TrimSpace(grant.ToolName) == "" { grant.ToolName = name diff --git a/internal/sandbox/grants_lock_unix.go b/internal/sandbox/grants_lock_unix.go new file mode 100644 index 000000000..ac36b674a --- /dev/null +++ b/internal/sandbox/grants_lock_unix.go @@ -0,0 +1,24 @@ +//go:build !windows + +package sandbox + +import ( + "errors" + "os" + "syscall" +) + +func tryLockGrantFile(file *os.File) (bool, error) { + err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB) + if err == nil { + return true, nil + } + if errors.Is(err, syscall.EWOULDBLOCK) || errors.Is(err, syscall.EAGAIN) { + return false, nil + } + return false, err +} + +func unlockGrantFile(file *os.File) error { + return syscall.Flock(int(file.Fd()), syscall.LOCK_UN) +} diff --git a/internal/sandbox/grants_lock_windows.go b/internal/sandbox/grants_lock_windows.go new file mode 100644 index 000000000..fa56261d3 --- /dev/null +++ b/internal/sandbox/grants_lock_windows.go @@ -0,0 +1,60 @@ +//go:build windows + +package sandbox + +import ( + "errors" + "os" + "syscall" + "unsafe" +) + +const ( + grantLockfileFailImmediately = 0x00000001 + grantLockfileExclusiveLock = 0x00000002 + grantErrorLockViolation = syscall.Errno(33) + grantErrorSharingViolation = syscall.Errno(32) +) + +var ( + grantKernel32 = syscall.NewLazyDLL("kernel32.dll") + grantProcLockFileEx = grantKernel32.NewProc("LockFileEx") + grantProcUnlockFileEx = grantKernel32.NewProc("UnlockFileEx") +) + +func tryLockGrantFile(file *os.File) (bool, error) { + var overlapped syscall.Overlapped + result, _, err := grantProcLockFileEx.Call( + file.Fd(), + uintptr(grantLockfileExclusiveLock|grantLockfileFailImmediately), + 0, + 1, + 0, + uintptr(unsafe.Pointer(&overlapped)), + ) + if result != 0 { + return true, nil + } + if errors.Is(err, grantErrorLockViolation) || errors.Is(err, grantErrorSharingViolation) { + return false, nil + } + if err == syscall.Errno(0) { + return false, nil + } + return false, err +} + +func unlockGrantFile(file *os.File) error { + var overlapped syscall.Overlapped + result, _, err := grantProcUnlockFileEx.Call( + file.Fd(), + 0, + 1, + 0, + uintptr(unsafe.Pointer(&overlapped)), + ) + if result != 0 || err == syscall.Errno(0) { + return nil + } + return err +} diff --git a/internal/sandbox/grants_test.go b/internal/sandbox/grants_test.go index ae0a81bce..08a17c7d9 100644 --- a/internal/sandbox/grants_test.go +++ b/internal/sandbox/grants_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) func TestGrantStorePersistsListsRevokesAndClears(t *testing.T) { @@ -70,6 +71,47 @@ func TestGrantStorePersistsListsRevokesAndClears(t *testing.T) { } } +func TestGrantStoreSerializesWritesAcrossStores(t *testing.T) { + path := filepath.Join(t.TempDir(), "sandbox-grants.json") + storeA, err := NewGrantStore(StoreOptions{FilePath: path}) + if err != nil { + t.Fatalf("NewGrantStore A returned error: %v", err) + } + storeB, err := NewGrantStore(StoreOptions{FilePath: path}) + if err != nil { + t.Fatalf("NewGrantStore B returned error: %v", err) + } + + unlock, err := storeA.lockStateFile() + if err != nil { + t.Fatalf("lockStateFile returned error: %v", err) + } + + done := make(chan error, 1) + go func() { + _, err := storeB.Grant(GrantInput{ToolName: "write_file", Decision: GrantAllow}) + done <- err + }() + + select { + case err := <-done: + unlock() + t.Fatalf("Grant completed while another store held the file lock: %v", err) + case <-time.After(100 * time.Millisecond): + // Expected: storeB is waiting for storeA to release the lock. + } + + unlock() + select { + case err := <-done: + if err != nil { + t.Fatalf("Grant returned error after file lock release: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("Grant did not complete after file lock release") + } +} + func TestGrantStoreReadsV1GrantFile(t *testing.T) { root := t.TempDir() path := filepath.Join(root, "sandbox-grants.json")