From 7bed5efe615a75b4a345b3f35f0ba15b53aee8a4 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Tue, 24 Feb 2026 22:05:44 -0500 Subject: [PATCH] feat(security): Enforce blacklist for discussion history files --- aggregate.py | 18 ++++++++++++++---- mcp_client.py | 25 +++++++++++++++++++++++-- tests/test_history_blacklist.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 tests/test_history_blacklist.py diff --git a/aggregate.py b/aggregate.py index a624ea6..294d978 100644 --- a/aggregate.py +++ b/aggregate.py @@ -37,14 +37,24 @@ def is_absolute_with_drive(entry: str) -> bool: def resolve_paths(base_dir: Path, entry: str) -> list[Path]: has_drive = is_absolute_with_drive(entry) is_wildcard = "*" in entry + + matches = [] if is_wildcard: root = Path(entry) if has_drive else base_dir / entry matches = [Path(p) for p in glob.glob(str(root), recursive=True) if Path(p).is_file()] - return sorted(matches) else: - if has_drive: - return [Path(entry)] - return [(base_dir / entry).resolve()] + p = Path(entry) if has_drive else (base_dir / entry).resolve() + matches = [p] + + # Blacklist filter + filtered = [] + for p in matches: + name = p.name.lower() + if name == "history.toml" or name.endswith("_history.toml"): + continue + filtered.append(p) + + return sorted(filtered) def build_discussion_section(history: list[str]) -> str: sections = [] diff --git a/mcp_client.py b/mcp_client.py index 713b94b..63c9c48 100644 --- a/mcp_client.py +++ b/mcp_client.py @@ -87,7 +87,14 @@ def _is_allowed(path: Path) -> bool: - it is contained within (or equal to) one of the _base_dirs All paths are resolved (follows symlinks) before comparison to prevent symlink-based path traversal. + + CRITICAL: Blacklisted files (history) are NEVER allowed. """ + # Blacklist check + name = path.name.lower() + if name == "history.toml" or name.endswith("_history.toml"): + return False + try: rp = path.resolve(strict=True) except (OSError, ValueError): @@ -153,11 +160,18 @@ def list_directory(path: str) -> str: try: entries = sorted(p.iterdir(), key=lambda e: (e.is_file(), e.name.lower())) lines = [f"Directory: {p}", ""] + count = 0 for entry in entries: + # Blacklist check + name = entry.name.lower() + if name == "history.toml" or name.endswith("_history.toml"): + continue + kind = "file" if entry.is_file() else "dir " size = f"{entry.stat().st_size:>10,} bytes" if entry.is_file() else "" lines.append(f" [{kind}] {entry.name:<40} {size}") - lines.append(f" ({len(entries)} entries)") + count += 1 + lines.append(f" ({count} entries)") return "\n".join(lines) except Exception as e: return f"ERROR listing '{path}': {e}" @@ -178,11 +192,18 @@ def search_files(path: str, pattern: str) -> str: if not matches: return f"No files matched '{pattern}' in {path}" lines = [f"Search '{pattern}' in {p}:", ""] + count = 0 for m in matches: + # Blacklist check + name = m.name.lower() + if name == "history.toml" or name.endswith("_history.toml"): + continue + rel = m.relative_to(p) kind = "file" if m.is_file() else "dir " lines.append(f" [{kind}] {rel}") - lines.append(f" ({len(matches)} match(es))") + count += 1 + lines.append(f" ({count} match(es))") return "\n".join(lines) except Exception as e: return f"ERROR searching '{path}': {e}" diff --git a/tests/test_history_blacklist.py b/tests/test_history_blacklist.py new file mode 100644 index 0000000..712637a --- /dev/null +++ b/tests/test_history_blacklist.py @@ -0,0 +1,32 @@ +import pytest +from pathlib import Path +import mcp_client +import aggregate + +def test_mcp_blacklist(tmp_path): + # Setup a "history" file + hist_file = tmp_path / "my_project_history.toml" + hist_file.write_text("secret history", encoding="utf-8") + + # Configure MCP client with the tmp_path as allowed + mcp_client.configure([{"path": str(hist_file)}], extra_base_dirs=[str(tmp_path)]) + + # Try to read it - should fail + result = mcp_client.read_file(str(hist_file)) + assert "ACCESS DENIED" in result or "BLACKLISTED" in result + + # Try to list it + result = mcp_client.list_directory(str(tmp_path)) + assert "my_project_history.toml" not in result + +def test_aggregate_blacklist(tmp_path): + # Setup a "history" file + hist_file = tmp_path / "my_project_history.toml" + hist_file.write_text("secret history", encoding="utf-8") + + # Try to resolve paths including the history file + paths = aggregate.resolve_paths(tmp_path, "*_history.toml") + assert hist_file not in paths + + paths = aggregate.resolve_paths(tmp_path, "*") + assert hist_file not in paths