mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
cc281856d6
Remove installed overlays tier to enforce clean separation of concerns: - workflow add installs workflows only (no overlay copying) - workflow overlay add installs overlays only (project-local) Changes: - Remove InstalledOverlaySource class and all references - Remove overlay-copying logic from _validate_and_install_local() - Update WorkflowResolver to 2-tier: project overlays + base workflow - Fix --priority override timing: apply before validation, not after - Remove tests for installed overlays (no longer applicable) Rationale: If upstream controls both base workflow and shipped overlays, and both get overwritten on bundle update, there's no reason to ship overlays separately. Overlays only make sense when someone other than the base author adds them. Resolves all three review findings from PR #3557: - r3594064677: workflow add no longer copies overlays from all call sites - r3594064705: --priority override now applied before validation - r3594064726: no stale installed overlays (tier removed entirely) Assisted-by: Claude (model: claude-opus-4-7, autonomous)
254 lines
9.9 KiB
Python
254 lines
9.9 KiB
Python
"""Security tests for workflow overlay path handling."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
import yaml
|
|
from typer.testing import CliRunner
|
|
|
|
from specify_cli import app
|
|
|
|
|
|
runner = CliRunner()
|
|
|
|
|
|
@pytest.fixture
|
|
def project_dir(tmp_path):
|
|
"""Create a mock spec-kit project with ``.specify/workflows/`` directory."""
|
|
workflows_dir = tmp_path / ".specify" / "workflows"
|
|
workflows_dir.mkdir(parents=True, exist_ok=True)
|
|
return tmp_path
|
|
|
|
|
|
def _write_workflow(project_root: Path, workflow_id: str, data: dict) -> Path:
|
|
wf_dir = project_root / ".specify" / "workflows" / workflow_id
|
|
wf_dir.mkdir(parents=True, exist_ok=True)
|
|
wf_path = wf_dir / "workflow.yml"
|
|
wf_path.write_text(yaml.safe_dump(data), encoding="utf-8")
|
|
return wf_path
|
|
|
|
|
|
def _write_overlay(project_root: Path, workflow_id: str, overlay_id: str, data: dict) -> Path:
|
|
ov_dir = project_root / ".specify" / "workflows" / "overlays" / workflow_id
|
|
ov_dir.mkdir(parents=True, exist_ok=True)
|
|
ov_path = ov_dir / f"{overlay_id}.yml"
|
|
ov_path.write_text(yaml.safe_dump(data), encoding="utf-8")
|
|
return ov_path
|
|
|
|
|
|
class TestOverlayPathTraversal:
|
|
"""Overlay CLI must stay inside the overlay directory."""
|
|
|
|
def test_overlay_add_rejects_traversal_in_workflow_id(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
overlay_file = project_dir / "overlay.yml"
|
|
overlay_file.write_text(
|
|
yaml.safe_dump(
|
|
{
|
|
"id": "ov1",
|
|
"extends": "../wf",
|
|
"priority": 10,
|
|
"edits": [
|
|
{
|
|
"operation": "insert_after",
|
|
"anchor": "a",
|
|
"step": {"id": "new", "type": "command", "command": "echo"},
|
|
}
|
|
],
|
|
}
|
|
),
|
|
encoding="utf-8",
|
|
)
|
|
|
|
result = runner.invoke(
|
|
app, ["workflow", "overlay", "add", str(overlay_file), "--priority", "5"]
|
|
)
|
|
assert result.exit_code != 0, result.output
|
|
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
|
|
|
|
def test_overlay_add_rejects_traversal_in_overlay_id(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
_write_workflow(
|
|
project_dir,
|
|
"wf",
|
|
{
|
|
"schema_version": "1.0",
|
|
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
|
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
|
},
|
|
)
|
|
overlay_file = project_dir / "overlay.yml"
|
|
overlay_file.write_text(
|
|
yaml.safe_dump(
|
|
{
|
|
"id": "../../ov1",
|
|
"extends": "wf",
|
|
"priority": 10,
|
|
"edits": [
|
|
{
|
|
"operation": "insert_after",
|
|
"anchor": "a",
|
|
"step": {"id": "new", "type": "command", "command": "echo"},
|
|
}
|
|
],
|
|
}
|
|
),
|
|
encoding="utf-8",
|
|
)
|
|
|
|
result = runner.invoke(
|
|
app, ["workflow", "overlay", "add", str(overlay_file), "--priority", "5"]
|
|
)
|
|
assert result.exit_code != 0, result.output
|
|
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
|
|
|
|
def test_overlay_remove_cannot_escape_overlays_dir(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
_write_workflow(
|
|
project_dir,
|
|
"wf",
|
|
{
|
|
"schema_version": "1.0",
|
|
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
|
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
|
},
|
|
)
|
|
# Create a base workflow file that would be the traversal target.
|
|
target = project_dir / ".specify" / "workflows" / "wf" / "workflow.yml"
|
|
assert target.is_file()
|
|
|
|
result = runner.invoke(
|
|
app, ["workflow", "overlay", "remove", "wf", "../wf/workflow"]
|
|
)
|
|
assert result.exit_code != 0, result.output
|
|
assert target.is_file()
|
|
assert "Invalid" in result.output or "traversal" in result.output.lower()
|
|
|
|
def test_overlay_remove_rejects_symlink(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
_write_workflow(
|
|
project_dir,
|
|
"wf",
|
|
{
|
|
"schema_version": "1.0",
|
|
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
|
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
|
},
|
|
)
|
|
_write_overlay(
|
|
project_dir,
|
|
"wf",
|
|
"ov1",
|
|
{
|
|
"id": "ov1",
|
|
"extends": "wf",
|
|
"priority": 10,
|
|
"edits": [
|
|
{
|
|
"operation": "insert_after",
|
|
"anchor": "a",
|
|
"step": {"id": "new", "type": "command", "command": "echo"},
|
|
}
|
|
],
|
|
},
|
|
)
|
|
|
|
overlay_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf"
|
|
real_file = overlay_dir / "ov1.yml"
|
|
symlink_file = overlay_dir / "symlink.yml"
|
|
symlink_file.symlink_to(real_file)
|
|
|
|
result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "symlink"])
|
|
assert result.exit_code != 0, result.output
|
|
assert real_file.is_file()
|
|
assert "symlink" in result.output.lower() or "Invalid" in result.output
|
|
|
|
def test_overlay_operations_reject_overlays_as_workflow_id(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
result = runner.invoke(app, ["workflow", "overlay", "list", "overlays"])
|
|
assert result.exit_code != 0, result.output
|
|
assert "Invalid" in result.output or "reserved" in result.output.lower()
|
|
|
|
def test_overlay_set_priority_rejects_traversal(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
_write_workflow(
|
|
project_dir,
|
|
"wf",
|
|
{
|
|
"schema_version": "1.0",
|
|
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
|
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
|
},
|
|
)
|
|
result = runner.invoke(
|
|
app, ["workflow", "overlay", "set-priority", "wf", "../other", "10"]
|
|
)
|
|
assert result.exit_code != 0, result.output
|
|
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
|
|
|
|
def test_overlay_enable_rejects_traversal(self, project_dir, monkeypatch):
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
_write_workflow(
|
|
project_dir,
|
|
"wf",
|
|
{
|
|
"schema_version": "1.0",
|
|
"workflow": {"id": "wf", "name": "WF", "version": "1.0.0"},
|
|
"steps": [{"id": "a", "type": "command", "command": "echo"}],
|
|
},
|
|
)
|
|
result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "../other"])
|
|
assert result.exit_code != 0, result.output
|
|
assert "invalid" in result.output.lower() or "traversal" in result.output.lower()
|
|
|
|
def test_overlay_rejects_symlinked_overlays_dir(self, project_dir, monkeypatch, tmp_path):
|
|
"""Overlay commands must reject a symlinked .specify/workflows/overlays directory."""
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
|
|
# Create a symlinked overlays directory pointing outside the project
|
|
outside_dir = tmp_path / "outside"
|
|
outside_dir.mkdir()
|
|
overlays_dir = project_dir / ".specify" / "workflows" / "overlays"
|
|
overlays_dir.symlink_to(outside_dir)
|
|
|
|
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
|
|
assert result.exit_code != 0, result.output
|
|
assert "symlink" in result.output.lower()
|
|
|
|
def test_overlay_list_rejects_symlinked_per_workflow_dir(self, project_dir, monkeypatch, tmp_path):
|
|
"""Overlay list must reject a symlinked per-workflow overlay directory."""
|
|
monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir)
|
|
|
|
# Create a real overlay directory outside the project.
|
|
outside_dir = tmp_path / "outside_wf"
|
|
outside_dir.mkdir()
|
|
outside_dir.joinpath("evil.yml").write_text(
|
|
yaml.safe_dump(
|
|
{
|
|
"id": "evil",
|
|
"extends": "wf",
|
|
"priority": 100,
|
|
"edits": [
|
|
{
|
|
"operation": "insert_after",
|
|
"anchor": "a",
|
|
"step": {"id": "evil-step", "type": "command", "command": "echo"},
|
|
}
|
|
],
|
|
}
|
|
),
|
|
encoding="utf-8",
|
|
)
|
|
|
|
# Symlink the per-workflow overlay directory to the outside location.
|
|
overlays_root = project_dir / ".specify" / "workflows" / "overlays"
|
|
overlays_root.mkdir(parents=True, exist_ok=True)
|
|
symlink_dir = overlays_root / "wf"
|
|
symlink_dir.symlink_to(outside_dir)
|
|
|
|
result = runner.invoke(app, ["workflow", "overlay", "list", "wf"])
|
|
assert result.exit_code != 0, result.output
|
|
assert "symlink" in result.output.lower()
|