fix: get_file_context honours the ignore contract (#147)

Author: nikolay-e

src/diffctx/mcp/server.py

@@ -313,7 +313,10 @@ def _build() -> str:
     "Examples:\n"
     '- patterns=["src/**/*.py"] — all Python files\n'
     '- patterns=["eval/*.py", "tests/conftest.py"] — specific sets\n'
-    '- patterns=["*.md"] with dry_run=true — preview what matches' + _UNTRUSTED_NOTICE
+    '- patterns=["*.md"] with dry_run=true — preview what matches\n\n'
+    "Honours .gitignore and .diffctx/ignore: excluded files are never returned "
+    "and are not reported as truncated, so a glob cannot read what the repo "
+    "withholds from the other tools." + _UNTRUSTED_NOTICE
 )
 
 
@@ -325,8 +328,21 @@ def _is_contained(child: Path, root: Path) -> bool:
 
 
 def _collect_matched_files(validated_path: Path, patterns: list[str], max_files: int) -> tuple[list[Path], int]:
+    """Files matching `patterns`, minus anything the repo declared off-limits.
+
+    The ignore specs are applied here for the same reason `get_tree_map` applies
+    them: `.diffctx/ignore` is a security contract, and this tool accepts
+    `**/*`. Without the specs the two MCP tools disagreed about the same repo —
+    a key or an env file that diff mode and the tree map both withhold was
+    readable through an explicit glob, which makes the contract advisory rather
+    than a contract.
+    """
     import glob as globmod
 
+    from diffctx.ignore import get_ignore_specs
+
+    spec = get_ignore_specs(validated_path, None, False, None)
+
     matched: list[Path] = []
     seen: set[Path] = set()
     total_matched = 0
@@ -336,6 +352,15 @@ def _collect_matched_files(validated_path: Path, patterns: list[str], max_files:
             p = Path(match)
             if not p.is_file() or not _is_contained(p, validated_path):
                 continue
+            try:
+                rel = p.resolve().relative_to(validated_path.resolve()).as_posix()
+            except ValueError:
+                continue
+            if spec.match_file(rel):
+                # Excluded silently and NOT counted: `total_matched` drives the
+                # truncation notice, and reporting "3 more files" for files the
+                # repo refuses to expose would leak their existence.
+                continue
             resolved = p.resolve()
             if resolved in seen:
                 continue

tests/test_mcp.py

@@ -770,3 +770,49 @@ async def test_ordinary_range_is_unaffected(self, server, mcp_repo):
             {"repo_path": str(mcp_repo.path), "diff_range": "HEAD~1..HEAD"},
         )
         assert "calc.py" in _get_text(result)
+
+
+class TestFileContextHonoursTheIgnoreContract:
+    """`get_file_context` accepts `**/*`, so it is the widest read surface the
+    MCP server exposes. It used to glob without the ignore specs that
+    `get_tree_map` and diff mode both apply, which made `.diffctx/ignore` — a
+    security contract per QA.md — advisory for anyone who asked directly."""
+
+    @staticmethod
+    def _repo(tmp_path):
+        root = tmp_path / "ignored_repo"
+        (root / ".diffctx").mkdir(parents=True)
+        (root / ".diffctx" / "ignore").write_text("secrets/\n*.key\n")
+        (root / "secrets").mkdir()
+        (root / "secrets" / "prod.env").write_text("API_TOKEN=must-not-surface\n")
+        (root / "deploy.key").write_text("PRIVATE-KEY-must-not-surface\n")
+        (root / "app.py").write_text("def run():\n    return 1\n")
+        return root
+
+    @pytest.mark.asyncio
+    async def test_a_declared_exclusion_is_not_readable_through_a_glob(self, server, tmp_path):
+        root = self._repo(tmp_path)
+        result = await server.call_tool(
+            "get_file_context",
+            {"repo_path": str(root), "patterns": ["**/*"]},
+        )
+        text = _get_text(result)
+
+        assert "must-not-surface" not in text, "an ignored file's contents were returned"
+        assert "prod.env" not in text
+        assert "deploy.key" not in text
+        assert "app.py" in text, "the fix must not stop ordinary files being read"
+
+    @pytest.mark.asyncio
+    async def test_an_excluded_file_is_not_counted_in_the_truncation_notice(self, server, tmp_path):
+        """Reporting "N more files" for withheld files would leak that they
+        exist, which is most of what the exclusion is protecting."""
+        root = self._repo(tmp_path)
+        result = await server.call_tool(
+            "get_file_context",
+            {"repo_path": str(root), "patterns": ["**/*"], "dry_run": True},
+        )
+        text = _get_text(result)
+
+        assert "secrets" not in text
+        assert "deploy.key" not in text