From 83833fe1abd839ea0a17810d8dc20efcdce461a8 Mon Sep 17 00:00:00 2001 From: ProfRandom92 <159939812+ProfRandom92@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:06:33 +0200 Subject: [PATCH 1/3] fix(plugins): implement structured line-oriented parser for secret redaction --- hf_space/previews.py | 50 ++++++++++++++++++- plugins/pr-review-memory/renderer.py | 10 ++-- .../plugins/test_pr_review_memory_renderer.py | 4 +- tests/test_secret_redaction_safety.py | 38 ++++++++++++++ 4 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 tests/test_secret_redaction_safety.py diff --git a/hf_space/previews.py b/hf_space/previews.py index 3939733..dbacbdd 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,55 @@ 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"} + # Matches: var_name = value or var_name: value + assignment_pattern = re.compile( + r"\b([A-Za-z_][A-Za-z0-9_]*)(\s*[:=]\s*)(\"[^\"]*\"|'[^']*'|[^\s,;]+)" + ) + + lines = text.splitlines(keepends=True) + redacted_lines = [] + any_redacted = False + + for line in lines: + new_line = line + matches = list(assignment_pattern.finditer(line)) + for match in reversed(matches): + var_name = match.group(1) + if any(kw in var_name.lower() for kw in sensitive_keywords): + start, end = match.span(3) + val = match.group(3) + if val == "" or val == '""' or val == "''": + continue + if val.startswith('"') and val.endswith('"'): + replacement = '""' + elif val.startswith("'") and val.endswith("'"): + replacement = "''" + else: + replacement = "" + new_line = new_line[:start] + replacement + new_line[end:] + any_redacted = True + 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/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..2c357e3 --- /dev/null +++ b/tests/test_secret_redaction_safety.py @@ -0,0 +1,38 @@ +import pytest +import sys +from pathlib import Path + +# Add plugins/pr-review-memory/ and hf_space/ to path +sys.path.insert(0, str(Path(__file__).parent.parent / "hf_space")) +sys.path.insert(0, str(Path(__file__).parent.parent / "plugins" / "pr-review-memory")) + +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=''" + + assert _clean_text("NORMAL_VAR=value") == "NORMAL_VAR=value" From 592046dc84cc54e3e7c4131db2ecaa033e581d9b Mon Sep 17 00:00:00 2001 From: ProfRandom92 <159939812+ProfRandom92@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:04:14 +0200 Subject: [PATCH 2/3] fix(redaction): handle quoted JSON keys and escaped secret values --- hf_space/previews.py | 34 +++++++++++++++++---------- pyproject.toml | 3 ++- tests/conftest.py | 6 +++++ tests/test_secret_redaction_safety.py | 19 ++++++++++----- 4 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 tests/conftest.py diff --git a/hf_space/previews.py b/hf_space/previews.py index dbacbdd..837c6c8 100644 --- a/hf_space/previews.py +++ b/hf_space/previews.py @@ -29,9 +29,13 @@ def redact_secrets(text: str) -> tuple[str, bool]: return "", False sensitive_keywords = {"token", "secret", "password", "api_key", "access_key", "private_key", "credential"} - # Matches: var_name = value or var_name: value + + # 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"\b([A-Za-z_][A-Za-z0-9_]*)(\s*[:=]\s*)(\"[^\"]*\"|'[^']*'|[^\s,;]+)" + r"([A-Za-z0-9_\-'\".\s/\\()]+?)(\s*[:=]\s*)(\"(?:[^\"\\]|\\.)*\"|'(?:[^'\\]|\\.)*'|[^\s,;}]+)" ) lines = text.splitlines(keepends=True) @@ -39,23 +43,29 @@ def redact_secrets(text: str) -> tuple[str, bool]: any_redacted = False for line in lines: - new_line = line - matches = list(assignment_pattern.finditer(line)) - for match in reversed(matches): - var_name = match.group(1) - if any(kw in var_name.lower() for kw in sensitive_keywords): - start, end = match.span(3) - val = match.group(3) - if val == "" or val == '""' or val == "''": - continue + 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 = "" - new_line = new_line[:start] + replacement + new_line[end:] 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 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/test_secret_redaction_safety.py b/tests/test_secret_redaction_safety.py index 2c357e3..ecddb00 100644 --- a/tests/test_secret_redaction_safety.py +++ b/tests/test_secret_redaction_safety.py @@ -1,10 +1,4 @@ import pytest -import sys -from pathlib import Path - -# Add plugins/pr-review-memory/ and hf_space/ to path -sys.path.insert(0, str(Path(__file__).parent.parent / "hf_space")) -sys.path.insert(0, str(Path(__file__).parent.parent / "plugins" / "pr-review-memory")) from previews import scan_secrets from renderer import _clean_text @@ -35,4 +29,17 @@ def test_secret_redaction_renderer_matrix(): 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=""' + + # Line boundary checks (no processing across lines) + from previews import redact_secrets + assert redact_secrets("TOKEN=\n'value'") == ("TOKEN=\n'value'", False) + assert _clean_text("NORMAL_VAR=value") == "NORMAL_VAR=value" From affeb43d298275bf5725b89d36feede6472d0af0 Mon Sep 17 00:00:00 2001 From: ProfRandom92 <159939812+ProfRandom92@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:41:18 +0200 Subject: [PATCH 3/3] test(previews): expand safety redaction matrix for escaped quotes and newlines --- tests/test_secret_redaction_safety.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_secret_redaction_safety.py b/tests/test_secret_redaction_safety.py index ecddb00..2541e8a 100644 --- a/tests/test_secret_redaction_safety.py +++ b/tests/test_secret_redaction_safety.py @@ -38,8 +38,13 @@ def test_secret_redaction_renderer_matrix(): assert _clean_text("TOKEN=val1 API_KEY=val2") == "TOKEN= API_KEY=" assert _clean_text('TOKEN=""') == 'TOKEN=""' - # Line boundary checks (no processing across lines) - from previews import redact_secrets - assert redact_secrets("TOKEN=\n'value'") == ("TOKEN=\n'value'", False) - + # 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)