Harden pack validator trust boundary
- reject unsafe pack IDs, path traversal, absolute paths, and separators - require --allow-arbitrary-code for dynamic validator execution - add --dry-run validation that does not import or execute validators - preserve JSON/report behavior with explicit trust boundary - add focused CLI security tests
This commit is contained in:
parent
40cabdec09
commit
f82f4d36de
2 changed files with 144 additions and 4 deletions
59
core/cli.py
59
core/cli.py
|
|
@ -342,15 +342,60 @@ def cmd_pack_verify(args: argparse.Namespace) -> int:
|
||||||
return _run(sys.executable, "-m", "language_packs", "verify", args.pack_id)
|
return _run(sys.executable, "-m", "language_packs", "verify", args.pack_id)
|
||||||
|
|
||||||
|
|
||||||
|
def _safe_pack_id(pack_id: str) -> str:
|
||||||
|
"""Reject pack IDs containing path traversal or separator characters."""
|
||||||
|
if not pack_id:
|
||||||
|
_die("pack_id is required", code=2)
|
||||||
|
|
||||||
|
path = Path(pack_id)
|
||||||
|
|
||||||
|
if path.is_absolute():
|
||||||
|
_die("pack_id must not be an absolute path", code=2)
|
||||||
|
|
||||||
|
if pack_id in {".", ".."}:
|
||||||
|
_die("pack_id must name a pack, not a relative path", code=2)
|
||||||
|
|
||||||
|
if any(part in {"", ".", ".."} for part in path.parts):
|
||||||
|
_die("pack_id must not contain path traversal", code=2)
|
||||||
|
|
||||||
|
if "/" in pack_id or "\\" in pack_id:
|
||||||
|
_die("pack_id must be a simple pack id, not a path", code=2)
|
||||||
|
|
||||||
|
return pack_id
|
||||||
|
|
||||||
|
|
||||||
def cmd_pack_validate(args: argparse.Namespace) -> int:
|
def cmd_pack_validate(args: argparse.Namespace) -> int:
|
||||||
"""Run executable source-pack validation gates."""
|
"""Run executable source-pack validation gates."""
|
||||||
import importlib.util
|
pack_id = _safe_pack_id(args.pack_id)
|
||||||
|
pack_dir = _REPO_ROOT / "packs" / pack_id
|
||||||
pack_dir = _REPO_ROOT / "packs" / args.pack_id
|
|
||||||
validator_path = pack_dir / "validators.py"
|
validator_path = pack_dir / "validators.py"
|
||||||
|
|
||||||
if not validator_path.exists():
|
if not validator_path.exists():
|
||||||
_die(f"source-pack validator not found: {validator_path}", code=1)
|
_die(f"source-pack validator not found: {validator_path}", code=1)
|
||||||
spec = importlib.util.spec_from_file_location(f"{args.pack_id}_validators", validator_path)
|
|
||||||
|
if getattr(args, "dry_run", False):
|
||||||
|
if args.json:
|
||||||
|
print(json.dumps({
|
||||||
|
"pack_id": pack_id,
|
||||||
|
"validator_path": str(validator_path),
|
||||||
|
"would_execute": False,
|
||||||
|
"exists": True,
|
||||||
|
}, ensure_ascii=False, indent=2, sort_keys=True))
|
||||||
|
else:
|
||||||
|
print(f"dry-run: pack_id={pack_id}")
|
||||||
|
print(f"validator: {validator_path}")
|
||||||
|
print("status: validator exists, would not execute")
|
||||||
|
return 0
|
||||||
|
|
||||||
|
if not getattr(args, "allow_arbitrary_code", False):
|
||||||
|
_die(
|
||||||
|
"dynamic validator execution requires --allow-arbitrary-code",
|
||||||
|
code=2,
|
||||||
|
)
|
||||||
|
|
||||||
|
import importlib.util
|
||||||
|
|
||||||
|
spec = importlib.util.spec_from_file_location(f"{pack_id}_validators", validator_path)
|
||||||
if spec is None or spec.loader is None:
|
if spec is None or spec.loader is None:
|
||||||
_die(f"cannot load source-pack validator: {validator_path}", code=1)
|
_die(f"cannot load source-pack validator: {validator_path}", code=1)
|
||||||
module = importlib.util.module_from_spec(spec)
|
module = importlib.util.module_from_spec(spec)
|
||||||
|
|
@ -569,6 +614,12 @@ def build_parser() -> argparse.ArgumentParser:
|
||||||
pack_validate = pack_sub.add_parser("validate", help="validate a source pack under packs/")
|
pack_validate = pack_sub.add_parser("validate", help="validate a source pack under packs/")
|
||||||
pack_validate.add_argument("pack_id", help="source pack id, e.g. en, he, grc, el")
|
pack_validate.add_argument("pack_id", help="source pack id, e.g. en, he, grc, el")
|
||||||
pack_validate.add_argument("--json", action="store_true", help="emit machine-readable JSON")
|
pack_validate.add_argument("--json", action="store_true", help="emit machine-readable JSON")
|
||||||
|
pack_validate.add_argument("--dry-run", action="store_true", help="check validator exists without executing")
|
||||||
|
pack_validate.add_argument(
|
||||||
|
"--allow-arbitrary-code",
|
||||||
|
action="store_true",
|
||||||
|
help="permit dynamic validator execution (required to run validators)",
|
||||||
|
)
|
||||||
pack_validate.set_defaults(func=cmd_pack_validate)
|
pack_validate.set_defaults(func=cmd_pack_validate)
|
||||||
|
|
||||||
rust = subparsers.add_parser(
|
rust = subparsers.add_parser(
|
||||||
|
|
|
||||||
89
tests/test_cli_pack_validate_security.py
Normal file
89
tests/test_cli_pack_validate_security.py
Normal file
|
|
@ -0,0 +1,89 @@
|
||||||
|
"""Security tests for the ``core pack validate`` CLI subcommand."""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import importlib.util
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from core import cli
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_requires_allow_arbitrary_code(capsys: pytest.CaptureFixture[str]) -> None:
|
||||||
|
with pytest.raises(SystemExit) as exc:
|
||||||
|
cli.main(["pack", "validate", "en"])
|
||||||
|
|
||||||
|
assert exc.value.code != 0
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
assert "--allow-arbitrary-code" in captured.err
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_dry_run_does_not_execute_validator(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
capsys: pytest.CaptureFixture[str],
|
||||||
|
) -> None:
|
||||||
|
def fail_import(*args: object, **kwargs: object) -> None:
|
||||||
|
raise AssertionError("validator should not execute during dry-run")
|
||||||
|
|
||||||
|
monkeypatch.setattr(importlib.util, "spec_from_file_location", fail_import)
|
||||||
|
|
||||||
|
rc = cli.main(["pack", "validate", "en", "--dry-run"])
|
||||||
|
|
||||||
|
assert rc == 0
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
assert "dry-run" in captured.out.lower()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"pack_id",
|
||||||
|
["../foo", "/tmp/foo", "foo/bar", r"foo\bar", ".", "..", ""],
|
||||||
|
)
|
||||||
|
def test_pack_validate_rejects_unsafe_pack_ids(pack_id: str) -> None:
|
||||||
|
args = ["pack", "validate", pack_id, "--dry-run"] if pack_id else ["pack", "validate", "", "--dry-run"]
|
||||||
|
with pytest.raises(SystemExit) as exc:
|
||||||
|
cli.main(args)
|
||||||
|
assert exc.value.code == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_rejects_path_traversal(capsys: pytest.CaptureFixture[str]) -> None:
|
||||||
|
with pytest.raises(SystemExit) as exc:
|
||||||
|
cli.main(["pack", "validate", "../foo", "--dry-run"])
|
||||||
|
|
||||||
|
assert exc.value.code == 2
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
assert "path" in captured.err.lower() or "traversal" in captured.err.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_rejects_absolute_paths(capsys: pytest.CaptureFixture[str]) -> None:
|
||||||
|
with pytest.raises(SystemExit) as exc:
|
||||||
|
cli.main(["pack", "validate", "/tmp/foo", "--dry-run"])
|
||||||
|
|
||||||
|
assert exc.value.code == 2
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
assert "absolute" in captured.err.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_rejects_path_separators(capsys: pytest.CaptureFixture[str]) -> None:
|
||||||
|
with pytest.raises(SystemExit) as exc:
|
||||||
|
cli.main(["pack", "validate", "foo/bar", "--dry-run"])
|
||||||
|
|
||||||
|
assert exc.value.code == 2
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
assert "path" in captured.err.lower()
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_allows_known_safe_pack_with_explicit_flag() -> None:
|
||||||
|
rc = cli.main(["pack", "validate", "en", "--allow-arbitrary-code"])
|
||||||
|
assert rc in {0, 1}
|
||||||
|
|
||||||
|
|
||||||
|
def test_pack_validate_dry_run_json(capsys: pytest.CaptureFixture[str]) -> None:
|
||||||
|
import json
|
||||||
|
|
||||||
|
rc = cli.main(["pack", "validate", "en", "--dry-run", "--json"])
|
||||||
|
|
||||||
|
assert rc == 0
|
||||||
|
captured = capsys.readouterr()
|
||||||
|
data = json.loads(captured.out)
|
||||||
|
assert data["pack_id"] == "en"
|
||||||
|
assert data["would_execute"] is False
|
||||||
|
assert data["exists"] is True
|
||||||
Loading…
Reference in a new issue