mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-02-18 00:17:39 +01:00
fix(sensor-proxy): lock file permissions and deadlock prevention
Final security hardening based on second Codex review:
**Lock File Permission Fix (Security)**
- Lock file now created with 0600 instead of 0644
- Prevents unprivileged users from opening lock and holding LOCK_EX
- Without this, any local user could DoS the installer/self-heal
- Added f.Chmod(0600) to fix permissions on existing lock files
**Deadlock Prevention (Future-Proofing)**
- Added documentation for future multi-file locking scenarios
- Specifies consistent lock ordering requirement (config.yaml.lock before allowed_nodes.yaml.lock)
- Prevents potential deadlocks if future commands modify multiple files
- Current implementation only locks one file, so no immediate issue
**Testing:**
✅ Lock file created as `-rw-------` (0600)
✅ Existing lock files with wrong perms get fixed
✅ Unprivileged users can no longer DoS the lock
**Codex Validation:**
- Locking is now correct (persistent .lock file, held during entire operation)
- Atomic writes complete while lock is held
- Validation honors actual config paths
- Empty lists supported for operational flexibility
- Error propagation prevents silent failures
- No remaining race conditions or security issues
Phase 2 is now complete and Codex-verified as secure.
Related to Phase 2 fixes commit 804a638ea
This commit is contained in:
@@ -237,14 +237,23 @@ func setAllowedNodes(path string, newNodes []string, replace bool) error {
|
||||
}
|
||||
|
||||
// withLockedFile opens a lock file with exclusive locking and runs a callback
|
||||
//
|
||||
// IMPORTANT: If future commands need to modify multiple files, use consistent lock ordering
|
||||
// to avoid deadlocks (e.g., always lock config.yaml.lock before allowed_nodes.yaml.lock)
|
||||
func withLockedFile(lockPath string, fn func(f *os.File) error) error {
|
||||
// Open or create the lock file (never deleted, persists across renames)
|
||||
f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, 0644)
|
||||
// Use 0600 to prevent unprivileged users from holding LOCK_EX and DoS'ing the installer
|
||||
f, err := os.OpenFile(lockPath, os.O_RDWR|os.O_CREATE, 0600)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to open lock file: %w", err)
|
||||
}
|
||||
defer f.Close()
|
||||
|
||||
// Ensure correct permissions even if file already exists with broader perms
|
||||
if err := f.Chmod(0600); err != nil {
|
||||
return fmt.Errorf("failed to set lock file permissions: %w", err)
|
||||
}
|
||||
|
||||
// Acquire exclusive lock
|
||||
if err := unix.Flock(int(f.Fd()), unix.LOCK_EX); err != nil {
|
||||
return fmt.Errorf("failed to acquire file lock: %w", err)
|
||||
|
||||
Reference in New Issue
Block a user