fix(drone): external projects can resolve AIPass branches (#618)
resolve_branch() validated branch path containment against the primary registry root even when the branch was found via the AIPASS_HOME fallback, so external projects (Vera, Daemon) were blocked from calling @api and any other AIPass branch with 'path escapes project root'. Add get_branch_with_registry() (non-breaking sibling to get_branch_by_name) that returns the branch plus the registry it was found in. resolve_branch() now validates containment against that registry's root. Security preserved: each branch stays contained within its own declaring registry; genuine escapes still blocked. 4 new cross-project resolver tests, 58 resolver tests pass, drone suite 702 pass, seedgo @drone 99%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
cc449c4a18
commit
bedf58e7b5
@@ -12,6 +12,15 @@ and this project uses [Calendar Versioning](https://calver.org/) in the format
|
||||
|
||||
### Fixed
|
||||
|
||||
- **External projects can call AIPass branches via drone** — `drone @api ...`
|
||||
(and any `drone @X`) now resolves from a non-AIPass project CWD instead of
|
||||
being blocked with "path escapes project root." The resolver was validating a
|
||||
branch's path against the *primary* registry root even when the branch was
|
||||
found via the `AIPASS_HOME` fallback, so any external project (Vera Studio,
|
||||
Daemon) hit a false security block. `resolve_branch()` now validates
|
||||
containment against the registry the branch was actually found in. Security is
|
||||
unchanged — each branch is still contained within its own declaring registry's
|
||||
root; genuine path escapes remain blocked. (#618)
|
||||
- **`aipass <command>` runs instead of printing an introspection banner** —
|
||||
`aipass` is a user-facing binary, so `aipass doctor` (and every other command)
|
||||
must execute, not describe itself. All 7 modules (`doctor`, `doctor_fix`,
|
||||
|
||||
@@ -396,3 +396,35 @@ def get_branch_by_name(name: str) -> Optional[Dict[str, Any]]:
|
||||
logger.warning("get_branch_by_name: AIPass home registry unavailable for '%s': %s", name, exc)
|
||||
|
||||
return None
|
||||
|
||||
|
||||
def get_branch_with_registry(name: str) -> Optional[tuple]:
|
||||
"""Get a branch and the registry path it was found in.
|
||||
|
||||
Same two-step lookup as get_branch_by_name (primary then AIPASS_HOME),
|
||||
but returns (branch_dict, registry_path) so callers can determine
|
||||
which project root the branch belongs to.
|
||||
"""
|
||||
lower_name = name.lower()
|
||||
|
||||
try:
|
||||
primary_path = get_registry_path()
|
||||
registry = load_registry()
|
||||
branch = registry.get("branches", {}).get(lower_name)
|
||||
if branch is not None:
|
||||
return branch, primary_path
|
||||
except (RegistryNotFoundError, RegistryCorruptError, RegistryPermissionError) as exc:
|
||||
logger.warning("get_branch_with_registry: primary registry unavailable for '%s': %s", name, exc)
|
||||
primary_path = None
|
||||
|
||||
home_path = _get_aipass_home_registry_path()
|
||||
if home_path is not None and home_path != primary_path:
|
||||
try:
|
||||
home_data = _load_registry_data(home_path)
|
||||
branch = home_data.get("branches", {}).get(lower_name)
|
||||
if branch is not None:
|
||||
return branch, home_path
|
||||
except (RegistryNotFoundError, RegistryCorruptError, RegistryPermissionError) as exc:
|
||||
logger.warning("get_branch_with_registry: AIPass home registry unavailable for '%s': %s", name, exc)
|
||||
|
||||
return None
|
||||
|
||||
@@ -25,7 +25,7 @@ from aipass.drone.apps.handlers.registry_handler import (
|
||||
load_registry,
|
||||
get_all_branches,
|
||||
get_branch_by_name,
|
||||
get_registry_path,
|
||||
get_branch_with_registry,
|
||||
_validate_branch_path,
|
||||
)
|
||||
|
||||
@@ -156,13 +156,14 @@ def resolve_branch(symbolic_name: str) -> str:
|
||||
raise BranchNotFoundError(f"Branch name must use @ prefix: '@{symbolic_name}' (got '{symbolic_name}')")
|
||||
|
||||
name = normalize_branch_name(symbolic_name).lower()
|
||||
branch = get_branch_by_name(name)
|
||||
result = get_branch_with_registry(name)
|
||||
|
||||
if branch is None:
|
||||
if result is None:
|
||||
raise BranchNotFoundError(f"Branch '{symbolic_name}' not found in registry")
|
||||
|
||||
branch, source_registry = result
|
||||
branch_path = Path(branch["path"])
|
||||
project_root = get_registry_path().parent
|
||||
project_root = source_registry.parent
|
||||
if not branch_path.is_absolute():
|
||||
branch_path = project_root / branch_path
|
||||
if not _validate_branch_path(branch_path, project_root, name):
|
||||
|
||||
@@ -447,3 +447,95 @@ class TestResolverPathContainment:
|
||||
assert "legit" in result
|
||||
finally:
|
||||
reset_registry_path()
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Cross-project resolution (issue #618)
|
||||
# ===========================================================================
|
||||
|
||||
|
||||
class TestCrossProjectResolution:
|
||||
"""resolve_branch() uses the source registry root for containment, not always the primary."""
|
||||
|
||||
def test_cross_project_resolves_via_aipass_home(self, tmp_path: Path, monkeypatch):
|
||||
"""Branch found via AIPASS_HOME resolves even when primary registry is a different project."""
|
||||
# External project with its own registry
|
||||
ext_project = tmp_path / "external_project"
|
||||
ext_project.mkdir()
|
||||
ext_reg = ext_project / "EXT_REGISTRY.json"
|
||||
_write_registry(ext_reg, [_make_branch("local_branch", "src/local_branch")])
|
||||
|
||||
# AIPass project with @target branch
|
||||
aipass_root = tmp_path / "aipass"
|
||||
target_dir = aipass_root / "src" / "aipass" / "target"
|
||||
target_dir.mkdir(parents=True)
|
||||
aipass_reg = aipass_root / "AIPASS_REGISTRY.json"
|
||||
_write_registry(
|
||||
aipass_reg,
|
||||
[_make_branch("target", str(target_dir))],
|
||||
)
|
||||
|
||||
# Primary registry = external project, AIPASS_HOME = aipass root
|
||||
set_registry_path(ext_reg)
|
||||
monkeypatch.setenv("AIPASS_HOME", str(aipass_root))
|
||||
try:
|
||||
result = resolve_branch("@target")
|
||||
assert str(target_dir) in result
|
||||
finally:
|
||||
reset_registry_path()
|
||||
|
||||
def test_genuine_escape_still_blocked(self, tmp_path: Path, monkeypatch):
|
||||
"""Branch whose path escapes its OWN declaring registry root is still blocked."""
|
||||
ext_project = tmp_path / "external_project"
|
||||
ext_project.mkdir()
|
||||
ext_reg = ext_project / "EXT_REGISTRY.json"
|
||||
_write_registry(ext_reg, [])
|
||||
|
||||
aipass_root = tmp_path / "aipass"
|
||||
aipass_root.mkdir()
|
||||
aipass_reg = aipass_root / "AIPASS_REGISTRY.json"
|
||||
_write_registry(
|
||||
aipass_reg,
|
||||
[_make_branch("evil", "../../../tmp/escape")],
|
||||
)
|
||||
|
||||
set_registry_path(ext_reg)
|
||||
monkeypatch.setenv("AIPASS_HOME", str(aipass_root))
|
||||
try:
|
||||
with pytest.raises(BranchNotFoundError):
|
||||
resolve_branch("@evil")
|
||||
finally:
|
||||
reset_registry_path()
|
||||
|
||||
def test_same_project_regression(self, tmp_path: Path, monkeypatch):
|
||||
"""Same-project resolution still works (no regression)."""
|
||||
branch_dir = tmp_path / "src" / "aipass" / "mybranch"
|
||||
branch_dir.mkdir(parents=True)
|
||||
reg_file = tmp_path / "AIPASS_REGISTRY.json"
|
||||
_write_registry(
|
||||
reg_file,
|
||||
[_make_branch("mybranch", "src/aipass/mybranch")],
|
||||
)
|
||||
|
||||
set_registry_path(reg_file)
|
||||
monkeypatch.delenv("AIPASS_HOME", raising=False)
|
||||
try:
|
||||
result = resolve_branch("@mybranch")
|
||||
assert "mybranch" in result
|
||||
finally:
|
||||
reset_registry_path()
|
||||
|
||||
def test_branch_exists_still_works(self, tmp_path: Path, monkeypatch):
|
||||
"""branch_exists() is unaffected by the get_branch_with_registry change."""
|
||||
branch_dir = tmp_path / "src" / "aipass" / "alpha"
|
||||
branch_dir.mkdir(parents=True)
|
||||
reg_file = tmp_path / "AIPASS_REGISTRY.json"
|
||||
_write_registry(reg_file, [_make_branch("alpha", "src/aipass/alpha")])
|
||||
|
||||
set_registry_path(reg_file)
|
||||
monkeypatch.delenv("AIPASS_HOME", raising=False)
|
||||
try:
|
||||
assert branch_exists("@alpha") is True
|
||||
assert branch_exists("@nonexistent") is False
|
||||
finally:
|
||||
reset_registry_path()
|
||||
|
||||
Reference in New Issue
Block a user