fix(security): keep profile exports out of source and image contexts
Route automatic profile exports to a managed store instead of the current checkout, and enforce a CI/Docker boundary that rejects archive files before they can be published.
This commit is contained in:
+3
-2
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
+3
-2
@@ -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
|
||||
|
||||
@@ -460,7 +460,11 @@ class CLICommandsMixin:
|
||||
/export <profile> — export a named profile
|
||||
/export [profile] -o <path> — 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)
|
||||
|
||||
+2
-2
@@ -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}")
|
||||
|
||||
@@ -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 ``<name>.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`.
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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())
|
||||
@@ -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)}
|
||||
@@ -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
|
||||
@@ -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 → <name>.tar.gz
|
||||
/export # the active profile → managed profile-exports/<name>-<timestamp>.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
|
||||
|
||||
Reference in New Issue
Block a user