🛡️ Sentinel: Secure bootstrapping and input validation hardening

.jules/sentinel.md

@@ -34,6 +34,13 @@
 
 **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.
 
+## 2026-10-24 - Insecure Bootstrapping Order (.env Loading)
+
+**Vulnerability:** The application called `load_dotenv()` at module level, *before* running security checks on `.env` file permissions. This created a race window where secrets could be read from a world-readable file before the permission fix was applied.
+
+**Learning:** Global initialization code (like `load_dotenv()`) runs immediately on import. Security checks that must run before sensitive operations should be placed in `main()` or an initialization function, and sensitive operations (loading secrets) should be deferred until checks pass.
+
+**Prevention:** Move `load_dotenv()` inside `main()` after `check_env_permissions()`. Re-initialize global variables (like `TOKEN`) that depend on environment variables to ensure they pick up the loaded values.
 ## 2026-02-14 - CSV Injection via Log Sanitization
 **Vulnerability:** `sanitize_for_log` in `main.py` was stripping quotes from `repr()` output for aesthetic reasons, inadvertently exposing strings starting with `=`, `+`, `-`, `@` to formula injection if logs are viewed in Excel.
 **Learning:** Security controls (like `repr()`) can be weakened by subsequent "cleanup" steps. Always consider downstream consumption of logs (e.g., CSV export).

main.py

@@ -39,7 +39,7 @@
 # --------------------------------------------------------------------------- #
 # 0. Bootstrap – load secrets and configure logging
 # --------------------------------------------------------------------------- #
-load_dotenv()
+# SECURITY: load_dotenv() moved to main() to ensure permissions are checked first
 
 # Respect NO_COLOR standard (https://no-color.org/)
 if os.getenv("NO_COLOR"):
@@ -756,6 +756,16 @@ def is_valid_folder_name(name: str) -> bool:
     if any(c in _DANGEROUS_FOLDER_CHARS or c in _BIDI_CONTROL_CHARS for c in name):
         return False
 
+    # Security: Block path traversal attempts
+    # Check stripped name to prevent whitespace bypass (e.g. " . ")
+    clean_name = name.strip()
+    if clean_name in (".", ".."):
+        return False
+
+    # Security: Block command option injection (if name is passed to shell)
+    if clean_name.startswith("-"):
+        return False
+
     return True
 
 
@@ -1845,9 +1855,14 @@ def parse_args() -> argparse.Namespace:
 
 def main():
     # SECURITY: Check .env permissions (after Colors is defined for NO_COLOR support)
+    # This must happen BEFORE load_dotenv() to prevent reading secrets from world-readable files
     check_env_permissions()
+    load_dotenv()
 
     global TOKEN
+    # Re-initialize TOKEN to pick up values from .env (since load_dotenv was delayed)
+    TOKEN = _clean_env_kv(os.getenv("TOKEN"), "TOKEN")
+
     args = parse_args()
 
     # Load persistent cache from disk (graceful degradation on any error)

tests/test_bootstrapping.py

@@ -0,0 +1,69 @@
+import os
+import sys
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+# We need to import main to test it, but we want to test its behavior when run
+import main
+
+
+def test_main_reloads_token_from_env(monkeypatch):
+    """
+    Verify that main() reloads TOKEN from environment variables after load_dotenv().
+    This ensures that secrets from .env are picked up even if the module was imported earlier.
+    """
+    # 1. Setup initial state
+    monkeypatch.setenv("TOKEN", "initial_token")
+    # Force reload main to pick up initial token
+    import importlib
+    importlib.reload(main)
+    assert main.TOKEN == "initial_token"
+
+    # 2. Mock dependencies
+    # Mock load_dotenv to simulate loading a .env file that changes TOKEN
+    def mock_load_dotenv():
+        os.environ["TOKEN"] = "loaded_from_env_file"
+
+    # Mock other parts of main to prevent actual execution
+    mock_args = MagicMock()
+    mock_args.profiles = "p1"
+    mock_args.folder_url = None
+    mock_args.dry_run = True  # Dry run to exit early
+    mock_args.no_delete = False
+    mock_args.plan_json = None
+
+    # Mock check_env_permissions to avoid filesystem checks
+    mock_check_perms = MagicMock()
+
+    # Mock warm_up_cache to avoid network calls
+    mock_warm_up = MagicMock()
+
+    # Mock sync_profile to avoid logic
+    mock_sync = MagicMock(return_value=True)
+
+    # Apply mocks
+    # Patch main.load_dotenv because main.py imports it as 'from dotenv import load_dotenv'
+    with patch("main.load_dotenv", side_effect=mock_load_dotenv) as mock_load_dotenv_call, \
+         patch("main.parse_args", return_value=mock_args), \
+         patch("main.check_env_permissions", mock_check_perms), \
+         patch("main.warm_up_cache", mock_warm_up), \
+         patch("main.sync_profile", mock_sync), \
+         patch("sys.stdin.isatty", return_value=False):  # Non-interactive
+
+        # 3. Run main()
+        # This should call load_dotenv (our mock), which updates env var,
+        # then main should update global TOKEN from env var.
+        with pytest.raises(SystemExit):
+            main.main()
+
+        # 4. Verify
+        # load_dotenv must have been called
+        assert mock_load_dotenv_call.called
+
+        # check_env_permissions must have been called BEFORE load_dotenv
+        # We can check order by checking if check_perms was called
+        # But verifying exact order is hard with separate mocks unless we use a manager
+        # However, if main.TOKEN is updated, we know the logic ran.
+
+        assert main.TOKEN == "loaded_from_env_file"

tests/test_folder_validation.py

@@ -72,5 +72,23 @@
                     is False
                 ), f"Bidi character {description} (U+{ord(char):04X}) should be blocked in '{test_name}'"
 
+        # Case 8: Path Traversal (Security Hardening)
+        # Block '.' and '..' which could be used for path traversal
+        path_traversal_cases = [".", ".."]
+        for pt_name in path_traversal_cases:
+            pt_data = {"group": {"group": pt_name}}
+            assert (
+                main.validate_folder_data(pt_data, f"http://pt-{pt_name}.com") is False
+            ), f"Path traversal name '{pt_name}' should be blocked"
+
+        # Case 9: Command Option Injection (Security Hardening)
+        # Block names starting with '-' which could be interpreted as flags
+        option_injection_cases = ["-flag", "--flag", "-v", "--verbose"]
+        for opt_name in option_injection_cases:
+            opt_data = {"group": {"group": opt_name}}
+            assert (
+                main.validate_folder_data(opt_data, f"http://opt-{opt_name}.com") is False
+            ), f"Option injection name '{opt_name}' should be blocked"
+
     finally:
         main.log = original_log

tests/test_push_rules_perf.py

@@ -11,6 +11,11 @@
 
 class TestPushRulesPerf(unittest.TestCase):
     def setUp(self):
+        # Ensure we are using the current main module instance (in case of reloads)
+        global main
+        if 'main' in sys.modules:
+            main = sys.modules['main']
+
         self.client = MagicMock()
         self.profile_id = "test_profile"
         self.folder_name = "test_folder"