🛡️ Sentinel: Fix TOCTOU vulnerability in file permission check

.jules/sentinel.md

@@ -25,3 +25,11 @@
 **Learning:** `os.chmod` and `open()` follow symlinks by default in Python (and most POSIX environments).
 
 **Prevention:** Always use `os.path.islink()` to check for symlinks before modifying file metadata or content if the path is user-controlled or in a writable directory. Use `os.open` with `O_CREAT | O_WRONLY | O_TRUNC` and `os.chmod(fd)` on the file descriptor to avoid race conditions (TOCTOU) and ensure operations apply to the file itself, not a symlink target.
+
+## 2026-10-24 - TOCTOU Race Condition in File Permission Checks
+
+**Vulnerability:** The `check_env_permissions` function checked for symlinks (`os.path.islink`) and then modified permissions (`os.chmod`) using the file path. This created a Time-of-Check Time-of-Use (TOCTOU) race condition where an attacker could swap the file with a symlink between the check and the modification.
+
+**Learning:** Path-based checks (`os.path.islink`, `os.stat`) followed by path-based operations (`os.chmod`) are inherently racy. File descriptors are the only way to pin the resource.
+
+**Prevention:** Use `os.open` with `O_NOFOLLOW` to open the file securely (failing if it's a symlink). Then use file-descriptor-based operations: `os.fstat(fd)` to check modes and `os.fchmod(fd, mode)` to modify permissions. This ensures operations apply to the exact inode opened.

main.py

@@ -136,25 +136,31 @@ def check_env_permissions(env_path: str = ".env") -> None:
         return
 
     try:
-        file_stat = os.stat(env_path)
-        # Check if group or others have any permission
-        if file_stat.st_mode & (stat.S_IRWXG | stat.S_IRWXO):
-            perms = format(stat.S_IMODE(file_stat.st_mode), "03o")
+        # Security: Use low-level file descriptor operations to avoid TOCTOU (Time-of-Check Time-of-Use)
+        # race conditions. We open the file with O_NOFOLLOW to ensure we don't follow symlinks.
+        fd = os.open(env_path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0))
+        try:
+            file_stat = os.fstat(fd)
+            # Check if group or others have any permission
+            if file_stat.st_mode & (stat.S_IRWXG | stat.S_IRWXO):
+                perms = format(stat.S_IMODE(file_stat.st_mode), "03o")
 
-            # Auto-fix: Set to 600 (owner read/write only)
-            try:
-                os.chmod(env_path, 0o600)
-                sys.stderr.write(
-                    f"{Colors.GREEN}✓ Fixed {env_path} permissions "
-                    f"(was {perms}, now set to 600){Colors.ENDC}\n"
-                )
-            except OSError as fix_error:
-                # Auto-fix failed, show warning with instructions
-                sys.stderr.write(
-                    f"{Colors.WARNING}⚠️  Security Warning: {env_path} is "
-                    f"readable by others ({perms})! Auto-fix failed: {fix_error}. "
-                    f"Please run: chmod 600 {env_path}{Colors.ENDC}\n"
-                )
+                # Auto-fix: Set to 600 (owner read/write only) using fchmod on the open descriptor
+                try:
+                    os.fchmod(fd, 0o600)
+                    sys.stderr.write(
+                        f"{Colors.GREEN}✓ Fixed {env_path} permissions "
+                        f"(was {perms}, now set to 600){Colors.ENDC}\n"
+                    )
+                except OSError as fix_error:
+                    # Auto-fix failed, show warning with instructions
+                    sys.stderr.write(
+                        f"{Colors.WARNING}⚠️  Security Warning: {env_path} is "
+                        f"readable by others ({perms})! Auto-fix failed: {fix_error}. "
+                        f"Please run: chmod 600 {env_path}{Colors.ENDC}\n"
+                    )
+        finally:
+            os.close(fd)
     except OSError as error:
         # More specific exception type as suggested by bot review
         exception_type = type(error).__name__

test_main.py

@@ -631,3 +631,67 @@
     combined = "".join(writes)
     # With width 48, at 50% progress we should have 24 filled chars
     assert "█" * 24 in combined or len([c for c in combined if c == "█"]) == 24
+
+
+# Case 18: check_env_permissions uses secure file operations
+def test_check_env_permissions_secure(monkeypatch):
+    m = reload_main_with_env(monkeypatch)
+
+    # Mock os.path.exists and os.path.islink
+    monkeypatch.setattr("os.path.exists", lambda x: True)
+    monkeypatch.setattr("os.path.islink", lambda x: False)
+    monkeypatch.setattr("os.name", "posix")
+
+    # Mock low-level file operations
+    mock_open = MagicMock(return_value=123)
+    mock_close = MagicMock()
+    mock_fstat = MagicMock()
+    mock_fchmod = MagicMock()
+
+    monkeypatch.setattr("os.open", mock_open)
+    monkeypatch.setattr("os.close", mock_close)
+    monkeypatch.setattr("os.fstat", mock_fstat)
+    monkeypatch.setattr("os.fchmod", mock_fchmod)
+
+    # Mock stat result: world readable (needs fix)
+    # 0o666 = rw-rw-rw-
+    mock_stat_result = MagicMock()
+    mock_stat_result.st_mode = 0o100666 # Regular file, rw-rw-rw-
+    mock_fstat.return_value = mock_stat_result
+
+    # Capture stderr
+    mock_stderr = MagicMock()
+    monkeypatch.setattr(sys, "stderr", mock_stderr)
+
+    # Run
+    m.check_env_permissions(".env")
+
+    # Verify os.open called with O_NOFOLLOW
+    assert mock_open.called
+    args, _ = mock_open.call_args
+    # Check flags
+    expected_flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)
+    assert args[1] == expected_flags
+
+    # Verify fchmod called
+    mock_fchmod.assert_called_with(123, 0o600)
+
+    # Verify close called
+    mock_close.assert_called_with(123)
+
+    # Verify success message
+    writes = [args[0] for args, _ in mock_stderr.write.call_args_list]
+    combined = "".join(writes)
+    assert "Fixed .env permissions" in combined
+
+    # Test case where permissions are already fine
+    mock_open.reset_mock()
+    mock_fchmod.reset_mock()
+    mock_close.reset_mock()
+    mock_stat_result.st_mode = 0o100600 # rw-------
+
+    m.check_env_permissions(".env")
+
+    assert mock_open.called
+    assert not mock_fchmod.called
+    mock_close.assert_called_with(123)