Merge pull request #148 from CyberSecDef/copilot/fix-environment-variable-paths

fix: reject absolute paths in directory env vars

Author: CyberSecDef

novelforge/config.py

@@ -233,17 +233,35 @@ def _parse_llm_providers() -> list[ProviderConfig]:
 # Flask secret key – override via SECRET_KEY environment variable in production
 SECRET_KEY = os.environ.get("SECRET_KEY", "change-me-in-production")
 
+
+def _resolve_dir(env_var: str, default: str) -> str:
+    """Return the absolute path for a directory env var, anchored to PROJECT_ROOT.
+
+    Raises:
+        ValueError: if the env var is set to an absolute path.  All directory
+            env vars must be relative to *PROJECT_ROOT* to prevent arbitrary
+            filesystem access.
+    """
+    raw = os.environ.get(env_var, default)
+    if os.path.isabs(raw):
+        raise ValueError(
+            f"{env_var} must be a relative path (got {raw!r}). "
+            "Directory env vars are resolved relative to the project root."
+        )
+    return str(PROJECT_ROOT / raw)
+
+
 # Directory where Flask-Session stores server-side session files
-SESSION_FILE_DIR = str(PROJECT_ROOT / os.environ.get("SESSION_FILE_DIR", "sessions/flask"))
+SESSION_FILE_DIR = _resolve_dir("SESSION_FILE_DIR", "sessions/flask")
 
 # Directory where exported novel files are stored temporarily
-EXPORT_DIR = str(PROJECT_ROOT / os.environ.get("EXPORT_DIR", "exports"))
+EXPORT_DIR = _resolve_dir("EXPORT_DIR", "exports")
 
 # Directory where novel session JSON files are stored
-NOVELS_DIR = str(PROJECT_ROOT / os.environ.get("NOVELS_DIR", "sessions/novels"))
+NOVELS_DIR = _resolve_dir("NOVELS_DIR", "sessions/novels")
 
 # Directory for log files
-LOGS_DIR = str(PROJECT_ROOT / os.environ.get("LOGS_DIR", "logs"))
+LOGS_DIR = _resolve_dir("LOGS_DIR", "logs")
 
 
 class ConfigurationError(Exception):

tests/test_validate_config.py

@@ -4,6 +4,50 @@
 import novelforge.config as cfg
 
 
+# ---------------------------------------------------------------------------
+# _resolve_dir helper
+# ---------------------------------------------------------------------------
+
+class TestResolveDir:
+    """Unit tests for the _resolve_dir() helper."""
+
+    def test_returns_project_root_relative_path_when_unset(self, monkeypatch):
+        monkeypatch.delenv("NF_TEST_DIR", raising=False)
+        result = cfg._resolve_dir("NF_TEST_DIR", "my/subdir")
+        assert result == str(cfg.PROJECT_ROOT / "my/subdir")
+
+    def test_returns_project_root_relative_path_for_relative_env_var(self, monkeypatch):
+        monkeypatch.setenv("NF_TEST_DIR", "custom/path")
+        result = cfg._resolve_dir("NF_TEST_DIR", "default/path")
+        assert result == str(cfg.PROJECT_ROOT / "custom/path")
+
+    def test_raises_for_absolute_path(self, monkeypatch):
+        monkeypatch.setenv("NF_TEST_DIR", "/etc/passwd")
+        with pytest.raises(ValueError, match="NF_TEST_DIR"):
+            cfg._resolve_dir("NF_TEST_DIR", "default")
+
+    def test_error_message_contains_bad_value(self, monkeypatch):
+        monkeypatch.setenv("NF_TEST_DIR", "/absolute/path")
+        with pytest.raises(ValueError, match="/absolute/path"):
+            cfg._resolve_dir("NF_TEST_DIR", "default")
+
+    def test_raises_for_root_path(self, monkeypatch):
+        monkeypatch.setenv("NF_TEST_DIR", "/")
+        with pytest.raises(ValueError):
+            cfg._resolve_dir("NF_TEST_DIR", "default")
+
+    def test_relative_default_accepted_when_env_var_unset(self, monkeypatch):
+        monkeypatch.delenv("NF_TEST_DIR", raising=False)
+        # Should not raise; default is relative
+        result = cfg._resolve_dir("NF_TEST_DIR", "logs")
+        assert result.endswith("logs")
+
+    def test_result_is_string(self, monkeypatch):
+        monkeypatch.delenv("NF_TEST_DIR", raising=False)
+        result = cfg._resolve_dir("NF_TEST_DIR", "data")
+        assert isinstance(result, str)
+
+
 # ---------------------------------------------------------------------------
 # get_env_int helper
 # ---------------------------------------------------------------------------