diff --git a/src/github_repo_auditor/cache.py b/src/github_repo_auditor/cache.py index 077829d..45c2779 100644 --- a/src/github_repo_auditor/cache.py +++ b/src/github_repo_auditor/cache.py @@ -15,30 +15,47 @@ "access_token", "api_key", "apikey", + "auth_token", "authorization", "client_secret", "credential", "password", "private_key", + "refresh_token", "github_token", "secret", "token", + "x_api_key", } ) +_SENSITIVE_ASSIGNMENT = re.compile( + r"(?i)(? str: + return re.sub(r"[^a-z0-9]+", "_", str(value).casefold()).strip("_") + + def contains_sensitive_data(value: Any) -> bool: """Return whether JSON-compatible data contains credential data.""" if isinstance(value, dict): return any( - str(key).lower() in _SENSITIVE_FIELD_NAMES or contains_sensitive_data(item) + _normalized_field_name(key) in _SENSITIVE_FIELD_NAMES + or contains_sensitive_data(item) for key, item in value.items() ) if isinstance(value, list): @@ -46,12 +63,27 @@ def contains_sensitive_data(value: Any) -> bool: if isinstance(value, tuple): return any(contains_sensitive_data(item) for item in value) if isinstance(value, str): - return any(pattern.search(value) for pattern in _SENSITIVE_VALUE_PATTERNS) + return ( + any(pattern.search(value) for pattern in _SENSITIVE_VALUE_PATTERNS) + or _SENSITIVE_ASSIGNMENT.search(value) is not None + or _URL_WITH_USERINFO.search(value) is not None + ) return False -def _url_has_sensitive_query(url: str) -> bool: - return any(name.lower() in _SENSITIVE_FIELD_NAMES for name, _value in parse_qsl(urlparse(url).query)) +def _url_has_sensitive_components(url: str) -> bool: + parsed = urlparse(url) + names = ( + name + for component in (parsed.query, parsed.fragment) + for name, _value in parse_qsl(component) + ) + return any(_normalized_field_name(name) in _SENSITIVE_FIELD_NAMES for name in names) + + +def _url_has_embedded_credentials(url: str) -> bool: + parsed = urlparse(url) + return parsed.username is not None or parsed.password is not None class ResponseCache: @@ -95,7 +127,13 @@ def put( response: object, ) -> None: """Store response data with current timestamp.""" - if _url_has_sensitive_query(url) or contains_sensitive_data(params) or contains_sensitive_data(response): + if ( + _url_has_sensitive_components(url) + or _url_has_embedded_credentials(url) + or contains_sensitive_data(url) + or contains_sensitive_data(params) + or contains_sensitive_data(response) + ): return path = self._path(url, params) entry = { diff --git a/src/github_repo_auditor/cli_output.py b/src/github_repo_auditor/cli_output.py index 1df6f29..c81d8b6 100644 --- a/src/github_repo_auditor/cli_output.py +++ b/src/github_repo_auditor/cli_output.py @@ -26,8 +26,10 @@ _stdout_console = Console() if HAS_RICH else None _SENSITIVE_ASSIGNMENT = re.compile( - r"(?i)(?P(?P['\"]?)(?:access_token|api_key|apikey|" - r"client_secret|credential|github_token|password|private_key|secret|token)" + r"(?i)(?(?P['\"]?)(?:access[-_]?token|" + r"auth[-_]?token|refresh[-_]?token|x[-_]?api[-_]?key|api[-_]?key|apikey|" + r"client[-_]?secret|credential|github[-_]?token|password|private[-_]?key|" + r"secret|token)" r"(?P=key_quote)\s*[:=]\s*)" r"(?:(?P\"(?:\\.|[^\"\\])*\"|'(?:\\.|[^'\\])*')|" r"(?P[^\r\n,;}\]]+))" @@ -38,21 +40,25 @@ r"(?P[^\r\n}\]]+))" ) _PRIVATE_KEY_BLOCK = re.compile( - r"-----BEGIN (?:RSA |EC |DSA )?PRIVATE KEY-----.*?" - r"-----END (?:RSA |EC |DSA )?PRIVATE KEY-----", + r"-----BEGIN (?P(?:(?:RSA|EC|DSA|OPENSSH|ENCRYPTED) )?)PRIVATE KEY-----.*?" + r"-----END (?P=key_kind)PRIVATE KEY-----", re.DOTALL, ) +_URL_USERINFO = re.compile(r"(?Phttps?://)[^/@\s]+@", re.IGNORECASE) _SENSITIVE_TOKENS = ( re.compile(r"\bAKIA[0-9A-Z]{16}\b"), re.compile(r"\bgh[pousr]_[A-Za-z0-9]{36,}\b"), re.compile(r"\bgithub_pat_[A-Za-z0-9_]{20,}\b"), re.compile(r"\bxox[bpors]-[A-Za-z0-9-]{10,}\b"), + re.compile(r"\bsecret_[A-Za-z0-9]{20,}\b"), + re.compile(r"\bntn_[A-Za-z0-9]{20,}\b"), ) def redact_sensitive_text(msg: str) -> str: """Redact credential-shaped values before terminal output.""" redacted = _PRIVATE_KEY_BLOCK.sub("", str(msg)) + redacted = _URL_USERINFO.sub(r"\g@", redacted) redacted = _AUTHORIZATION_VALUE.sub(_redact_assignment, redacted) redacted = _SENSITIVE_ASSIGNMENT.sub(_redact_assignment, redacted) for pattern in _SENSITIVE_TOKENS: diff --git a/src/github_repo_auditor/operator_control_center_artifacts.py b/src/github_repo_auditor/operator_control_center_artifacts.py index 28ee4f5..86c95c5 100644 --- a/src/github_repo_auditor/operator_control_center_artifacts.py +++ b/src/github_repo_auditor/operator_control_center_artifacts.py @@ -3,10 +3,12 @@ from __future__ import annotations import json +import re from datetime import datetime from pathlib import Path from github_repo_auditor.cache import contains_sensitive_data +from github_repo_auditor.cli_output import redact_sensitive_text from github_repo_auditor.operator_artifact_paths import control_center_paths from github_repo_auditor.operator_control_center import ( control_center_artifact_payload, @@ -18,6 +20,16 @@ write_weekly_command_center_artifacts, ) +_SENSITIVE_LABELLED_VALUE = re.compile( + r"(?i)\b(?:[a-z0-9]+[-_])*(?:secret|token|password|credential)" + r"(?:[-_][a-z0-9]+)+\b" +) +_SAFE_MARKDOWN_TEXT = "" +_SAFE_MARKDOWN_LANES = frozenset({"blocked", "urgent", "ready", "deferred"}) +_SAFE_MARKDOWN_STATUSES = frozenset( + {"ok", "ready", "current", "warning", "blocked", "error", "unknown"} +) + def should_print_control_center_item(item: dict) -> bool: catalog = item.get("portfolio_catalog") or {} @@ -50,6 +62,88 @@ def filter_snapshot_for_default_view(snapshot: dict) -> dict: return snapshot +def _looks_sensitive_key(key: str) -> bool: + lowered = str(key or "").strip().lower() + return any( + marker in lowered + for marker in ( + "secret", + "token", + "password", + "passwd", + "credential", + "api_key", + "apikey", + "private_key", + "access_key", + ) + ) + + +def _redact_sensitive_values(value: object) -> object: + if isinstance(value, dict): + redacted: dict = {} + for k, v in value.items(): + if _looks_sensitive_key(str(k)): + redacted[k] = "[REDACTED]" + else: + redacted[k] = _redact_sensitive_values(v) + return redacted + if isinstance(value, list): + return [_redact_sensitive_values(item) for item in value] + if isinstance(value, tuple): + return tuple(_redact_sensitive_values(item) for item in value) + if isinstance(value, str): + redacted_text = redact_sensitive_text(value) + return _SENSITIVE_LABELLED_VALUE.sub("", redacted_text) + return value + + +def _sanitized_snapshot_for_rendering(snapshot: dict) -> dict: + """Build a fail-closed projection for the persisted Markdown artifact. + + Operator prose, identifiers, paths, and URLs may contain opaque secrets that + no pattern-based redactor can distinguish from ordinary text. Persist only + fixed explanatory text plus finite status/lane values and numeric counts. + """ + raw_setup = snapshot.get("operator_setup_health") + setup = raw_setup if isinstance(raw_setup, dict) else {} + status = str(setup.get("status", "unknown")).strip().lower() + safe_status = status if status in _SAFE_MARKDOWN_STATUSES else "unknown" + + def safe_count(value: object) -> int: + return value if isinstance(value, int) and value >= 0 else 0 + + raw_queue = snapshot.get("operator_queue") + safe_queue = [] + if isinstance(raw_queue, list): + for item in raw_queue: + if not isinstance(item, dict): + continue + lane = str(item.get("lane", "deferred")).strip().lower() + safe_queue.append( + { + "lane": lane if lane in _SAFE_MARKDOWN_LANES else "deferred", + "repo": "", + "title": _SAFE_MARKDOWN_TEXT, + "summary": _SAFE_MARKDOWN_TEXT, + "lane_reason": _SAFE_MARKDOWN_TEXT, + "recommended_action": _SAFE_MARKDOWN_TEXT, + } + ) + + return { + "operator_summary": {"headline": _SAFE_MARKDOWN_TEXT}, + "operator_setup_health": { + "status": safe_status, + "blocking_errors": safe_count(setup.get("blocking_errors")), + "warnings": safe_count(setup.get("warnings")), + }, + "operator_queue": safe_queue, + "operator_recent_changes": [], + } + + def write_control_center_artifacts( report_data: dict, snapshot: dict, @@ -77,26 +171,48 @@ def write_control_center_artifacts( report_reference=report_reference, generated_at=generated_at.isoformat(), ) - payload = control_center_artifact_payload(report_data, snapshot) - payload["weekly_command_center_digest_v1"] = weekly_digest + if contains_sensitive_data(weekly_digest): + raise ValueError("control-center artifacts must not persist credential fields") + markdown_snapshot = _sanitized_snapshot_for_rendering(snapshot) + sanitized_report_data = _redact_sensitive_values(report_data) + if not isinstance(sanitized_report_data, dict): + raise ValueError("control-center artifacts must not persist credential fields") + payload = control_center_artifact_payload(sanitized_report_data, markdown_snapshot) + sanitized_weekly_digest_for_payload = _redact_sensitive_values(weekly_digest) + payload["weekly_command_center_digest_v1"] = sanitized_weekly_digest_for_payload + sanitized_payload = _redact_sensitive_values(payload) + if not isinstance(sanitized_payload, dict): + raise ValueError("control-center artifacts must not persist credential fields") + payload = sanitized_payload if contains_sensitive_data(payload) or contains_sensitive_data(snapshot): raise ValueError("control-center artifacts must not persist credential fields") + if contains_sensitive_data(markdown_snapshot): + raise ValueError("control-center artifacts must not persist credential fields") + rendered_markdown = render_control_center_markdown( + markdown_snapshot, "operator", generated_at.date().isoformat() + ) + if contains_sensitive_data(rendered_markdown): + raise ValueError("control-center artifacts must not persist credential fields") + safe_rendered_markdown = rendered_markdown + if contains_sensitive_data(safe_rendered_markdown): + raise ValueError("control-center artifacts must not persist credential fields") + sanitized_weekly_digest = _redact_sensitive_values(weekly_digest) + if not isinstance(sanitized_weekly_digest, dict): + raise ValueError("control-center artifacts must not persist credential fields") + if contains_sensitive_data(sanitized_weekly_digest): + raise ValueError("control-center artifacts must not persist credential fields") weekly_json, weekly_md = write_weekly_command_center_artifacts( output_dir, username=username, generated_at=generated_at, - digest=weekly_digest, + digest=sanitized_weekly_digest, ) payload["weekly_command_center_reference"] = { "json_path": str(weekly_json), "markdown_path": str(weekly_md), } - # Credential-shaped data is rejected above. - # codeql[py/clear-text-storage-sensitive-data] + # The payload is recursively sanitized and checked before persistence. json_path.write_text(json.dumps(payload, indent=2)) - # Credential-shaped data is rejected above. - # codeql[py/clear-text-storage-sensitive-data] - md_path.write_text( - render_control_center_markdown(snapshot, username, generated_at.isoformat()) - ) + # The exact rendered value is redacted and rechecked immediately before persistence. + md_path.write_text(safe_rendered_markdown) return json_path, md_path, weekly_json, weekly_md, payload diff --git a/tests/test_cache.py b/tests/test_cache.py index 97bac20..b64dda2 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -3,6 +3,8 @@ import json import time +import pytest + from github_repo_auditor.cache import ResponseCache @@ -65,6 +67,47 @@ def test_put_skips_payloads_that_contain_credentials(self, tmp_path): assert not cache._path(url, {"access_token": "secret-param", "per_page": "10"}).exists() + def test_put_skips_embedded_url_credentials(self, tmp_path): + cache = ResponseCache(cache_dir=tmp_path / "cache", ttl=3600) + url = "https://operator:opaque-value@example.invalid/repos" + + cache.put(url, None, {"status": "ok"}) + + assert not cache._path(url, None).exists() + + def test_put_skips_hyphenated_credential_alias(self, tmp_path): + cache = ResponseCache(cache_dir=tmp_path / "cache", ttl=3600) + url = "https://api.github.com/repos/user/repo" + + cache.put(url, None, {"access-token": "opaque-value"}) + + assert not cache._path(url, None).exists() + + @pytest.mark.parametrize("alias", ["auth-token", "refresh_token", "x-api-key"]) + def test_put_skips_additional_credential_aliases(self, tmp_path, alias): + cache = ResponseCache(cache_dir=tmp_path / "cache", ttl=3600) + url = "https://api.github.com/repos/user/repo" + + cache.put(url, None, {alias: "opaque-value"}) + + assert not cache._path(url, None).exists() + + def test_put_skips_sensitive_url_fragment(self, tmp_path): + cache = ResponseCache(cache_dir=tmp_path / "cache", ttl=3600) + url = "https://example.invalid/callback#access-token=opaque-value" + + cache.put(url, None, {"status": "ok"}) + + assert not cache._path(url, None).exists() + + def test_put_skips_additional_provider_token_family(self, tmp_path): + cache = ResponseCache(cache_dir=tmp_path / "cache", ttl=3600) + url = "https://api.github.com/repos/user/repo" + + cache.put(url, None, {"note": "secret_" + ("a" * 40)}) + + assert not cache._path(url, None).exists() + def test_put_skips_credential_shaped_value_under_benign_key(self, tmp_path): cache = ResponseCache(cache_dir=tmp_path / "cache", ttl=3600) url = "https://api.github.com/repos/user/repo" diff --git a/tests/test_cli_output.py b/tests/test_cli_output.py index a84c1cf..22c991a 100644 --- a/tests/test_cli_output.py +++ b/tests/test_cli_output.py @@ -1,3 +1,5 @@ +import pytest + from github_repo_auditor.cli_output import ( HAS_RICH, create_progress, @@ -7,6 +9,7 @@ print_warning, redact_sensitive_text, ) +from github_repo_auditor import cli_output class TestRichAvailable: @@ -65,6 +68,42 @@ def test_preserves_noncredential_status_text(self): "scanned=5 high=11 critical=0" ) + def test_redacts_embedded_url_credentials_and_additional_private_key(self): + url = "https://operator:opaque-value@example.invalid/repos" + private_key = ( + "-----BEGIN OPENSSH PRIVATE KEY-----\n" + "opaque-material\n" + "-----END OPENSSH PRIVATE KEY-----" + ) + + assert "opaque-value" not in redact_sensitive_text(url) + assert "opaque-material" not in redact_sensitive_text(private_key) + + @pytest.mark.parametrize("alias", ["auth-token", "refresh_token", "x-api-key"]) + def test_redacts_additional_credential_aliases(self, alias): + output = redact_sensitive_text(f"{alias}=opaque-value") + + assert "opaque-value" not in output + assert "" in output + + def test_redacts_sensitive_url_fragment(self): + output = redact_sensitive_text( + "https://example.invalid/callback#access-token=opaque-value" + ) + + assert "opaque-value" not in output + + def test_print_info_redacts_additional_provider_token_family( + self, capsys, monkeypatch + ): + monkeypatch.setattr(cli_output, "HAS_RICH", False) + + print_info("provider=secret_" + ("a" * 40)) + + captured = capsys.readouterr() + assert "secret_" not in captured.err + assert "" in captured.err + def test_print_status_no_crash(self, capsys): print_status("Testing status") # Should not raise diff --git a/tests/test_operator_control_center_artifacts.py b/tests/test_operator_control_center_artifacts.py index dc0ac45..11788e9 100644 --- a/tests/test_operator_control_center_artifacts.py +++ b/tests/test_operator_control_center_artifacts.py @@ -66,6 +66,110 @@ def test_control_center_artifacts_reject_credential_value_before_any_write( assert weekly_writes == [] +def test_control_center_artifacts_redact_opaque_sensitive_free_text_before_writing( + tmp_path, monkeypatch +): + json_path, md_path, weekly_writes = _stub_artifact_dependencies( + tmp_path, + monkeypatch, + weekly_digest={"status": "current"}, + ) + weekly_digests = [] + + def write_weekly(*_args, **kwargs): + weekly_writes.append(True) + weekly_digests.append(kwargs["digest"]) + return tmp_path / "weekly.json", tmp_path / "weekly.md" + + monkeypatch.setattr(artifacts, "write_weekly_command_center_artifacts", write_weekly) + + result = artifacts.write_control_center_artifacts( + {}, + {"operator_summary": {"headline": "opaque-secret-value"}}, + tmp_path, + username="user", + generated_at=datetime.now(timezone.utc), + report_reference="report", + ) + + assert result[0:2] == (json_path, md_path) + assert "opaque-secret-value" not in json_path.read_text() + assert "opaque-secret-value" not in md_path.read_text() + assert "opaque-secret-value" not in str(weekly_digests[0]) + assert "" in md_path.read_text() + assert weekly_writes == [True] + + +def test_control_center_artifact_sanitizer_preserves_ordinary_security_text(): + value = {"headline": "secret scanning is enabled; token budget is healthy"} + + assert artifacts._redact_sensitive_values(value) == value + + +def test_control_center_artifact_sanitizer_redacts_compound_sensitive_labels(): + value = {"headline": "secret_scanning_value and api-token-prod"} + + assert artifacts._redact_sensitive_values(value) == { + "headline": " and " + } + + +def test_control_center_markdown_projection_drops_unlabelled_free_text(): + projection = artifacts._sanitized_snapshot_for_rendering( + { + "operator_summary": {"headline": "opaque-value"}, + "operator_setup_health": {"status": "unexpected", "warnings": "secret"}, + "operator_queue": [ + {"lane": "urgent", "title": "opaque-value", "summary": "opaque-value"}, + "not-a-queue-item", + ], + "operator_recent_changes": [{"summary": "opaque-value"}], + } + ) + + assert projection == { + "operator_summary": {"headline": ""}, + "operator_setup_health": { + "status": "unknown", + "blocking_errors": 0, + "warnings": 0, + }, + "operator_queue": [ + { + "lane": "urgent", + "repo": "", + "title": "", + "summary": "", + "lane_reason": "", + "recommended_action": "", + } + ], + "operator_recent_changes": [], + } + + +def test_control_center_artifacts_reject_hyphenated_credential_alias( + tmp_path, monkeypatch +): + json_path, md_path, weekly_writes = _stub_artifact_dependencies( + tmp_path, monkeypatch + ) + + with pytest.raises(ValueError, match="must not persist credential fields"): + artifacts.write_control_center_artifacts( + {"access-token": "opaque-value"}, + {}, + tmp_path, + username="user", + generated_at=datetime.now(timezone.utc), + report_reference="report", + ) + + assert not json_path.exists() + assert not md_path.exists() + assert weekly_writes == [] + + def test_control_center_artifacts_reject_sensitive_derived_digest_before_writing( tmp_path, monkeypatch ): @@ -96,6 +200,82 @@ def test_control_center_artifacts_reject_sensitive_derived_digest_before_writing assert weekly_writes == [] +def test_control_center_artifacts_reject_sensitive_rendered_markdown_before_writing( + tmp_path, monkeypatch +): + json_path, md_path, weekly_writes = _stub_artifact_dependencies( + tmp_path, + monkeypatch, + weekly_digest={"status": "current"}, + ) + monkeypatch.setattr( + artifacts, + "control_center_artifact_payload", + lambda *_args: {"status": "current"}, + ) + monkeypatch.setattr( + artifacts, + "render_control_center_markdown", + lambda *_args: "secret_" + ("a" * 40), + ) + + with pytest.raises(ValueError, match="must not persist credential fields"): + artifacts.write_control_center_artifacts( + {}, + {}, + tmp_path, + username="user", + generated_at=datetime.now(timezone.utc), + report_reference="report", + ) + + assert not json_path.exists() + assert not md_path.exists() + assert weekly_writes == [] + + +@pytest.mark.parametrize( + "rendered", + [ + "x-api-key=opaque-value", + "https://user:opaque-value@example.invalid/report", + "https://example.invalid/callback#refresh_token=opaque-value", + ], +) +def test_control_center_artifacts_reject_additional_rendered_credentials( + tmp_path, monkeypatch, rendered +): + json_path, md_path, weekly_writes = _stub_artifact_dependencies( + tmp_path, + monkeypatch, + weekly_digest={"status": "current"}, + ) + monkeypatch.setattr( + artifacts, + "control_center_artifact_payload", + lambda *_args: {"status": "current"}, + ) + monkeypatch.setattr( + artifacts, + "render_control_center_markdown", + lambda *_args: rendered, + ) + + with pytest.raises(ValueError, match="must not persist credential fields"): + artifacts.write_control_center_artifacts( + {}, + {}, + tmp_path, + username="user", + generated_at=datetime.now(timezone.utc), + report_reference="report", + ) + + assert not json_path.exists() + assert not md_path.exists() + assert weekly_writes == [] + + def test_control_center_artifacts_preserve_normal_artifact_writes( tmp_path, monkeypatch ):