From e1f9b50c3df9dff98a55e9c55412a8e330bd5662 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 19 Jan 2026 01:25:52 +0000 Subject: [PATCH] Sentinel: Fix GitHub repo traversal vulnerability Prevent arbitrary file write and repo traversal by strictly validating `github_repo` and `file_path` inputs in `update_github_cdn_urls`. Added `validate_github_repo` and `validate_file_path` functions to enforce strict whitelisting of characters and reject path traversal sequences. Added comprehensive unit tests in `tests/test_github_validation.py`. --- .jules/sentinel.md | 5 ++ discordsend_utils/github_integration.py | 42 ++++++++++- tests/test_github_validation.py | 99 +++++++++++++++++++++++++ 3 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 tests/test_github_validation.py diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 66b6f62..0161adc 100644 --- a/.jules/sentinel.md +++ b/.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. diff --git a/discordsend_utils/github_integration.py b/discordsend_utils/github_integration.py index eaecdd0..84bd16f 100644 --- a/discordsend_utils/github_integration.py +++ b/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}" diff --git a/tests/test_github_validation.py b/tests/test_github_validation.py new file mode 100644 index 0000000..dd5dcb7 --- /dev/null +++ b/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()