From 456e974ecf4c7cdfbd9166977ed283c495940524 Mon Sep 17 00:00:00 2001 From: Santhi Prakash Date: Sat, 25 Jul 2026 06:40:49 +0000 Subject: [PATCH 1/2] fix(brainstorming): prevent path traversal in resolve_detail() - Problem: resolve_detail() joined an unvalidated detail path onto csv_dir and read the result with no containment check, allowing --extra overlays (or a crafted CSV) to read arbitrary files (e.g. ../.env, /etc/passwd) into the LLM context during a normal bmad-brainstorming session. - Fix: resolve both the base directory and the target path, then reject the read if the resolved path escapes the base via is_relative_to(). This also closes the symlink-escape variant. - Verification: all 31 pytest tests pass, including 2 new traversal tests (relative and absolute). Full npm test suite also passes. --- .../bmad-brainstorming/scripts/brain.py | 6 +++++- .../scripts/tests/test_brain.py | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/core-skills/bmad-brainstorming/scripts/brain.py b/src/core-skills/bmad-brainstorming/scripts/brain.py index 1028cf6edb..b559535f1d 100644 --- a/src/core-skills/bmad-brainstorming/scripts/brain.py +++ b/src/core-skills/bmad-brainstorming/scripts/brain.py @@ -130,7 +130,11 @@ def resolve_detail(row: dict, csv_dir: Path) -> str | None: (or the file is missing — a missing file is reported to stderr, not fatal).""" if not row.get("detail"): return None - path = (csv_dir / row["detail"]).resolve() + base = csv_dir.resolve() + path = (base / row["detail"]).resolve() + if not path.is_relative_to(base): + print(f"# detail path escapes the catalog directory for {row['technique_name']}: {row['detail']}", file=sys.stderr) + return None if not path.is_file(): print(f"# detail file not found for {row['technique_name']}: {row['detail']}", file=sys.stderr) return None diff --git a/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py b/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py index 329a6a5f3c..0580153701 100644 --- a/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py +++ b/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py @@ -74,6 +74,27 @@ def test_resolve_detail_missing_file_warns_not_fatal(lib, capsys): assert "not found" in capsys.readouterr().err +def test_resolve_detail_refuses_path_traversal(lib, tmp_path, capsys): + """Paths that escape the catalog directory must be refused, not read.""" + rows = brain.load(lib) + secret = tmp_path.parent / ".env" + secret.write_text("SECRET=hunter2", encoding="utf-8") + # relative traversal + rows[1]["detail"] = "../.env" + assert brain.resolve_detail(rows[1], lib.parent) is None + assert "escapes" in capsys.readouterr().err + # absolute path (pathlib '/' lets the right operand win) + rows[1]["detail"] = str(secret.resolve()) + assert brain.resolve_detail(rows[1], lib.parent) is None + assert "escapes" in capsys.readouterr().err + + +def test_resolve_detail_still_reads_valid_detail(lib): + """A legitimate detail file inside the catalog directory still resolves.""" + row = next(r for r in brain.load(lib) if r["detail"]) + assert "multi-step instructions" in brain.resolve_detail(row, lib.parent) + + def test_show_inlines_detail(lib, capsys): assert brain.main(["--file", str(lib), "show", "Quantum Superposition"]) == 0 out = capsys.readouterr().out From f4d1c3bfdd5509588250bc227fa67fa353bc3e74 Mon Sep 17 00:00:00 2001 From: Santhi Prakash Date: Sat, 25 Jul 2026 14:41:07 +0000 Subject: [PATCH 2/2] test(brainstorming): add symlink escape regression test in resolve_detail - Problem: CodeRabbit review flagged that resolve_detail() claims symlink protection but the test only covers relative traversal and absolute paths. - Fix: Add in-catalog symlink test case that creates a symlink inside the catalog directory pointing to a file outside it, then asserts resolve_detail() returns None and stderr contains 'escapes'. - Verification: all 31 pytest tests pass. Co-Authored-By: Paperclip --- .../bmad-brainstorming/scripts/tests/test_brain.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py b/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py index 0580153701..549def26b6 100644 --- a/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py +++ b/src/core-skills/bmad-brainstorming/scripts/tests/test_brain.py @@ -87,6 +87,12 @@ def test_resolve_detail_refuses_path_traversal(lib, tmp_path, capsys): rows[1]["detail"] = str(secret.resolve()) assert brain.resolve_detail(rows[1], lib.parent) is None assert "escapes" in capsys.readouterr().err + # symlink inside catalog pointing outside + link = lib.parent / "techniques" / "escape.md" + link.symlink_to(secret) + rows[1]["detail"] = "techniques/escape.md" + assert brain.resolve_detail(rows[1], lib.parent) is None + assert "escapes" in capsys.readouterr().err def test_resolve_detail_still_reads_valid_detail(lib):