fix: handle log file permission errors gracefully

install/deploy/pre_deploy.sh

@@ -101,6 +101,32 @@ if ! git diff --quiet 2>/dev/null; then
     git status --short
 fi
 
+# Check 8: Verify logs directory ownership
+LOGS_DIR="$INSTALL_FOLDER/logs"
+WEB_USER="${WEB_USER:-www-data}"
+if [ -d "$LOGS_DIR" ]; then
+    LOGS_OWNER=$(stat -c '%U' "$LOGS_DIR" 2>/dev/null || echo "unknown")
+    if [ "$LOGS_OWNER" != "$WEB_USER" ]; then
+        echo "WARNING: Logs directory owned by '$LOGS_OWNER', should be '$WEB_USER'"
+        echo "Fixing ownership..."
+        chown -R "$WEB_USER:$WEB_USER" "$LOGS_DIR" 2>/dev/null || {
+            echo "ERROR: Failed to fix logs ownership. Run manually:"
+            echo "  sudo chown -R $WEB_USER:$WEB_USER $LOGS_DIR"
+            exit 1
+        }
+        echo "✓ Logs directory ownership fixed"
+    else
+        echo "✓ Logs directory ownership OK ($WEB_USER)"
+    fi
+else
+    echo "Creating logs directory with correct ownership..."
+    mkdir -p "$LOGS_DIR"
+    chown "$WEB_USER:$WEB_USER" "$LOGS_DIR" 2>/dev/null || {
+        echo "WARNING: Could not set logs ownership (run as root)"
+    }
+    echo "✓ Logs directory created"
+fi
+
 # Export backup directory for other scripts
 echo "$BACKUP_DIR" > /tmp/sp-deploy-backup-dir.txt
 

log_configuration.py

@@ -3,9 +3,10 @@
 import logging
 import logging.handlers
 import os
+import sys
 from logging import Logger, StreamHandler
 from logging.handlers import RotatingFileHandler
-from typing import Union
+from typing import Optional, Union
 
 
 class LogConfiguration:
@@ -19,21 +20,47 @@ def __init__(self, folder: str, filename: str, debug: bool = False) -> None:
             self._consoleLogger.setLevel(logging.DEBUG)
         else:
             self._consoleLogger.setLevel(logging.INFO)
-        # create a file handler
-        path = os.path.join(folder, 'logs', f'{filename}.log')
-        self._fileLogger = logging.handlers.RotatingFileHandler(path, maxBytes=1024 * 1024, backupCount=20)
-        self._fileLogger.setLevel(logging.DEBUG)
-        # create a logging format
-        formatter = logging.Formatter('[%(name)s][%(levelname)s][%(asctime)s] %(message)s')
-        self._fileLogger.setFormatter(formatter)
+
+        # create a file handler with permission error handling
+        self._fileLogger: Optional[RotatingFileHandler] = None
+        log_dir = os.path.join(folder, 'logs')
+        path = os.path.join(log_dir, f'{filename}.log')
+
+        try:
+            # Ensure logs directory exists
+            os.makedirs(log_dir, exist_ok=True)
+
+            self._fileLogger = logging.handlers.RotatingFileHandler(
+                path, maxBytes=1024 * 1024, backupCount=20
+            )
+            self._fileLogger.setLevel(logging.DEBUG)
+            # create a logging format
+            formatter = logging.Formatter('[%(name)s][%(levelname)s][%(asctime)s] %(message)s')
+            self._fileLogger.setFormatter(formatter)
+        except PermissionError as e:
+            # Log file owned by different user (e.g., root vs www-data)
+            # Fall back to console-only logging rather than crashing
+            print(
+                f"[WARNING] Cannot write to log file {path}: {e}. "
+                f"Falling back to console-only logging. "
+                f"Fix: sudo chown www-data:www-data {log_dir} -R",
+                file=sys.stderr
+            )
+        except OSError as e:
+            # Other filesystem errors (disk full, etc.)
+            print(
+                f"[WARNING] Cannot create log file {path}: {e}. "
+                f"Falling back to console-only logging.",
+                file=sys.stderr
+            )
 
     @property
-    def file_logger(self) -> RotatingFileHandler:
+    def file_logger(self) -> Optional[RotatingFileHandler]:
         """
         Get file logger.
 
-        :return: file logger
-        :rtype: logging.handlers.RotatingFileHandler
+        :return: file logger or None if file logging unavailable
+        :rtype: Optional[logging.handlers.RotatingFileHandler]
         """
         return self._fileLogger
 
@@ -59,7 +86,8 @@ def create_logger(self, name: str) -> Logger:
         logger = logging.getLogger(name)
         logger.setLevel(logging.DEBUG)
         # add the handlers to the logger
-        logger.addHandler(self.file_logger)
-        logger.addHandler(self.console_logger)
+        if self._fileLogger is not None:
+            logger.addHandler(self._fileLogger)
+        logger.addHandler(self._consoleLogger)
 
         return logger

tests/test_log_configuration.py

@@ -15,42 +15,42 @@ class TestLogConfiguration(unittest.TestCase):
 
     def _test_init_with_log_value(self, debug, result_level):
         """Test logger initialization with specific debug and level."""
-        joined_path = 'baz'
         folder = 'foo'
         filename = 'bar'
+        log_dir = 'foo/logs'
+        log_path = 'foo/logs/bar.log'
         console_format = '[%(levelname)s] %(message)s'
         file_format = '[%(name)s][%(levelname)s][%(asctime)s] %(message)s'
         with mock.patch('logging.handlers.RotatingFileHandler') as mock_fh:
             with mock.patch('logging.StreamHandler') as mock_sh:
                 with mock.patch('logging.Formatter') as mock_formatter:
-                    with mock.patch('os.path.join',
-                                    return_value=joined_path) as mock_join:
-                        log_config = LogConfiguration(folder, filename, debug)
-
-                        mock_sh().setLevel.assert_called_once_with(
-                            result_level)
-                        mock_sh().setFormatter.assert_called_once_with(
-                            mock_formatter())
-                        mock_fh.assert_called_once_with(joined_path,
-                                                        maxBytes=1024 * 1024,
-                                                        backupCount=20)
-                        mock_fh().setLevel.assert_called_once_with(
-                            logging.DEBUG)
-                        mock_fh().setFormatter.assert_called_once_with(
-                            mock_formatter())
-                        mock_formatter.assert_has_calls([
-                            mock.call(console_format),
-                            mock.call(file_format)
-                        ])
-                        mock_join.assert_called_once_with(folder, 'logs',
-                                                          '%s.log' % filename)
+                    with mock.patch('os.path.join', side_effect=[log_dir, log_path]):
+                        with mock.patch('os.makedirs') as mock_makedirs:
+                            log_config = LogConfiguration(folder, filename, debug)
 
-                        self.assertEqual(log_config._consoleLogger, mock_sh())
-                        self.assertEqual(log_config.console_logger, mock_sh())
-                        self.assertEqual(log_config._fileLogger, mock_fh())
-                        self.assertEqual(log_config.file_logger, mock_fh())
+                            mock_makedirs.assert_called_once_with(log_dir, exist_ok=True)
+                            mock_sh().setLevel.assert_called_once_with(
+                                result_level)
+                            mock_sh().setFormatter.assert_called_once_with(
+                                mock_formatter())
+                            mock_fh.assert_called_once_with(log_path,
+                                                            maxBytes=1024 * 1024,
+                                                            backupCount=20)
+                            mock_fh().setLevel.assert_called_once_with(
+                                logging.DEBUG)
+                            mock_fh().setFormatter.assert_called_once_with(
+                                mock_formatter())
+                            mock_formatter.assert_has_calls([
+                                mock.call(console_format),
+                                mock.call(file_format)
+                            ])
+
+                            self.assertEqual(log_config._consoleLogger, mock_sh())
+                            self.assertEqual(log_config.console_logger, mock_sh())
+                            self.assertEqual(log_config._fileLogger, mock_fh())
+                            self.assertEqual(log_config.file_logger, mock_fh())
 
-                        return log_config
+                            return log_config
 
     def test_init_correctly_initializes_the_instance_when_debug(self):
         """Test log initialization with debug mode and level."""
@@ -60,24 +60,82 @@ def test_init_correctly_initializes_the_instance_when_no_debug(self):
         """Test log initialization with info level."""
         self._test_init_with_log_value(False, logging.INFO)
 
+    def test_init_handles_permission_error(self):
+        """Test that permission errors fall back to console-only logging."""
+        folder = 'foo'
+        filename = 'bar'
+        log_dir = 'foo/logs'
+        log_path = 'foo/logs/bar.log'
+        with mock.patch('logging.handlers.RotatingFileHandler') as mock_fh:
+            mock_fh.side_effect = PermissionError("Permission denied")
+            with mock.patch('logging.StreamHandler') as mock_sh:
+                with mock.patch('os.path.join', side_effect=[log_dir, log_path]):
+                    with mock.patch('os.makedirs'):
+                        log_config = LogConfiguration(folder, filename, False)
+
+                        # Console logger should still be set up
+                        self.assertEqual(log_config._consoleLogger, mock_sh())
+                        # File logger should be None
+                        self.assertIsNone(log_config._fileLogger)
+                        self.assertIsNone(log_config.file_logger)
+
+    def test_init_handles_os_error(self):
+        """Test that OSError falls back to console-only logging."""
+        folder = 'foo'
+        filename = 'bar'
+        log_dir = 'foo/logs'
+        log_path = 'foo/logs/bar.log'
+        with mock.patch('logging.handlers.RotatingFileHandler') as mock_fh:
+            mock_fh.side_effect = OSError("Disk full")
+            with mock.patch('logging.StreamHandler') as mock_sh:
+                with mock.patch('os.path.join', side_effect=[log_dir, log_path]):
+                    with mock.patch('os.makedirs'):
+                        log_config = LogConfiguration(folder, filename, False)
+
+                        # Console logger should still be set up
+                        self.assertEqual(log_config._consoleLogger, mock_sh())
+                        # File logger should be None
+                        self.assertIsNone(log_config._fileLogger)
+
     def test_create_logger(self):
         """Test logger creation."""
         with mock.patch.object(LogConfiguration, '__init__',
                                return_value=None):
             with mock.patch('logging.getLogger') as mock_get:
-                with mock.patch.object(LogConfiguration, 'file_logger'):
-                    with mock.patch.object(LogConfiguration, 'console_logger'):
-                        log_config = LogConfiguration('foo', 'bar')
-                        name = 'foobar'
-
-                        result = log_config.create_logger(name)
-
-                        mock_get.assert_called_once_with(name)
-                        mock_get().setLevel.assert_called_once_with(
-                            logging.DEBUG)
-                        mock_get().addHandler.assert_has_calls([
-                            mock.call(log_config.file_logger),
-                            mock.call(log_config.console_logger)
-                        ])
-
-                        self.assertEqual(result, mock_get())
+                log_config = LogConfiguration('foo', 'bar')
+                log_config._fileLogger = mock.MagicMock()
+                log_config._consoleLogger = mock.MagicMock()
+                name = 'foobar'
+
+                result = log_config.create_logger(name)
+
+                mock_get.assert_called_once_with(name)
+                mock_get().setLevel.assert_called_once_with(
+                    logging.DEBUG)
+                mock_get().addHandler.assert_has_calls([
+                    mock.call(log_config._fileLogger),
+                    mock.call(log_config._consoleLogger)
+                ])
+
+                self.assertEqual(result, mock_get())
+
+    def test_create_logger_without_file_logger(self):
+        """Test logger creation when file logger is unavailable."""
+        with mock.patch.object(LogConfiguration, '__init__',
+                               return_value=None):
+            with mock.patch('logging.getLogger') as mock_get:
+                log_config = LogConfiguration('foo', 'bar')
+                log_config._fileLogger = None
+                log_config._consoleLogger = mock.MagicMock()
+                name = 'foobar'
+
+                result = log_config.create_logger(name)
+
+                mock_get.assert_called_once_with(name)
+                mock_get().setLevel.assert_called_once_with(
+                    logging.DEBUG)
+                # Only console logger should be added
+                mock_get().addHandler.assert_called_once_with(
+                    log_config._consoleLogger)
+
+                self.assertEqual(result, mock_get())