fix: fail closed when no safe export destination exists; polish salvage edges
- _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 <name>.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.
This commit is contained in:
@@ -111,6 +111,5 @@ plans/
|
|||||||
/log.txt
|
/log.txt
|
||||||
/sqlite_leak_fix.png
|
/sqlite_leak_fix.png
|
||||||
/*.png.bak
|
/*.png.bak
|
||||||
/default.tar.gz
|
|
||||||
*.tar.gz
|
*.tar.gz
|
||||||
*.tgz
|
*.tgz
|
||||||
|
|||||||
@@ -2154,13 +2154,14 @@ def _profile_export_directory() -> Path:
|
|||||||
for candidate in candidates:
|
for candidate in candidates:
|
||||||
if not _inside_git_checkout(candidate):
|
if not _inside_git_checkout(candidate):
|
||||||
return candidate
|
return candidate
|
||||||
logger.warning(
|
# Fail closed: writing a secret-bearing archive into a source tree is the
|
||||||
"Every managed export destination resolves inside a Git checkout; "
|
# incident this helper exists to prevent (#92457). A warning on stderr
|
||||||
"falling back to %s. Pass an explicit output path to avoid staging "
|
# would not stop a scripted export from recreating it.
|
||||||
"profile archives in a source tree.",
|
raise ValueError(
|
||||||
export_dir,
|
"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:
|
def get_profile_export_path(name: str, *, timestamp: Optional[str] = None) -> Path:
|
||||||
|
|||||||
@@ -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("profile_name", help="Profile to export")
|
||||||
profile_export.add_argument(
|
profile_export.add_argument(
|
||||||
"-o", "--output", default=None, help="Output file (default: <name>.tar.gz)"
|
"-o", "--output", default=None,
|
||||||
|
help="Output file (default: a managed profile-exports/<name>-<timestamp>.tar.gz "
|
||||||
|
"under the default Hermes home)",
|
||||||
)
|
)
|
||||||
|
|
||||||
profile_import = profile_subparsers.add_parser(
|
profile_import = profile_subparsers.add_parser(
|
||||||
|
|||||||
@@ -172,3 +172,43 @@ async def test_profile_export_api_uses_the_shared_managed_destination(
|
|||||||
result = await router_mod.export_profile_endpoint("default", ProfileExport())
|
result = await router_mod.export_profile_endpoint("default", ProfileExport())
|
||||||
|
|
||||||
assert result == {"ok": True, "archive": str(managed)}
|
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")
|
||||||
|
|||||||
Reference in New Issue
Block a user