mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-02 09:18:06 +00:00
ChuckBuilds
2a99cf6e64
CodeQL flagged 3 clear-text-logging-of-secrets alerts in install_requirements_file() (src/common/permission_utils.py:353,360,371). Pre-existing on main, unrelated to this PR's own diff, but now visible since the path-injection alerts that previously took priority in the annotation list are fixed. The underlying risk is real: pip can echo a private index URL's embedded basic-auth credentials (from a requirements.txt --index-url line or PIP_INDEX_URL) back verbatim in its own stderr/stdout on failure, and this function both logs that output directly and returns it to callers -- store_manager.py's _install_dependencies() logs result.stderr from this same function too. Added _redact_url_credentials(), applied immediately after each of the two subprocess.run() calls (mutating result.stderr/stdout in place) rather than patching each log call site individually. This closes the leak at the source: every downstream use -- the three flagged log lines, the "note" string embedded in the returned stdout, and store_manager.py's own logging of the returned result -- gets the redacted text for free. Verified the fixed-phrase "denied" check (`"a password is required" in result.stderr`) is unaffected, since URL syntax and those phrases don't overlap -- covered explicitly by test_does_not_touch_denied_check_phrases. Added test/test_permission_utils.py (6 tests) covering the redaction helper directly and both subprocess.run() call sites (the sudo-wrapper branch, which this repo's scripts/fix_perms/safe_pip_install.sh makes live, and the no-wrapper fallback branch). All pass.
84 lines
3.7 KiB
Python
84 lines
3.7 KiB
Python
"""
|
|
Tests for src.common.permission_utils's URL-credential redaction.
|
|
|
|
Covers the fix for a CodeQL clear-text-logging-of-secrets alert:
|
|
install_requirements_file() must never let a private index URL's embedded
|
|
user:pass@ credentials reach logs or its returned CompletedProcess, since
|
|
pip can echo that URL back verbatim in its own stderr/stdout on failure.
|
|
"""
|
|
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
from src.common.permission_utils import _redact_url_credentials, install_requirements_file
|
|
|
|
|
|
class TestRedactUrlCredentials:
|
|
def test_redacts_embedded_basic_auth(self):
|
|
text = "Could not fetch URL https://alice:s3cr3t@pypi.example.com/simple/: 403"
|
|
redacted = _redact_url_credentials(text)
|
|
assert "s3cr3t" not in redacted
|
|
assert "alice" not in redacted
|
|
assert "https://***:***@pypi.example.com/simple/" in redacted
|
|
|
|
def test_leaves_credential_free_text_unchanged(self):
|
|
text = "ERROR: Could not find a version that satisfies the requirement foo==1.0"
|
|
assert _redact_url_credentials(text) == text
|
|
|
|
def test_handles_none_and_empty(self):
|
|
assert _redact_url_credentials(None) == ""
|
|
assert _redact_url_credentials("") == ""
|
|
|
|
def test_does_not_touch_denied_check_phrases(self):
|
|
"""The fixed phrases install_requirements_file greps for must survive
|
|
redaction untouched -- they don't overlap with URL syntax, but this
|
|
pins that assumption so a regex change can't silently break it."""
|
|
text = "sudo: a password is required"
|
|
assert _redact_url_credentials(text) == text
|
|
|
|
|
|
class TestInstallRequirementsFileRedaction:
|
|
@patch('src.common.permission_utils.subprocess.run')
|
|
def test_wrapper_path_redacts_stderr_and_stdout(self, mock_run, tmp_path):
|
|
"""safe_pip_install.sh exists in this repo, so install_requirements_file
|
|
takes the sudo-wrapper branch; a failing result must come back
|
|
with any embedded index-URL credentials already redacted."""
|
|
req_file = tmp_path / "requirements.txt"
|
|
req_file.write_text("requests\n")
|
|
|
|
mock_run.return_value = MagicMock(
|
|
returncode=1,
|
|
stdout="Looking in indexes: https://bob:hunter2@pypi.internal/simple\n",
|
|
stderr="ERROR https://bob:hunter2@pypi.internal/simple/foo: 401",
|
|
)
|
|
|
|
result = install_requirements_file(req_file, timeout=5)
|
|
|
|
assert "hunter2" not in result.stdout
|
|
assert "hunter2" not in result.stderr
|
|
assert "https://***:***@pypi.internal" in result.stdout
|
|
assert "https://***:***@pypi.internal" in result.stderr
|
|
|
|
@patch('src.common.permission_utils.subprocess.run')
|
|
@patch('src.common.permission_utils.Path.exists', return_value=False)
|
|
def test_no_wrapper_fallback_path_redacts_stderr_and_stdout(self, mock_exists, mock_run, tmp_path):
|
|
"""No safe_pip_install.sh wrapper -> falls straight to the
|
|
sys.executable pip fallback (the second subprocess.run call site);
|
|
its result must come back redacted too, independent of the wrapper
|
|
branch's own redaction above."""
|
|
req_file = tmp_path / "requirements.txt"
|
|
req_file.write_text("requests\n")
|
|
|
|
mock_run.return_value = MagicMock(
|
|
returncode=1,
|
|
stdout="Looking in indexes: https://carol:swordfish@pypi.internal/simple\n",
|
|
stderr="ERROR https://carol:swordfish@pypi.internal/simple/foo: 401",
|
|
)
|
|
|
|
result = install_requirements_file(req_file, timeout=5)
|
|
|
|
assert "swordfish" not in result.stdout
|
|
assert "swordfish" not in result.stderr
|
|
assert "https://***:***@pypi.internal" in result.stdout
|
|
assert "https://***:***@pypi.internal" in result.stderr
|