Skip to content

Conversation

@ChuckBuilds
Copy link
Owner

@ChuckBuilds ChuckBuilds commented Jan 26, 2026

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

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced system stability by adding safeguards to prevent permission modifications on critical system directories
    • Temporary and system directories are now properly excluded from permission changes during snapshot operations

✏️ Tip: You can customize this high-level summary in your review settings.

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: Claude Sonnet 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

The changes introduce safeguards to prevent permission modifications on system directories, specifically /tmp, by adding a protected directory check in the permission utilities module and conditionally skipping permission enforcement in the snapshot writing logic.

Changes

Cohort / File(s) Summary
System Directory Protection
src/common/permission_utils.py
Added PROTECTED_SYSTEM_DIRECTORIES constant; modified ensure_directory_permissions() with early guard logic to skip permission modifications for protected paths, verify usability, and handle errors appropriately for non-existent protected directories.
Snapshot Write Guard
src/display_manager.py
Modified _write_snapshot_if_due() to conditionally skip permission modification for /tmp directory while preserving existing snapshot write flow (atomic writes, file permissions, timestamp updates).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through /tmp with care,
Protecting sacred systems fair!
No reckless chmod shall dare encroach,
On directories we must approach.
Permissions safe, now all can share! 🔐

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and concisely summarizes the main change: preventing /tmp permission corruption that breaks system updates, which directly corresponds to the primary objective of the PR.
Linked Issues check ✅ Passed The PR implements safeguards to prevent modifying /tmp permissions (via PROTECTED_SYSTEM_DIRECTORIES constant and explicit check in display_manager.py), directly addressing issue #202's requirement to restore /tmp to correct permissions (1777/drwxrwxrwt).
Out of Scope Changes check ✅ Passed All changes are directly scoped to preventing /tmp permission corruption: a new PROTECTED_SYSTEM_DIRECTORIES constant, defensive guards in permission_utils.py and display_manager.py, with no unrelated modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/tmp folder permission issue

2 participants