From 4fcbb57d23e718fb8833987dcdc085bf635528df Mon Sep 17 00:00:00 2001 From: Bash Bandicoot Date: Fri, 6 Mar 2026 12:41:31 -0800 Subject: [PATCH] Fix orbit crash loop on incorrect file permissions (#40887) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - `checkPermFile` in `pkg/secure/secure.go` now self-heals incorrect file permissions via `os.Chmod` instead of returning a fatal error - Fixes orbit crash-looping indefinitely when `/opt/orbit/updates-metadata.json` has mode 755 instead of the expected 600 ## Problem Orbit refuses to start when `updates-metadata.json` has wrong permissions (e.g. 755 instead of 600), entering an infinite restart loop (`systemd` restart counter observed at 3447+). The manual workaround is `chmod 600 /opt/orbit/updates-metadata.json`, but the root cause — an external process changing file permissions — is intermittent and hard to track. The `checkPermFile` function in `pkg/secure/secure.go` was designed as a security check, but its behavior of fatally erroring on any permission mismatch causes a denial-of-service on the legitimate user. For comparison, `checkPermPath` (the directory equivalent) already tolerates permissions that are less permissive than expected. ## Fix When `checkPermFile` detects a permission mismatch, it now attempts `os.Chmod` to correct the permissions before proceeding. It only returns an error if the chmod itself fails (e.g. insufficient privileges). This preserves the security intent — files end up with correct permissions — while making orbit resilient to external permission drift. ## Test plan - [ ] `go test ./pkg/secure/ -v -run TestOpenFile` — verifies self-healing behavior - [ ] `go test ./pkg/secure/ -v -run TestMkdirAll` — unchanged, verifies directory checks still work - [ ] Manual: create `/opt/orbit/updates-metadata.json` with mode 755, start orbit, confirm it self-heals and starts normally --------- Co-authored-by: Bash Bandicoot --- changes/40887-fix-file-permission-self-heal | 1 + orbit/changes/40887-fix-file-permission-self-heal | 1 + pkg/secure/secure.go | 6 ++++-- pkg/secure/secure_test.go | 15 +++++++++------ 4 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 changes/40887-fix-file-permission-self-heal create mode 100644 orbit/changes/40887-fix-file-permission-self-heal diff --git a/changes/40887-fix-file-permission-self-heal b/changes/40887-fix-file-permission-self-heal new file mode 100644 index 0000000000..913df73c12 --- /dev/null +++ b/changes/40887-fix-file-permission-self-heal @@ -0,0 +1 @@ +* Fixed `secure.OpenFile` to self-heal incorrect file permissions via `chmod` instead of returning a fatal error. diff --git a/orbit/changes/40887-fix-file-permission-self-heal b/orbit/changes/40887-fix-file-permission-self-heal new file mode 100644 index 0000000000..c866e35428 --- /dev/null +++ b/orbit/changes/40887-fix-file-permission-self-heal @@ -0,0 +1 @@ +* Fixed orbit crash loop when `updates-metadata.json` has incorrect file permissions by self-healing via `chmod` instead of fatally erroring. diff --git a/pkg/secure/secure.go b/pkg/secure/secure.go index f89db0efd5..9bc9c714ad 100644 --- a/pkg/secure/secure.go +++ b/pkg/secure/secure.go @@ -66,8 +66,10 @@ func checkPermPath(path string, perm os.FileMode) error { func checkPermFile(filePath string, perm os.FileMode) error { if f, err := os.Stat(filePath); !errors.Is(err, os.ErrNotExist) && f != nil && f.Mode() != perm { - return fmt.Errorf( - "File %s already exists with mode %o instead of the expected %o", filePath, f.Mode(), perm) + if chmodErr := os.Chmod(filePath, perm); chmodErr != nil { + return fmt.Errorf( + "file %s has mode %o instead of the expected %o and chmod failed: %w", filePath, f.Mode(), perm, chmodErr) + } } if err := checkPermPath(path.Dir(filePath), perm); err != nil { return err diff --git a/pkg/secure/secure_test.go b/pkg/secure/secure_test.go index 984a2ed923..7b68ae13a1 100644 --- a/pkg/secure/secure_test.go +++ b/pkg/secure/secure_test.go @@ -44,13 +44,16 @@ func TestOpenFile(t *testing.T) { require.NotNil(t, fd) require.NoError(t, fd.Close()) - _, err = OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0677) - require.Error(t, err) - expectedFileErr := fmt.Sprintf( - "File %s already exists with mode 755 instead of the expected %o", filePath, 0677) - require.Equal(t, expectedFileErr, err.Error()) + // Opening with a different perm should self-heal via chmod rather than error. + fd, err = OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0600) + require.NoError(t, err) + fd.Close() + info, err := os.Stat(filePath) + require.NoError(t, err) + require.Equal(t, os.FileMode(0600), info.Mode()) - fd, err = OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0755) + // Re-open with the now-correct mode still works. + fd, err = OpenFile(filePath, os.O_CREATE|os.O_WRONLY, 0600) require.NoError(t, err) fd.Close() }