fix(plugins): implement structured line-oriented parser for secret redaction

hf_space/previews.py

@@ -13,16 +13,72 @@
     "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),
 }
 
 
 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 ('"<redacted>"', "'<redacted>'", "<redacted>"):
+                    return match.group(0)
+
+                if val.startswith('"') and val.endswith('"'):
+                    replacement = '"<redacted>"'
+                elif val.startswith("'") and val.endswith("'"):
+                    replacement = "'<redacted>'"
+                else:
+                    replacement = "<redacted>"
+                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]:

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)}=<redacted>", cleaned)
-    return cleaned
+
+    redacted_text, _ = redact_secrets(cleaned)
+    return redacted_text
 
 
 def _format_pr(pr_number: Any, pr_url: Any) -> str:

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]

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"))

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=<redacted>" in markdown
-    assert "secret=<redacted>" in markdown
+    assert 'token="<redacted>"' in markdown
+    assert "secret='<redacted>'" in markdown
 
 
 def test_renderer_handles_general_iterable_items_as_bullets() -> None:

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=<redacted>"
+    assert _clean_text("HF_TOKEN=value") == "HF_TOKEN=<redacted>"
+    assert _clean_text("_HF_TOKEN=value") == "_HF_TOKEN=<redacted>"
+    assert _clean_text("MY_TOKEN_1=value") == "MY_TOKEN_1=<redacted>"
+    assert _clean_text("api_key=value") == "api_key=<redacted>"
+    assert _clean_text("SERVICE_API_KEY=value") == "SERVICE_API_KEY=<redacted>"
+    assert _clean_text("TOKEN  =  value") == "TOKEN  =  <redacted>"
+    assert _clean_text('TOKEN="value"') == 'TOKEN="<redacted>"'
+    assert _clean_text("TOKEN='value'") == "TOKEN='<redacted>'"
+
+    # New edge cases
+    assert _clean_text('TOKEN="abc\\"def"') == 'TOKEN="<redacted>"'
+    assert _clean_text('"TOKEN": "value"') == '"TOKEN": "<redacted>"'
+    assert _clean_text("'API_KEY': 'value'") == "'API_KEY': '<redacted>'"
+    assert _clean_text('{"api_key": "value"}') == '{"api_key": "<redacted>"}'
+    assert _clean_text('{"normal": "value"}') == '{"normal": "value"}'
+    assert _clean_text("TOKEN=val1 API_KEY=val2") == "TOKEN=<redacted> API_KEY=<redacted>"
+    assert _clean_text('TOKEN="<redacted>"') == 'TOKEN="<redacted>"'
+
+    # Explicit cases required by Thread C
+    assert _clean_text('TOKEN="value\\"with\\"escapes"') == 'TOKEN="<redacted>"'
+    assert _clean_text("TOKEN='value\\'with\\'escapes'") == "TOKEN='<redacted>'"
+    assert _clean_text("HF_TOKEN=") == "HF_TOKEN="
+    assert _clean_text("HF_TOKEN=\n'value'") == "HF_TOKEN= '<redacted>'"  # _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)