From 525dd12da7729bd03d131edaa0c2a7a45caeba52 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 23 Aug 2026 12:53:21 +0530 Subject: [PATCH] fix: fail closed when no safe export destination exists; polish salvage edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _profile_export_directory(): when the managed store, the home-sibling store, AND the temp dir all resolve inside Git checkouts, raise a clear ValueError instead of warning and proceeding — a stderr warning would not stop a scripted export from staging a secret-bearing archive in a source tree, which is the exact #92457 incident class. All three callers already surface ValueError cleanly (CLI/TUI print Error: + exit, API returns 400). - .dockerignore: drop the /default.tar.gz line made redundant by the global *.tar.gz pattern this PR adds. - hermes profile export -o help text: stop advertising the old .tar.gz cwd default. - Tests: cwd-in-unrelated-checkout topology (the second production shape from the blocking review) and the fail-closed path. Mutation-checked: both fail on the pre-fix helper. --- .dockerignore | 1 - hermes_cli/profiles.py | 13 +++--- hermes_cli/subcommands/profile.py | 4 +- .../test_profile_export_default_path.py | 40 +++++++++++++++++++ 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/.dockerignore b/.dockerignore index 9bf276a08f..ae00a18bb9 100644 --- a/.dockerignore +++ b/.dockerignore @@ -111,6 +111,5 @@ plans/ /log.txt /sqlite_leak_fix.png /*.png.bak -/default.tar.gz *.tar.gz *.tgz diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 5f06358d33..9f681037f2 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -2154,13 +2154,14 @@ def _profile_export_directory() -> Path: for candidate in candidates: if not _inside_git_checkout(candidate): return candidate - logger.warning( - "Every managed export destination resolves inside a Git checkout; " - "falling back to %s. Pass an explicit output path to avoid staging " - "profile archives in a source tree.", - export_dir, + # Fail closed: writing a secret-bearing archive into a source tree is the + # incident this helper exists to prevent (#92457). A warning on stderr + # would not stop a scripted export from recreating it. + raise ValueError( + "No safe automatic export destination: every candidate directory is " + "inside a Git checkout. Pass an explicit output path outside the " + "checkout (e.g. -o /path/outside/repo/profile.tar.gz)." ) - return export_dir def get_profile_export_path(name: str, *, timestamp: Optional[str] = None) -> Path: diff --git a/hermes_cli/subcommands/profile.py b/hermes_cli/subcommands/profile.py index abcc53cd41..ee5e361a57 100644 --- a/hermes_cli/subcommands/profile.py +++ b/hermes_cli/subcommands/profile.py @@ -135,7 +135,9 @@ def build_profile_parser(subparsers, *, cmd_profile: Callable) -> None: ) profile_export.add_argument("profile_name", help="Profile to export") profile_export.add_argument( - "-o", "--output", default=None, help="Output file (default: .tar.gz)" + "-o", "--output", default=None, + help="Output file (default: a managed profile-exports/-.tar.gz " + "under the default Hermes home)", ) profile_import = profile_subparsers.add_parser( diff --git a/tests/hermes_cli/test_profile_export_default_path.py b/tests/hermes_cli/test_profile_export_default_path.py index baf09c5489..af60becc1d 100644 --- a/tests/hermes_cli/test_profile_export_default_path.py +++ b/tests/hermes_cli/test_profile_export_default_path.py @@ -172,3 +172,43 @@ async def test_profile_export_api_uses_the_shared_managed_destination( result = await router_mod.export_profile_endpoint("default", ProfileExport()) assert result == {"ok": True, "archive": str(managed)} + + +def test_cwd_in_unrelated_checkout_does_not_prove_safety( + tmp_path, monkeypatch, profiles +): + """cwd inside unrelated checkout A must not stand in for the safety proof + of a HERMES_HOME inside checkout B.""" + checkout_b = tmp_path / "checkout-b" + checkout_b.mkdir() + (checkout_b / ".git").mkdir() + checkout_a = tmp_path / "checkout-a" + checkout_a.mkdir() + (checkout_a / ".git").write_text("gitdir: ../git\n", encoding="utf-8") + monkeypatch.chdir(checkout_a) + monkeypatch.setattr(Path, "home", lambda: tmp_path / "home") + monkeypatch.setattr(profiles, "_get_default_hermes_home", lambda: checkout_b) + + result = profiles.get_profile_export_path("default", timestamp="20260823-120000") + + assert not result.resolve().is_relative_to(checkout_b.resolve()) + assert not result.resolve().is_relative_to(checkout_a.resolve()) + + +def test_every_candidate_inside_a_checkout_fails_closed( + tmp_path, monkeypatch, profiles +): + """When home, sibling store, and tempdir all resolve inside checkouts the + helper must refuse — a warning would not stop a scripted export from + recreating the #92457 incident artifact.""" + import tempfile + + checkout = tmp_path / "checkout" + checkout.mkdir() + (checkout / ".git").mkdir() + monkeypatch.setattr(Path, "home", lambda: checkout / "home") + monkeypatch.setattr(profiles, "_get_default_hermes_home", lambda: checkout) + monkeypatch.setattr(tempfile, "gettempdir", lambda: str(checkout / "tmp")) + + with pytest.raises(ValueError, match="No safe automatic export destination"): + profiles.get_profile_export_path("default")