fix: mechanical security hardening — symlinks, markdown links, .dev.vars

.gitignore

@@ -11,6 +11,8 @@ build/
 coverage.xml
 test-results.xml
 .dev.vars
+.env
+.env.*
 .claude/
 .claude-flow/
 .serena/

.secrets.baseline

@@ -204,7 +204,7 @@
         "filename": "tests/test_grippy_codebase.py",
         "hashed_secret": "3acfb2c2b433c0ea7ff107e33df91b18e52f960f",
         "is_verified": false,
-        "line_number": 561
+        "line_number": 562
       }
     ],
     "tests/test_grippy_embedder.py": [
@@ -254,21 +254,21 @@
         "filename": "tests/test_grippy_review.py",
         "hashed_secret": "3e536cc49ad17f2f50dc6f0aab08495bddbb2833",
         "is_verified": false,
-        "line_number": 906
+        "line_number": 907
       },
       {
         "type": "Secret Keyword",
         "filename": "tests/test_grippy_review.py",
         "hashed_secret": "80c3eb3a746f82974a9696275d8b52a37fba449b",
         "is_verified": false,
-        "line_number": 938
+        "line_number": 939
       },
       {
         "type": "AWS Access Key",
         "filename": "tests/test_grippy_review.py",
         "hashed_secret": "1d5bc0e7232d07b336f4d35db4c0200142962f4a",
         "is_verified": false,
-        "line_number": 1463
+        "line_number": 1464
       }
     ],
     "tests/test_grippy_rule_secrets.py": [
@@ -295,5 +295,5 @@
       }
     ]
   },
-  "generated_at": "2026-03-15T06:39:33Z"
+  "generated_at": "2026-04-03T17:34:56Z"
 }

CLAUDE.md

@@ -158,7 +158,7 @@ Graph enrichment: `enrich_results()` (rules/enrichment.py) post-processes findin
 ### Codebase Tools (`codebase.py`) — LLM-facing, security-critical
 
 - Path traversal: `Path.is_relative_to()` (not `startswith`)
-- Symlink: `grep_code` uses `-S` flag (no follow)
+- Symlink: `grep_code` uses `-r` (not `-R`), which does not follow symlinks on GNU grep. BSD grep `-r` does follow symlinks; `-S` is BSD-only and not used. See DEBT-INT-009.
 - Glob timeout: 5-second `time.monotonic()` deadline
 - Result caps: 5,000 files indexed, 500 glob results, 12,000 chars per tool response
 - Sanitization: `tool_hooks` middleware applies `navi_sanitize.clean()` + XML-escape + 12K truncation to all tool outputs before LLM sees them

src/grippy/codebase.py

@@ -731,6 +731,27 @@ def grep_code(pattern: str, glob: str = "*.py", context_lines: int = 2) -> str:
     return grep_code
 
 
+def _reject_symlinks(path: Path, repo_root: Path) -> str | None:
+    """Check if any component of *path* is a symlink.
+
+    Defense-in-depth: rejects symlinks *before* resolve() so a symlink
+    inside the repo that resolves to an external target cannot bypass the
+    is_relative_to() guard.
+
+    Returns an error message string if a symlink is found, or None if safe.
+    """
+    try:
+        relative = path.relative_to(repo_root)
+    except ValueError:
+        return "Error: path outside repo root."
+    current = repo_root
+    for part in relative.parts:
+        current = current / part
+        if current.is_symlink():
+            return "Error: symlinks not allowed."
+    return None
+
+
 def _make_read_file(repo_root: Path) -> Any:
     """Create a read_file tool function bound to a repo root."""
 
@@ -742,7 +763,11 @@ def read_file(path: str, start_line: int = 0, end_line: int = 0) -> str:
         :param end_line: last line to read (1-based, 0 = to end)
         """
         target = repo_root / path
-        # Prevent path traversal
+        # Layer 1: reject symlinks before resolve()
+        symlink_err = _reject_symlinks(target, repo_root)
+        if symlink_err is not None:
+            return symlink_err
+        # Layer 2: resolve + is_relative_to (catches traversal via ..)
         try:
             target = target.resolve()
             if not target.is_relative_to(repo_root.resolve()):
@@ -795,7 +820,11 @@ def list_files(path: str = ".", glob_pattern: str = "*") -> str:
         :param glob_pattern: glob pattern to filter files (default "*")
         """
         target = repo_root / path
-        # Prevent path traversal
+        # Layer 1: reject symlinks before resolve()
+        symlink_err = _reject_symlinks(target, repo_root)
+        if symlink_err is not None:
+            return symlink_err
+        # Layer 2: resolve + is_relative_to (catches traversal via ..)
         try:
             target = target.resolve()
             if not target.is_relative_to(repo_root.resolve()):

src/grippy/github_review.py

@@ -160,6 +160,16 @@ def _sanitize_comment_text(text: str) -> str:
     # Strip markdown images (tracking pixels) and external links (phishing)
     text = re.sub(r"!\[[^\]]*\]\([^)]+\)", "", text)
     text = re.sub(r"\[([^\]]*)\]\(https?://[^)]+\)", r"\1", text)
+    # Strip reference-style link definitions: [id]: url (optional title)
+    text = re.sub(r"^\s{0,3}\[[^\]]+\]:\s+\S[^\n]*$", "", text, flags=re.MULTILINE)
+    # Strip reference-style link references: [text][id] and collapsed [text][]
+    text = re.sub(r"\[([^\]]*)\]\[[^\]]*\]", r"\1", text)
+    # Defang bare URL autolinks: <https://...> → hxxps://... (not clickable on GitHub)
+    text = re.sub(
+        r"<https?://[^>]+>",
+        lambda m: m.group(0)[1:-1].replace("https://", "hxxps://").replace("http://", "hxxp://"),
+        text,
+    )
     # Loop unquote until stable — prevents multi-layer URL encoding bypass
     # (e.g., %2561 -> %61 -> a) that could smuggle dangerous schemes past
     # the regex check.
@@ -174,7 +184,18 @@ def _sanitize_comment_text(text: str) -> str:
     # URL-encoded to bypass the earlier stripping pass.
     text = re.sub(r"!\[[^\]]*\]\([^)]+\)", "", text)
     text = re.sub(r"\[([^\]]*)\]\(https?://[^)]+\)", r"\1", text)
+    text = re.sub(r"^\s{0,3}\[[^\]]+\]:\s+\S[^\n]*$", "", text, flags=re.MULTILINE)
+    text = re.sub(r"\[([^\]]*)\]\[[^\]]*\]", r"\1", text)
+    # Defang autolinks after decode too
+    text = re.sub(
+        r"<https?://[^>]+>",
+        lambda m: m.group(0)[1:-1].replace("https://", "hxxps://").replace("http://", "hxxp://"),
+        text,
+    )
     text = _DANGEROUS_SCHEME_RE.sub("", text)
+    # Defang bare URLs that survived all prior stripping — GitHub auto-linkifies these
+    text = re.sub(r"https://", "hxxps://", text)
+    text = re.sub(r"http://", "hxxp://", text)
     return text
 
 

src/grippy/review.py

@@ -22,7 +22,9 @@
 
 import itertools
 import json
+import logging
 import os
+import subprocess
 import sys
 from collections import Counter
 from collections.abc import Callable
@@ -42,10 +44,35 @@
 from grippy.rules import RuleResult, RuleSeverity, check_gate, load_profile, run_rules
 from grippy.rules.enrichment import enrich_results, persist_rule_findings
 
+log = logging.getLogger(__name__)
+
 # Max diff size sent to the LLM — configurable for local models with smaller context
 MAX_DIFF_CHARS = int(os.environ.get("GRIPPY_MAX_DIFF_CHARS", "500000"))
 
 
+def _is_git_tracked(path: str) -> bool:
+    """Check if a file is tracked by git. Returns True if tracked.
+
+    Fail-open: returns False if git is unavailable or times out.
+    This means a tracked .dev.vars loads in that case — acceptable
+    trade-off for local dev usability. CI environments are excluded
+    by a separate check before this function is called.
+    """
+    try:
+        file_path = Path(path)
+        repo_dir = file_path.parent
+        # Use relative path — git ls-files expects paths relative to cwd/repo root
+        result = subprocess.run(
+            ["git", "ls-files", "--error-unmatch", file_path.name],
+            capture_output=True,
+            timeout=5,
+            cwd=str(repo_dir),
+        )
+        return result.returncode == 0
+    except (subprocess.TimeoutExpired, FileNotFoundError):
+        return False
+
+
 _ERROR_HINTS: dict[str, str] = {
     "CONFIG ERROR": "Valid `GRIPPY_TRANSPORT` values: `openai`, `anthropic`, `google`, `groq`, `mistral`, `local`.",
     "TIMEOUT": "Increase `GRIPPY_TIMEOUT` or reduce PR diff size.",
@@ -286,11 +313,17 @@ def main(*, profile: str | None = None) -> None:
     if not os.environ.get("CI"):
         dev_vars_path = Path(__file__).resolve().parent.parent.parent / ".dev.vars"
         if dev_vars_path.is_file():
-            for line in dev_vars_path.read_text().splitlines():
-                line = line.strip()
-                if line and not line.startswith("#") and "=" in line:
-                    key, _, value = line.partition("=")
-                    os.environ.setdefault(key.strip(), value.strip())
+            if _is_git_tracked(str(dev_vars_path)):
+                log.warning(
+                    ".dev.vars is tracked by git — refusing to load. "
+                    "Remove it from tracking: git rm --cached .dev.vars"
+                )
+            else:
+                for line in dev_vars_path.read_text().splitlines():
+                    line = line.strip()
+                    if line and not line.startswith("#") and "=" in line:
+                        key, _, value = line.partition("=")
+                        os.environ.setdefault(key.strip(), value.strip())
 
     # Required env
     token = os.environ.get("GITHUB_TOKEN", "")

tests/test_grippy_codebase.py

@@ -23,6 +23,7 @@
     _make_list_files,
     _make_read_file,
     _make_search_code,
+    _reject_symlinks,
     _write_manifest,
     chunk_file,
     sanitize_tool_hook,
@@ -1480,3 +1481,136 @@ def test_payload_non_dict_after_json_parse(self) -> None:
         row = {"payload": json.dumps([1, 2, 3])}  # list, not dict
         results = CodebaseIndex._parse_results_static([row])
         assert results == []
+
+
+# --- Symlink rejection tests ---
+
+
+class TestRejectSymlinks:
+    """_reject_symlinks prevents symlink-based path traversal before resolve()."""
+
+    def test_rejects_symlink_file_to_outside(self, tmp_path: Path) -> None:
+        """Symlink file pointing outside repo root is rejected."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        outside = tmp_path / "secret.txt"
+        outside.write_text("stolen")
+        link = repo / "evil.txt"
+        link.symlink_to(outside)
+
+        result = _reject_symlinks(repo / "evil.txt", repo)
+        assert result is not None
+        assert "symlink" in result.lower()
+
+    def test_rejects_symlink_directory_to_outside(self, tmp_path: Path) -> None:
+        """Symlink directory pointing outside repo root is rejected."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        outside_dir = tmp_path / "outside_dir"
+        outside_dir.mkdir()
+        (outside_dir / "secret.py").write_text("stolen")
+        link = repo / "evil_dir"
+        link.symlink_to(outside_dir)
+
+        result = _reject_symlinks(repo / "evil_dir", repo)
+        assert result is not None
+        assert "symlink" in result.lower()
+
+    def test_allows_regular_file(self, tmp_path: Path) -> None:
+        """Regular file inside repo returns None (no error)."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        (repo / "safe.py").write_text("safe")
+
+        result = _reject_symlinks(repo / "safe.py", repo)
+        assert result is None
+
+    def test_allows_nested_path_without_symlinks(self, tmp_path: Path) -> None:
+        """Nested path with no symlinks in any component returns None."""
+        repo = tmp_path / "repo"
+        (repo / "src" / "pkg").mkdir(parents=True)
+        (repo / "src" / "pkg" / "mod.py").write_text("code")
+
+        result = _reject_symlinks(repo / "src" / "pkg" / "mod.py", repo)
+        assert result is None
+
+    def test_rejects_symlink_in_intermediate_component(self, tmp_path: Path) -> None:
+        """Symlink in a middle path component is detected."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        outside_dir = tmp_path / "outside"
+        outside_dir.mkdir()
+        (outside_dir / "secret.py").write_text("stolen")
+        link = repo / "linked_dir"
+        link.symlink_to(outside_dir)
+
+        result = _reject_symlinks(repo / "linked_dir" / "secret.py", repo)
+        assert result is not None
+        assert "symlink" in result.lower()
+
+    def test_rejects_path_outside_repo_root(self, tmp_path: Path) -> None:
+        """Path that is not relative to repo root is rejected."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        outside = tmp_path / "other" / "file.py"
+
+        result = _reject_symlinks(outside, repo)
+        assert result is not None
+        assert "outside" in result.lower()
+
+
+class TestSymlinkDefenseInReadFile:
+    """read_file rejects symlinks via _reject_symlinks before resolve()."""
+
+    def test_read_file_rejects_symlink_to_outside(self, tmp_path: Path) -> None:
+        """read_file returns error for symlink pointing outside repo."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        (repo / "safe.py").write_text("safe")
+        outside = tmp_path / "secret.py"
+        outside.write_text("stolen data")
+        link = repo / "evil_link.py"
+        link.symlink_to(outside)
+
+        read_fn = _make_read_file(repo)
+        result = read_fn("evil_link.py")
+        assert "symlink" in result.lower() or "not allowed" in result.lower()
+
+    def test_read_file_allows_regular_file(self, tmp_path: Path) -> None:
+        """read_file works for normal files after symlink check."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        (repo / "safe.py").write_text("safe content here")
+
+        read_fn = _make_read_file(repo)
+        result = read_fn("safe.py")
+        assert "safe content here" in result
+
+
+class TestSymlinkDefenseInListFiles:
+    """list_files rejects symlinks via _reject_symlinks before resolve()."""
+
+    def test_list_files_rejects_symlink_directory(self, tmp_path: Path) -> None:
+        """list_files returns error for symlinked directory pointing outside."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        (repo / "safe.py").write_text("safe")
+        outside_dir = tmp_path / "outside"
+        outside_dir.mkdir()
+        (outside_dir / "secret.py").write_text("stolen")
+        link = repo / "evil_dir"
+        link.symlink_to(outside_dir)
+
+        list_fn = _make_list_files(repo)
+        result = list_fn("evil_dir")
+        assert "symlink" in result.lower() or "not allowed" in result.lower()
+
+    def test_list_files_allows_regular_directory(self, tmp_path: Path) -> None:
+        """list_files works for normal directories after symlink check."""
+        repo = tmp_path / "repo"
+        (repo / "src").mkdir(parents=True)
+        (repo / "src" / "main.py").write_text("code")
+
+        list_fn = _make_list_files(repo)
+        result = list_fn("src")
+        assert "main.py" in result