diff --git a/.dockerignore b/.dockerignore index 8b4cb63c32..9bf276a08f 100644 --- a/.dockerignore +++ b/.dockerignore @@ -107,9 +107,10 @@ plans/ .hadolint.yaml .mailmap -# Repo-root debug/export artifacts — must never reach image layers (COPY . .) +# Debug/export artifacts — must never reach image layers (COPY . .) /log.txt /sqlite_leak_fix.png /*.png.bak /default.tar.gz -/*.tar.gz +*.tar.gz +*.tgz diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index caf4e2d3cb..2f3798346b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -166,6 +166,11 @@ jobs: needs: detect uses: ./.github/workflows/infographic-check.yml + profile-artifact-check: + name: Profile artifact check + needs: detect + uses: ./.github/workflows/profile-artifact-check.yml + lockfile-diff: name: package-lock.json diff needs: detect @@ -230,6 +235,7 @@ jobs: - uv-lockfile - lockfile-diff - docker-lint + - profile-artifact-check - supply-chain - review-labels - osv-scanner diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index f245708486..7e6b916f2a 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -94,6 +94,9 @@ jobs: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Reject profile exports in the build context + run: python3 scripts/ci/check_profile_archive_boundary.py + # Retry once on transient Docker Hub / buildkit pull failures # (connection reset, auth token timeout, rate limiting). The action # generates a unique builder name per invocation so the retry doesn't @@ -206,6 +209,9 @@ jobs: - name: Checkout trusted source uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Reject profile exports in the build context + run: python3 scripts/ci/check_profile_archive_boundary.py + # Retry once on transient Docker Hub / buildkit pull failures. # See build job for rationale; same pattern. - name: Set up Docker Buildx diff --git a/.github/workflows/profile-artifact-check.yml b/.github/workflows/profile-artifact-check.yml new file mode 100644 index 0000000000..0e3ff286e9 --- /dev/null +++ b/.github/workflows/profile-artifact-check.yml @@ -0,0 +1,21 @@ +name: Profile Artifact Boundary + +# A reusable, unconditional guard for the incident class in #92457. Ignore +# files reduce accidental staging; this job is the enforcement boundary that +# still catches `git add -f` and generated files present during a build. + +on: + workflow_call: + +permissions: + contents: read + +jobs: + check-profile-artifacts: + name: Reject profile archives + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Reject profile exports in the checkout + run: python3 scripts/ci/check_profile_archive_boundary.py diff --git a/.gitignore b/.gitignore index f1d51492aa..2d5279bca4 100644 --- a/.gitignore +++ b/.gitignore @@ -99,11 +99,12 @@ apps/desktop/src/**/*.d.ts !apps/desktop/src/global.d.ts !apps/desktop/src/vite-env.d.ts -# Repo-root build/debug artifacts that must never be committed +# Build/debug artifacts that must never be committed /log.txt /sqlite_leak_fix.png /*.png.bak -/default.tar.gz +*.tar.gz +*.tgz apps/shared/src/**/*.js apps/shared/src/**/*.js.map apps/shared/src/**/*.d.ts diff --git a/hermes_cli/cli_commands_mixin.py b/hermes_cli/cli_commands_mixin.py index 31782deaca..a48f6e0e91 100644 --- a/hermes_cli/cli_commands_mixin.py +++ b/hermes_cli/cli_commands_mixin.py @@ -460,7 +460,11 @@ class CLICommandsMixin: /export — export a named profile /export [profile] -o — choose the output path """ - from hermes_cli.profiles import export_profile, get_active_profile_name + from hermes_cli.profiles import ( + export_profile, + get_active_profile_name, + get_profile_export_path, + ) parts = command.split()[1:] output = None @@ -474,7 +478,7 @@ class CLICommandsMixin: name = parts[0] if parts else (get_active_profile_name() or "default") if not output: - output = f"{name}.tar.gz" + output = str(get_profile_export_path(name)) try: result = export_profile(name, output) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 82e3933255..5a346222f2 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -11476,10 +11476,10 @@ def cmd_profile(args): sys.exit(1) elif action == "export": - from hermes_cli.profiles import export_profile + from hermes_cli.profiles import export_profile, get_profile_export_path name = args.profile_name - output = args.output or f"{name}.tar.gz" + output = args.output or str(get_profile_export_path(name)) try: result_path = export_profile(name, output) print(f"✓ Exported '{name}' to {result_path}") diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 91d997b8a5..574047965a 100644 --- a/hermes_cli/profiles.py +++ b/hermes_cli/profiles.py @@ -2115,6 +2115,63 @@ def get_active_profile_name() -> str: # Export / Import # --------------------------------------------------------------------------- +def _profile_export_directory() -> Path: + """Choose an export directory that cannot become source-tree input.""" + export_dir = _get_default_hermes_home() / "profile-exports" + try: + cwd = Path.cwd().resolve() + except OSError: + return export_dir + + checkout_root = next( + ( + candidate + for candidate in (cwd, *cwd.parents) + if (candidate / ".git").exists() + ), + None, + ) + if checkout_root is None: + return export_dir + + try: + export_dir.resolve().relative_to(checkout_root.resolve()) + except ValueError: + return export_dir + + # A custom deployment may point HERMES_HOME at its source checkout. Do + # not put the automatic archive under that tree; use a sibling store and + # fall back to the OS temp directory only for the unusual case where the + # user's home itself is the checkout. + candidates = [ + Path.home() / ".hermes-profile-exports", + ] + import tempfile + + candidates.append(Path(tempfile.gettempdir()) / "hermes-profile-exports") + for candidate in candidates: + try: + candidate.resolve().relative_to(checkout_root.resolve()) + except ValueError: + return candidate + return export_dir + + +def get_profile_export_path(name: str, *, timestamp: Optional[str] = None) -> Path: + """Return a managed destination for an export with no explicit output. + + Keep automatic exports outside the current working directory and outside + every named profile. The CLI is commonly run from a source checkout; its + old ``.tar.gz`` default therefore made a profile snapshot look like + a repository artifact and allowed it to be committed accidentally. + """ + canon = normalize_profile_name(name) + validate_profile_name(canon) + export_dir = _profile_export_directory() + export_dir.mkdir(parents=True, exist_ok=True) + stamp = timestamp or time.strftime("%Y%m%d-%H%M%S") + return export_dir / f"{canon}-{stamp}.tar.gz" + def _default_export_ignore(root_dir: Path): """Return an *ignore* callable for :func:`shutil.copytree`. diff --git a/hermes_cli/web_routers/profiles.py b/hermes_cli/web_routers/profiles.py index c0669510a4..0e08d63142 100644 --- a/hermes_cli/web_routers/profiles.py +++ b/hermes_cli/web_routers/profiles.py @@ -1173,14 +1173,12 @@ async def export_profile_endpoint(name: str, body: ProfileExport): output = (body.output or "").strip() if not output: - from hermes_constants import get_hermes_home - staging = get_hermes_home() / "profile-exports" try: - staging.mkdir(parents=True, exist_ok=True) + output = str(profiles_mod.get_profile_export_path(name)) + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) except OSError as exc: raise HTTPException(status_code=500, detail=f"Could not create export directory: {exc}") - stamp = time.strftime("%Y%m%d-%H%M%S") - output = str(staging / f"{profiles_mod.normalize_profile_name(name)}-{stamp}.tar.gz") loop = asyncio.get_running_loop() try: diff --git a/scripts/ci/check_profile_archive_boundary.py b/scripts/ci/check_profile_archive_boundary.py new file mode 100644 index 0000000000..2a8a198dd7 --- /dev/null +++ b/scripts/ci/check_profile_archive_boundary.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +"""Reject profile export archives before publication. + +``.gitignore`` and ``.dockerignore`` are useful first-line filters, but both +can be bypassed (for example with ``git add -f`` or a non-standard build +context). This check is the blocking, executable policy at the CI and image +publication boundaries. It intentionally checks the filesystem rather than +Git's index so a generated archive cannot enter a build after checkout. +""" + +from __future__ import annotations + +import argparse +import os +from pathlib import Path + +_PROFILE_ARCHIVE_SUFFIXES = (".tar.gz", ".tgz") + + +def find_forbidden_profile_archives(root: Path) -> list[Path]: + """Return profile archive paths anywhere in the checkout.""" + root = root.resolve() + if not root.is_dir(): + raise ValueError(f"repository root is not a directory: {root}") + + offenders: list[Path] = [] + for directory, dirnames, filenames in os.walk(root, followlinks=False): + dirnames[:] = [ + name + for name in dirnames + if name not in {".git", ".venv", "venv", "node_modules", "__pycache__"} + ] + for name in (*dirnames, *filenames): + if name.casefold().endswith(_PROFILE_ARCHIVE_SUFFIXES): + offenders.append((Path(directory) / name).relative_to(root)) + + return sorted(offenders, key=lambda path: path.as_posix().casefold()) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Reject profile export archives in the checkout." + ) + parser.add_argument( + "--root", + type=Path, + default=Path.cwd(), + help="repository root to inspect (default: current directory)", + ) + args = parser.parse_args(argv) + + try: + offenders = find_forbidden_profile_archives(args.root) + except ValueError as exc: + parser.error(str(exc)) + + if not offenders: + print("No profile export archives detected in the checkout.") + return 0 + + print( + "::error::profile export archives are forbidden " + "in source and Docker build contexts" + ) + for path in offenders: + print(f" {path.as_posix()}") + print( + "Move the archive outside the checkout or pass an explicit external " + "output path to the profile export command." + ) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/hermes_cli/test_profile_export_default_path.py b/tests/hermes_cli/test_profile_export_default_path.py new file mode 100644 index 0000000000..d06da31059 --- /dev/null +++ b/tests/hermes_cli/test_profile_export_default_path.py @@ -0,0 +1,122 @@ +"""Regression coverage for safe automatic profile-export destinations.""" + +from __future__ import annotations + +from argparse import Namespace +from pathlib import Path + +import pytest + +from hermes_cli import profiles +from hermes_cli.cli_commands_mixin import CLICommandsMixin +from hermes_cli.main import cmd_profile +from hermes_cli.profiles import get_profile_export_path + + +def test_default_export_path_is_managed_and_outside_named_profiles( + tmp_path, monkeypatch +): + default_home = tmp_path / ".hermes" + default_home.mkdir() + monkeypatch.setattr( + "hermes_cli.profiles._get_default_hermes_home", lambda: default_home + ) + + result = get_profile_export_path("Research-Bot", timestamp="20260823-120000") + + assert ( + result + == default_home / "profile-exports" / "research-bot-20260823-120000.tar.gz" + ) + assert result.parent.is_dir() + assert not (default_home / "profiles" / "research-bot" / result.name).exists() + + +def test_custom_hermes_home_inside_a_checkout_uses_a_sibling_store( + tmp_path, monkeypatch +): + checkout = tmp_path / "checkout" + checkout.mkdir() + (checkout / ".git").write_text("gitdir: ../git\n", encoding="utf-8") + monkeypatch.chdir(checkout) + monkeypatch.setattr(Path, "home", lambda: tmp_path / "home") + monkeypatch.setattr( + "hermes_cli.profiles._get_default_hermes_home", lambda: checkout + ) + + result = get_profile_export_path("default", timestamp="20260823-120000") + + assert not result.resolve().is_relative_to(checkout.resolve()) + + +def test_cli_export_default_does_not_write_into_the_current_checkout( + tmp_path, monkeypatch, capsys +): + default_home = tmp_path / ".hermes" + default_home.mkdir() + (default_home / "config.yaml").write_text("model: test\n", encoding="utf-8") + checkout = tmp_path / "checkout" + checkout.mkdir() + monkeypatch.chdir(checkout) + monkeypatch.setattr( + "hermes_cli.profiles._get_default_hermes_home", lambda: default_home + ) + monkeypatch.setattr( + "hermes_constants.get_default_hermes_root", lambda: default_home + ) + + cmd_profile( + Namespace( + profile_action="export", + profile_name="default", + output=None, + ) + ) + + exported = list((default_home / "profile-exports").glob("default-*.tar.gz")) + assert len(exported) == 1 + assert exported[0].parent == default_home / "profile-exports" + assert not (checkout / "default.tar.gz").exists() + assert str(exported[0]) in capsys.readouterr().out + + +def test_slash_export_uses_the_same_managed_destination(tmp_path, monkeypatch): + default_home = tmp_path / ".hermes" + default_home.mkdir() + monkeypatch.setattr( + "hermes_cli.profiles._get_default_hermes_home", lambda: default_home + ) + monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default") + calls = [] + monkeypatch.setattr( + profiles, + "export_profile", + lambda name, output: calls.append((name, output)) or output, + ) + + CLICommandsMixin()._handle_export_command("/export") + + assert len(calls) == 1 + assert calls[0][0] == "default" + assert Path(calls[0][1]).parent == default_home / "profile-exports" + assert not (tmp_path / "default.tar.gz").exists() + + +@pytest.mark.asyncio +async def test_profile_export_api_uses_the_shared_managed_destination( + tmp_path, monkeypatch +): + from hermes_cli.web_models import ProfileExport + from hermes_cli.web_routers.profiles import export_profile_endpoint + + managed = tmp_path / "profile-exports" / "default-20260823-120000.tar.gz" + monkeypatch.setattr(profiles, "get_profile_export_path", lambda name: managed) + monkeypatch.setattr( + profiles, + "export_profile", + lambda name, output, extra_files=None: output, + ) + + result = await export_profile_endpoint("default", ProfileExport()) + + assert result == {"ok": True, "archive": str(managed)} diff --git a/tests/scripts/test_check_profile_archive_boundary.py b/tests/scripts/test_check_profile_archive_boundary.py new file mode 100644 index 0000000000..d0e4acd5ba --- /dev/null +++ b/tests/scripts/test_check_profile_archive_boundary.py @@ -0,0 +1,57 @@ +"""Behavioral tests for the profile archive CI guard.""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + + +SCRIPT = ( + Path(__file__).resolve().parents[2] + / "scripts" + / "ci" + / "check_profile_archive_boundary.py" +) + + +def _run(root: Path) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(SCRIPT), "--root", str(root)], + capture_output=True, + text=True, + check=False, + ) + + +def test_clean_root_passes(tmp_path): + result = _run(tmp_path) + + assert result.returncode == 0 + assert "No profile export archives" in result.stdout + + +def test_root_profile_archive_fails_without_printing_contents(tmp_path): + default_archive = tmp_path / "default.tar.gz" + alternate_archive = tmp_path / "backup.TGZ" + default_archive.write_bytes(b"profile webhook secret must never be printed") + alternate_archive.write_bytes(b"another archive") + + result = _run(tmp_path) + + assert result.returncode == 1 + assert "default.tar.gz" in result.stdout + assert "backup.TGZ" in result.stdout + assert "profile webhook secret" not in result.stdout + assert "another archive" not in result.stdout + + +def test_nested_profile_archive_is_also_rejected(tmp_path): + nested = tmp_path / "fixtures" + nested.mkdir() + (nested / "fixture.tar.gz").write_bytes(b"test fixture") + + result = _run(tmp_path) + + assert result.returncode == 1 + assert "fixtures/fixture.tar.gz" in result.stdout diff --git a/website/docs/user-guide/profile-distributions.md b/website/docs/user-guide/profile-distributions.md index 46f0660210..4cf8a295b0 100644 --- a/website/docs/user-guide/profile-distributions.md +++ b/website/docs/user-guide/profile-distributions.md @@ -624,11 +624,18 @@ When you don't need versioning, skip the repo. `/export` packs a profile into a In the CLI, TUI, or desktop chat: ``` -/export # the active profile → .tar.gz +/export # the active profile → managed profile-exports/-.tar.gz /export research-bot # a named profile /export research-bot -o ~/Desktop/research-bot.tar.gz ``` +Without `-o`, the CLI and TUI place the archive in Hermes's managed +`profile-exports/` directory under the default Hermes home, not in the current +working directory. This keeps routine exports out of source checkouts and +prevents a generated profile snapshot from being mistaken for a repository +source file. An explicit `-o` path is still honored when you intentionally +choose where to save the archive. + Or from a shell, same machinery: ```bash