Merge pull request #26 from AEmotionStudio/sentinel-github-validation-17856768252331476057

🛡️ Sentinel: [HIGH] Fix GitHub repo traversal vulnerability

.jules/sentinel.md

@@ -12,3 +12,8 @@
 **Vulnerability:** Upstream APIs (like GitHub) may echo back sensitive request headers or credentials in their error response bodies (e.g., "Bad credentials: [TOKEN] is invalid"). Including raw `response.text` in application error messages or logs leaked these credentials.
 **Learning:** Never assume external API error responses are safe to log or display. They may contain sensitive data sent in the request (headers, body) or specific to the failure context.
 **Prevention:** Sanitize raw response bodies (`response.text`) before including them in error logs or exception messages. Scrub known sensitive patterns (like API tokens) from any external input before outputting it.
+
+## 2025-10-27 - GitHub Repo Traversal and Arbitrary File Write
+**Vulnerability:** `update_github_cdn_urls` accepted `github_repo` strings like `user/repo/../../victim/target` and `file_path` strings like `../secret.txt`, allowing path traversal. This could enable an attacker to manipulate files in arbitrary repositories the user's token had access to.
+**Learning:** Checking for the presence of a separator (like `/`) is not sufficient validation. Simple string concatenation for URL construction is dangerous when inputs can contain traversal sequences like `..`.
+**Prevention:** Strictly validate structural inputs against a whitelist regex (e.g., `^[\w.-]+/[\w.-]+$`). Reject any input containing path traversal sequences (`..`) or unexpected characters before using them in API calls.

discordsend_utils/github_integration.py

@@ -6,11 +6,43 @@ Handles updating GitHub repositories with Discord CDN URLs.
 
 import base64
 import time
+import re
 from typing import List, Optional, Tuple
 
 import requests
 
 
+def validate_github_repo(repo: str) -> bool:
+    """
+    Validate GitHub repository format (username/repo).
+    Strictly enforces alphanumeric, hyphens, underscores, and periods.
+    Prevents path traversal and injection.
+    """
+    if not repo:
+        return False
+    # Pattern: username/repo
+    # GitHub usernames: alphanumeric, hyphens (max 39 chars)
+    # Repo names: alphanumeric, hyphens, periods, underscores
+    pattern = r"^[a-zA-Z0-9-]+/[\w.-]+$"
+    return bool(re.match(pattern, repo))
+
+
+def validate_file_path(path: str) -> bool:
+    """
+    Validate file path for GitHub API.
+    Prevents path traversal (..) and absolute paths.
+    """
+    if not path:
+        return False
+    # Prevent traversal
+    if ".." in path:
+        return False
+    # Prevent absolute paths (GitHub API treats paths as relative to root)
+    if path.startswith("/"):
+        return False
+    return True
+
+
 def update_github_cdn_urls(
     github_repo: str,
     github_token: str,
@@ -44,9 +76,13 @@ def update_github_cdn_urls(
     if not cdn_urls:
         return False, "No CDN URLs to update"
     
-    # Ensure repository format is valid
-    if "/" not in github_repo:
-        return False, f"Invalid GitHub repository format: {github_repo}. Expected format: username/repo"
+    # Strictly validate repository format to prevent traversal/injection
+    if not validate_github_repo(github_repo):
+        return False, f"Invalid GitHub repository format: {github_repo}. Expected format: username/repo (alphanumeric, hyphens, periods, underscores only)"
+
+    # Strictly validate file path to prevent traversal
+    if not validate_file_path(file_path):
+        return False, f"Invalid file path: {file_path}. Path traversal (..) and absolute paths are not allowed."
     
     # Setup API endpoint
     api_url = f"https://api.github.com/repos/{github_repo}/contents/{file_path}"

tests/test_github_validation.py

@@ -0,0 +1,99 @@
+
+import unittest
+import sys
+from unittest.mock import MagicMock
+
+# Mock torch and other heavy dependencies
+sys.modules["torch"] = MagicMock()
+sys.modules["numpy"] = MagicMock()
+sys.modules["PIL"] = MagicMock()
+sys.modules["cv2"] = MagicMock()
+sys.modules["folder_paths"] = MagicMock()
+sys.modules["comfy"] = MagicMock()
+sys.modules["comfy.cli_args"] = MagicMock()
+
+# Now we can safely import
+from discordsend_utils.github_integration import validate_github_repo, validate_file_path, update_github_cdn_urls
+
+class TestGitHubValidation(unittest.TestCase):
+
+    def test_validate_github_repo_valid(self):
+        """Test valid GitHub repository formats."""
+        valid_repos = [
+            "username/repo",
+            "user-name/repo-name",
+            "user-name/repo.name",  # Dot in repo is valid
+            "user-name/repo_name",  # Underscore in repo is valid
+            "0123/4567"
+        ]
+        for repo in valid_repos:
+            with self.subTest(repo=repo):
+                self.assertTrue(validate_github_repo(repo), f"Failed for {repo}")
+
+    def test_validate_github_repo_invalid(self):
+        """Test invalid GitHub repository formats (traversal, injection, invalid chars)."""
+        invalid_repos = [
+            "username/repo/../other",  # Traversal
+            "username/repo?query=1",   # Query injection
+            "username",                # Missing slash
+            "/repo",                   # Missing username
+            "user/",                   # Missing repo
+            "user/repo/",              # Trailing slash (strict check)
+            "../../user/repo",         # Traversal at start
+            "user/repo#fragment",      # Fragment
+            "user/repo;rm -rf",        # Command injection style
+            "user/repo\nnewline",      # Newline
+            "user.name/repo",          # Dot in username (invalid)
+            "user_name/repo",          # Underscore in username (invalid)
+        ]
+        for repo in invalid_repos:
+            with self.subTest(repo=repo):
+                self.assertFalse(validate_github_repo(repo), f"Should have failed for {repo}")
+
+    def test_validate_file_path_valid(self):
+        """Test valid file paths."""
+        valid_paths = [
+            "file.txt",
+            "path/to/file.txt",
+            "folder/subfolder/file.md",
+            "README.md",
+            "docs/image.png"
+        ]
+        for path in valid_paths:
+            with self.subTest(path=path):
+                self.assertTrue(validate_file_path(path), f"Failed for {path}")
+
+    def test_validate_file_path_invalid(self):
+        """Test invalid file paths (traversal, absolute)."""
+        invalid_paths = [
+            "../file.txt",             # Traversal
+            "path/../file.txt",        # Traversal inside
+            "/etc/passwd",             # Absolute path
+            "/file.txt",               # Absolute path
+            "../../secret",            # Deep traversal
+            "",                        # Empty
+            None                       # None
+        ]
+        for path in invalid_paths:
+            with self.subTest(path=path):
+                self.assertFalse(validate_file_path(path), f"Should have failed for {path}")
+
+    def test_update_github_cdn_urls_rejects_invalid_repo(self):
+        """Test that update_github_cdn_urls rejects invalid repo before making requests."""
+        repo = "user/repo/../malicious"
+        success, message = update_github_cdn_urls(repo, "token", "file.md", [("f", "u")])
+
+        self.assertFalse(success)
+        self.assertIn("Invalid GitHub repository format", message)
+
+    def test_update_github_cdn_urls_rejects_invalid_path(self):
+        """Test that update_github_cdn_urls rejects invalid path before making requests."""
+        path = "../../../secret.txt"
+        success, message = update_github_cdn_urls("user/repo", "token", path, [("f", "u")])
+
+        self.assertFalse(success)
+        self.assertIn("Invalid file path", message)
+        self.assertIn("Path traversal", message)
+
+if __name__ == "__main__":
+    unittest.main()