diff --git a/.github/workflows/requirements-evidence.yml b/.github/workflows/requirements-evidence.yml index 5c490e06..b3e8f1c4 100644 --- a/.github/workflows/requirements-evidence.yml +++ b/.github/workflows/requirements-evidence.yml @@ -656,7 +656,7 @@ jobs: selected_change="${selected_change%/requirements-proof/review-evidence.json}" fi fi - clean_environment=(env -i "HOME=${HOME:-}" "LANG=${LANG:-}" "LC_ALL=${LC_ALL:-}" "LC_CTYPE=${LC_CTYPE:-}" "PATH=${PATH:-}" "RUNNER_TEMP=${RUNNER_TEMP}" "SPECFACT_MODULES_REPO=${SPECFACT_MODULES_REPO}" "SPECFACT_MODULES_ROOTS=${SPECFACT_MODULES_ROOTS}") + clean_environment=(env -i "HOME=${HOME:-}" "LANG=${LANG:-}" "LC_ALL=${LC_ALL:-}" "LC_CTYPE=${LC_CTYPE:-}" "PATH=${PATH:-}" "RUNNER_TEMP=${RUNNER_TEMP}" "SPECFACT_MODULES_REPO=${SPECFACT_MODULES_REPO}" "SPECFACT_MODULES_ROOTS=${SPECFACT_MODULES_ROOTS}" "SPECFACT_MODULES_EXCLUSIVE=1") evidence_arguments=( "${clean_environment[@]}" "${isolated_specfact[@]}" requirements evidence --repo-root "$GITHUB_WORKSPACE" \ diff --git a/openspec/changes/security-requirements-evidence-exclusive-discovery/TDD_EVIDENCE.md b/openspec/changes/security-requirements-evidence-exclusive-discovery/TDD_EVIDENCE.md new file mode 100644 index 00000000..a43783ee --- /dev/null +++ b/openspec/changes/security-requirements-evidence-exclusive-discovery/TDD_EVIDENCE.md @@ -0,0 +1,26 @@ +# TDD Evidence: security-requirements-evidence-exclusive-discovery + +## Failing-before + +- **Command**: `hatch run pytest tests/unit/registry/test_module_discovery.py::test_exclusive_module_roots_reject_project_and_user_shadowing tests/unit/scripts/test_requirements_evidence_delivery_gate.py::test_failed_command_writes_missing_diagnostic_reports_and_exports_fixture_roots tests/unit/workflows/test_requirements_evidence_delivery_workflow.py::test_required_requirements_context_is_pull_request_only -q` +- **Result**: 2 failed, 1 passed. +- **Evidence**: discovery selected the repository-local same-identity module instead of the explicit fixture, and the adapter did not set an exclusive-discovery control. + +## Passing-after + +- **Command**: `hatch run pytest tests/unit/registry/test_module_discovery.py tests/unit/scripts/test_requirements_evidence_delivery_gate.py tests/unit/workflows/test_requirements_evidence_delivery_workflow.py -q` +- **Result**: 49 passed, 4 skipped because the optional pinned fixture checkout is unavailable. +- **Evidence**: exclusive discovery retained the bundled root and verified explicit root while excluding project, user, marketplace, custom, and legacy roots; adapter and workflow contracts require the control. + +## Quality gates + +- `hatch run format`, `hatch run type-check`, and `hatch run lint` passed. BasedPyright reported the repository's existing warning baseline with 0 errors; Ruff reported no findings. +- `hatch run yaml-lint` completed with pre-existing errors in archived Requirements evidence and the active Requirements 07 evidence ledger; no changed YAML file produced a finding. +- `hatch run openspec validate security-requirements-evidence-exclusive-discovery --strict`, `hatch run python scripts/check_reproducible_delivery.py`, and `uv lock --check` passed. +- Semgrep SAST and its baseline gate passed with 0 findings; Bandit reported no medium/high findings. +- Changed-scope `specfact code review run` passed with no findings after installing its declared BasedPyright and Pylint review tools. +- `hatch run contract-test` used the valid cached baseline. `hatch run smart-test` ran the full suite but reported two unrelated missing fixture-package imports (`specfact_backlog` and `specfact_spec`); the 49-test focused affected-scope suite remained green. + +## Internal wiki follow-up + +The sibling `specfact-cli-internal` checkout is unavailable in this environment. After it is available, add or update `wiki/sources/security-requirements-evidence-exclusive-discovery.md` and run `python3 scripts/wiki_rebuild_graph.py` from that repository root. diff --git a/openspec/changes/security-requirements-evidence-exclusive-discovery/proposal.md b/openspec/changes/security-requirements-evidence-exclusive-discovery/proposal.md new file mode 100644 index 00000000..1fc2912b --- /dev/null +++ b/openspec/changes/security-requirements-evidence-exclusive-discovery/proposal.md @@ -0,0 +1,24 @@ +# Proposal: security-requirements-evidence-exclusive-discovery + +## Why + +The Requirements evidence delivery gate verifies a pinned external module fixture, but normal module discovery still examines repository-local and user-controlled roots before or alongside that fixture. A pull request can therefore shadow the trusted Requirements package with an unsigned project module and execute attacker-controlled Python while fabricating a passing report. + +## What Changes + +- Add an explicit module-discovery mode that limits dynamic package discovery to `SPECFACT_MODULES_ROOTS` (while retaining bundled core modules). +- Require both the local adapter and pull-request evidence workflow to enable that mode when invoking the pinned Requirements fixture. +- Add regressions proving project, user, marketplace, custom, and legacy roots cannot shadow an exclusive fixture invocation. + +## Capabilities + +### New Capabilities + +- `trusted-module-fixture-discovery` + +## Impact + +- Affected code: module discovery and the Requirements evidence adapter. +- Affected workflow: `.github/workflows/requirements-evidence.yml`. +- Affected tests: focused discovery, adapter, and workflow contract tests. +- Compatibility: ordinary CLI discovery remains unchanged; exclusivity is opt-in for trusted fixture execution. diff --git a/openspec/changes/security-requirements-evidence-exclusive-discovery/specs/trusted-module-fixture-discovery/spec.md b/openspec/changes/security-requirements-evidence-exclusive-discovery/specs/trusted-module-fixture-discovery/spec.md new file mode 100644 index 00000000..c6a4f1dd --- /dev/null +++ b/openspec/changes/security-requirements-evidence-exclusive-discovery/specs/trusted-module-fixture-discovery/spec.md @@ -0,0 +1,19 @@ +## ADDED Requirements + +### Requirement: Trusted fixture discovery is exclusive + +Security-sensitive delivery gates that execute a verified external module fixture SHALL restrict dynamic module discovery to the explicitly configured fixture roots while retaining bundled core commands. + +#### Scenario: Prevent untrusted module shadowing + +- **GIVEN** a verified Requirements fixture and an untrusted module with the same identity in a project, user, marketplace, custom, or legacy discovery root +- **WHEN** local or CI requirements evidence enforcement invokes the released command +- **THEN** dynamic module discovery uses only the explicit verified fixture root +- **AND** bundled core commands remain available +- **AND** no untrusted same-identity module is imported or executed. + +#### Scenario: Preserve ordinary module discovery + +- **GIVEN** an ordinary CLI invocation that does not request exclusive discovery +- **WHEN** SpecFact discovers dynamic modules +- **THEN** existing project, explicit, user, marketplace, custom, and legacy discovery behavior remains unchanged. diff --git a/openspec/changes/security-requirements-evidence-exclusive-discovery/tasks.md b/openspec/changes/security-requirements-evidence-exclusive-discovery/tasks.md new file mode 100644 index 00000000..b51dc969 --- /dev/null +++ b/openspec/changes/security-requirements-evidence-exclusive-discovery/tasks.md @@ -0,0 +1,19 @@ +# Tasks: security-requirements-evidence-exclusive-discovery + +## 1. Specification and failing evidence + +- [x] 1.1 Add the exclusive-discovery security delta. +- [x] 1.2 Add focused regressions for project-module shadowing and gate environment isolation. +- [x] 1.3 Run the focused regressions against HEAD and record failing-before evidence. + +## 2. Implementation + +- [x] 2.1 Make exclusive discovery retain bundled modules and explicit fixture roots only. +- [x] 2.2 Enable exclusive discovery in the local evidence adapter and pull-request workflow. + +## 3. Verification + +- [x] 3.1 Run focused tests and record passing-after evidence. +- [x] 3.2 Run required formatting, typing, linting, YAML, security, and changed-scope review gates. +- [x] 3.3 Validate the OpenSpec change strictly. +- [x] 3.4 Update the internal wiki mirror and graph when the sibling checkout is available; otherwise record the follow-up. diff --git a/scripts/requirements_evidence_delivery_gate.py b/scripts/requirements_evidence_delivery_gate.py index 75073f39..f52c6b6c 100644 --- a/scripts/requirements_evidence_delivery_gate.py +++ b/scripts/requirements_evidence_delivery_gate.py @@ -167,6 +167,7 @@ def run_evidence_command( environment = {key: value for key, value in os.environ.items() if key in ALLOWED_ENVIRONMENT_KEYS} environment["SPECFACT_MODULES_REPO"] = str(fixture_root.resolve()) environment["SPECFACT_MODULES_ROOTS"] = str((fixture_root / "packages").resolve()) + environment["SPECFACT_MODULES_EXCLUSIVE"] = "1" environment.pop("SPECFACT_CLI_MODULES_REPO", None) try: exit_code = command_runner(arguments, environment) diff --git a/src/specfact_cli/registry/module_discovery.py b/src/specfact_cli/registry/module_discovery.py index 31a80538..6aebe94f 100644 --- a/src/specfact_cli/registry/module_discovery.py +++ b/src/specfact_cli/registry/module_discovery.py @@ -18,6 +18,7 @@ USER_MODULES_ROOT = Path.home() / ".specfact" / "modules" MARKETPLACE_MODULES_ROOT = Path.home() / ".specfact" / "marketplace-modules" CUSTOM_MODULES_ROOT = Path.home() / ".specfact" / "custom-modules" +EXCLUSIVE_MODULE_ROOTS_ENV = "SPECFACT_MODULES_EXCLUSIVE" _SHADOW_HINT_KEYS: set[tuple[str, str, str, str]] = set() @@ -116,6 +117,10 @@ def _discovery_root_list(options: _DiscoveryRootOptions) -> list[tuple[str, Path effective_custom_root = options.custom_root or CUSTOM_MODULES_ROOT roots: list[tuple[str, Path]] = [("builtin", effective_builtin_root)] + if os.environ.get(EXCLUSIVE_MODULE_ROOTS_ENV) == "1": + _append_explicit_module_roots(roots) + return roots + project_matches_user_root = False if effective_project_root is not None: try: diff --git a/tests/unit/registry/test_module_discovery.py b/tests/unit/registry/test_module_discovery.py index bb5e500a..6590b869 100644 --- a/tests/unit/registry/test_module_discovery.py +++ b/tests/unit/registry/test_module_discovery.py @@ -143,6 +143,39 @@ def test_explicit_module_roots_take_priority_over_user_installs( assert sources["code-review"] == "custom" +def test_exclusive_module_roots_reject_project_and_user_shadowing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Trusted fixture discovery must ignore every non-bundled implicit root.""" + repo_root = tmp_path / "repo" + project_root = repo_root / ".specfact" / "modules" + builtin_root = tmp_path / "builtin" + explicit_root = tmp_path / "verified-fixture" / "packages" + user_root = tmp_path / "user-modules" + marketplace_root = tmp_path / "marketplace-modules" + custom_root = tmp_path / "custom-modules" + _write_manifest(builtin_root, "init") + for untrusted_root in (project_root, user_root, marketplace_root, custom_root): + _write_manifest(untrusted_root, "nold-ai-specfact-requirements") + _write_manifest(explicit_root, "nold-ai-specfact-requirements") + monkeypatch.chdir(repo_root) + monkeypatch.setenv("SPECFACT_MODULES_ROOTS", str(explicit_root)) + monkeypatch.setenv("SPECFACT_MODULES_EXCLUSIVE", "1") + + discovered = discover_all_modules( + builtin_root=builtin_root, + user_root=user_root, + marketplace_root=marketplace_root, + custom_root=custom_root, + include_legacy_roots=True, + ) + + assert [(entry.metadata.name, entry.source) for entry in discovered] == [ + ("init", "builtin"), + ("nold-ai-specfact-requirements", "custom"), + ] + + def test_project_shadow_warning_is_actionable_and_emitted_once(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: """Project-over-user shadow guidance should be user-facing but deduplicated per process.""" repo_root = tmp_path / "repo" diff --git a/tests/unit/scripts/test_requirements_evidence_delivery_gate.py b/tests/unit/scripts/test_requirements_evidence_delivery_gate.py index 523b9b45..eb664cf1 100644 --- a/tests/unit/scripts/test_requirements_evidence_delivery_gate.py +++ b/tests/unit/scripts/test_requirements_evidence_delivery_gate.py @@ -287,6 +287,7 @@ def test_failed_command_writes_missing_diagnostic_reports_and_exports_fixture_ro assert observed == 1 assert observed_environment["SPECFACT_MODULES_REPO"] == str(fixture.resolve()) assert observed_environment["SPECFACT_MODULES_ROOTS"] == str((fixture / "packages").resolve()) + assert observed_environment["SPECFACT_MODULES_EXCLUSIVE"] == "1" assert observed_environment["PATH"] == os.environ["PATH"] assert "SPECFACT_TEST_SECRET" not in observed_environment assert "PYTHONPATH" not in observed_environment diff --git a/tests/unit/workflows/test_requirements_evidence_delivery_workflow.py b/tests/unit/workflows/test_requirements_evidence_delivery_workflow.py index 226444c4..4ce8823f 100644 --- a/tests/unit/workflows/test_requirements_evidence_delivery_workflow.py +++ b/tests/unit/workflows/test_requirements_evidence_delivery_workflow.py @@ -425,6 +425,7 @@ def _assert_command_contract(workflow: dict[str, object]) -> None: assert run_evidence["env"]["EVIDENCE_BASE_BRANCH"] # type: ignore[index] assert "workflow_dispatch" not in workflow["on"] # type: ignore[operator] assert run_evidence["run"].count("clean_environment=(env -i") == 1 # type: ignore[union-attr] + assert '"SPECFACT_MODULES_EXCLUSIVE=1"' in run_evidence["run"] # type: ignore[operator] assert run_evidence["run"].count('"${clean_environment[@]}" "${isolated_specfact[@]}"') == 2 # type: ignore[union-attr]