From e830c85adfc39c5526d3c896598c428caa3067eb Mon Sep 17 00:00:00 2001 From: Aling33 <152494711+Aling33@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:26:55 +0800 Subject: [PATCH] fix(scripts): reject a directory passed to support_bundle --out (#7256) * fix(scripts): reject a directory passed to support_bundle --out `--out` names the zip, but the path went straight to `zipfile.ZipFile`, so pointing it at an existing directory ended in a raw `PermissionError` traceback (Windows) instead of the CLI's usual `error: ...` + exit 2. `create_support_bundle` now raises the `ValueError` the CLI already maps to a clean message, and the regression test pins exit 2 with no traceback. * docs(scripts): say "zip file path" for support_bundle --out Review nit: the help text said only "Output zip path", so the file-not-directory contract the new validation enforces was invisible until a run failed. --------- Co-authored-by: Willem Jiang --- backend/tests/test_support_bundle.py | 14 ++++++++++++++ scripts/support_bundle.py | 6 +++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/backend/tests/test_support_bundle.py b/backend/tests/test_support_bundle.py index fa5f3240ef..a107b009c1 100644 --- a/backend/tests/test_support_bundle.py +++ b/backend/tests/test_support_bundle.py @@ -902,6 +902,20 @@ def test_main_reports_invalid_thread_id_without_traceback(tmp_path, capsys): assert "Traceback" not in captured.err +def test_main_reports_directory_out_without_traceback(tmp_path, capsys): + project_root = tmp_path / "project" + project_root.mkdir() + out_dir = tmp_path / "bundles" + out_dir.mkdir() + + exit_code = support_bundle.main(["--project-root", str(project_root), "--out", str(out_dir)]) + + captured = capsys.readouterr() + assert exit_code == 2 + assert "--out must be a file path, not a directory" in captured.err + assert "Traceback" not in captured.err + + def test_main_prints_reporter_next_steps_and_optional_upload(tmp_path, capsys): project_root = tmp_path / "project" project_root.mkdir() diff --git a/scripts/support_bundle.py b/scripts/support_bundle.py index fdb3461fb9..5e3bdeadd3 100644 --- a/scripts/support_bundle.py +++ b/scripts/support_bundle.py @@ -899,6 +899,10 @@ def create_support_bundle( config_path = (config_path or project_root / "config.yaml").resolve() extensions_config_path = (extensions_config_path or project_root / "extensions_config.json").resolve() out_path = (out_path or _default_out_path(project_root)).resolve() + # `--out` names the zip, so a directory there would only fail inside + # zipfile with a raw PermissionError/IsADirectoryError. + if out_path.is_dir(): + raise ValueError(f"--out must be a file path, not a directory: {out_path}") out_path.parent.mkdir(parents=True, exist_ok=True) if thread_id: _validate_thread_id(thread_id) @@ -964,7 +968,7 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: parser.add_argument("--config", type=Path, default=None, help="Path to config.yaml") parser.add_argument("--extensions-config", type=Path, default=None, help="Path to extensions_config.json") parser.add_argument("--thread-id", default=None, help="Optional thread id to include file manifests for") - parser.add_argument("--out", type=Path, default=None, help="Output zip path") + parser.add_argument("--out", type=Path, default=None, help="Output zip file path") parser.add_argument("--include-doctor", action="store_true", help="Include redacted make doctor output") return parser.parse_args(argv)