fix: prevent /tmp permission corruption breaking system updates (#209)

Issue: LEDMatrix was changing /tmp permissions from 1777 (drwxrwxrwt)
to 2775 (drwxrwsr-x), breaking apt update and other system tools.

Root cause: display_manager.py's _write_snapshot_if_due() called
ensure_directory_permissions() on /tmp when writing snapshots to
/tmp/led_matrix_preview.png. This removed the sticky bit and
world-writable permissions that /tmp requires.

Fix:
- Added PROTECTED_SYSTEM_DIRECTORIES safelist to permission_utils.py
  to prevent modifying permissions on /tmp and other system directories
- Added explicit check in display_manager.py to skip /tmp
- Defense-in-depth approach prevents similar issues in other code paths

The sticky bit (1xxx) is critical for /tmp - it prevents users from
deleting files they don't own. Without world-writable permissions,
regular users cannot create temp files.

Fixes #202

Co-authored-by: Chuck <chuck@example.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-01-26 10:56:30 -05:00
committed by GitHub
parent f9de9fa29e
commit 384ed096ff
2 changed files with 46 additions and 6 deletions

View File

@@ -13,27 +13,63 @@ from typing import Optional
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
# System directories that should never have their permissions modified
# These directories have special system-level permissions that must be preserved
PROTECTED_SYSTEM_DIRECTORIES = {
'/tmp',
'/var/tmp',
'/dev',
'/proc',
'/sys',
'/run',
'/var/run',
'/etc',
'/boot',
'/var',
'/usr',
'/lib',
'/lib64',
'/bin',
'/sbin',
}
def ensure_directory_permissions(path: Path, mode: int = 0o775) -> None: def ensure_directory_permissions(path: Path, mode: int = 0o775) -> None:
""" """
Create directory and set permissions. Create directory and set permissions.
If the directory already exists and we cannot change its permissions, If the directory already exists and we cannot change its permissions,
we check if it's usable (readable/writable). If so, we continue without we check if it's usable (readable/writable). If so, we continue without
raising an exception. This allows the system to work even when running raising an exception. This allows the system to work even when running
as a non-root user who cannot change permissions on existing directories. as a non-root user who cannot change permissions on existing directories.
Protected system directories (like /tmp, /etc, /var) are never modified
to prevent breaking system functionality.
Args: Args:
path: Directory path to create/ensure path: Directory path to create/ensure
mode: Permission mode (default: 0o775 for group-writable directories) mode: Permission mode (default: 0o775 for group-writable directories)
Raises: Raises:
OSError: If directory creation fails or directory exists but is not usable OSError: If directory creation fails or directory exists but is not usable
""" """
try: try:
# Never modify permissions on system directories
path_str = str(path.resolve() if path.is_absolute() else path)
if path_str in PROTECTED_SYSTEM_DIRECTORIES:
logger.debug(f"Skipping permission modification on protected system directory: {path_str}")
# Verify the directory is usable
if path.exists() and os.access(path, os.R_OK | os.W_OK):
return
elif path.exists():
logger.warning(f"Protected system directory {path_str} exists but is not writable")
return
else:
raise OSError(f"Protected system directory {path_str} does not exist")
# Create directory if it doesn't exist # Create directory if it doesn't exist
path.mkdir(parents=True, exist_ok=True) path.mkdir(parents=True, exist_ok=True)
# Try to set permissions # Try to set permissions
try: try:
os.chmod(path, mode) os.chmod(path, mode)

View File

@@ -816,8 +816,12 @@ class DisplayManager:
get_assets_file_mode get_assets_file_mode
) )
snapshot_path_obj = Path(self._snapshot_path) snapshot_path_obj = Path(self._snapshot_path)
if snapshot_path_obj.parent: # Only ensure permissions on non-system directories
ensure_directory_permissions(snapshot_path_obj.parent, get_assets_dir_mode()) # Never modify /tmp permissions - it has special system permissions (1777)
# that must not be changed or it breaks apt and other system tools
parent_dir = snapshot_path_obj.parent
if parent_dir and str(parent_dir) != '/tmp':
ensure_directory_permissions(parent_dir, get_assets_dir_mode())
# Write atomically: temp then replace # Write atomically: temp then replace
tmp_path = f"{self._snapshot_path}.tmp" tmp_path = f"{self._snapshot_path}.tmp"
self.image.save(tmp_path, format='PNG') self.image.save(tmp_path, format='PNG')