diff --git a/hf_space/previews.py b/hf_space/previews.py index 3939733..837c6c8 100644 --- a/hf_space/previews.py +++ b/hf_space/previews.py @@ -13,7 +13,6 @@ "openai-key": re.compile(r"\bsk-(?:proj-)?[A-Za-z0-9_-]{20,}\b"), "aws-access-key": re.compile(r"\bAKIA[0-9A-Z]{16}\b"), "bearer-token": re.compile(r"\bBearer\s+[A-Za-z0-9._~+/=-]{16,}", re.I), - "secret-assignment": re.compile(r"\b(?:API_KEY|TOKEN|SECRET|PASSWORD)\s*=\s*[^\s]+", re.I), } @@ -21,8 +20,65 @@ def _sha256(text: str) -> str: return "sha256:" + hashlib.sha256(text.encode("utf-8")).hexdigest() +def redact_secrets(text: str) -> tuple[str, bool]: + """Redact sensitive variable assignments line-by-line. + + Returns the redacted text and a boolean indicating if any secret assignment was redacted. + """ + if not text: + return "", False + + sensitive_keywords = {"token", "secret", "password", "api_key", "access_key", "private_key", "credential"} + + # Key-value assignment pattern: + # Group 1: Key (can include quotes, word characters, dashes, dots, slashes) + # Group 2: Separator (: or = with surrounding whitespace) + # Group 3: Value (double-quoted with escapes, single-quoted with escapes, or unquoted characters) + assignment_pattern = re.compile( + r"([A-Za-z0-9_\-'\".\s/\\()]+?)(\s*[:=]\s*)(\"(?:[^\"\\]|\\.)*\"|'(?:[^'\\]|\\.)*'|[^\s,;}]+)" + ) + + lines = text.splitlines(keepends=True) + redacted_lines = [] + any_redacted = False + + for line in lines: + def repl(match): + nonlocal any_redacted + var_name = match.group(1).strip() + sep = match.group(2) + val = match.group(3) + + # Check if clean key contains any sensitive keyword + clean_key = var_name.replace('"', '').replace("'", "").lower() + if any(kw in clean_key for kw in sensitive_keywords): + if val in ('""', "''", ""): + return match.group(0) + + if val.startswith('"') and val.endswith('"'): + replacement = '""' + elif val.startswith("'") and val.endswith("'"): + replacement = "''" + else: + replacement = "" + any_redacted = True + return match.group(1) + sep + replacement + return match.group(0) + + new_line = assignment_pattern.sub(repl, line) + redacted_lines.append(new_line) + + return "".join(redacted_lines), any_redacted + + def scan_secrets(text: str) -> list[str]: - return sorted(name for name, pattern in _SECRET_PATTERNS.items() if pattern.search(text or "")) + matches = [name for name, pattern in _SECRET_PATTERNS.items() if pattern.search(text or "")] + + _, has_assignment = redact_secrets(text or "") + if has_assignment: + matches.append("secret-assignment") + + return sorted(matches) def _constraints(text: str) -> list[str]: diff --git a/plugins/pr-review-memory/renderer.py b/plugins/pr-review-memory/renderer.py index 0533018..9a09201 100644 --- a/plugins/pr-review-memory/renderer.py +++ b/plugins/pr-review-memory/renderer.py @@ -9,9 +9,8 @@ REQUIRED_FIELDS = ("repository", "pr_number", "branch", "head_sha", "validation_summary", "next_action") DIFF_MARKER_PREFIXES = ("diff --git", "index ", "@@", "+++", "---") -SECRET_PATTERN = re.compile( - r"(?i)\b(api[_-]?key|secret|token|password)\b\s*[:=]\s*(?:\"[^\"]*\"|'[^']*'|[^\s,;]+)" -) + +from hf_space.previews import redact_secrets def render_pr_review_memory_handoff(data: dict[str, Any]) -> str: @@ -67,8 +66,9 @@ def _clean_text(value: Any) -> str: continue kept_lines.append(stripped) cleaned = " ".join(part for part in kept_lines if part) - cleaned = SECRET_PATTERN.sub(lambda match: f"{match.group(1)}=", cleaned) - return cleaned + + redacted_text, _ = redact_secrets(cleaned) + return redacted_text def _format_pr(pr_number: Any, pr_url: Any) -> str: diff --git a/pyproject.toml b/pyproject.toml index 7cdd2bc..b8616b1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,8 @@ dev = [ "pytest>=8", "pytest-asyncio>=0.24", "tomli>=2.0; python_version < '3.11'", - "build>=1.2" + "build>=1.2", + "pandas" ] [project.scripts] diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..57b2c80 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,6 @@ +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "hf_space")) +sys.path.insert(0, str(ROOT / "plugins" / "pr-review-memory")) diff --git a/tests/plugins/test_pr_review_memory_renderer.py b/tests/plugins/test_pr_review_memory_renderer.py index afe660a..924516b 100644 --- a/tests/plugins/test_pr_review_memory_renderer.py +++ b/tests/plugins/test_pr_review_memory_renderer.py @@ -276,8 +276,8 @@ def test_renderer_redacts_quoted_multi_word_secret_values() -> None: assert "multi word secret" not in markdown assert "another multi word value" not in markdown - assert "token=" in markdown - assert "secret=" in markdown + assert 'token=""' in markdown + assert "secret=''" in markdown def test_renderer_handles_general_iterable_items_as_bullets() -> None: diff --git a/tests/test_secret_redaction_safety.py b/tests/test_secret_redaction_safety.py new file mode 100644 index 0000000..2541e8a --- /dev/null +++ b/tests/test_secret_redaction_safety.py @@ -0,0 +1,50 @@ +import pytest + +from previews import scan_secrets +from renderer import _clean_text + +def test_secret_scan_previews_matrix(): + assert "secret-assignment" in scan_secrets("TOKEN=value") + assert "secret-assignment" in scan_secrets("HF_TOKEN=value") + assert "secret-assignment" in scan_secrets("_HF_TOKEN=value") + assert "secret-assignment" in scan_secrets("MY_TOKEN_1=value") + assert "secret-assignment" in scan_secrets("api_key=value") + assert "secret-assignment" in scan_secrets("SERVICE_API_KEY=value") + assert "secret-assignment" in scan_secrets("TOKEN = value") + assert "secret-assignment" in scan_secrets('TOKEN="value"') + assert "secret-assignment" in scan_secrets("TOKEN='value'") + assert "secret-assignment" in scan_secrets("export HF_TOKEN=value") + + assert "secret-assignment" not in scan_secrets("HF_TOKEN=\n'value'") + assert "secret-assignment" not in scan_secrets("NORMAL_VAR=value") + +def test_secret_redaction_renderer_matrix(): + assert _clean_text("TOKEN=value") == "TOKEN=" + assert _clean_text("HF_TOKEN=value") == "HF_TOKEN=" + assert _clean_text("_HF_TOKEN=value") == "_HF_TOKEN=" + assert _clean_text("MY_TOKEN_1=value") == "MY_TOKEN_1=" + assert _clean_text("api_key=value") == "api_key=" + assert _clean_text("SERVICE_API_KEY=value") == "SERVICE_API_KEY=" + assert _clean_text("TOKEN = value") == "TOKEN = " + assert _clean_text('TOKEN="value"') == 'TOKEN=""' + assert _clean_text("TOKEN='value'") == "TOKEN=''" + + # New edge cases + assert _clean_text('TOKEN="abc\\"def"') == 'TOKEN=""' + assert _clean_text('"TOKEN": "value"') == '"TOKEN": ""' + assert _clean_text("'API_KEY': 'value'") == "'API_KEY': ''" + assert _clean_text('{"api_key": "value"}') == '{"api_key": ""}' + assert _clean_text('{"normal": "value"}') == '{"normal": "value"}' + assert _clean_text("TOKEN=val1 API_KEY=val2") == "TOKEN= API_KEY=" + assert _clean_text('TOKEN=""') == 'TOKEN=""' + + # Explicit cases required by Thread C + assert _clean_text('TOKEN="value\\"with\\"escapes"') == 'TOKEN=""' + assert _clean_text("TOKEN='value\\'with\\'escapes'") == "TOKEN=''" + assert _clean_text("HF_TOKEN=") == "HF_TOKEN=" + assert _clean_text("HF_TOKEN=\n'value'") == "HF_TOKEN= ''" # _clean_text collapses newlines to space first + assert _clean_text("NORMAL_VAR=value") == "NORMAL_VAR=value" + + # Line boundary checks (no processing across lines on raw multiline text) + from previews import redact_secrets + assert redact_secrets("HF_TOKEN=\n'value'") == ("HF_TOKEN=\n'value'", False)