diff --git a/internal/pkg/execsession/ancestry.go b/internal/pkg/execsession/ancestry.go deleted file mode 100644 index 8dc2b4b..0000000 --- a/internal/pkg/execsession/ancestry.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright IBM Corp. 2026 -// SPDX-License-Identifier: MPL-2.0 - -package execsession - -import "os" - -// AncestryFn returns the parent PID of pid, with ok=false when it cannot be -// determined or pid is invalid/dead. It is the seam that lets consumer logic be -// tested with a fake process tree. -type AncestryFn func(pid int) (ppid int, ok bool) - -// maxAncestryDepth bounds the parent walk to guard against cycles and -// pathological trees. -const maxAncestryDepth = 64 - -// IsAncestor walks parents starting at self up to a bounded depth, returning -// true if target is encountered. It fails closed (returns false) when ancestry -// cannot be resolved on this platform, when parentOf is nil, or when a cycle is -// detected. Because a dead PID will not appear in the walk, this also serves as -// a liveness check on target. -func IsAncestor(target, self int, parentOf AncestryFn) bool { - if parentOf == nil { - return false - } - - pid := self - for i := 0; i < maxAncestryDepth && pid > 1; i++ { - if pid == target { - return true - } - ppid, ok := parentOf(pid) - if !ok { - return false - } - if ppid == pid { - return false // self-cycle - } - pid = ppid - } - return pid == target -} - -// selfPID returns the current process id. It exists as a helper so tests can -// reference the process without importing os. -func selfPID() int { - return os.Getpid() -} diff --git a/internal/pkg/execsession/ancestry_darwin.go b/internal/pkg/execsession/ancestry_darwin.go deleted file mode 100644 index 2a6fc2b..0000000 --- a/internal/pkg/execsession/ancestry_darwin.go +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright IBM Corp. 2026 -// SPDX-License-Identifier: MPL-2.0 - -//go:build darwin - -package execsession - -import "golang.org/x/sys/unix" - -// ParentPID returns the parent pid of pid using the kern.proc.pid sysctl. -// Returns ok=false if the process does not exist or the lookup fails. -func ParentPID(pid int) (int, bool) { - kproc, err := unix.SysctlKinfoProc("kern.proc.pid", pid) - if err != nil || kproc == nil { - return 0, false - } - return int(kproc.Eproc.Ppid), true -} diff --git a/internal/pkg/execsession/ancestry_linux.go b/internal/pkg/execsession/ancestry_linux.go deleted file mode 100644 index d9d62ff..0000000 --- a/internal/pkg/execsession/ancestry_linux.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright IBM Corp. 2026 -// SPDX-License-Identifier: MPL-2.0 - -//go:build linux - -package execsession - -import ( - "os" - "strconv" - "strings" -) - -// ParentPID returns the parent pid of pid by reading /proc//stat. The -// second field (comm) is wrapped in parentheses and may itself contain spaces -// and parentheses, so the state and ppid fields are parsed relative to the last -// ')'. Returns ok=false if the process does not exist or cannot be parsed. -func ParentPID(pid int) (int, bool) { - data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") - if err != nil { - return 0, false - } - - contents := string(data) - rparen := strings.LastIndexByte(contents, ')') - if rparen < 0 || rparen+1 >= len(contents) { - return 0, false - } - - // After ") " the fields are: state ppid pgrp ... - fields := strings.Fields(contents[rparen+1:]) - if len(fields) < 2 { - return 0, false - } - - ppid, err := strconv.Atoi(fields[1]) - if err != nil { - return 0, false - } - return ppid, true -} diff --git a/internal/pkg/execsession/ancestry_test.go b/internal/pkg/execsession/ancestry_test.go deleted file mode 100644 index 6177c0e..0000000 --- a/internal/pkg/execsession/ancestry_test.go +++ /dev/null @@ -1,110 +0,0 @@ -// Copyright IBM Corp. 2026 -// SPDX-License-Identifier: MPL-2.0 - -package execsession - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -// fakeAncestry builds an AncestryFn from a child->parent map. A pid that is -// absent from the map is treated as dead/unknown (ok=false). -func fakeAncestry(parents map[int]int) AncestryFn { - return func(pid int) (int, bool) { - ppid, ok := parents[pid] - return ppid, ok - } -} - -func TestIsAncestor(t *testing.T) { - t.Parallel() - - t.Run("direct parent", func(t *testing.T) { - t.Parallel() - parents := map[int]int{100: 50, 50: 1} - assert.True(t, IsAncestor(50, 100, fakeAncestry(parents))) - }) - - t.Run("deep chain", func(t *testing.T) { - t.Parallel() - parents := map[int]int{100: 90, 90: 80, 80: 70, 70: 1} - assert.True(t, IsAncestor(80, 100, fakeAncestry(parents))) - }) - - t.Run("self is ancestor", func(t *testing.T) { - t.Parallel() - parents := map[int]int{100: 50} - assert.True(t, IsAncestor(100, 100, fakeAncestry(parents))) - }) - - t.Run("not an ancestor", func(t *testing.T) { - t.Parallel() - parents := map[int]int{100: 50, 50: 1} - assert.False(t, IsAncestor(999, 100, fakeAncestry(parents))) - }) - - t.Run("dead target pid not in chain", func(t *testing.T) { - t.Parallel() - // 100 -> 50 -> 1, target 777 is not in the walk. - parents := map[int]int{100: 50, 50: 1} - assert.False(t, IsAncestor(777, 100, fakeAncestry(parents))) - }) - - t.Run("unknown self pid fails closed", func(t *testing.T) { - t.Parallel() - parents := map[int]int{} // self 100 not resolvable - assert.False(t, IsAncestor(50, 100, fakeAncestry(parents))) - }) - - t.Run("cycle guard", func(t *testing.T) { - t.Parallel() - // 100 -> 50 -> 100 (cycle, neither is target 7) - parents := map[int]int{100: 50, 50: 100} - assert.False(t, IsAncestor(7, 100, fakeAncestry(parents))) - }) - - t.Run("self-cycle parent equals pid", func(t *testing.T) { - t.Parallel() - parents := map[int]int{100: 100} - assert.False(t, IsAncestor(7, 100, fakeAncestry(parents))) - }) - - t.Run("depth cap", func(t *testing.T) { - t.Parallel() - // Build a long chain where target sits beyond the depth cap. - parents := make(map[int]int) - for i := 1000; i > 100; i-- { - parents[i] = i - 1 - } - // target 100 is ~900 hops from 1000, beyond the 64 cap. - assert.False(t, IsAncestor(100, 1000, fakeAncestry(parents))) - }) - - t.Run("nil parentOf fails closed", func(t *testing.T) { - t.Parallel() - assert.False(t, IsAncestor(50, 100, nil)) - }) - - t.Run("reaching init without target", func(t *testing.T) { - t.Parallel() - parents := map[int]int{100: 1} - assert.False(t, IsAncestor(2, 100, fakeAncestry(parents))) - }) -} - -func TestParentPID_Self(t *testing.T) { - t.Parallel() - - // The platform ParentPID for our own process should resolve to our actual - // parent (the test runner / shell). We can't assert the exact value but it - // should be resolvable and positive on supported platforms. - self := selfPID() - ppid, ok := ParentPID(self) - if !ok { - t.Skip("ParentPID not supported on this platform") - } - assert.Positive(t, ppid) - assert.NotEqual(t, self, ppid) -} diff --git a/internal/pkg/execsession/ancestry_windows.go b/internal/pkg/execsession/ancestry_windows.go deleted file mode 100644 index 487a224..0000000 --- a/internal/pkg/execsession/ancestry_windows.go +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright IBM Corp. 2026 -// SPDX-License-Identifier: MPL-2.0 - -//go:build windows - -package execsession - -import ( - "unsafe" - - "golang.org/x/sys/windows" -) - -// ParentPID returns the parent pid of pid by walking a Toolhelp32 process -// snapshot. Returns ok=false if the snapshot fails or the process is not found. -func ParentPID(pid int) (int, bool) { - snapshot, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPPROCESS, 0) - if err != nil { - return 0, false - } - defer windows.CloseHandle(snapshot) - - var entry windows.ProcessEntry32 - entry.Size = uint32(unsafe.Sizeof(entry)) - - if err := windows.Process32First(snapshot, &entry); err != nil { - return 0, false - } - - for { - if int(entry.ProcessID) == pid { - return int(entry.ParentProcessID), true - } - if err := windows.Process32Next(snapshot, &entry); err != nil { - return 0, false - } - } -} diff --git a/internal/pkg/execsession/execsession.go b/internal/pkg/execsession/execsession.go index 120db08..b8480de 100644 --- a/internal/pkg/execsession/execsession.go +++ b/internal/pkg/execsession/execsession.go @@ -44,8 +44,8 @@ const ( ReasonNoSession = "no-session" // ReasonStale indicates the env var was set but the session file is gone. ReasonStale = "stale" - // ReasonNotAnAncestor indicates the granting process is not a live ancestor. - ReasonNotAnAncestor = "not-an-ancestor" + // ReasonNotLive indicates the granting process is no longer alive. + ReasonNotLive = "not-live" // ReasonClassNotGranted indicates the resource class was not permitted. ReasonClassNotGranted = "class-not-granted" // ReasonGranted indicates the delete is authorized. @@ -140,7 +140,8 @@ func (s *Store) path(token string) (string, bool) { } // Create issues a new token, writes the session file with 0600 permissions, and -// acquires an exclusive advisory lock held open in the returned Handle. +// acquires a shared advisory lock held open in the returned Handle. The lock is +// held for the process lifetime so authorizers can detect liveness. func (s *Store) Create(perms Permissions, pid int) (*Handle, error) { if err := os.MkdirAll(s.Dir, 0o700); err != nil { return nil, fmt.Errorf("failed to create exec session directory %q: %w", s.Dir, err) @@ -235,14 +236,20 @@ type Decision struct { Reason string } +// LivenessFn reports whether the process that granted the session file at path +// is still alive. It is the seam that lets authorization be tested without real +// processes. It reports alive=true when the granting process still holds its +// shared lock on the file, and alive=false once that lock has been released +// (the process exited or was killed). +type LivenessFn func(path string) (alive bool, err error) + // EnvAuthorizer is the runtime Authorizer. It reads the session token from the -// environment, loads the session, and verifies liveness/ancestry before -// checking the granted classes. +// environment, loads the session, and verifies the granting process is still +// alive before checking the granted classes. type EnvAuthorizer struct { Store *Store Getenv func(string) string // default os.Getenv - Ancestry AncestryFn // default ParentPID (platform) - Self int // default os.Getpid() + Liveness LivenessFn // default probeLiveness } func (a *EnvAuthorizer) getenv(key string) string { @@ -252,18 +259,11 @@ func (a *EnvAuthorizer) getenv(key string) string { return os.Getenv(key) } -func (a *EnvAuthorizer) ancestry() AncestryFn { - if a.Ancestry != nil { - return a.Ancestry - } - return ParentPID -} - -func (a *EnvAuthorizer) self() int { - if a.Self != 0 { - return a.Self +func (a *EnvAuthorizer) liveness() LivenessFn { + if a.Liveness != nil { + return a.Liveness } - return selfPID() + return probeLiveness } // AuthorizeDelete implements Authorizer. @@ -282,10 +282,21 @@ func (a *EnvAuthorizer) AuthorizeDelete(class string) (Decision, error) { return Decision{}, fmt.Errorf("failed to load exec session: %w", err) } - // Liveness + anti-leak: the granting PID must be a live ancestor of this - // process. A dead PID will not appear in the ancestor walk. - if !IsAncestor(sess.PID, a.self(), a.ancestry()) { - return Decision{Allowed: false, Token: sess.Token, Reason: ReasonNotAnAncestor}, nil + // Liveness: the granting process holds a shared lock on the session file for + // its lifetime. If we can take an exclusive lock, the holder is gone and the + // grant no longer applies. The token itself is the authorization boundary; + // possession plus a live holder is sufficient. + path, ok := a.Store.path(token) + if !ok { + // Load already validated the token, so this should be unreachable. + return Decision{Allowed: false, Reason: ReasonStale}, nil + } + alive, err := a.liveness()(path) + if err != nil { + return Decision{}, fmt.Errorf("failed to check exec session liveness: %w", err) + } + if !alive { + return Decision{Allowed: false, Token: sess.Token, Reason: ReasonNotLive}, nil } if !AllowsDelete(sess.AllowDelete, class) { diff --git a/internal/pkg/execsession/execsession_test.go b/internal/pkg/execsession/execsession_test.go index bf61aad..0e91f8d 100644 --- a/internal/pkg/execsession/execsession_test.go +++ b/internal/pkg/execsession/execsession_test.go @@ -47,6 +47,35 @@ func TestStoreCreateLoadRoundTrip(t *testing.T) { assert.True(t, os.IsNotExist(err), "file should be removed after Close") } +func TestProbeLiveness(t *testing.T) { + t.Parallel() + + store := &Store{Dir: t.TempDir()} + handle, err := store.Create(Permissions{AllowDelete: []string{"workspaces"}}, os.Getpid()) + require.NoError(t, err) + + path := filepath.Join(store.Dir, handle.Token()+".hcl") + + // While the handle holds its shared lock, the holder reads as alive. + alive, err := probeLiveness(path) + require.NoError(t, err) + assert.True(t, alive, "holder should read as alive while lock is held") + + // After Close releases the lock and removes the file, it reads as not alive. + require.NoError(t, handle.Close()) + alive, err = probeLiveness(path) + require.NoError(t, err) + assert.False(t, alive, "holder should read as not alive after Close") +} + +func TestProbeLivenessMissingFile(t *testing.T) { + t.Parallel() + + alive, err := probeLiveness(filepath.Join(t.TempDir(), "nope.hcl")) + require.NoError(t, err) + assert.False(t, alive) +} + func TestStoreLoadMissing(t *testing.T) { t.Parallel() @@ -94,13 +123,17 @@ func TestEnvAuthorizerAuthorizeDelete(t *testing.T) { return store, h.Token() } + // fakeLiveness returns a LivenessFn reporting the given liveness for any path. + fakeLiveness := func(alive bool) LivenessFn { + return func(string) (bool, error) { return alive, nil } + } + t.Run("no env returns no-session", func(t *testing.T) { t.Parallel() a := &EnvAuthorizer{ Store: &Store{Dir: t.TempDir()}, Getenv: func(string) string { return "" }, - Self: 100, - Ancestry: fakeAncestry(map[int]int{}), + Liveness: fakeLiveness(true), } d, err := a.AuthorizeDelete("workspaces") require.NoError(t, err) @@ -114,8 +147,7 @@ func TestEnvAuthorizerAuthorizeDelete(t *testing.T) { a := &EnvAuthorizer{ Store: &Store{Dir: t.TempDir()}, Getenv: func(string) string { return "GHOSTTOKEN234567ABCDEF" }, - Self: 100, - Ancestry: fakeAncestry(map[int]int{}), + Liveness: fakeLiveness(true), } d, err := a.AuthorizeDelete("workspaces") require.NoError(t, err) @@ -123,30 +155,28 @@ func TestEnvAuthorizerAuthorizeDelete(t *testing.T) { assert.Equal(t, ReasonStale, d.Reason) }) - t.Run("pid not an ancestor is denied", func(t *testing.T) { + t.Run("dead holder is denied", func(t *testing.T) { t.Parallel() store, token := newStoreWithSession(t, Permissions{AllowDelete: []string{"workspaces"}}, 4242) a := &EnvAuthorizer{ Store: store, Getenv: func(string) string { return token }, - Self: 100, - Ancestry: fakeAncestry(map[int]int{100: 50, 50: 1}), // 4242 absent + Liveness: fakeLiveness(false), } d, err := a.AuthorizeDelete("workspaces") require.NoError(t, err) assert.False(t, d.Allowed) - assert.Equal(t, ReasonNotAnAncestor, d.Reason) + assert.Equal(t, ReasonNotLive, d.Reason) assert.Equal(t, token, d.Token, "token surfaced for audit even when denied") }) - t.Run("ancestor ok but class not granted is denied", func(t *testing.T) { + t.Run("live holder but class not granted is denied", func(t *testing.T) { t.Parallel() store, token := newStoreWithSession(t, Permissions{AllowDelete: []string{"runs"}}, 4242) a := &EnvAuthorizer{ Store: store, Getenv: func(string) string { return token }, - Self: 100, - Ancestry: fakeAncestry(map[int]int{100: 4242, 4242: 1}), + Liveness: fakeLiveness(true), } d, err := a.AuthorizeDelete("workspaces") require.NoError(t, err) @@ -155,14 +185,13 @@ func TestEnvAuthorizerAuthorizeDelete(t *testing.T) { assert.Equal(t, token, d.Token) }) - t.Run("ancestor ok and class granted is allowed", func(t *testing.T) { + t.Run("live holder and class granted is allowed", func(t *testing.T) { t.Parallel() store, token := newStoreWithSession(t, Permissions{AllowDelete: []string{"workspaces"}}, 4242) a := &EnvAuthorizer{ Store: store, Getenv: func(string) string { return token }, - Self: 100, - Ancestry: fakeAncestry(map[int]int{100: 4242, 4242: 1}), + Liveness: fakeLiveness(true), } d, err := a.AuthorizeDelete("workspaces") require.NoError(t, err) @@ -177,8 +206,7 @@ func TestEnvAuthorizerAuthorizeDelete(t *testing.T) { a := &EnvAuthorizer{ Store: store, Getenv: func(string) string { return token }, - Self: 100, - Ancestry: fakeAncestry(map[int]int{100: 4242, 4242: 1}), + Liveness: fakeLiveness(true), } d, err := a.AuthorizeDelete("projects") require.NoError(t, err) @@ -186,19 +214,15 @@ func TestEnvAuthorizerAuthorizeDelete(t *testing.T) { assert.Equal(t, ReasonClassNotGranted, d.Reason) }) - t.Run("deep descendant is allowed", func(t *testing.T) { + t.Run("liveness probe error is surfaced", func(t *testing.T) { t.Parallel() store, token := newStoreWithSession(t, Permissions{AllowDelete: []string{"workspaces"}}, 4242) a := &EnvAuthorizer{ - Store: store, - Getenv: func(string) string { return token }, - Self: 100, - // 100 -> 90 -> 80 -> 4242 -> 1 - Ancestry: fakeAncestry(map[int]int{100: 90, 90: 80, 80: 4242, 4242: 1}), + Store: store, + Getenv: func(string) string { return token }, + Liveness: func(string) (bool, error) { return false, assert.AnError }, } - d, err := a.AuthorizeDelete("workspaces") - require.NoError(t, err) - assert.True(t, d.Allowed) - assert.Equal(t, ReasonGranted, d.Reason) + _, err := a.AuthorizeDelete("workspaces") + require.Error(t, err) }) } diff --git a/internal/pkg/execsession/lock_unix.go b/internal/pkg/execsession/lock_unix.go index d7cfe42..a33a5b9 100644 --- a/internal/pkg/execsession/lock_unix.go +++ b/internal/pkg/execsession/lock_unix.go @@ -6,18 +6,48 @@ package execsession import ( + "errors" "os" "golang.org/x/sys/unix" ) -// acquireLock takes an exclusive, non-blocking advisory lock on f. It returns -// an error (unix.EWOULDBLOCK) if another process already holds the lock. +// acquireLock takes a shared, non-blocking advisory lock on f. The granting +// process holds this lock for its lifetime; authorizers detect liveness by +// trying to take an exclusive lock (see probeLiveness). It returns an error if +// an incompatible lock is already held. func acquireLock(f *os.File) error { - return unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB) + return unix.Flock(int(f.Fd()), unix.LOCK_SH|unix.LOCK_NB) } // releaseLock releases the advisory lock held on f. func releaseLock(f *os.File) error { return unix.Flock(int(f.Fd()), unix.LOCK_UN) } + +// probeLiveness reports whether the process that created the session file at +// path is still alive. The holder keeps a shared lock for its lifetime, so an +// exclusive lock can only be acquired once the holder has exited. A successful +// exclusive acquire therefore means the holder is dead; EWOULDBLOCK means it is +// still alive. +func probeLiveness(path string) (bool, error) { + f, err := os.Open(path) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + defer func() { _ = f.Close() }() + + if err := unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB); err != nil { + if errors.Is(err, unix.EWOULDBLOCK) { + return true, nil // holder still alive + } + return false, err + } + // Acquired an exclusive lock: no live holder. Release immediately so we do + // not falsely signal liveness to a concurrent probe. + _ = unix.Flock(int(f.Fd()), unix.LOCK_UN) + return false, nil +} diff --git a/internal/pkg/execsession/lock_windows.go b/internal/pkg/execsession/lock_windows.go index 205a3c1..f2b1795 100644 --- a/internal/pkg/execsession/lock_windows.go +++ b/internal/pkg/execsession/lock_windows.go @@ -6,17 +6,19 @@ package execsession import ( + "errors" "os" "golang.org/x/sys/windows" ) -// acquireLock takes an exclusive, non-blocking lock on f via LockFileEx. It -// returns an error if another process already holds the lock. +// acquireLock takes a shared, non-blocking lock on f via LockFileEx. The +// granting process holds this lock for its lifetime; authorizers detect +// liveness by trying to take an exclusive lock (see probeLiveness). func acquireLock(f *os.File) error { return windows.LockFileEx( windows.Handle(f.Fd()), - windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY, + windows.LOCKFILE_FAIL_IMMEDIATELY, // shared lock (no EXCLUSIVE flag) 0, 1, 0, &windows.Overlapped{}, @@ -32,3 +34,38 @@ func releaseLock(f *os.File) error { &windows.Overlapped{}, ) } + +// probeLiveness reports whether the process that created the session file at +// path is still alive. The holder keeps a shared lock for its lifetime, so an +// exclusive lock can only be acquired once the holder has exited. A successful +// exclusive acquire therefore means the holder is dead; a lock violation means +// it is still alive. +func probeLiveness(path string) (bool, error) { + f, err := os.Open(path) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + defer func() { _ = f.Close() }() + + h := windows.Handle(f.Fd()) + ol := &windows.Overlapped{} + if err := windows.LockFileEx( + h, + windows.LOCKFILE_EXCLUSIVE_LOCK|windows.LOCKFILE_FAIL_IMMEDIATELY, + 0, + 1, 0, + ol, + ); err != nil { + if errors.Is(err, windows.ERROR_LOCK_VIOLATION) { + return true, nil // holder still alive + } + return false, err + } + // Acquired an exclusive lock: no live holder. Release immediately so we do + // not falsely signal liveness to a concurrent probe. + _ = windows.UnlockFileEx(h, 0, 1, 0, ol) + return false, nil +}