From c0d476e8d3dfee8563d79f52e6e781b43ba58e22 Mon Sep 17 00:00:00 2001 From: howlcipher Date: Wed, 12 Aug 2026 16:54:53 -0400 Subject: [PATCH 1/8] Add RealTerraformTool: real fmt/init/validate/plan via subprocess Shells out to a real terraform binary for fmt/init/validate/plan only -- apply/destroy stay on SimulatedTerraformTool until Milestone 3. Adds tools/_subprocess_safety.py for explicit timeouts and secret redaction on every real subprocess call, since state/auth output can contain secrets. Plan parsing extracts only {address, action} from terraform show -json, never raw attribute values, feeding the existing risk classifier unchanged. Also renders LearningMoment.related_artifact, which had no real caller before this milestone. Co-Authored-By: Claude Sonnet 5 --- src/devops_learn/explanations/service.py | 2 + src/devops_learn/tools/_subprocess_safety.py | 86 ++++++ src/devops_learn/tools/terraform_tool.py | 281 ++++++++++++++++++- tests/explanations/test_service.py | 49 ++++ tests/fixtures/fake_terraform/terraform | 130 +++++++++ tests/tools/test_real_terraform_tool.py | 189 +++++++++++++ tests/tools/test_subprocess_safety.py | 52 ++++ 7 files changed, 785 insertions(+), 4 deletions(-) create mode 100644 src/devops_learn/tools/_subprocess_safety.py create mode 100644 tests/explanations/test_service.py create mode 100755 tests/fixtures/fake_terraform/terraform create mode 100644 tests/tools/test_real_terraform_tool.py create mode 100644 tests/tools/test_subprocess_safety.py diff --git a/src/devops_learn/explanations/service.py b/src/devops_learn/explanations/service.py index fb921e4..6654109 100644 --- a/src/devops_learn/explanations/service.py +++ b/src/devops_learn/explanations/service.py @@ -48,4 +48,6 @@ def render_learning_moment( lines.append(moment.why_it_matters) if depth >= ExplanationDepth.DEEP: lines.append(moment.deep_explanation) + if moment.related_artifact and depth >= ExplanationDepth.LEARNING: + lines.append(f"See: {moment.related_artifact}") return "\n".join(lines) diff --git a/src/devops_learn/tools/_subprocess_safety.py b/src/devops_learn/tools/_subprocess_safety.py new file mode 100644 index 0000000..f5d8e97 --- /dev/null +++ b/src/devops_learn/tools/_subprocess_safety.py @@ -0,0 +1,86 @@ +"""Shared subprocess-safety helpers for Real*Tool implementations. + +Extracted because RealTerraformTool needs both an explicit timeout and output +redaction, neither of which RealDockerTool/RealPythonTool have today: those +two tools never set a subprocess timeout and put stdout/stderr into +ToolResult.details verbatim. That gap matters more for Terraform, since +provider auth errors and init/plan output can contain secret-shaped values. +Deliberately generic (no Terraform-specific knowledge) so those two tools +could adopt it later; retrofitting them is out of scope for this milestone. +""" + +from __future__ import annotations + +import re +import subprocess +from dataclasses import dataclass + +DEFAULT_TIMEOUT_SECONDS = 120 +MAX_OUTPUT_CHARS = 20_000 + +_SECRET_PATTERN = re.compile( + r"(?i)\b([A-Z0-9_]*(?:SECRET|TOKEN|PASSWORD|API_KEY)[A-Z0-9_]*)\s*[=:]\s*(\S+)" +) + + +@dataclass(frozen=True) +class SafeRunResult: + returncode: int + stdout: str + stderr: str + timed_out: bool + + +def redact(text: str) -> str: + """Masks KEY=value-shaped secrets in free text. + + Defense in depth only. The primary defense against leaking sensitive + Terraform data is never extracting attribute values from structured + output in the first place -- see the module docstring in + tools/terraform_tool.py for why plan parsing never touches + resource_changes[].change.before/after. + """ + return _SECRET_PATTERN.sub(lambda m: f"{m.group(1)}=", text) + + +def _truncate(text: str) -> str: + if len(text) <= MAX_OUTPUT_CHARS: + return text + hidden = len(text) - MAX_OUTPUT_CHARS + return text[:MAX_OUTPUT_CHARS] + f"\n... [truncated, {hidden} more characters] ...\n" + + +def _sanitize(text: str) -> str: + return _truncate(redact(text)) + + +def run_safely( + command: list[str], *, cwd: str | None, timeout: int = DEFAULT_TIMEOUT_SECONDS +) -> SafeRunResult: + """Runs `command` as an argument array (never shell=True) with a timeout, + and always redacts + truncates stdout/stderr before returning, so a call + site can never forget to sanitize output before it reaches a ToolResult. + """ + try: + completed = subprocess.run( + command, + cwd=cwd, + capture_output=True, + text=True, + check=False, + timeout=timeout, + ) + except subprocess.TimeoutExpired as exc: + partial_stdout = exc.stdout if isinstance(exc.stdout, str) else "" + return SafeRunResult( + returncode=-1, + stdout=_sanitize(partial_stdout), + stderr=_sanitize(f"Command timed out after {timeout}s: {' '.join(command)}"), + timed_out=True, + ) + return SafeRunResult( + returncode=completed.returncode, + stdout=_sanitize(completed.stdout), + stderr=_sanitize(completed.stderr), + timed_out=False, + ) diff --git a/src/devops_learn/tools/terraform_tool.py b/src/devops_learn/tools/terraform_tool.py index 330b3dd..4d65bea 100644 --- a/src/devops_learn/tools/terraform_tool.py +++ b/src/devops_learn/tools/terraform_tool.py @@ -1,16 +1,32 @@ -"""Simulated TerraformTool. No real terraform binary invoked in V1; see docs/safety.md. +"""TerraformTool implementations: simulated (no binary invoked, used by +`analyze`/`review`) and real (used by `devops-learn terraform`). -plan() derives its resource count from templates/terraform/main.tf.reference -instead of a hardcoded string, so the learner's interpretation question stays -meaningful and the reference config can change without a second edit site. +SimulatedTerraformTool.plan() derives its resource count from +templates/terraform/main.tf.reference instead of a hardcoded string, so the +learner's interpretation question stays meaningful and the reference config +can change without a second edit site. + +RealTerraformTool shells out to a real `terraform` binary for fmt/init/ +validate/plan only -- see its class docstring for why apply/destroy are +deliberately not declared here. Its plan parsing never extracts raw resource +attribute values (before/after) from `terraform show -json`, only +{address, action} pairs: this is the primary defense against leaking +sensitive plan data, since Terraform's own "sensitive" markers can vary by +provider/version and are not a defense to rely on alone. Free-text +stdout/stderr from every real subprocess call is still redacted and +truncated via tools/_subprocess_safety.py as defense in depth. """ from __future__ import annotations +import json import re +import shutil +import tempfile from pathlib import Path from typing import Any, Mapping +from devops_learn.tools import _subprocess_safety from devops_learn.tools.approval import ApprovalRecord from devops_learn.tools.base import RiskLevel, Tool, ToolOperationSpec, ToolResult @@ -126,3 +142,260 @@ def execute( was_dry_run=dry_run, approval=approval, ) + + +_REAL_OPERATIONS = ( + ToolOperationSpec( + name="fmt", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), + ToolOperationSpec( + name="init", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), + ToolOperationSpec( + name="validate", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), + ToolOperationSpec( + name="plan", + risk_level=RiskLevel.SAFE, + supports_dry_run=False, + requires_approval=False, + is_destructive=False, + ), +) + + +def _terraform_command() -> str: + command = shutil.which("terraform") + if command is None: + raise FileNotFoundError( + "Terraform CLI not found. Install Terraform or use simulation mode." + ) + return command + + +def _details_from_plan_json(plan_json: Mapping[str, Any]) -> dict[str, Any]: + """Extracts only {address, action} pairs from a real `terraform show -json` + plan -- never the resource_changes[].change.before/after attribute maps, + which can contain sensitive values. See the module docstring.""" + create = change = replace = destroy = 0 + resources: list[dict[str, str]] = [] + for resource_change in plan_json.get("resource_changes", []): + actions = tuple(resource_change.get("change", {}).get("actions", ())) + if actions == ("create",): + create += 1 + action = "create" + elif actions == ("update",): + change += 1 + action = "update" + elif actions in (("delete", "create"), ("create", "delete")): + replace += 1 + action = "replace" + elif actions == ("delete",): + destroy += 1 + action = "destroy" + else: + continue # ("no-op",) / ("read",): not a change + address = str(resource_change.get("address", "unknown")) + resources.append({"address": address, "action": action}) + return { + "create": create, + "change": change, + "replace": replace, + "destroy": destroy, + "resources": resources, + } + + +class RealTerraformTool(Tool): + """Runs real terraform fmt/init/validate/plan via subprocess. + + Deliberately does not declare apply_approved_plan or + destroy_approved_environment: those stay simulated until Milestone 3, per + docs/roadmap.md. A caller invoking either operation on this tool gets a + KeyError from Tool.spec_for rather than a result that looks real but + silently isn't -- apply/destroy remain real capabilities of the + "terraform" tool name only via SimulatedTerraformTool, exactly how + cli/main.py already selects real-vs-simulated per command today. See + docs/adr/0004-controlled-tool-execution.md. + """ + + @property + def name(self) -> str: + return "terraform" + + @property + def operations(self) -> tuple[ToolOperationSpec, ...]: + return _REAL_OPERATIONS + + def execute( + self, + operation: str, + params: Mapping[str, Any], + *, + dry_run: bool, + approval: ApprovalRecord | None, + ) -> ToolResult: + spec = self.spec_for(operation) + assert not spec.requires_approval or dry_run or (approval is not None and approval.granted) + + if dry_run: + return ToolResult( + success=True, + summary=f"Would run terraform {operation} (dry run)", + details={"dry_run": True}, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + + working_dir = params.get("path") + timeout_override = params.get("timeout_seconds") + + def _timeout(default: int) -> int: + return int(timeout_override) if timeout_override is not None else default + + try: + terraform = _terraform_command() + if operation == "fmt": + result = _subprocess_safety.run_safely( + [terraform, "fmt", "-check", "-diff", "-no-color"], + cwd=working_dir, + timeout=_timeout(30), + ) + success = result.returncode == 0 + summary = "Configuration already formatted" if success else ( + "Configuration needs formatting" + ) + details: dict[str, Any] = { + "returncode": result.returncode, + "stdout": result.stdout, + "stderr": result.stderr, + } + elif operation == "init": + result = _subprocess_safety.run_safely( + [terraform, "init", "-input=false", "-no-color"], + cwd=working_dir, + timeout=_timeout(120), + ) + success = result.returncode == 0 + success_lines = [ + line + for line in result.stdout.strip().splitlines() + if "successfully initialized" in line.lower() + ] + if success_lines: + summary = success_lines[0].strip() + elif success: + summary = "Terraform has been initialized" + else: + summary = "terraform init failed" + details = { + "returncode": result.returncode, + "stdout": result.stdout, + "stderr": result.stderr, + } + elif operation == "validate": + result = _subprocess_safety.run_safely( + [terraform, "validate", "-json"], cwd=working_dir, timeout=_timeout(30) + ) + try: + parsed = json.loads(result.stdout) if result.stdout else {} + except json.JSONDecodeError: + parsed = {} + diagnostics = parsed.get("diagnostics", []) + success = result.returncode == 0 and bool(parsed.get("valid", False)) + summary = ( + "Success! The configuration is valid." + if success + else f"Validation failed: {len(diagnostics)} diagnostic(s)" + ) + details = { + "returncode": result.returncode, + "valid": parsed.get("valid"), + "diagnostic_count": len(diagnostics), + "stderr": result.stderr, + } + else: # plan + with tempfile.TemporaryDirectory() as tmp_dir: + plan_path = str(Path(tmp_dir) / "tfplan.out") + plan_result = _subprocess_safety.run_safely( + [terraform, "plan", "-input=false", "-no-color", f"-out={plan_path}"], + cwd=working_dir, + timeout=_timeout(180), + ) + if plan_result.returncode != 0: + return ToolResult( + success=False, + summary="(real, failed) terraform plan failed", + details={ + "returncode": plan_result.returncode, + "stdout": plan_result.stdout, + "stderr": plan_result.stderr, + }, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + show_result = _subprocess_safety.run_safely( + [terraform, "show", "-json", plan_path], cwd=working_dir, timeout=30 + ) + if show_result.returncode != 0: + return ToolResult( + success=False, + summary="(real, failed) terraform show -json failed", + details={ + "returncode": show_result.returncode, + "stderr": show_result.stderr, + }, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + try: + plan_json = json.loads(show_result.stdout) + except json.JSONDecodeError: + return ToolResult( + success=False, + summary="(real, failed) Could not parse terraform show -json output", + details={"error": "malformed JSON from terraform show -json"}, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + details = _details_from_plan_json(plan_json) + success = True + summary = ( + f"Plan: {details['create']} to add, {details['change']} to change, " + f"{details['replace']} to replace, {details['destroy']} to destroy." + ) + except (OSError, FileNotFoundError) as exc: + return ToolResult( + success=False, + summary=f"(real, failed) {exc}", + details={"error": str(exc)}, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) + + prefix = "(real) " if success else "(real, failed) " + return ToolResult( + success=success, + summary=prefix + summary, + details=details, + risk_level=spec.risk_level, + was_dry_run=dry_run, + approval=approval, + ) diff --git a/tests/explanations/test_service.py b/tests/explanations/test_service.py new file mode 100644 index 0000000..e69d3ef --- /dev/null +++ b/tests/explanations/test_service.py @@ -0,0 +1,49 @@ +from devops_learn.domain.enums import ExecutionMode, ExplanationDepth +from devops_learn.domain.explanation_models import LearningMoment +from devops_learn.explanations.service import ExplanationService + + +def _moment(**overrides: object) -> LearningMoment: + defaults: dict[str, object] = dict( + concept="Terraform state", + trigger="before init", + summary="State tracks what Terraform believes exists.", + deep_explanation="Full detail about state internals.", + why_it_matters="Understanding state matters for trustworthy plans.", + related_artifact="docs/terraform-state.md", + ) + defaults.update(overrides) + return LearningMoment(**defaults) # type: ignore[arg-type] + + +def test_related_artifact_is_rendered_at_learning_depth() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(), mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.LEARNING + ) + assert rendered is not None + assert "docs/terraform-state.md" in rendered + + +def test_related_artifact_is_omitted_below_learning_depth() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(), mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.NORMAL + ) + assert rendered is not None + assert "docs/terraform-state.md" not in rendered + + +def test_related_artifact_absent_when_not_set() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(related_artifact=None), + mode=ExecutionMode.COLLABORATIVE, + depth=ExplanationDepth.DEEP, + ) + assert rendered is not None + assert "See:" not in rendered + + +def test_learning_moment_suppressed_in_autonomous_mode_below_deep_depth() -> None: + rendered = ExplanationService().render_learning_moment( + _moment(), mode=ExecutionMode.AUTONOMOUS, depth=ExplanationDepth.LEARNING + ) + assert rendered is None diff --git a/tests/fixtures/fake_terraform/terraform b/tests/fixtures/fake_terraform/terraform new file mode 100755 index 0000000..329d163 --- /dev/null +++ b/tests/fixtures/fake_terraform/terraform @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +"""A fake `terraform` binary for tests/tools/test_real_terraform_tool.py. + +Behavior is selected by the FAKE_TERRAFORM_SCENARIO env var so a single +script covers every scenario instead of one file per case. Never touches +real infrastructure; only prints canned output and writes a placeholder +plan file when asked to `-out=`. +""" +from __future__ import annotations + +import json +import os +import sys +import time + +SCENARIO = os.environ.get("FAKE_TERRAFORM_SCENARIO", "success") + + +def _plan_out_path(args: list[str]) -> str | None: + for arg in args: + if arg.startswith("-out="): + return arg[len("-out="):] + return None + + +def main() -> int: + args = sys.argv[1:] + if not args: + return 1 + subcommand = args[0] + + if SCENARIO == "timeout": + time.sleep(5) + return 0 + + if subcommand == "fmt": + if SCENARIO == "fmt_needs_formatting": + print("main.tf\n+ location = \"eastus\"", file=sys.stdout) + return 3 + return 0 + + if subcommand == "init": + if SCENARIO == "init_failure": + print("Error: Failed to install provider", file=sys.stderr) + return 1 + print("Terraform has been successfully initialized!") + return 0 + + if subcommand == "validate": + if SCENARIO == "validate_failure": + print(json.dumps({ + "valid": False, + "diagnostics": [{"severity": "error", "summary": "Missing required argument"}], + })) + return 1 + print(json.dumps({"valid": True, "diagnostics": []})) + return 0 + + if subcommand == "plan": + out_path = _plan_out_path(args) + if SCENARIO == "plan_failure_with_secret": + # Mirrors real `terraform plan` output observed with no Azure credentials: + # a genuine "az login"/"Authorizer" auth failure, with a secret-shaped + # value mixed in to also exercise redaction. + print( + "Error: building AzureRM Client: obtain a token using " + "ARM_CLIENT_SECRET=super-secret-value-123 failed: could not configure " + "AzureCli Authorizer: obtaining tenant ID: running Azure CLI: exit status 1: " + "ERROR: Please run 'az login' to setup account.", + file=sys.stderr, + ) + return 1 + if out_path: + with open(out_path, "wb") as handle: + handle.write(b"fake-plan-binary-content") + return 0 + + if subcommand == "show": + if SCENARIO == "show_json_malformed": + print("{not valid json") + return 0 + if SCENARIO == "plan_with_sensitive_attribute": + print(json.dumps({ + "resource_changes": [ + { + "address": "azurerm_resource_group.main", + "change": {"actions": ["create"]}, + }, + { + "address": "azurerm_key_vault_secret.admin", + "change": { + "actions": ["create"], + "after": {"admin_password": "hunter2"}, + }, + }, + ] + })) + return 0 + if SCENARIO == "plan_mixed_actions": + print(json.dumps({ + "resource_changes": [ + {"address": "azurerm_resource_group.main", + "change": {"actions": ["create"]}}, + {"address": "azurerm_container_registry.main", + "change": {"actions": ["update"]}}, + {"address": "azurerm_log_analytics_workspace.main", + "change": {"actions": ["delete", "create"]}}, + {"address": "azurerm_storage_account.old", + "change": {"actions": ["delete"]}}, + {"address": "azurerm_resource_group.untouched", + "change": {"actions": ["no-op"]}}, + ] + })) + return 0 + # default "success" scenario: two creates + print(json.dumps({ + "resource_changes": [ + {"address": "azurerm_resource_group.main", + "change": {"actions": ["create"]}}, + {"address": "azurerm_container_registry.main", + "change": {"actions": ["create"]}}, + ] + })) + return 0 + + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/tools/test_real_terraform_tool.py b/tests/tools/test_real_terraform_tool.py new file mode 100644 index 0000000..24ec66a --- /dev/null +++ b/tests/tools/test_real_terraform_tool.py @@ -0,0 +1,189 @@ +import json +import os +from pathlib import Path + +import pytest + +from devops_learn.tools.base import RiskLevel +from devops_learn.tools.terraform_tool import RealTerraformTool +from devops_learn.validation import terraform_plan_analysis + +FIXTURES_DIR = Path(__file__).resolve().parents[1] / "fixtures" / "fake_terraform" + + +def _use_fake_terraform(monkeypatch, scenario: str = "success") -> None: + monkeypatch.setenv("PATH", f"{FIXTURES_DIR}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("FAKE_TERRAFORM_SCENARIO", scenario) + + +def test_real_terraform_tool_has_safe_operations() -> None: + tool = RealTerraformTool() + assert tool.name == "terraform" + names = {spec.name for spec in tool.operations} + assert names == {"fmt", "init", "validate", "plan"} + for spec in tool.operations: + assert spec.risk_level is RiskLevel.SAFE + assert not spec.requires_approval + + +def test_real_terraform_tool_does_not_expose_apply_or_destroy() -> None: + tool = RealTerraformTool() + with pytest.raises(KeyError): + tool.spec_for("apply_approved_plan") + with pytest.raises(KeyError): + tool.spec_for("destroy_approved_environment") + + +def test_missing_binary_reports_failure(monkeypatch, tmp_path) -> None: + monkeypatch.setenv("PATH", str(tmp_path / "empty-path")) + (tmp_path / "empty-path").mkdir() + result = RealTerraformTool().execute( + "fmt", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "Terraform CLI not found" in result.summary + + +def test_fmt_success(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "fmt", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "(real)" in result.summary + + +def test_fmt_needs_formatting(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "fmt_needs_formatting") + result = RealTerraformTool().execute( + "fmt", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "needs formatting" in result.summary.lower() + + +def test_init_success(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "init", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "(real)" in result.summary + + +def test_init_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "init_failure") + result = RealTerraformTool().execute( + "init", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "(real, failed)" in result.summary + + +def test_validate_success(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "validate", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "valid" in result.summary.lower() + + +def test_validate_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "validate_failure") + result = RealTerraformTool().execute( + "validate", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert result.details["diagnostic_count"] == 1 + + +def test_plan_success_produces_classifier_shaped_details(monkeypatch, tmp_path) -> None: + """Proves the real plan output needs zero changes to + validation/terraform_plan_analysis.analyze().""" + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert result.details.keys() >= {"create", "change", "replace", "destroy"} + assert result.details["create"] == 2 + + summary = terraform_plan_analysis.analyze(result.details) + assert summary.risk_level is RiskLevel.SAFE + + +def test_plan_mixed_actions_classified_correctly(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "plan_mixed_actions") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert result.details["create"] == 1 + assert result.details["change"] == 1 + assert result.details["replace"] == 1 + assert result.details["destroy"] == 1 + + summary = terraform_plan_analysis.analyze(result.details) + assert summary.risk_level is RiskLevel.DESTRUCTIVE + + +def test_plan_failure_is_reported_and_secret_is_redacted(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "plan_failure_with_secret") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + dumped = json.dumps(result.details) + assert "super-secret-value-123" not in dumped + assert "ARM_CLIENT_SECRET=" in dumped + + +def test_plan_show_json_malformed_is_a_clean_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "show_json_malformed") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "Could not parse" in result.summary + + +def test_plan_details_never_include_raw_attribute_values(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "plan_with_sensitive_attribute") + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "hunter2" not in json.dumps(result.details) + + +def test_timeout_is_reported_not_hung(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch, "timeout") + result = RealTerraformTool().execute( + "fmt", + {"path": str(tmp_path), "timeout_seconds": 1}, + dry_run=False, + approval=None, + ) + assert not result.success + assert "timed out" in result.details.get("stderr", "").lower() + + +def test_invalid_working_directory_reports_failure(monkeypatch, tmp_path) -> None: + _use_fake_terraform(monkeypatch) + result = RealTerraformTool().execute( + "fmt", + {"path": str(tmp_path / "does-not-exist")}, + dry_run=False, + approval=None, + ) + assert not result.success + + +def test_dry_run_never_invokes_subprocess(monkeypatch, tmp_path) -> None: + monkeypatch.setenv("PATH", str(tmp_path)) # empty dir: would fail loudly if invoked + result = RealTerraformTool().execute( + "plan", {"path": str(tmp_path)}, dry_run=True, approval=None + ) + assert result.success + assert "dry run" in result.summary.lower() diff --git a/tests/tools/test_subprocess_safety.py b/tests/tools/test_subprocess_safety.py new file mode 100644 index 0000000..af9e108 --- /dev/null +++ b/tests/tools/test_subprocess_safety.py @@ -0,0 +1,52 @@ +from devops_learn.tools import _subprocess_safety + + +def test_redact_masks_secret_shaped_key_value_pairs() -> None: + text = "Error: ARM_CLIENT_SECRET=abc123XYZ failed to authenticate" + redacted = _subprocess_safety.redact(text) + assert "abc123XYZ" not in redacted + assert "ARM_CLIENT_SECRET=" in redacted + + +def test_redact_leaves_non_secret_text_untouched() -> None: + text = "Plan: 2 to add, 0 to change, 0 to destroy." + assert _subprocess_safety.redact(text) == text + + +def test_truncate_caps_long_output_with_a_visible_marker() -> None: + long_text = "x" * (_subprocess_safety.MAX_OUTPUT_CHARS + 500) + result = _subprocess_safety._truncate(long_text) + assert len(result) < len(long_text) + assert "truncated" in result + + +def test_run_safely_returns_sanitized_result_on_success() -> None: + result = _subprocess_safety.run_safely( + ["python3", "-c", "print('hello')"], cwd=None, timeout=10 + ) + assert result.returncode == 0 + assert "hello" in result.stdout + assert not result.timed_out + + +def test_run_safely_reports_timeout_without_hanging() -> None: + result = _subprocess_safety.run_safely( + ["python3", "-c", "import time; time.sleep(5)"], cwd=None, timeout=1 + ) + assert result.timed_out + assert result.returncode != 0 + assert "timed out" in result.stderr.lower() + + +def test_run_safely_redacts_secret_shaped_stderr() -> None: + result = _subprocess_safety.run_safely( + [ + "python3", + "-c", + "import sys; sys.stderr.write('API_KEY=super-secret-value')", + ], + cwd=None, + timeout=10, + ) + assert "super-secret-value" not in result.stderr + assert "API_KEY=" in result.stderr From 814fdb592d5e179bbd4ab0d053aa9809f97364fd Mon Sep 17 00:00:00 2001 From: howlcipher Date: Wed, 12 Aug 2026 16:55:04 -0400 Subject: [PATCH 2/8] Add devops-learn terraform command and its vertical-slice workflow workflows/terraform_flow.py runs fmt -> init -> validate -> plan -> risk analysis, teaching provider/state concepts just-in-time (gated by learner profile, matching local_flow's precedent) and diagnosing an auth failure on plan as an expected, honest outcome rather than a bug. cli/main.py wires RealTerraformTool in only for this command, keeping every other command's terraform tool simulated. Co-Authored-By: Claude Sonnet 5 --- src/devops_learn/cli/commands/terraform.py | 44 +++ src/devops_learn/cli/main.py | 31 +- src/devops_learn/workflows/terraform_flow.py | 360 +++++++++++++++++++ tests/workflows/test_terraform_flow.py | 165 +++++++++ 4 files changed, 595 insertions(+), 5 deletions(-) create mode 100644 src/devops_learn/cli/commands/terraform.py create mode 100644 src/devops_learn/workflows/terraform_flow.py create mode 100644 tests/workflows/test_terraform_flow.py diff --git a/src/devops_learn/cli/commands/terraform.py b/src/devops_learn/cli/commands/terraform.py new file mode 100644 index 0000000..f48c5e3 --- /dev/null +++ b/src/devops_learn/cli/commands/terraform.py @@ -0,0 +1,44 @@ +"""`devops-learn terraform [path]`: run the real Terraform vertical slice. + +Runs real fmt/init/validate/plan against /infra/terraform via +RealTerraformTool. This milestone ships real execution only against the +bundled projects/api_platform sample; generating configuration for arbitrary +projects is future work (see docs/roadmap.md). +""" + +from __future__ import annotations + +import argparse +from pathlib import Path + +from devops_learn.bootstrap import Platform +from devops_learn.cli.terminal_ui import TerminalUi +from devops_learn.workflows.terraform_flow import TerraformOptions, run_terraform_flow + + +def register(subparsers: "argparse._SubParsersAction[argparse.ArgumentParser]") -> None: + parser = subparsers.add_parser( + "terraform", + help="Run the real Terraform vertical slice: fmt, init, validate, plan, risk analysis", + ) + parser.add_argument( + "path", + nargs="?", + default="projects/api_platform", + help="Project containing infra/terraform/ (defaults to the bundled sample project)", + ) + parser.set_defaults(handler=run) + + +def run(args: argparse.Namespace, platform: Platform) -> None: + infra_root = f"{args.path}/infra/terraform" + if not Path(infra_root).is_dir(): + print(f"No Terraform configuration found at {infra_root}.") + print( + "This milestone ships real Terraform execution only against the bundled " + "projects/api_platform sample; generating configuration for arbitrary projects " + "is future work (see docs/roadmap.md)." + ) + return + options = TerraformOptions(project_root=args.path, infra_root=infra_root) + run_terraform_flow(platform, TerminalUi(), options) diff --git a/src/devops_learn/cli/main.py b/src/devops_learn/cli/main.py index 9c669ee..80b7289 100644 --- a/src/devops_learn/cli/main.py +++ b/src/devops_learn/cli/main.py @@ -7,21 +7,30 @@ from devops_learn.ai.anthropic_provider import AnthropicProvider from devops_learn.bootstrap import build_platform -from devops_learn.cli.commands import analyze, explain, history, init, local, profile, review +from devops_learn.cli.commands import ( + analyze, + explain, + history, + init, + local, + profile, + review, + terraform, +) from devops_learn.config.settings import load_settings from devops_learn.domain.enums import ExecutionMode from devops_learn.learning.persistence.connection import connect from devops_learn.tools.approval import ApprovalGate, CliApprovalGate, ThresholdApprovalGate from devops_learn.tools.base import RiskLevel, Tool -from devops_learn.tools.docker_tool import RealDockerTool +from devops_learn.tools.docker_tool import RealDockerTool, SimulatedDockerTool from devops_learn.tools.git_tool import SimulatedGitTool from devops_learn.tools.kubernetes_tool import SimulatedKubernetesTool -from devops_learn.tools.python_tool import RealPythonTool -from devops_learn.tools.terraform_tool import SimulatedTerraformTool +from devops_learn.tools.python_tool import RealPythonTool, SimulatedPythonTool +from devops_learn.tools.terraform_tool import RealTerraformTool, SimulatedTerraformTool from devops_learn.tools.validation_tool import SimulatedValidationTool from devops_learn.tools.cloud_tool import SimulatedCloudTool -_COMMAND_MODULES = (analyze, review, history, explain, profile, init, local) +_COMMAND_MODULES = (analyze, review, history, explain, profile, init, local, terraform) def build_parser() -> argparse.ArgumentParser: @@ -69,6 +78,18 @@ def _approval_gate_for_args(args: argparse.Namespace) -> ApprovalGate: def _tools_for_args(args: argparse.Namespace) -> dict[str, Tool] | None: + if args.command == "terraform": + # This command only ever invokes the "terraform" tool -- keep the rest + # simulated rather than provisioning real python/docker tools it never uses. + return { + "python": SimulatedPythonTool(), + "git": SimulatedGitTool(), + "docker": SimulatedDockerTool(), + "terraform": RealTerraformTool(), + "kubernetes": SimulatedKubernetesTool(), + "cloud": SimulatedCloudTool(), + "validation": SimulatedValidationTool(), + } if getattr(args, "real_tools", False) or args.command == "local": return { "python": RealPythonTool(), diff --git a/src/devops_learn/workflows/terraform_flow.py b/src/devops_learn/workflows/terraform_flow.py new file mode 100644 index 0000000..b6330f1 --- /dev/null +++ b/src/devops_learn/workflows/terraform_flow.py @@ -0,0 +1,360 @@ +"""Real Terraform vertical slice: fmt -> init -> validate -> plan -> risk analysis. + +Runs RealTerraformTool against a real, committed Terraform working directory +(projects/api_platform/infra/terraform/ by default). Never applies or +destroys -- see docs/roadmap.md (Milestone 3) and docs/safety.md. A plan +failure due to missing Azure credentials is treated as an expected, honest +outcome to teach from, not something to hide. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime, timezone +from typing import Any + +from devops_learn.bootstrap import Platform +from devops_learn.domain.enums import ( + AuditEventType, + CloudProviderKind, + CostPriority, + EnvironmentKind, + ExecutionMode, + ExperienceState, + ExplanationDepth, +) +from devops_learn.domain.explanation_models import Explanation, LearningMoment +from devops_learn.domain.learner_profile_models import CompetencyArea +from devops_learn.domain.plan_models import TerraformPlanSummary +from devops_learn.domain.session_models import EngagementSession +from devops_learn.tools.approval import RiskLevel +from devops_learn.tools.base import ToolResult +from devops_learn.validation import terraform_plan_analysis +from devops_learn.workflows import presenters +from devops_learn.workflows.ui import Ui + + +@dataclass(frozen=True) +class TerraformOptions: + project_root: str + infra_root: str + + +def run_terraform_flow( + platform: Platform, ui: Ui, options: TerraformOptions +) -> EngagementSession: + session = platform.session_service.start( + project_root=options.project_root, + mode=ExecutionMode.COLLABORATIVE, + explanation_depth=ExplanationDepth.LEARNING, + cloud=CloudProviderKind.AZURE, + environment=EnvironmentKind.DEV, + cost_priority=CostPriority.BALANCED, + simulation_mode=False, + ) + assert session.id is not None + session_id = session.id + + _audit(platform, session_id, AuditEventType.SESSION_STARTED, "Terraform workflow started") + + fmt_result = _run_terraform_step( + platform, ui, session_id, "fmt", options.infra_root, "Terraform fmt" + ) + if not fmt_result.success: + ui.present( + f"Configuration is not formatted. Run `terraform fmt` in {options.infra_root} " + "to fix this before continuing." + ) + + _teach_provider_concept(platform, ui, session_id) + init_result = _run_terraform_step( + platform, ui, session_id, "init", options.infra_root, "Terraform init" + ) + if not init_result.success: + _diagnose_and_stop(platform, ui, session_id, "terraform init failed") + platform.session_service.complete(session) + return session + + validate_result = _run_terraform_step( + platform, ui, session_id, "validate", options.infra_root, "Terraform validate" + ) + if not validate_result.success: + _diagnose_and_stop(platform, ui, session_id, "terraform validate failed") + platform.session_service.complete(session) + return session + + _teach_state_concepts(platform, ui, session_id) + _audit(platform, session_id, AuditEventType.TERRAFORM_PLAN_STARTED, "Terraform plan started") + plan_result = _run_terraform_step( + platform, ui, session_id, "plan", options.infra_root, "Terraform plan" + ) + if not plan_result.success: + _diagnose_plan_failure(platform, ui, session_id, plan_result) + platform.session_service.complete(session) + return session + + summary = terraform_plan_analysis.analyze(plan_result.details) + ui.present(presenters.render_terraform_plan(summary)) + _audit(platform, session_id, AuditEventType.TERRAFORM_PLAN_COMPLETED, summary.reason) + _explain(platform, ui, "What to check", _what_to_check_explanation(summary)) + + platform.experience_tracker.record( + session_id, "Terraform", "Ran real fmt/init/validate/plan", ExperienceState.PRACTICED + ) + platform.experience_tracker.record( + session_id, "Azure", "Reviewed a real Azure resource plan", ExperienceState.INTRODUCED + ) + + _print_history_and_experience(platform, ui, session_id) + platform.session_service.complete(session) + return session + + +def _audit( + platform: Platform, + session_id: int, + event_type: AuditEventType, + summary: str, + payload: dict[str, Any] | None = None, +) -> None: + platform.audit_service.record( + session_id=session_id, + event_type=event_type, + occurred_at=_now(), + summary=summary, + payload=payload or {}, + ) + + +def _run_terraform_step( + platform: Platform, + ui: Ui, + session_id: int, + operation: str, + infra_root: str, + label: str, +) -> ToolResult: + _explain(platform, ui, label, _terraform_explanation(operation)) + result = platform.tool_service.invoke("terraform", operation, {"path": infra_root}) + ui.present(result.summary) + _audit( + platform, + session_id, + AuditEventType.TOOL_INVOKED, + f"terraform.{operation}", + {"success": result.success}, + ) + return result + + +def _diagnose_and_stop( + platform: Platform, ui: Ui, session_id: int, context: str +) -> None: + ui.present(f"{context}. Diagnosis starts from the output above.") + _audit(platform, session_id, AuditEventType.DEPLOYMENT_FAILED, context) + platform.experience_tracker.record( + session_id, "Troubleshooting", f"Investigated {context}", ExperienceState.INTRODUCED + ) + + +def _diagnose_plan_failure( + platform: Platform, ui: Ui, session_id: int, result: ToolResult +) -> None: + stderr = str(result.details.get("stderr", "")).lower() + is_auth_failure = "az login" in stderr or "authoriz" in stderr or "authent" in stderr + if is_auth_failure: + ui.present( + "DIAGNOSIS: terraform plan failed because Terraform could not authenticate to " + "Azure. The azurerm provider needs credentials to build API requests, even for a " + "read-only plan.\n\n" + "Fix by doing ONE of:\n" + " - `az login` (uses your interactive Azure CLI session), or\n" + " - setting ARM_CLIENT_ID, ARM_CLIENT_SECRET, ARM_TENANT_ID, and " + "ARM_SUBSCRIPTION_ID for a service principal.\n\n" + "This is an expected, honest failure on a machine with no Azure credentials -- " + "not a bug. See docs/safety.md." + ) + else: + ui.present("terraform plan failed. Diagnosis starts from the output above.") + _audit(platform, session_id, AuditEventType.DEPLOYMENT_FAILED, "terraform plan failed") + platform.experience_tracker.record( + session_id, + "Troubleshooting", + "Diagnosed a terraform plan authentication failure" + if is_auth_failure + else "Diagnosed a terraform plan failure", + ExperienceState.PRACTICED, + ) + + +def _should_teach(platform: Platform) -> bool: + profile = platform.learner_profile_service.load() + return not profile.is_strong(CompetencyArea.TERRAFORM) + + +def _teach_state_concepts(platform: Platform, ui: Ui, session_id: int) -> None: + if not _should_teach(platform): + return + moment = LearningMoment( + concept="Terraform state", + trigger="before running terraform plan", + summary=( + "Terraform tracks the infrastructure it manages in a state file. State is " + "Terraform's record of what it believes exists; the real Azure resources are the " + "ground truth, and the two can drift apart." + ), + deep_explanation=( + "Every managed resource gets an address (e.g. azurerm_resource_group.main) " + "recorded in state alongside the real Azure resource ID it maps to. Terraform " + "diffs your configuration against state, then confirms that against Azure during " + "plan/apply. State can contain resource attribute values, including sensitive " + "ones, which is why it is never committed to git (see .gitignore) -- and why " + "deleting a state file does NOT delete the real Azure resources; it only makes " + "Terraform forget about them." + ), + why_it_matters=( + "Understanding state is the difference between reading a `terraform plan` and " + "actually knowing what it will do." + ), + related_artifact="docs/terraform-state.md", + ) + _render_learning_moment(platform, ui, moment) + platform.experience_tracker.record( + session_id, + "Terraform state", + "Explained desired vs. actual state", + ExperienceState.INTRODUCED, + ) + + +def _teach_provider_concept(platform: Platform, ui: Ui, session_id: int) -> None: + if not _should_teach(platform): + return + moment = LearningMoment( + concept="Terraform provider", + trigger="before running terraform init", + summary=( + "A provider is the plugin Terraform uses to talk to a specific platform -- here, " + "the azurerm provider talks to Azure's Resource Manager API. `terraform init` " + "downloads it." + ), + deep_explanation=( + "A resource block's type prefix (azurerm_) tells Terraform which provider owns it " + "and which real API calls to make. A resource address like " + "azurerm_resource_group.main combines the resource type and a local name; " + "Terraform uses addresses to track dependencies between resources, such as the " + "container registry needing the resource group to exist first." + ), + why_it_matters=( + "init failing almost always means a provider or network problem, not a mistake in " + "your .tf files -- a different failure mode than validate or plan." + ), + related_artifact="docs/terraform-state.md", + ) + _render_learning_moment(platform, ui, moment) + platform.experience_tracker.record( + session_id, + "Terraform", + "Explained provider and resource address", + ExperienceState.INTRODUCED, + ) + + +def _render_learning_moment(platform: Platform, ui: Ui, moment: LearningMoment) -> None: + rendered = platform.explanation_service.render_learning_moment( + moment, mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.LEARNING + ) + if rendered: + ui.present(rendered) + + +def _explain(platform: Platform, ui: Ui, label: str, explanation: Explanation) -> None: + rendered = platform.explanation_service.render( + explanation, mode=ExecutionMode.COLLABORATIVE, depth=ExplanationDepth.LEARNING + ) + ui.present(rendered) + + +def _terraform_explanation(operation: str) -> Explanation: + if operation == "fmt": + return Explanation( + action="Checking Terraform formatting.", + why="Consistent formatting makes configuration easy to review and diff.", + ) + if operation == "init": + return Explanation( + action="Initializing the Terraform working directory.", + why=( + "Terraform must download the azurerm provider before it can validate, plan, " + "or apply anything." + ), + what_to_understand=( + "init also creates a local .terraform/ plugin cache, which is never committed " + "to git -- it is regenerated by running init again." + ), + ) + if operation == "validate": + return Explanation( + action="Validating the configuration.", + why="Validate catches syntax and type errors locally, before Terraform talks to Azure.", + ) + return Explanation( + action="Running terraform plan.", + why="Plan shows exactly what Terraform would change, without changing anything yet.", + what_to_understand=( + "Plan compares your configuration against Terraform's state file, then confirms " + "that against Azure. It requires Azure authentication even though it makes no " + "changes." + ), + ) + + +def _what_to_check_explanation(summary: TerraformPlanSummary) -> Explanation: + if summary.risk_level is RiskLevel.DESTRUCTIVE: + result = ( + "This plan would destroy real resources. Never approve an apply from a plan like " + "this without independently confirming that destruction is intended." + ) + elif summary.risk_level is RiskLevel.HIGH: + result = ( + "Terraform must destroy and recreate at least one resource. Confirm you " + "understand *why* before ever approving an apply: is the resource stateful, " + "would its address or endpoint change, and would that cause downtime?" + ) + elif summary.risk_level is RiskLevel.LOW: + result = "Confirm the in-place changes match what you intended to modify." + else: + result = ( + "Confirm the resource names in this plan won't collide with anything already in " + "the target subscription." + ) + return Explanation( + action="Reviewing what this plan means before any apply.", + what_to_understand=( + "Terraform state can drift from real infrastructure if something changes " + "resources outside Terraform. A plan is only trustworthy if state and Azure " + "haven't drifted apart." + ), + result=result, + ) + + +def _print_history_and_experience(platform: Platform, ui: Ui, session_id: int) -> None: + events = platform.audit_service.history(session_id) + lines = ["AUDIT HISTORY", ""] + for event in events: + lines.append(f"{event.sequence_no:>3}. {event.event_type.value}: {event.summary}") + ui.present("\n".join(lines)) + + grouped = platform.experience_tracker.summary(session_id) + lines = ["EXPERIENCE SUMMARY", ""] + for concept, entries in grouped.items(): + lines.append(concept) + for entry in entries: + lines.append(f" ✓ {entry.item}") + lines.append("") + ui.present("\n".join(lines).rstrip()) + + +def _now() -> datetime: + return datetime.now(timezone.utc) diff --git a/tests/workflows/test_terraform_flow.py b/tests/workflows/test_terraform_flow.py new file mode 100644 index 0000000..e8f7f9f --- /dev/null +++ b/tests/workflows/test_terraform_flow.py @@ -0,0 +1,165 @@ +import os +import sqlite3 +from pathlib import Path + +from devops_learn.bootstrap import build_platform +from devops_learn.domain.enums import AuditEventType +from devops_learn.domain.learner_profile_models import ( + CompetencyArea, + LearnerProfile, + ProficiencyLevel, +) +from devops_learn.domain.question_models import ClarifyingQuestion +from devops_learn.tools.approval import AutoApproveApprovalGate +from devops_learn.tools.terraform_tool import RealTerraformTool +from devops_learn.workflows.terraform_flow import TerraformOptions, run_terraform_flow +from devops_learn.workflows.ui import Ui + +FIXTURES_DIR = Path(__file__).resolve().parents[1] / "fixtures" / "fake_terraform" + + +class ScriptedUi(Ui): + def __init__(self) -> None: + self.presented: list[str] = [] + + def present(self, text: str) -> None: + self.presented.append(text) + + def ask_choice(self, question: ClarifyingQuestion) -> str: + return question.options[0] if question.options else "yes" + + def confirm(self, prompt: str, *, default: bool = False) -> bool: + return True + + +def _platform(conn: sqlite3.Connection): + return build_platform( + conn, tools={"terraform": RealTerraformTool()}, approval_gate=AutoApproveApprovalGate() + ) + + +def _use_fake_terraform(monkeypatch, scenario: str) -> None: + monkeypatch.setenv("PATH", f"{FIXTURES_DIR}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("FAKE_TERRAFORM_SCENARIO", scenario) + + +def test_successful_plan_reaches_risk_analysis_and_records_experience( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "success") + platform = _platform(conn) + ui = ScriptedUi() + session = run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + assert session.id is not None + + joined = "\n".join(ui.presented) + assert "TERRAFORM PLAN" in joined + assert "Risk: SAFE" in joined + + events = {e.event_type for e in platform.audit_service.history(session.id)} + assert AuditEventType.TERRAFORM_PLAN_COMPLETED in events + assert AuditEventType.DEPLOYMENT_FAILED not in events + + experience = platform.experience_tracker.summary(session.id) + assert "Terraform" in experience + assert "Azure" in experience + + +def test_plan_auth_failure_produces_credential_diagnosis( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "plan_failure_with_secret") + platform = _platform(conn) + ui = ScriptedUi() + session = run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + assert session.id is not None + + joined = "\n".join(ui.presented) + assert "az login" in joined + assert "ARM_CLIENT_SECRET" in joined + assert "super-secret-value-123" not in joined # redacted before ever reaching the UI + + events = {e.event_type for e in platform.audit_service.history(session.id)} + assert AuditEventType.DEPLOYMENT_FAILED in events + assert AuditEventType.TERRAFORM_PLAN_COMPLETED not in events + + +def test_generic_plan_failure_does_not_claim_an_auth_problem( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "show_json_malformed") + platform = _platform(conn) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + + joined = "\n".join(ui.presented) + assert "az login" not in joined + assert "Diagnosis starts from the output above" in joined + + +def test_init_failure_stops_before_plan( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "init_failure") + platform = _platform(conn) + ui = ScriptedUi() + session = run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + assert session.id is not None + + events = {e.event_type for e in platform.audit_service.history(session.id)} + assert AuditEventType.DEPLOYMENT_FAILED in events + assert AuditEventType.TERRAFORM_PLAN_STARTED not in events + + +def test_beginner_learner_sees_teaching_moments( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "success") + platform = _platform(conn) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + joined = "\n".join(ui.presented) + assert "LEARNING MOMENT: Terraform provider" in joined + assert "LEARNING MOMENT: Terraform state" in joined + + +def test_learner_strong_in_terraform_sees_no_teaching_moments( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "success") + platform = _platform(conn) + platform.learner_profile_service.save( + LearnerProfile( + proficiencies={CompetencyArea.TERRAFORM: ProficiencyLevel.STRONG}, + learning_focus=(), + ) + ) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + joined = "\n".join(ui.presented) + assert "LEARNING MOMENT" not in joined + + +def test_mixed_actions_plan_is_classified_high_risk( + conn: sqlite3.Connection, monkeypatch, tmp_path +) -> None: + _use_fake_terraform(monkeypatch, "plan_mixed_actions") + platform = _platform(conn) + ui = ScriptedUi() + run_terraform_flow( + platform, ui, TerraformOptions(project_root=str(tmp_path), infra_root=str(tmp_path)) + ) + joined = "\n".join(ui.presented) + assert "Risk: DESTRUCTIVE" in joined From 47045970eafa3c60662dbff494afeaf50980ce56 Mon Sep 17 00:00:00 2001 From: howlcipher Date: Wed, 12 Aug 2026 16:55:16 -0400 Subject: [PATCH 3/8] Add real Terraform config for the bundled sample project projects/api_platform/infra/terraform/: resource group, container registry, Log Analytics workspace, tagged for easy identification and cleanup. Split into providers/variables/main/outputs.tf as a readability convention (Terraform still merges the whole directory). Fixes an ACR naming bug (hyphens aren't valid in that resource's name, unlike most others) caught by live terraform validate during review. .gitignore now excludes .terraform/ and *.tfstate* while keeping .terraform.lock.hcl committed. ProjectAnalyzer correctly now detects this real IaC. Co-Authored-By: Claude Sonnet 5 --- .gitignore | 17 ++++++++ .../infra/terraform/.terraform.lock.hcl | 22 ++++++++++ .../api_platform/infra/terraform/README.md | 34 +++++++++++++++ projects/api_platform/infra/terraform/main.tf | 42 +++++++++++++++++++ .../api_platform/infra/terraform/outputs.tf | 7 ++++ .../api_platform/infra/terraform/providers.tf | 18 ++++++++ .../api_platform/infra/terraform/variables.tf | 11 +++++ tests/analysis/test_project_analyzer.py | 4 +- 8 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 projects/api_platform/infra/terraform/.terraform.lock.hcl create mode 100644 projects/api_platform/infra/terraform/README.md create mode 100644 projects/api_platform/infra/terraform/main.tf create mode 100644 projects/api_platform/infra/terraform/outputs.tf create mode 100644 projects/api_platform/infra/terraform/providers.tf create mode 100644 projects/api_platform/infra/terraform/variables.tf diff --git a/.gitignore b/.gitignore index e6c3749..01305ff 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,20 @@ htmlcov/ *.sqlite3 .env .devops_learn/ + +# Terraform working-directory artifacts (see projects/api_platform/infra/terraform/). +# .terraform.lock.hcl is deliberately NOT listed here: it pins exact provider +# versions/hashes so `terraform init` is reproducible across machines and CI, +# which is normal, recommended Terraform practice -- unlike .terraform/ (a +# regenerable local plugin cache) and *.tfstate (can contain sensitive values, +# and since this milestone never applies, would only ever hold stale/empty +# state if committed). +**/.terraform/ +*.tfstate +*.tfstate.* +crash.log +crash.*.log +override.tf +override.tf.json +*_override.tf +*_override.tf.json diff --git a/projects/api_platform/infra/terraform/.terraform.lock.hcl b/projects/api_platform/infra/terraform/.terraform.lock.hcl new file mode 100644 index 0000000..eb0286e --- /dev/null +++ b/projects/api_platform/infra/terraform/.terraform.lock.hcl @@ -0,0 +1,22 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/azurerm" { + version = "3.117.1" + constraints = "~> 3.0" + hashes = [ + "h1:3c9iOEtBMnHrpJLlhbQ0sCZPWhE/2dvEPcL8KkXAh7w=", + "zh:0c513676836e3c50d004ece7d2624a8aff6faac14b833b96feeac2e4bc2c1c12", + "zh:50ea01ada95bae2f187db9e926e463f45d860767a85ebc59160414e00e76c35d", + "zh:52c2a9edacc06b3f72153f5ef6daca0761c6292158815961fe37f60bc576a3d7", + "zh:618eed2a06b19b1a025b45b05891846d570a6a1cca4d23f4942f5a99e1f747ae", + "zh:61cde5d3165d7e5ec311d5d89486819cd605c1b2d54611b5c97bd4e97dba2762", + "zh:6a873358d5031fc222f5e05f029d1237f3dce8345c767665f393283dfa2627f6", + "zh:afdd80064b2a04da311856feb4ed45f77ff4df6c356e8c2b10afb51fe7e61c70", + "zh:b09113df7e0e8c8959539bd22bae6c39faeb269ba3c4cd948e742f5cf58c35fb", + "zh:d340db7973109761cfc27d52aa02560363337c908b2c99b3628adc5a70a99d5b", + "zh:d5a577226ebc8c65e8f19384878a86acc4b51ede4b4a82d37c3b331b0efcd4a7", + "zh:e2962b147f9e71732df8dbc74940c10d20906f3c003cbfaa1eb9fabbf601a9f0", + "zh:f569b65999264a9416862bca5cd2a6177d94ccb0424f3a4ef424428912b9cb3c", + ] +} diff --git a/projects/api_platform/infra/terraform/README.md b/projects/api_platform/infra/terraform/README.md new file mode 100644 index 0000000..d2b808f --- /dev/null +++ b/projects/api_platform/infra/terraform/README.md @@ -0,0 +1,34 @@ +# infra/terraform + +This is a real, valid Terraform configuration for a minimal Azure footprint: +a resource group, a container registry, and a Log Analytics workspace. + +## Files in this directory + +Terraform merges every `*.tf` file in a directory into a single +configuration. The split here (`providers.tf` / `variables.tf` / `main.tf` / +`outputs.tf`) is a naming convention for readability, not a module boundary +-- Terraform does not treat these as separate modules, and there is no +requirement to name files this way. + +## What runs for real + +`devops-learn terraform` runs real `fmt`, `init`, `validate`, and `plan` +against this directory via `RealTerraformTool` +(`src/devops_learn/tools/terraform_tool.py`). `init` downloads the `azurerm` +provider plugin (network access, no Azure credentials needed). `plan` +requires Azure authentication (`az login`, or `ARM_CLIENT_ID` / +`ARM_CLIENT_SECRET` / `ARM_TENANT_ID` / `ARM_SUBSCRIPTION_ID` environment +variables) to build the provider client -- without it, `plan` fails cleanly +and the workflow explains why rather than faking a result. + +`apply`/`destroy` are not implemented against this directory yet; see +`docs/roadmap.md` (Milestone 3) and `docs/terraform-state.md`. + +## Local state + +Running `init`/`plan` here creates `.terraform/` and, if this configuration +is ever applied, `terraform.tfstate*` -- both are gitignored (see +`docs/terraform-state.md` for why state isn't committed). +`.terraform.lock.hcl` is committed on purpose: it pins the exact provider +version/hash so `init` is reproducible on every machine. diff --git a/projects/api_platform/infra/terraform/main.tf b/projects/api_platform/infra/terraform/main.tf new file mode 100644 index 0000000..1ec84b1 --- /dev/null +++ b/projects/api_platform/infra/terraform/main.tf @@ -0,0 +1,42 @@ +# The resources this configuration declares: everything the learner sees +# planned/applied against Azure by `devops-learn terraform`. +# +# Tagged consistently so every resource this project creates is obvious in +# the Azure portal and easy to find and delete. + +locals { + tags = { + project = "devops-learn-by-doing" + environment = "learning" + managed-by = "terraform" + purpose = "devops-learning" + } + + # Azure Container Registry names must be alphanumeric only (no hyphens), + # unlike most other Azure resource names -- strip them here rather than + # relaxing var.project_name itself. + registry_name = replace(var.project_name, "-", "") +} + +resource "azurerm_resource_group" "main" { + name = "${var.project_name}-rg" + location = var.location + tags = local.tags +} + +resource "azurerm_container_registry" "main" { + name = "${local.registry_name}acr" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + sku = "Basic" + tags = local.tags +} + +resource "azurerm_log_analytics_workspace" "main" { + name = "${var.project_name}-logs" + resource_group_name = azurerm_resource_group.main.name + location = azurerm_resource_group.main.location + sku = "PerGB2018" + retention_in_days = 30 + tags = local.tags +} diff --git a/projects/api_platform/infra/terraform/outputs.tf b/projects/api_platform/infra/terraform/outputs.tf new file mode 100644 index 0000000..ea34d4c --- /dev/null +++ b/projects/api_platform/infra/terraform/outputs.tf @@ -0,0 +1,7 @@ +output "resource_group_name" { + value = azurerm_resource_group.main.name +} + +output "container_registry_login_server" { + value = azurerm_container_registry.main.login_server +} diff --git a/projects/api_platform/infra/terraform/providers.tf b/projects/api_platform/infra/terraform/providers.tf new file mode 100644 index 0000000..10afbfc --- /dev/null +++ b/projects/api_platform/infra/terraform/providers.tf @@ -0,0 +1,18 @@ +# Which platform Terraform talks to, and which version of it. +# +# Terraform merges every *.tf file in this directory into a single +# configuration -- providers.tf / variables.tf / main.tf / outputs.tf is a +# naming convention for readability, not a module boundary. See README.md. + +terraform { + required_providers { + azurerm = { + source = "hashicorp/azurerm" + version = "~> 3.0" + } + } +} + +provider "azurerm" { + features {} +} diff --git a/projects/api_platform/infra/terraform/variables.tf b/projects/api_platform/infra/terraform/variables.tf new file mode 100644 index 0000000..93feebe --- /dev/null +++ b/projects/api_platform/infra/terraform/variables.tf @@ -0,0 +1,11 @@ +variable "project_name" { + description = "Short name used to prefix every resource this configuration creates." + type = string + default = "api-platform" +} + +variable "location" { + description = "Azure region to create resources in." + type = string + default = "eastus" +} diff --git a/tests/analysis/test_project_analyzer.py b/tests/analysis/test_project_analyzer.py index a8f95cc..059298f 100644 --- a/tests/analysis/test_project_analyzer.py +++ b/tests/analysis/test_project_analyzer.py @@ -14,7 +14,9 @@ def test_analyzes_the_bundled_example_project() -> None: assert assessment.containerization_status is MaturityStatus.GOOD assert assessment.healthcheck_status is MaturityStatus.GOOD assert assessment.ci_cd_status is MaturityStatus.MISSING - assert assessment.iac_status is MaturityStatus.MISSING + # projects/api_platform/infra/terraform/ is real Terraform config as of Milestone 2 + # (docs/roadmap.md) -- ProjectAnalyzer correctly detects it. + assert assessment.iac_status is MaturityStatus.GOOD def test_detects_missing_everything_for_an_empty_project(tmp_path: Path) -> None: From 4e42692b09f4bd67ccedf2a62b6b928643d3d155 Mon Sep 17 00:00:00 2001 From: howlcipher Date: Wed, 12 Aug 2026 16:55:22 -0400 Subject: [PATCH 4/8] Add real Terraform checks to CI fmt -check, init -backend=false, and validate against projects/api_platform/infra/terraform/ -- no Azure credentials needed, no plan/apply in CI. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd2b278..5c2a466 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,3 +23,14 @@ jobs: run: cd projects/api_platform && mypy app --strict - name: Test run: pytest + - name: Setup Terraform + uses: hashicorp/setup-terraform@v3 + with: + terraform_version: "1.15.8" + terraform_wrapper: false + - name: Terraform fmt check + run: terraform fmt -check -recursive projects/api_platform/infra/terraform + - name: Terraform init (no backend, no credentials) + run: terraform -chdir=projects/api_platform/infra/terraform init -backend=false -input=false + - name: Terraform validate + run: terraform -chdir=projects/api_platform/infra/terraform validate From c049c419e1c54ce9e12dd768f4cdbaaf5f922ad5 Mon Sep 17 00:00:00 2001 From: howlcipher Date: Wed, 12 Aug 2026 16:55:33 -0400 Subject: [PATCH 5/8] Document Milestone 2 and teach Terraform state as a first-class concept docs/terraform-state.md is the reference the in-flow teaching moments link back to: desired vs. actual vs. Terraform state, provider, resource address, dependencies, drift, local vs. remote state and locking, state sensitivity, why deleting state isn't deleting infrastructure, why hand-editing state is dangerous. Updates README/roadmap/safety/ development docs to describe exactly what's real this milestone (fmt/ init/validate/plan, no apply, no real Azure execution) and documents the RUN_AZURE_INTEGRATION_TESTS convention Milestone 3 will need. Co-Authored-By: Claude Sonnet 5 --- CONTRIBUTING.md | 11 +++-- README.md | 14 +++++- docs/development.md | 17 +++++++- docs/learning-model.md | 6 +++ docs/roadmap.md | 23 ++++++---- docs/safety.md | 28 ++++++++++++ docs/terraform-state.md | 94 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 178 insertions(+), 15 deletions(-) create mode 100644 docs/terraform-state.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8674c22..a15439b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,6 +29,11 @@ existing `CloudConcept`. See `docs/cloud-model.md`. ## Adding a tool -All tool operations are simulated in V1. Implement the `Tool` interface in -`src/devops_learn/tools/`, declare accurate `risk_level`/`is_destructive` metadata, and -never bypass `ToolService.invoke`. See `docs/safety.md`. +Implement the `Tool` interface in `src/devops_learn/tools/`, declare accurate +`risk_level`/`is_destructive` metadata, and never bypass `ToolService.invoke`. Provide a +`Simulated*Tool` for tests/offline use and a `Real*Tool` that shells out to the actual binary +via an argument list (never `shell=True`), an explicit timeout, and +`tools/_subprocess_safety.py`'s redaction. See `docs/safety.md` and `docs/development.md`. + +Any test needing real Azure credentials must be gated behind `RUN_AZURE_INTEGRATION_TESTS=1` so +normal CI never attempts to reach Azure. diff --git a/README.md b/README.md index 43882cc..ed3a6c7 100644 --- a/README.md +++ b/README.md @@ -24,8 +24,14 @@ Project -> Analysis -> Requirements -> Questions -> Recommendations -> Architect questions that cannot be inferred safely. - **Real local execution** (`devops-learn local`) runs actual `pytest`, `flake8`, `docker build`, `docker run`, and HTTP health checks against the project. -- **Simulated cloud/Terraform/Kubernetes** execution for safe learning without - credentials or cost (`devops-learn analyze` without `--real-tools`). +- **Real Terraform execution** (`devops-learn terraform`) runs actual + `terraform fmt`/`init`/`validate`/`plan` against a real, committed Azure + configuration, with structured `terraform show -json` parsing feeding a + deterministic risk classifier. No `apply`/`destroy` yet; `plan` requires + Azure authentication and fails cleanly, with an explanation, without it. +- **Simulated cloud/Kubernetes** execution for safe learning without + credentials or cost (`devops-learn analyze` without `--real-tools`; + Terraform in `analyze`/`review` also stays simulated). - **Controlled tool execution**: every capability is a `Tool` with declared risk level, dry-run support, and human approval; `ToolService` is the only caller of `Tool.execute`. @@ -53,6 +59,10 @@ devops-learn init projects/api_platform # Run a real local vertical slice: test -> lint -> docker build -> run -> verify devops-learn local projects/api_platform +# Run a real Terraform vertical slice: fmt -> init -> validate -> plan -> risk analysis +# (plan requires Azure credentials; fails cleanly with an explanation without them) +devops-learn terraform + # Simulate the full cloud workflow (no credentials, no cost) devops-learn analyze projects/api_platform --mode collaborative --depth learning diff --git a/docs/development.md b/docs/development.md index b150976..01c5247 100644 --- a/docs/development.md +++ b/docs/development.md @@ -35,6 +35,11 @@ devops-learn init projects/api_platform # Real local vertical slice: test -> lint -> docker build -> run -> verify devops-learn local projects/api_platform +# Real Terraform vertical slice: fmt -> init -> validate -> plan -> risk analysis +# (plan requires Azure credentials -- az login, or ARM_CLIENT_ID/ARM_CLIENT_SECRET/ +# ARM_TENANT_ID/ARM_SUBSCRIPTION_ID -- and fails cleanly, with an explanation, without them) +devops-learn terraform + # Simulate the full cloud workflow (no credentials, no cost) devops-learn analyze projects/api_platform --mode collaborative --depth learning @@ -84,7 +89,17 @@ declaration at construction time. Provide a `Simulated*Tool` for tests and environments where the real dependency is unavailable, and a `Real*Tool` that shells out to the actual binary. See `tools/python_tool.py` and -`tools/docker_tool.py` for the pattern. +`tools/docker_tool.py` for the base pattern, and `tools/terraform_tool.py` for a `Real*Tool` +that also needs an explicit subprocess timeout and output redaction — reuse +`tools/_subprocess_safety.py`'s `run_safely()`/`redact()` rather than calling `subprocess.run` +directly, so secret-shaped output can never accidentally reach a `ToolResult` unredacted. + +Any future test that requires real Azure credentials must be gated behind +`RUN_AZURE_INTEGRATION_TESTS=1` (e.g. +`@pytest.mark.skipif(os.environ.get("RUN_AZURE_INTEGRATION_TESTS") != "1", reason="requires az +login")`), so ordinary CI and `pytest` runs never attempt to reach Azure. Nothing in the +platform needs this yet — it is the convention Milestone 3 (`docs/roadmap.md`) will use as soon +as real Azure calls exist. ## Adding a workflow command diff --git a/docs/learning-model.md b/docs/learning-model.md index b2ca996..1484e5a 100644 --- a/docs/learning-model.md +++ b/docs/learning-model.md @@ -92,6 +92,12 @@ course. When the platform generates or runs Terraform, it explains: The depth of each of those points depends on the learner profile and the current explanation depth. +`devops-learn terraform` applies this to Terraform state specifically: +`workflows/terraform_flow.py` surfaces 2-3 state/provider concepts per run, +tied to the command about to execute, rather than the full picture at once. +See `docs/terraform-state.md` for the complete reference those moments link +back to. + ## AI-generated work must be reviewable Before significant execution the platform shows: diff --git a/docs/roadmap.md b/docs/roadmap.md index 9c479d1..3680643 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -26,15 +26,20 @@ allow-listed subprocess wrapper, gated by `ToolService.invoke`. offline/no-Docker environments. The `devops-learn local` command runs the full local vertical slice end to end and teaches Docker concepts in context. -## Milestone 2: real Terraform workflow - -Add `RealTerraformTool` shelling out to a real `terraform` binary for -`fmt`/`validate`/`plan` against generated configuration, with structured -parsing of `terraform show -json` feeding the existing -`validation/terraform_plan_analysis.analyze()` risk classifier unchanged. No -`apply` and no Azure credentials required yet; -`apply_approved_plan`/`destroy_approved_environment` stay simulated until -Milestone 3. +## Milestone 2: real Terraform workflow (done) + +`RealTerraformTool` (`tools/terraform_tool.py`) shells out to a real +`terraform` binary for `fmt`/`init`/`validate`/`plan` against a real, +committed configuration (`projects/api_platform/infra/terraform/`), with +structured parsing of `terraform show -json` feeding the existing +`validation/terraform_plan_analysis.analyze()` risk classifier unchanged. +`devops-learn terraform` runs the full vertical slice (fmt -> init -> +validate -> plan -> risk analysis) and teaches Terraform state, provider, +and resource-address concepts just-in-time (`docs/terraform-state.md`). No +`apply`; `apply_approved_plan`/`destroy_approved_environment` are not even +declared on `RealTerraformTool` and stay simulated until Milestone 3. `plan` +requires Azure authentication and fails cleanly, with an explanation, on a +machine without it — `init`/`validate`/`fmt` need no credentials. ## Milestone 3: first Azure vertical slice diff --git a/docs/safety.md b/docs/safety.md index a5547fa..eb15dea 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -10,6 +10,13 @@ - `devops-learn local ` runs real `pytest`, `flake8`, `docker build`, `docker run`, `docker logs`, and `docker stop` against the project you provide, and performs a real HTTP health check against the running container. +- `devops-learn terraform` runs real `terraform fmt`, `init`, `validate`, and `plan` (including + `terraform show -json`) against `projects/api_platform/infra/terraform/`. It never applies or + destroys anything. `init` downloads the `azurerm` provider plugin (network access, no Azure + credentials required); `plan` requires Azure authentication (`az login`, or + `ARM_CLIENT_ID`/`ARM_CLIENT_SECRET`/`ARM_TENANT_ID`/`ARM_SUBSCRIPTION_ID`) to build the + provider client and fails cleanly, with an explanation, without it — see "Real Terraform + execution" below. - The platform's own persistence (SQLite), audit log, decisions, experience tracking, and learner profile are real. - The reference Dockerfile, Terraform configuration, Kubernetes manifest, and GitHub Actions @@ -60,6 +67,27 @@ resources, applies Terraform, or modifies remote state. It: - stops the container at the end of the workflow, - clearly labels every real result with `(real)`. +## Real Terraform execution + +`devops-learn terraform` is the only command that uses `RealTerraformTool` +(`tools/terraform_tool.py`) — `devops-learn analyze --real-tools` and `devops-learn local` both +still use `SimulatedTerraformTool` for the `terraform` tool name; only `python`/`docker` become +real for those commands. `RealTerraformTool` declares only `fmt`/`init`/`validate`/`plan` — +`apply_approved_plan`/`destroy_approved_environment` are not implemented on it at all, so +invoking either raises `KeyError` rather than silently falling back to simulated behavior. Real +apply/destroy stay simulated until Milestone 3 (`docs/roadmap.md`). + +Every subprocess call goes through `tools/_subprocess_safety.py::run_safely`, which sets an +explicit timeout and always redacts secret-shaped `KEY=value` text and truncates long output +before it reaches a `ToolResult`. `plan()`'s parsing of `terraform show -json` never extracts +raw resource attribute values (`before`/`after`) — only `{address, action}` per resource change +— which is the primary defense against leaking sensitive plan data; free-text redaction is +defense in depth on top of that. + +A `terraform plan` failure due to missing Azure credentials is an expected, honest outcome on a +machine with no `az login`/`ARM_*` environment variables configured — `workflows/terraform_flow.py` +diagnoses it explicitly rather than treating it as a bug, and never fabricates a successful plan. + ## Terraform plan risk `validation/terraform_plan_analysis.analyze()` classifies a plan's `create`/`change`/`replace`/ diff --git a/docs/terraform-state.md b/docs/terraform-state.md new file mode 100644 index 0000000..d384451 --- /dev/null +++ b/docs/terraform-state.md @@ -0,0 +1,94 @@ +# Terraform state + +`devops-learn terraform` (`workflows/terraform_flow.py`) surfaces a subset of +these concepts just-in-time, right before the command that makes them +relevant -- not as a front-loaded course. This doc is the fuller reference +those in-flow moments link back to. + +## Desired state vs. actual infrastructure vs. Terraform state + +Three things, easy to conflate: + +- **Desired state**: what your `.tf` files declare should exist. +- **Actual infrastructure**: what really exists in Azure right now. +- **Terraform state**: Terraform's own record of what it believes exists, + stored in `terraform.tfstate`. This is neither of the other two -- it's + Terraform's memory, built from the last successful apply (or import). + +`terraform plan` diffs desired state against Terraform's state file, then +confirms that against the real provider API. All three can disagree with +each other; when state and actual infrastructure disagree, that's drift. + +## Provider + +A provider is the plugin Terraform uses to talk to a specific platform -- +`azurerm` for Azure. `terraform init` downloads it. Every resource type's +prefix (`azurerm_resource_group`, `azurerm_container_registry`) names the +provider that owns it. + +## Resource address + +A resource's address (e.g. `azurerm_resource_group.main`) combines its type +and a local name. Terraform uses addresses as keys into the state file and to +build the dependency graph between resources -- e.g. the container registry +in `main.tf` implicitly depends on the resource group because it references +`azurerm_resource_group.main.name`. + +## Dependencies + +Terraform builds a dependency graph from resource references (not file +order) and applies/destroys in graph order: dependencies before dependents on +create, dependents before dependencies on destroy. + +## Drift + +Drift is when the real infrastructure no longer matches what Terraform's +state file says it should be -- usually because someone changed a resource +outside Terraform (Azure Portal, `az` CLI, another tool). `terraform plan` +detects drift by refreshing state against the real provider before diffing, +so a plan can show changes even though nothing in your `.tf` files changed. + +## Local vs. remote state, and locking + +This milestone only uses **local state**: `terraform.tfstate` sits in the +working directory. That's fine solo, but two people (or two CI runs) applying +concurrently against local state can corrupt it. **Remote state** (e.g. an +Azure Storage blob) centralizes the file and typically adds **locking**, so +only one `apply` can run against a given state file at a time. Remote state +and locking are out of scope for this milestone; see `docs/roadmap.md`. + +## State sensitivity + +State can contain resource attribute values, including ones marked sensitive +by a provider (connection strings, generated passwords, keys). This is why +`terraform.tfstate*` is gitignored (see `.gitignore`) -- committing it would +commit those values in plaintext into version control history. + +## Why deleting state does not delete infrastructure + +`terraform.tfstate` is Terraform's memory, not a control plane. Deleting it +does not touch Azure at all -- it only makes Terraform forget every resource +it was tracking. Run `terraform plan` afterward and every real resource would +show up as something Terraform wants to *create*, because as far as +Terraform now knows, nothing exists yet. The real resources are still there, +now unmanaged and orphaned from Terraform's perspective. + +## Why hand-editing state is dangerous + +State's internal schema is version-specific and includes fields (resource +dependency ordering, provider-specific metadata) that aren't meant to be +edited by hand. A manual edit that doesn't match reality can cause Terraform +to plan a destructive "fix" -- e.g. destroying and recreating a resource +Terraform now believes doesn't match its record, even though the real +resource is fine. Use `terraform state` subcommands (`mv`, `rm`, `import`) +instead of editing the file directly; even those require care and are +DESTRUCTIVE-adjacent operations in this platform's risk model (see +`docs/safety.md`). + +## Real vs. simulated in this platform + +`devops-learn terraform` runs real `fmt`/`init`/`validate`/`plan` against +`projects/api_platform/infra/terraform/` and never applies or destroys. +`apply_approved_plan`/`destroy_approved_environment` -- and therefore any +real state file this platform would ever write -- stay simulated until +Milestone 3. See `docs/safety.md` and `docs/roadmap.md`. From da6a1a2394b58118720f698fff6ae0a1b1951c17 Mon Sep 17 00:00:00 2001 From: howlcipher Date: Wed, 12 Aug 2026 16:55:45 -0400 Subject: [PATCH 6/8] Add project CLAUDE.md, review subagents, and a destructive-command hook CLAUDE.md: durable rules on product direction, the Tool/ToolService gateway, real-vs-simulated labeling, and quality gates. .claude/agents/: terraform-reviewer, security-reviewer, learning-reviewer -- read-only review personas used during this milestone's own development. .claude/hooks/confirm_destructive_command.py: a PreToolUse Bash hook that forces explicit confirmation before terraform apply/destroy, az group/ resource delete, git push --force, and similar commands run directly via Bash, as a backstop for when a command bypasses ToolService entirely. Co-Authored-By: Claude Sonnet 5 --- .claude/agents/learning-reviewer.md | 19 +++++ .claude/agents/security-reviewer.md | 20 +++++ .claude/agents/terraform-reviewer.md | 20 +++++ .claude/hooks/confirm_destructive_command.py | 70 +++++++++++++++ .claude/settings.json | 15 ++++ CLAUDE.md | 90 ++++++++++++++++++++ 6 files changed, 234 insertions(+) create mode 100644 .claude/agents/learning-reviewer.md create mode 100644 .claude/agents/security-reviewer.md create mode 100644 .claude/agents/terraform-reviewer.md create mode 100755 .claude/hooks/confirm_destructive_command.py create mode 100644 .claude/settings.json create mode 100644 CLAUDE.md diff --git a/.claude/agents/learning-reviewer.md b/.claude/agents/learning-reviewer.md new file mode 100644 index 0000000..e3c9db6 --- /dev/null +++ b/.claude/agents/learning-reviewer.md @@ -0,0 +1,19 @@ +--- +name: learning-reviewer +description: Verifies the Terraform workflow actually teaches, not just automates — checks just-in-time moments, ExplanationDepth gating, and evidence recording. Use after any diff touching src/devops_learn/workflows/terraform_flow.py, docs/terraform-state.md, docs/learning-model.md, docs/roadmap.md, or docs/safety.md. +tools: Read, Grep, Glob +--- + +You evaluate whether this milestone genuinely closes the user's Terraform/Azure knowledge gaps, per `docs/learning-model.md`, or merely automates the work. You are read-only: report findings, do not edit anything. + +Check specifically: + +- `workflows/terraform_flow.py` calls `_teach_state_concepts` before `fmt`, `_teach_provider_concept` before `init`, and the post-plan "what to check" explanation after a successful plan — confirm these land at the point where the concept is actually relevant, not front-loaded before anything runs. +- Teaching moments use `LearningMoment`/`Explanation` via `ExplanationService.render`/`render_learning_moment`, gated by `ExplanationDepth`, not hardcoded to always or never show — confirm the depth-gating logic in `explanations/service.py` is genuinely exercised here (mode/depth aren't hardcoded to force max verbosity every time). +- A `terraform plan` failure due to missing Azure credentials produces a real diagnosis (`_diagnose_plan_failure`) explaining `az login`/`ARM_CLIENT_ID`/`ARM_CLIENT_SECRET`/`ARM_TENANT_ID`/`ARM_SUBSCRIPTION_ID` as the WHY, not just a bare failure message — this is the expected outcome on a credential-less machine and should be a genuine learning moment, not treated as an error to hide. +- `ExperienceTracker.record` calls in `terraform_flow.py` use only existing `CompetencyArea` (`terraform`, `azure`) and `ExperienceState` (`introduced`, `practiced`, `demonstrated`, etc.) vocabulary — no invented competency areas or states. +- `docs/terraform-state.md` actually covers: desired vs. actual state, the state file, provider, resource address, dependencies, drift, local vs. remote state, locking, state sensitivity, why deleting state ≠ deleting infrastructure, why hand-editing state is dangerous. +- `docs/terraform-state.md` is cross-linked from `docs/learning-model.md`'s "Just-in-time learning" section. +- `docs/roadmap.md`/`docs/safety.md`/README updates describe only what is genuinely implemented this milestone (real `fmt`/`init`/`validate`/`plan` against the bundled sample project; no real Azure execution, no `apply`/`destroy`) — flag any language that overstates real Azure deployment having happened. + +Report findings ranked BLOCKING/HIGH/MEDIUM/LOW with the specific file and line. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 0000000..0c6df89 --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,20 @@ +--- +name: security-reviewer +description: Reviews for secret leakage, subprocess safety, and approval-gate integrity across RealTerraformTool, CI, and Claude Code hooks. Use after any diff touching src/devops_learn/tools/terraform_tool.py, src/devops_learn/tools/_subprocess_safety.py, .github/workflows/ci.yml, or .claude/settings.json. +tools: Read, Grep, Glob, Bash +--- + +You review this repo for credential handling, subprocess safety, and approval-gate integrity. You are read-only: report findings, do not fix them yourself. + +Check specifically: + +- `tools/_subprocess_safety.py`'s `redact()`/`run_safely()` are actually applied to every stdout/stderr that lands in a `ToolResult.details` from `RealTerraformTool` — grep for any `result.stdout`/`result.stderr` usage that bypasses `_subprocess_safety`. +- `RealTerraformTool.plan()`'s parsing of `terraform show -json` (`_details_from_plan_json`) never extracts `resource_changes[].change.before`/`.after` — only `{address, action}`. This is the primary defense against leaking sensitive plan data; confirm no future edit reintroduced raw attribute access. +- Every subprocess call in `terraform_tool.py` goes through `run_safely(..., timeout=...)` with an explicit timeout — none should call `subprocess.run` directly. +- `ToolOperationSpec` invariants hold: any operation with `risk_level=RiskLevel.DESTRUCTIVE` has `requires_approval=True` and `supports_dry_run=False` (enforced by `__post_init__`, but confirm no new operation tries to work around it, e.g. via a different risk level for something that is actually destructive). +- `.github/workflows/ci.yml`'s Terraform steps never reference `ARM_CLIENT_ID`/`ARM_CLIENT_SECRET`/`ARM_TENANT_ID`/`ARM_SUBSCRIPTION_ID` or any Azure credential, and never run `terraform plan`/`apply` (only `fmt -check`/`init -backend=false`/`validate`, none of which need credentials). +- `.gitignore` covers `.terraform/`, `*.tfstate*`, and crash logs; `.terraform.lock.hcl` is NOT ignored. +- `.claude/settings.json`'s `PreToolUse` hook (`.claude/hooks/confirm_destructive_command.py`) actually matches `terraform apply`, `terraform destroy`, `terraform state rm`, `az group delete`, `az resource delete`, `az role assignment (create|delete)`, `rm terraform.tfstate*`, `rm -rf .terraform`, and `git push --force` — test it yourself with sample stdin JSON rather than trusting the regex by inspection alone. +- No API keys, tokens, `.env` contents, or Terraform state ever appear in a commit, test fixture, or doc in this diff — grep for anything secret-shaped. + +Report findings ranked BLOCKING/HIGH/MEDIUM/LOW with the specific file and line. diff --git a/.claude/agents/terraform-reviewer.md b/.claude/agents/terraform-reviewer.md new file mode 100644 index 0000000..14a7e25 --- /dev/null +++ b/.claude/agents/terraform-reviewer.md @@ -0,0 +1,20 @@ +--- +name: terraform-reviewer +description: Reviews changes to *.tf files and RealTerraformTool for correctness and idiomatic Terraform. Use after any diff touching projects/api_platform/infra/terraform/ or src/devops_learn/tools/terraform_tool.py. +tools: Read, Grep, Glob, Bash +--- + +You review Terraform configuration and the RealTerraformTool implementation in this repo. You are read-only: report findings, do not rewrite the implementation. + +Check specifically: + +- Provider version is pinned (`~> X.Y`, never unpinned) in `providers.tf`. +- Resource naming and tags are consistent with `main.tf` (`project`, `environment`, `managed-by`, `purpose`). +- `.gitignore` correctly excludes `.terraform/` and `*.tfstate*` while `.terraform.lock.hcl` stays committed (verify it is not accidentally ignored). +- The `providers.tf`/`variables.tf`/`main.tf`/`outputs.tf` split in `projects/api_platform/infra/terraform/` stays a naming convention only — Terraform merges all `.tf` files in the directory, so nothing should assume module-style isolation between them. +- `RealTerraformTool` (`src/devops_learn/tools/terraform_tool.py`) only declares `fmt`/`init`/`validate`/`plan` operations, all `RiskLevel.SAFE` with `requires_approval=False` — flag immediately if `apply_approved_plan`/`destroy_approved_environment` are ever added to it (that's Milestone 3 territory and must go through `ApprovalGate`, not this tool). +- `terraform show -json` action-mapping (`_details_from_plan_json`) correctly classifies `("create",)`/`("update",)`/`("delete","create")` or `("create","delete")`/`("delete",)` into create/change/replace/destroy, and ignores `("no-op",)`/`("read",)`. +- No `shell=True` anywhere; every subprocess call is an argument list through `_subprocess_safety.run_safely`. +- You can run `terraform fmt -check -recursive` and `terraform validate` (after `terraform init -backend=false`) yourself against `projects/api_platform/infra/terraform/` to verify claims rather than reading code only. + +Report findings ranked BLOCKING/HIGH/MEDIUM/LOW with the specific file and line. diff --git a/.claude/hooks/confirm_destructive_command.py b/.claude/hooks/confirm_destructive_command.py new file mode 100755 index 0000000..b422c84 --- /dev/null +++ b/.claude/hooks/confirm_destructive_command.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +"""PreToolUse hook: forces explicit user confirmation before a Bash command +that could destroy infrastructure or discard history runs, instead of +letting it execute silently. + +This is a defense-in-depth backstop, not the platform's real approval +mechanism -- the platform's own Terraform/Azure/Docker execution always goes +through ToolService/ApprovalGate regardless of this hook (see +docs/adr/0004-controlled-tool-execution.md). This hook only matters when a +command bypasses that layer entirely, e.g. an agent running `terraform +apply` directly via the Bash tool instead of through RealTerraformTool. + +Reads the PreToolUse JSON payload from stdin, and if tool_input.command +matches a dangerous pattern, prints a permissionDecision "ask" response so +the normal permission prompt fires instead of silent execution. Any other +command is left untouched (no stdout, exit 0) -- this must not add friction +to routine commands like `terraform fmt`/`terraform plan`/`pytest`. +""" +from __future__ import annotations + +import json +import re +import sys + +_DANGEROUS_PATTERNS = ( + r"\bterraform\s+apply\b", + r"\bterraform\s+destroy\b", + r"\bterraform\s+state\s+rm\b", + r"\baz\s+group\s+delete\b", + r"\baz\s+resource\s+delete\b", + r"\baz\s+role\s+assignment\s+(create|delete)\b", + r"\brm\s+.*terraform\.tfstate", + r"\brm\s+-rf\s+.*\.terraform\b", + r"\bgit\s+push\b.*(--force\b|(? int: + try: + payload = json.load(sys.stdin) + except (json.JSONDecodeError, ValueError): + return 0 + + tool_input = payload.get("tool_input") if isinstance(payload, dict) else None + command = tool_input.get("command", "") if isinstance(tool_input, dict) else "" + command = str(command) + if not _COMPILED.search(command): + return 0 + + print( + json.dumps( + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "ask", + "permissionDecisionReason": ( + "This command can destroy infrastructure or discard history " + "outside the platform's own ToolService approval gate. " + "Confirm you intend to run it before proceeding." + ), + } + } + ) + ) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..993f542 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 \"${CLAUDE_PROJECT_DIR}/.claude/hooks/confirm_destructive_command.py\"" + } + ] + } + ] + } +} diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..285966d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,90 @@ +# DevOps-Learn-by-Doing + +## Product + +This is an AI-assisted DevOps mastery platform, not an automation tool and +not an anti-AI exercise. The goal is real engineering work performed +together with AI in a way that deliberately closes the user's knowledge +gaps. AI automation and learner understanding are two separate dimensions: +running a command for the user is not the same as the user understanding +why it ran or what it did. See `docs/learning-model.md`. Never equate manual +typing with expertise, and never let AI execution stand in for competency +evidence — evidence lives in `ExperienceTracker`/`ExperienceState` +(`experience/tracker.py`, `domain/enums.py`). + +## Architecture + +Modular monolith (`docs/architecture.md`, `docs/adr/0001`). `bootstrap.py` +is the single composition root — all concrete implementations are wired +there via constructor injection, never a service locator. `domain/` depends +on nothing; `tools/`/`cloud/`/`ai/` depend only on `domain/`; +`workflows/`/`bootstrap.py` depend on everything; `cli/` depends only on +`bootstrap.py`/`workflows/`/`domain/`. Preserve this layering unless there +is a strong reason to change it. + +Every capability the platform can execute is a `Tool` (`tools/base.py`). +`ToolService.invoke` (`tools/service.py`) is the only caller of +`Tool.execute` anywhere in the codebase — this is what makes "destructive +operations require human approval" structural rather than a convention any +one caller has to remember. Never bypass `ToolService` for convenience, and +never call `Tool.execute` directly outside it. + +LLMs explain and reason; deterministic code decides. `LLMProvider` +(`ai/provider.py`) has no method that invokes a tool or makes a decision — +it only produces freeform explanation text. Recommendations, risk +classification, and plans are typed dataclasses produced by deterministic +services (`docs/adr/0008-structured-ai-output.md`), never freeform LLM +output parsed at runtime. + +## Safety + +- Never fabricate successful execution. Every real result is explicitly + labeled `(real)`/`(real, failed)`; every simulated result says + `(simulated)`. See `docs/safety.md` for the current real-vs-simulated + boundary — keep it accurate as implementation changes. +- `ToolOperationSpec` (`tools/base.py`) enforces at construction time that a + `RiskLevel.DESTRUCTIVE` operation must require approval and must never + support a dry-run bypass of that approval. Do not work around this. +- `ApprovalGate`/`CliApprovalGate` (`tools/approval.py`) is the only path to + approving a HIGH/DESTRUCTIVE operation; `AutoApproveApprovalGate`/ + `AutoDenyApprovalGate` exist only for tests, never for the real CLI. +- Never commit credentials, Terraform state, or `.env` files. Real + subprocess-backed tools must redact secret-shaped output before it lands + in a `ToolResult` (see `tools/_subprocess_safety.py`) and must set an + explicit timeout on every subprocess call. +- Never expose unrestricted arbitrary shell execution through a `Tool`. Real + tools shell out to a narrow, allow-listed set of operations via argument + arrays (never `shell=True`) — see `tools/docker_tool.py`, + `tools/python_tool.py`, `tools/terraform_tool.py`. + +## Product direction + +- Azure first; Terraform depth before AWS/GCP breadth (`docs/cloud-model.md`). +- Do not add Kubernetes/AKS until a real cloud deployment path is proven + reliable without it — not because Kubernetes is technically harder, but + because cluster-level complexity should sit on top of a working real + deployment path, not substitute for one (`docs/roadmap.md`). +- Prefer understandable learning infrastructure over enterprise complexity — + no module registries, landing zones, Terragrunt, or multi-subscription + setups unless a milestone genuinely requires them. +- See `docs/roadmap.md` for current milestone status before proposing new + scope. + +## Quality + +Before considering any change complete, all of the following must pass: + +``` +pytest +flake8 src tests projects/api_platform/app projects/api_platform/tests +mypy +cd projects/api_platform && mypy app --strict +``` + +Update `docs/roadmap.md`/`docs/safety.md` when implementation reality +changes — this repo's credibility depends on never claiming more than what +is genuinely implemented (e.g. do not describe real Azure execution as +working unless it has actually been verified against a real subscription). + +Do not put task-specific notes, implementation plans, or milestone journals +in this file — it is for durable project knowledge only. From b951991e05ed169962db0a1a5d0a7a728a29d53b Mon Sep 17 00:00:00 2001 From: howlcipher Date: Thu, 13 Aug 2026 12:39:50 -0400 Subject: [PATCH 7/8] feat(security): add DevSecOps control plane --- .github/workflows/ci.yml | 40 +++ .gitignore | 1 + CLAUDE.md | 3 + README.md | 9 + docs/architecture.md | 9 + docs/devsecops.md | 148 +++++++++ docs/roadmap.md | 11 + docs/safety.md | 5 + policy/security/devsecops.rego | 81 +++++ projects/api_platform/app/main.py | 4 +- projects/api_platform/tests/test_health.py | 12 +- projects/api_platform/tests/test_info.py | 14 +- src/devops_learn/audit/service.py | 3 +- src/devops_learn/cli/commands/security.py | 45 +++ src/devops_learn/cli/main.py | 7 +- src/devops_learn/domain/enums.py | 54 ++++ src/devops_learn/domain/security_models.py | 76 +++++ src/devops_learn/security/__init__.py | 5 + src/devops_learn/security/change_analysis.py | 42 +++ src/devops_learn/security/eligibility.py | 38 +++ src/devops_learn/security/normalization.py | 129 ++++++++ src/devops_learn/security/policy.py | 70 +++++ src/devops_learn/security/redaction.py | 41 +++ src/devops_learn/security/remediation.py | 59 ++++ src/devops_learn/security/reporting.py | 66 ++++ src/devops_learn/tools/policy_tool.py | 107 +++++++ src/devops_learn/tools/python_tool.py | 20 +- .../tools/security_scanner_tool.py | 266 ++++++++++++++++ src/devops_learn/workflows/security_flow.py | 297 ++++++++++++++++++ tests/security/__init__.py | 1 + .../test_change_policy_eligibility.py | 109 +++++++ tests/security/test_normalization.py | 79 +++++ tests/security/test_secret_safety.py | 65 ++++ tests/tools/test_security_tools.py | 67 ++++ 34 files changed, 1966 insertions(+), 17 deletions(-) create mode 100644 docs/devsecops.md create mode 100644 policy/security/devsecops.rego create mode 100644 src/devops_learn/cli/commands/security.py create mode 100644 src/devops_learn/domain/security_models.py create mode 100644 src/devops_learn/security/__init__.py create mode 100644 src/devops_learn/security/change_analysis.py create mode 100644 src/devops_learn/security/eligibility.py create mode 100644 src/devops_learn/security/normalization.py create mode 100644 src/devops_learn/security/policy.py create mode 100644 src/devops_learn/security/redaction.py create mode 100644 src/devops_learn/security/remediation.py create mode 100644 src/devops_learn/security/reporting.py create mode 100644 src/devops_learn/tools/policy_tool.py create mode 100644 src/devops_learn/tools/security_scanner_tool.py create mode 100644 src/devops_learn/workflows/security_flow.py create mode 100644 tests/security/__init__.py create mode 100644 tests/security/test_change_policy_eligibility.py create mode 100644 tests/security/test_normalization.py create mode 100644 tests/security/test_secret_safety.py create mode 100644 tests/tools/test_security_tools.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c2a466..7cd4fd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,3 +34,43 @@ jobs: run: terraform -chdir=projects/api_platform/infra/terraform init -backend=false -input=false - name: Terraform validate run: terraform -chdir=projects/api_platform/infra/terraform validate + security: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + - name: Install platform + run: pip install -e ".[dev]" + - name: Install Trivy + uses: aquasecurity/setup-trivy@v0.2.6 + - name: Install Conftest + env: + CONFTEST_VERSION: 0.69.0 + run: | + curl --fail --location --silent --show-error \ + --output /tmp/conftest.tar.gz \ + "https://github.com/open-policy-agent/conftest/releases/download/v${CONFTEST_VERSION}/conftest_${CONFTEST_VERSION}_Linux_x86_64.tar.gz" + tar --extract --gzip --file /tmp/conftest.tar.gz --directory /usr/local/bin conftest + conftest --version + - name: Select comparison base + id: base + env: + PR_BASE: ${{ github.event.pull_request.base.sha }} + BEFORE: ${{ github.event.before }} + run: echo "ref=${PR_BASE:-$BEFORE}" >> "$GITHUB_OUTPUT" + - name: Security gate + run: >- + devops-learn security scan projects/api_platform + --base-ref "${{ steps.base.outputs.ref }}" + --report artifacts/security/security-report.json + - name: Upload sanitized security report + if: always() + uses: actions/upload-artifact@v4 + with: + name: security-report + path: artifacts/security/security-report.json + if-no-files-found: warn diff --git a/.gitignore b/.gitignore index 01305ff..9c0755c 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ htmlcov/ *.sqlite3 .env .devops_learn/ +artifacts/security/ # Terraform working-directory artifacts (see projects/api_platform/infra/terraform/). # .terraform.lock.hcl is deliberately NOT listed here: it pins exact provider diff --git a/CLAUDE.md b/CLAUDE.md index 285966d..e8ec56d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,6 +56,9 @@ output parsed at runtime. tools shell out to a narrow, allow-listed set of operations via argument arrays (never `shell=True`) — see `tools/docker_tool.py`, `tools/python_tool.py`, `tools/terraform_tool.py`. +- Security scanner output is evidence, not security truth from an LLM. Normalize and structurally + redact it before any console, audit, report, or explanation path; policy decisions are + deterministic and introduced findings must remain distinct from historical debt. ## Product direction diff --git a/README.md b/README.md index ed3a6c7..dfffd6c 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,10 @@ Project -> Analysis -> Requirements -> Questions -> Recommendations -> Architect - **Controlled tool execution**: every capability is a `Tool` with declared risk level, dry-run support, and human approval; `ToolService` is the only caller of `Tool.execute`. +- **DevSecOps security gate** (`devops-learn security scan`) runs real Trivy + evidence collection, compares an explicit Git base against the proposed + state, applies Conftest policy, redacts secrets structurally, and prevents + blocked changes from progressing toward deployment eligibility. - **Competency evidence tracking**: records what you were exposed to, practiced, or demonstrated, not fabricated certification. @@ -63,6 +67,10 @@ devops-learn local projects/api_platform # (plan requires Azure credentials; fails cleanly with an explanation without them) devops-learn terraform +# Check security prerequisites, then scan a proposed change against its base +devops-learn security doctor +devops-learn security scan projects/api_platform --base-ref origin/main + # Simulate the full cloud workflow (no credentials, no cost) devops-learn analyze projects/api_platform --mode collaborative --depth learning @@ -150,6 +158,7 @@ freeform explanation text (`docs/adr/0008-structured-ai-output.md`). - `docs/cloud-model.md` the concept-first, multi-cloud abstraction. - `docs/safety.md` simulation vs. real execution, approval gating, risk levels. - `docs/roadmap.md` the next milestones. +- `docs/devsecops.md` security evidence, policy gate, secret safety, CI, and demo. - `docs/adr/` architecture decision records. ## Development diff --git a/docs/architecture.md b/docs/architecture.md index 1db1890..ac60b19 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -15,6 +15,7 @@ workflows/analyze_flow.py sequences the services below for one engagement; no +-- architecture.ArchitectureService recommendations -> ArchitectureProposal (concept-first) +-- planning.PlanningService proposal -> ImplementationPlan of Tool operations +-- validation.terraform_plan_analysis pure risk classification of a Terraform plan + +-- security.normalize/change_analysis/policy normalized security evidence and gate +-- troubleshooting.TroubleshootingService gathers evidence, then diagnoses +-- tools.ToolService the only entry point into any Tool +-- explanations.ExplanationService renders Explanation/LearningMoment by mode+depth @@ -24,6 +25,7 @@ workflows/analyze_flow.py sequences the services below for one engagement; no +-- learning.SessionService EngagementSession lifecycle +-- learning.LearnerProfileService stores skill profile and learning focus +-- workflows.local_flow real local vertical slice (test -> Docker -> verify) + +-- workflows.security_flow Trivy -> base comparison -> Conftest policy -> gate +-- ai.LLMProvider MockLLMProvider (default) or AnthropicProvider ``` @@ -52,6 +54,13 @@ never in whether a HIGH/DESTRUCTIVE tool operation requires approval inspect -> test -> lint -> Docker build -> run -> HTTP verify -> logs -> stop. It uses real `python` and `docker` tools while keeping cloud/Terraform/Kubernetes simulated. +`workflows/security_flow.py` is the pre-deployment security stage. It invokes +Trivy through `SecurityScannerTool`, normalizes redacted findings, compares an +explicit Git base through a temporary archive, invokes Conftest through +`PolicyTool`, records audit and experience evidence, and emits a sanitized +report. Operational tool risk remains separate from the deployment decision. +See `docs/devsecops.md`. + ## Layers ```text diff --git a/docs/devsecops.md b/docs/devsecops.md new file mode 100644 index 0000000..a74d546 --- /dev/null +++ b/docs/devsecops.md @@ -0,0 +1,148 @@ +# DevSecOps security control plane + +## Purpose + +DevSecOps in this platform is a deployment decision stage, not a scanner next +to deployment. It answers what a change introduced, why it matters, what +policy permits, and whether the change can proceed. + +```mermaid +flowchart TD + A[PR or change] --> B[Validation] + B --> C[Trivy] + C --> D[Normalized findings] + D --> E[Base versus proposed] + E --> F[Conftest policy] + F --> G[Explanation and remediation] + G --> H[Human approval when required] + H --> I[Deployment eligibility] +``` + +`ProjectAnalyzer` remains a fast, deterministic structural assessment. It +does not start heavyweight scanners. The `security` workflow adds tool-backed +evidence immediately before future deployment eligibility. + +## Tools and boundaries + +Trivy is the sole scanner in this milestone. It covers filesystem dependency +vulnerabilities, secrets, Terraform and Dockerfile configuration, and an +optional already-built image. This keeps overlap and scanner maintenance low. + +Conftest evaluates the Rego policy in `policy/security/`. It is not a second +scanner. `SecurityScannerTool` invokes only allow-listed Trivy arguments and +`PolicyTool` invokes only Conftest policy evaluation. Both use `ToolService`. +They are SAFE operations because they are read-only; that operational risk is +separate from the policy gate that can still return `BLOCK`. + +Run a capability check before scanning: + +```bash +devops-learn security doctor +``` + +## Evidence, normalization, and redaction + +Trivy JSON is normalized into scanner-independent `SecurityFinding` values. +The domain carries stable identity, severity, category, file/package/resource, +versions, safe evidence, and change status. It never carries a raw secret +match. Before scanner output reaches a normalized finding, console text, +report, or audit event, secret-bearing fields are replaced with +`[REDACTED SECRET]`. Raw Trivy output is not persisted. + +The JSON report is written to `artifacts/security/security-report.json` below +the scan target and is ignored by Git. It contains normalized evidence, policy +result, scanner metadata, and base ref only. + +## Change-aware security + +Use an explicit Git base ref: + +```bash +devops-learn security scan projects/api_platform --base-ref origin/main +``` + +The scanner resolves the target repository, archives the base commit into a +temporary directory, scans that isolated state, then scans the current target. +It never checks out or alters the working tree. A SHA-256 fingerprint over +category, rule ID, target, location, resource, and installed version provides +stable comparison identity. Findings are `INTRODUCED`, `RESOLVED`, +`UNCHANGED`, or `UNCERTAIN`; duplicate identity collisions are deliberately +uncertain. Rename and dependency-version edge cases remain limitations. + +Without `--base-ref`, findings are `UNCERTAIN`, so the workflow does not claim +they were introduced by the current change. + +## Policy and eligibility + +The default policy is inspectable in `policy/security/devsecops.rego`. + +| Evidence | Default decision | +| --- | --- | +| Introduced critical issue or secret | BLOCK | +| Introduced public SSH/RDP exposure or privileged Kubernetes workload | BLOCK | +| Introduced high IaC or dependency issue | REQUIRE_APPROVAL | +| Introduced medium or uncertain high severity issue | WARN | +| Existing debt or resolved finding | Reported, not automatically blocking | + +`BLOCK` makes `devops-learn security scan` exit non-zero. Deployment eligibility +also requires validation success and, for `REQUIRE_APPROVAL`, a recorded human +approval. There is no `--ignore-security` flag and the AI cannot waive policy. +Any future override must be explicit, reasoned, scoped to a fingerprint, and +audited; it is intentionally not implemented yet. + +## Explanation and remediation + +The explanation service uses the existing execution mode and explanation depth. +It renders what was found, why it matters, policy result, safe evidence, and a +remediation classification. Scanner evidence and Rego remain truth; an LLM may +explain but cannot invent a finding, claim a rescan, or overrule policy. + +Credential removal/rotation is `HIGH_RISK`; network and identity changes need +human input; dependency and IaC corrections require review. No high-risk +infrastructure change is auto-remediated. + +## Demo + +The demo is deliberately isolated under `projects/devsecops_demo/` and uses a +synthetic value that cannot authenticate anywhere. It is not a deployable +application. The following creates an ephemeral Git repository so the current +insecure state is compared to a real base: + +```bash +demo_dir=$(mktemp -d) +cp -R projects/devsecops_demo/baseline "$demo_dir/demo" +git -C "$demo_dir/demo" init +git -C "$demo_dir/demo" config user.email demo@example.invalid +git -C "$demo_dir/demo" config user.name devsecops-demo +git -C "$demo_dir/demo" add . +git -C "$demo_dir/demo" commit -m baseline +cp -R projects/devsecops_demo/proposed/. "$demo_dir/demo/" +devops-learn security scan "$demo_dir/demo" --base-ref HEAD +cp -R projects/devsecops_demo/remediated/. "$demo_dir/demo/" +devops-learn security scan "$demo_dir/demo" --base-ref HEAD +``` + +The first command is expected to block once Trivy detects the synthetic secret +and configuration findings. The second is expected to be non-blocking. Scanner +rule IDs and Dockerfile findings can vary by Trivy release, so the demo’s +policy proof is based on the detected secret and public administrative ingress. + +## CI and future Azure path + +Pull-request CI installs Trivy and Conftest, runs a base-aware security scan of +the bundled real FastAPI project, uploads the sanitized report, and never uses +Azure credentials or runs `terraform apply`. GitHub PR metadata supplies the +base SHA, not `HEAD~1`. + +The future real path is: Terraform fmt, validate, plan, plan JSON risk +analysis, Trivy IaC/plan scan, normalized change-aware findings, Conftest +policy, deployment eligibility, human plan approval, real apply, then Azure +verification. Azure and apply are not implemented by this milestone. + +## Limitations + +Image scanning requires a locally built image and Docker availability. The +first comparison implementation does not solve every move, rename, or package +upgrade equivalence. Policy overrides, real Terraform plan scanning, Azure +apply, runtime detection, and Kubernetes deployment are intentionally out of +scope. diff --git a/docs/roadmap.md b/docs/roadmap.md index 3680643..4578277 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -51,6 +51,17 @@ first real cloud execution path is reliable — not because Kubernetes is technically harder, but because a working real deployment path should exist before adding cluster-level complexity on top of it. +## DevSecOps security control plane (done) + +Trivy-backed filesystem/config/image evidence normalizes into scanner-independent +findings, compares a base Git state to the proposed state, and feeds inspectable +Conftest policy. The deterministic gate includes structural secret redaction, +audit/experience evidence, remediation classification, a report artifact, demo, +and PR CI. See `docs/devsecops.md`. + +The next dependency is the real Terraform plan plus Azure deployment path, +where this gate must be required before any real apply. + ## Further out (not yet milestoned) - Real AWS/GCP `CloudProvider` implementations. diff --git a/docs/safety.md b/docs/safety.md index eb15dea..51c59c3 100644 --- a/docs/safety.md +++ b/docs/safety.md @@ -21,6 +21,11 @@ profile are real. - The reference Dockerfile, Terraform configuration, Kubernetes manifest, and GitHub Actions workflow under `templates/` are real, valid configuration files, shown as examples. +- `devops-learn security scan` is real, read-only local Trivy and Conftest execution when those + binaries are available. It does not install tools, alter the working tree, apply Terraform, or + contact Azure. Base refs are materialized with a temporary Git archive. Scanner evidence is + structurally redacted before it reaches reports, console output, audit events, or explanations; + raw scanner JSON is never persisted. ## What is simulated diff --git a/policy/security/devsecops.rego b/policy/security/devsecops.rego new file mode 100644 index 0000000..415c3c4 --- /dev/null +++ b/policy/security/devsecops.rego @@ -0,0 +1,81 @@ +package main + +# Conftest evaluates this deterministic policy over normalized, redacted JSON. +# Prefixes are parsed by security.policy; scanner severity alone never decides. + +deny contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.severity == "critical" + msg := sprintf("BLOCK: introduced critical finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} + +deny contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.category == "secret" + msg := sprintf("BLOCK: introduced secret finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} + +deny contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.category == "network" + contains(lower(finding.title), "public") + contains(lower(finding.title), "ssh") + msg := sprintf("BLOCK: introduced public administrative exposure %s", [finding.fingerprint]) +} + +deny contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.category == "network" + contains(lower(finding.title), "public") + contains(lower(finding.title), "rdp") + msg := sprintf("BLOCK: introduced public administrative exposure %s", [finding.fingerprint]) +} + +deny contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.category == "kubernetes" + contains(lower(finding.title), "privileged") + msg := sprintf("BLOCK: introduced privileged Kubernetes workload %s", [finding.fingerprint]) +} + +warn contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.severity == "high" + finding.category == "iac_misconfiguration" + msg := sprintf("REQUIRE_APPROVAL: introduced high IaC finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} + +warn contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.severity == "high" + finding.category == "dependency" + msg := sprintf("REQUIRE_APPROVAL: introduced high dependency finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} + +warn contains msg if { + finding := input.findings[_] + finding.change_status == "introduced" + finding.severity == "medium" + msg := sprintf("WARN: introduced medium finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} + +warn contains msg if { + finding := input.findings[_] + finding.change_status == "uncertain" + finding.severity == "high" + msg := sprintf("WARN: uncertain high-severity finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} + +warn contains msg if { + finding := input.findings[_] + finding.change_status == "uncertain" + finding.severity == "critical" + msg := sprintf("WARN: uncertain high-severity finding %s (%s)", [finding.rule_id, finding.fingerprint]) +} diff --git a/projects/api_platform/app/main.py b/projects/api_platform/app/main.py index 5687a42..747ede6 100644 --- a/projects/api_platform/app/main.py +++ b/projects/api_platform/app/main.py @@ -21,13 +21,13 @@ @app.get("/health") -def health() -> dict[str, str]: +async def health() -> dict[str, str]: logger.info("health check requested") return {"status": "ok"} @app.get("/info") -def info() -> dict[str, str]: +async def info() -> dict[str, str]: return { "service": settings.service_name, "version": settings.service_version, diff --git a/projects/api_platform/tests/test_health.py b/projects/api_platform/tests/test_health.py index 719eb52..4f53420 100644 --- a/projects/api_platform/tests/test_health.py +++ b/projects/api_platform/tests/test_health.py @@ -1,11 +1,17 @@ -from fastapi.testclient import TestClient +import asyncio + +import httpx from app.main import app -client = TestClient(app) + +async def _get(path: str) -> httpx.Response: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + return await client.get(path) def test_health_returns_ok() -> None: - response = client.get("/health") + response = asyncio.run(_get("/health")) assert response.status_code == 200 assert response.json() == {"status": "ok"} diff --git a/projects/api_platform/tests/test_info.py b/projects/api_platform/tests/test_info.py index 3d522b7..0bfb43a 100644 --- a/projects/api_platform/tests/test_info.py +++ b/projects/api_platform/tests/test_info.py @@ -1,12 +1,18 @@ -from fastapi.testclient import TestClient +import asyncio + +import httpx from app.main import app -client = TestClient(app) + +async def _get(path: str) -> httpx.Response: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + return await client.get(path) def test_info_returns_service_metadata() -> None: - response = client.get("/info") + response = asyncio.run(_get("/info")) assert response.status_code == 200 body = response.json() assert body["service"] == "api-platform" @@ -15,7 +21,7 @@ def test_info_returns_service_metadata() -> None: def test_info_never_leaks_secret_looking_keys() -> None: - response = client.get("/info") + response = asyncio.run(_get("/info")) body = response.json() forbidden = {"password", "secret", "token", "key", "credential"} assert not (set(k.lower() for k in body) & forbidden) diff --git a/src/devops_learn/audit/service.py b/src/devops_learn/audit/service.py index 73061cc..ec9c3c8 100644 --- a/src/devops_learn/audit/service.py +++ b/src/devops_learn/audit/service.py @@ -13,6 +13,7 @@ from devops_learn.domain.audit_models import AuditEvent from devops_learn.domain.enums import AuditEventType from devops_learn.learning.persistence.repositories.audit_repository import AuditRepository +from devops_learn.security.redaction import redact_data class AuditService: @@ -35,7 +36,7 @@ def record( event_type=event_type, occurred_at=occurred_at, summary=summary, - payload=payload or {}, + payload=redact_data(payload or {}), ) return self._audit_repository.append(event) diff --git a/src/devops_learn/cli/commands/security.py b/src/devops_learn/cli/commands/security.py new file mode 100644 index 0000000..ea3ccd4 --- /dev/null +++ b/src/devops_learn/cli/commands/security.py @@ -0,0 +1,45 @@ +"""`devops-learn security`: doctor and evidence-backed security scanning.""" + +from __future__ import annotations + +import argparse + +from devops_learn.bootstrap import Platform +from devops_learn.cli.terminal_ui import TerminalUi +from devops_learn.domain.enums import SecurityGateDecision +from devops_learn.workflows.security_flow import ( + SecurityOptions, + run_security_doctor, + run_security_scan, +) + + +def register(subparsers: "argparse._SubParsersAction[argparse.ArgumentParser]") -> None: + parser = subparsers.add_parser( + "security", help="Run DevSecOps evidence, policy, and gate checks" + ) + commands = parser.add_subparsers(dest="security_command", required=True) + doctor = commands.add_parser("doctor", help="Report security scanner prerequisites") + doctor.set_defaults(handler=run_doctor) + scan = commands.add_parser( + "scan", help="Scan filesystem/config, compare a Git base, and evaluate policy" + ) + scan.add_argument( + "path", nargs="?", default=".", help="Git repository or project directory to scan" + ) + scan.add_argument("--base-ref", help="Comparable Git base ref, such as origin/main") + scan.add_argument("--image", help="Optional already-built Docker image to scan with Trivy") + scan.add_argument("--report", help="Path for the sanitized JSON report") + scan.set_defaults(handler=run_scan) + + +def run_doctor(args: argparse.Namespace, platform: Platform) -> None: + run_security_doctor(platform, TerminalUi()) + + +def run_scan(args: argparse.Namespace, platform: Platform) -> None: + report = run_security_scan( + platform, TerminalUi(), SecurityOptions(args.path, args.base_ref, args.image, args.report) + ) + if report and report.policy.decision is SecurityGateDecision.BLOCK: + raise SystemExit(2) diff --git a/src/devops_learn/cli/main.py b/src/devops_learn/cli/main.py index 80b7289..d80f9c0 100644 --- a/src/devops_learn/cli/main.py +++ b/src/devops_learn/cli/main.py @@ -15,6 +15,7 @@ local, profile, review, + security, terraform, ) from devops_learn.config.settings import load_settings @@ -29,8 +30,10 @@ from devops_learn.tools.terraform_tool import RealTerraformTool, SimulatedTerraformTool from devops_learn.tools.validation_tool import SimulatedValidationTool from devops_learn.tools.cloud_tool import SimulatedCloudTool +from devops_learn.tools.policy_tool import PolicyTool +from devops_learn.tools.security_scanner_tool import SecurityScannerTool -_COMMAND_MODULES = (analyze, review, history, explain, profile, init, local, terraform) +_COMMAND_MODULES = (analyze, review, history, explain, profile, init, local, terraform, security) def build_parser() -> argparse.ArgumentParser: @@ -78,6 +81,8 @@ def _approval_gate_for_args(args: argparse.Namespace) -> ApprovalGate: def _tools_for_args(args: argparse.Namespace) -> dict[str, Tool] | None: + if args.command == "security": + return {"security_scanner": SecurityScannerTool(), "security_policy": PolicyTool()} if args.command == "terraform": # This command only ever invokes the "terraform" tool -- keep the rest # simulated rather than provisioning real python/docker tools it never uses. diff --git a/src/devops_learn/domain/enums.py b/src/devops_learn/domain/enums.py index 8949926..d4597e0 100644 --- a/src/devops_learn/domain/enums.py +++ b/src/devops_learn/domain/enums.py @@ -149,6 +149,12 @@ class AuditEventType(Enum): VALIDATION_COMPLETED = "validation_completed" TERRAFORM_PLAN_STARTED = "terraform_plan_started" TERRAFORM_PLAN_COMPLETED = "terraform_plan_completed" + SECURITY_SCAN_STARTED = "security_scan_started" + SECURITY_SCAN_COMPLETED = "security_scan_completed" + SECURITY_GATE_EVALUATED = "security_gate_evaluated" + SECURITY_OVERRIDE_APPROVED = "security_override_approved" + REMEDIATION_PROPOSED = "remediation_proposed" + REMEDIATION_APPLIED = "remediation_applied" USER_APPROVED_PLAN = "user_approved_plan" TOOL_INVOKED = "tool_invoked" DEPLOYMENT_STARTED = "deployment_started" @@ -158,3 +164,51 @@ class AuditEventType(Enum): DIAGNOSIS_PRODUCED = "diagnosis_produced" ROLLBACK_PERFORMED = "rollback_performed" SESSION_COMPLETED = "session_completed" + + +class FindingSeverity(Enum): + UNKNOWN = "unknown" + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + CRITICAL = "critical" + + +class FindingCategory(Enum): + VULNERABILITY = "vulnerability" + SECRET = "secret" + IAC_MISCONFIGURATION = "iac_misconfiguration" + CONTAINER = "container" + DEPENDENCY = "dependency" + IDENTITY = "identity" + NETWORK = "network" + KUBERNETES = "kubernetes" + POLICY = "policy" + UNKNOWN = "unknown" + + +class FindingChangeStatus(Enum): + INTRODUCED = "introduced" + RESOLVED = "resolved" + UNCHANGED = "unchanged" + UNCERTAIN = "uncertain" + + +class SecurityGateDecision(Enum): + ALLOW = "allow" + WARN = "warn" + REQUIRE_APPROVAL = "require_approval" + BLOCK = "block" + + +class RemediationRisk(Enum): + SAFE_TO_AUTO_FIX = "safe_to_auto_fix" + REQUIRES_REVIEW = "requires_review" + REQUIRES_HUMAN_INPUT = "requires_human_input" + HIGH_RISK = "high_risk" + + +class DeploymentEligibility(Enum): + ELIGIBLE = "eligible" + INELIGIBLE = "ineligible" + PENDING_APPROVAL = "pending_approval" diff --git a/src/devops_learn/domain/security_models.py b/src/devops_learn/domain/security_models.py new file mode 100644 index 0000000..9029d8e --- /dev/null +++ b/src/devops_learn/domain/security_models.py @@ -0,0 +1,76 @@ +"""Scanner-independent DevSecOps evidence and deterministic decisions. + +Raw scanner output is intentionally not represented here. In particular, a +secret match is never a field that downstream code can accidentally render. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Mapping + +from devops_learn.domain.enums import ( + DeploymentEligibility, + FindingCategory, + FindingChangeStatus, + FindingSeverity, + RemediationRisk, + SecurityGateDecision, +) + + +@dataclass(frozen=True) +class SecurityFinding: + fingerprint: str + scanner: str + rule_id: str + title: str + category: FindingCategory + severity: FindingSeverity + target: str + file: str | None = None + line: int | None = None + resource: str | None = None + installed_version: str | None = None + fixed_version: str | None = None + description: str | None = None + references: tuple[str, ...] = () + change_status: FindingChangeStatus = FindingChangeStatus.UNCERTAIN + evidence: str | None = None + metadata: Mapping[str, str] = field(default_factory=dict) + + +@dataclass(frozen=True) +class RemediationAction: + finding_fingerprint: str + recommended_change: str + why: str + risk: RemediationRisk + required_information: str | None + automation_available: bool + + +@dataclass(frozen=True) +class PolicyResult: + decision: SecurityGateDecision + reasons: tuple[str, ...] + policy_available: bool = True + + +@dataclass(frozen=True) +class SecurityReport: + findings: tuple[SecurityFinding, ...] + policy: PolicyResult + scanner_versions: Mapping[str, str] + base_ref: str | None + proposed_target: str + metadata: Mapping[str, str] = field(default_factory=dict) + + +@dataclass(frozen=True) +class DeploymentEligibilityResult: + validation_passed: bool + security_gate: SecurityGateDecision + approval_granted: bool + eligibility: DeploymentEligibility + reason: str diff --git a/src/devops_learn/security/__init__.py b/src/devops_learn/security/__init__.py new file mode 100644 index 0000000..0cdbe5e --- /dev/null +++ b/src/devops_learn/security/__init__.py @@ -0,0 +1,5 @@ +"""DevSecOps control-plane services. + +This package normalizes scanner evidence, compares states, plans remediation, +and turns policy evidence into deployment eligibility. It owns no tool I/O. +""" diff --git a/src/devops_learn/security/change_analysis.py b/src/devops_learn/security/change_analysis.py new file mode 100644 index 0000000..58e943a --- /dev/null +++ b/src/devops_learn/security/change_analysis.py @@ -0,0 +1,42 @@ +"""Stable base-versus-proposed finding classification.""" + +from __future__ import annotations + +from collections import Counter +from dataclasses import replace + +from devops_learn.domain.enums import FindingChangeStatus +from devops_learn.domain.security_models import SecurityFinding + + +def classify_changes( + base: tuple[SecurityFinding, ...], proposed: tuple[SecurityFinding, ...] +) -> tuple[SecurityFinding, ...]: + """Classify comparable reports without claiming certainty for collisions. + + A duplicated fingerprint within either report means a stable identity is not + specific enough to compare those occurrences; affected findings are marked + UNCERTAIN rather than arbitrarily paired. + """ + base_counts = Counter(finding.fingerprint for finding in base) + proposed_counts = Counter(finding.fingerprint for finding in proposed) + findings: list[SecurityFinding] = [] + for finding in proposed: + fingerprint = finding.fingerprint + if base_counts[fingerprint] > 1 or proposed_counts[fingerprint] > 1: + status = FindingChangeStatus.UNCERTAIN + elif fingerprint in base_counts: + status = FindingChangeStatus.UNCHANGED + else: + status = FindingChangeStatus.INTRODUCED + findings.append(replace(finding, change_status=status)) + for finding in base: + fingerprint = finding.fingerprint + if fingerprint not in proposed_counts: + status = ( + FindingChangeStatus.UNCERTAIN + if base_counts[fingerprint] > 1 + else FindingChangeStatus.RESOLVED + ) + findings.append(replace(finding, change_status=status)) + return tuple(findings) diff --git a/src/devops_learn/security/eligibility.py b/src/devops_learn/security/eligibility.py new file mode 100644 index 0000000..f49be05 --- /dev/null +++ b/src/devops_learn/security/eligibility.py @@ -0,0 +1,38 @@ +"""Security gate to deployment eligibility conversion.""" + +from __future__ import annotations + +from devops_learn.domain.enums import DeploymentEligibility, SecurityGateDecision +from devops_learn.domain.security_models import DeploymentEligibilityResult + + +def evaluate_deployment_eligibility( + *, validation_passed: bool, gate: SecurityGateDecision, approval_granted: bool = False +) -> DeploymentEligibilityResult: + if not validation_passed: + return DeploymentEligibilityResult( + False, gate, approval_granted, DeploymentEligibility.INELIGIBLE, "Validation failed." + ) + if gate is SecurityGateDecision.BLOCK: + return DeploymentEligibilityResult( + True, + gate, + approval_granted, + DeploymentEligibility.INELIGIBLE, + "Security policy blocked this change.", + ) + if gate is SecurityGateDecision.REQUIRE_APPROVAL and not approval_granted: + return DeploymentEligibilityResult( + True, + gate, + False, + DeploymentEligibility.PENDING_APPROVAL, + "Security policy requires human approval.", + ) + return DeploymentEligibilityResult( + True, + gate, + approval_granted, + DeploymentEligibility.ELIGIBLE, + "Validation and security requirements are satisfied.", + ) diff --git a/src/devops_learn/security/normalization.py b/src/devops_learn/security/normalization.py new file mode 100644 index 0000000..4234d44 --- /dev/null +++ b/src/devops_learn/security/normalization.py @@ -0,0 +1,129 @@ +"""Trivy-to-domain normalization with no raw-secret retention.""" + +from __future__ import annotations + +import hashlib +from typing import Any, Mapping + +from devops_learn.domain.enums import FindingCategory, FindingSeverity +from devops_learn.domain.security_models import SecurityFinding +from devops_learn.security.redaction import REDACTED_SECRET, redact_text + + +def _severity(value: object) -> FindingSeverity: + try: + return FindingSeverity(str(value).lower()) + except ValueError: + return FindingSeverity.UNKNOWN + + +def _category(result: Mapping[str, Any], item: Mapping[str, Any], kind: str) -> FindingCategory: + if kind == "secret": + return FindingCategory.SECRET + if kind == "misconfiguration": + text = " ".join(str(item.get(key, "")) for key in ("Type", "ID", "Title")) + lowered = text.lower() + if "kubernetes" in lowered or "k8s" in lowered: + return FindingCategory.KUBERNETES + if "docker" in lowered or "container" in lowered: + return FindingCategory.CONTAINER + if "network" in lowered or "port" in lowered or "ingress" in lowered: + return FindingCategory.NETWORK + if "identity" in lowered or "rbac" in lowered: + return FindingCategory.IDENTITY + return FindingCategory.IAC_MISCONFIGURATION + if kind == "vulnerability": + return FindingCategory.DEPENDENCY + return FindingCategory.UNKNOWN + + +def stable_fingerprint( + *, + category: FindingCategory, + rule_id: str, + target: str, + file: str | None, + resource: str | None, + installed_version: str | None, +) -> str: + """Hash stable identity fields, never scanner prose, ordering, or timestamps.""" + parts = (category.value, rule_id, target, file or "", resource or "", installed_version or "") + return hashlib.sha256("\x1f".join(parts).encode("utf-8")).hexdigest()[:24] + + +def _line(item: Mapping[str, Any]) -> int | None: + cause = item.get("CauseMetadata") + if isinstance(cause, Mapping) and isinstance(cause.get("StartLine"), int): + return int(cause["StartLine"]) + value = item.get("StartLine") + return value if isinstance(value, int) else None + + +def _finding(result: Mapping[str, Any], item: Mapping[str, Any], kind: str) -> SecurityFinding: + target = str(result.get("Target", "unknown")) + rule_id = str(item.get("VulnerabilityID") or item.get("ID") or item.get("RuleID") or "unknown") + resource = item.get("PkgName") or item.get("Resource") + resource_text = str(resource) if resource is not None else None + installed = item.get("InstalledVersion") + installed_text = str(installed) if installed is not None else None + file_value = item.get("File") or target + file = str(file_value) if file_value else None + category = _category(result, item, kind) + title = str(item.get("Title") or item.get("Message") or rule_id) + description = item.get("Description") + description_text = redact_text(str(description)) if description else None + references_raw = item.get("References", []) + references = tuple(str(item) for item in references_raw if isinstance(item, str)) + evidence = None + if kind == "secret": + evidence = f"Secret pattern detected; value {REDACTED_SECRET}" + elif file: + evidence = f"Trivy rule {rule_id} at {file}" + (f":{_line(item)}" if _line(item) else "") + return SecurityFinding( + fingerprint=stable_fingerprint( + category=category, + rule_id=rule_id, + target=target, + file=file, + resource=resource_text, + installed_version=installed_text, + ), + scanner="trivy", + rule_id=rule_id, + title=title, + category=category, + severity=_severity(item.get("Severity")), + target=target, + file=file, + line=_line(item), + resource=resource_text, + installed_version=installed_text, + fixed_version=str(item["FixedVersion"]) if item.get("FixedVersion") else None, + description=description_text, + references=references, + evidence=evidence, + metadata={"trivy_type": str(result.get("Type", "unknown")), "finding_kind": kind}, + ) + + +def normalize_trivy(document: Mapping[str, Any]) -> tuple[SecurityFinding, ...]: + """Normalize a complete Trivy JSON document; tolerate partial output.""" + results = document.get("Results", []) + if not isinstance(results, list): + return () + findings: list[SecurityFinding] = [] + for result in results: + if not isinstance(result, Mapping): + continue + for field, kind in ( + ("Vulnerabilities", "vulnerability"), + ("Misconfigurations", "misconfiguration"), + ("Secrets", "secret"), + ): + items = result.get(field, []) + if not isinstance(items, list): + continue + for item in items: + if isinstance(item, Mapping): + findings.append(_finding(result, item, kind)) + return tuple(findings) diff --git a/src/devops_learn/security/policy.py b/src/devops_learn/security/policy.py new file mode 100644 index 0000000..edef0bb --- /dev/null +++ b/src/devops_learn/security/policy.py @@ -0,0 +1,70 @@ +"""Conftest result parsing and policy input generation.""" + +from __future__ import annotations + +import json +from typing import Any, Mapping + +from devops_learn.domain.enums import SecurityGateDecision +from devops_learn.domain.security_models import PolicyResult, SecurityFinding + +_RANK = { + SecurityGateDecision.ALLOW: 0, + SecurityGateDecision.WARN: 1, + SecurityGateDecision.REQUIRE_APPROVAL: 2, + SecurityGateDecision.BLOCK: 3, +} + + +def policy_input(findings: tuple[SecurityFinding, ...]) -> dict[str, Any]: + return { + "findings": [ + { + "fingerprint": finding.fingerprint, + "rule_id": finding.rule_id, + "category": finding.category.value, + "severity": finding.severity.value, + "change_status": finding.change_status.value, + "title": finding.title, + "target": finding.target, + "file": finding.file, + "resource": finding.resource, + "fixed_version": finding.fixed_version, + } + for finding in findings + ] + } + + +def parse_conftest_output(output: str) -> PolicyResult: + """Map policy-owned message prefixes to a single conservative decision.""" + try: + data = json.loads(output) + except json.JSONDecodeError: + return PolicyResult( + SecurityGateDecision.BLOCK, ("Policy engine returned malformed JSON.",), False + ) + messages: list[str] = [] + if isinstance(data, list): + for result in data: + if not isinstance(result, Mapping): + continue + for failure in result.get("failures", []): + if isinstance(failure, Mapping) and isinstance(failure.get("msg"), str): + messages.append(failure["msg"]) + for warning in result.get("warnings", []): + if isinstance(warning, Mapping) and isinstance(warning.get("msg"), str): + messages.append(warning["msg"]) + decision = SecurityGateDecision.ALLOW + reasons: list[str] = [] + for message in messages: + prefix, _, reason = message.partition(":") + candidate = { + "BLOCK": SecurityGateDecision.BLOCK, + "WARN": SecurityGateDecision.WARN, + "REQUIRE_APPROVAL": SecurityGateDecision.REQUIRE_APPROVAL, + }.get(prefix.strip().upper(), SecurityGateDecision.WARN) + if _RANK[candidate] > _RANK[decision]: + decision = candidate + reasons.append(reason.strip() or message) + return PolicyResult(decision, tuple(reasons)) diff --git a/src/devops_learn/security/redaction.py b/src/devops_learn/security/redaction.py new file mode 100644 index 0000000..dd3b2f5 --- /dev/null +++ b/src/devops_learn/security/redaction.py @@ -0,0 +1,41 @@ +"""Structural redaction for security evidence. + +This is deliberately applied before evidence is normalized, audited, rendered, +or written to an artifact. It is not an instruction for an LLM to follow. +""" + +from __future__ import annotations + +import re +from typing import Any + +REDACTED_SECRET = "[REDACTED SECRET]" +_ASSIGNMENT = re.compile( + r"(?i)\b([A-Z0-9_]*(?:SECRET|TOKEN|PASSWORD|API[_-]?KEY)[A-Z0-9_]*)\s*[=:]\s*([^\s,]+)" +) +_BEARER = re.compile(r"(?i)\b(bearer\s+)[A-Za-z0-9._~+/=-]+") + + +def redact_text(value: str) -> str: + """Remove common credential-shaped values while retaining useful context.""" + value = _ASSIGNMENT.sub(lambda match: f"{match.group(1)}={REDACTED_SECRET}", value) + return _BEARER.sub(lambda match: f"{match.group(1)}{REDACTED_SECRET}", value) + + +def redact_data(value: Any, *, secret_context: bool = False) -> Any: + """Return a recursively safe copy of scanner data. + + Scanner fields named Match, Secret, Value, or Code are evidence-bearing + secret contexts even when their values do not look like a conventional key. + """ + if isinstance(value, dict): + result: dict[str, Any] = {} + for key, item in value.items(): + key_is_secret = key.lower() in {"match", "secret", "value", "code", "lines"} + result[str(key)] = redact_data(item, secret_context=secret_context or key_is_secret) + return result + if isinstance(value, list): + return [redact_data(item, secret_context=secret_context) for item in value] + if isinstance(value, str): + return REDACTED_SECRET if secret_context else redact_text(value) + return value diff --git a/src/devops_learn/security/remediation.py b/src/devops_learn/security/remediation.py new file mode 100644 index 0000000..eaec44f --- /dev/null +++ b/src/devops_learn/security/remediation.py @@ -0,0 +1,59 @@ +"""Deterministic, conservative remediation guidance.""" + +from __future__ import annotations + +from devops_learn.domain.enums import FindingCategory, RemediationRisk +from devops_learn.domain.security_models import RemediationAction, SecurityFinding + + +def plan_remediation(finding: SecurityFinding) -> RemediationAction: + if finding.category is FindingCategory.SECRET: + return RemediationAction( + finding.fingerprint, + ( + "Remove the fixture or credential from source and replace any real " + "credential outside this tool." + ), + "A committed secret can be copied from source history and logs.", + RemediationRisk.HIGH_RISK, + "Credential owner and rotation procedure, if this is not a synthetic fixture.", + False, + ) + if finding.category in (FindingCategory.NETWORK, FindingCategory.IDENTITY): + return RemediationAction( + finding.fingerprint, + "Restrict the exposure to an approved source range or managed access mechanism.", + "Network and identity changes can remove legitimate access or expand attack surface.", + RemediationRisk.REQUIRES_HUMAN_INPUT, + "Approved administrative source range or access strategy.", + False, + ) + if finding.category in (FindingCategory.IAC_MISCONFIGURATION, FindingCategory.KUBERNETES): + return RemediationAction( + finding.fingerprint, + "Apply the scanner's least-privilege recommendation and review the resulting plan.", + "Infrastructure configuration changes can affect availability and access.", + RemediationRisk.REQUIRES_REVIEW, + None, + False, + ) + if finding.category in (FindingCategory.DEPENDENCY, FindingCategory.VULNERABILITY): + return RemediationAction( + finding.fingerprint, + ( + f"Upgrade {finding.resource or 'the affected dependency'} to " + f"{finding.fixed_version or 'a supported fixed version'}." + ), + "A fixed version may require application compatibility testing.", + RemediationRisk.REQUIRES_REVIEW, + None, + False, + ) + return RemediationAction( + finding.fingerprint, + "Review the scanner evidence and make the smallest safe correction.", + "The finding has insufficient normalized context for safe automation.", + RemediationRisk.REQUIRES_REVIEW, + None, + False, + ) diff --git a/src/devops_learn/security/reporting.py b/src/devops_learn/security/reporting.py new file mode 100644 index 0000000..78b7938 --- /dev/null +++ b/src/devops_learn/security/reporting.py @@ -0,0 +1,66 @@ +"""Safe human and machine-readable reporting for normalized evidence.""" + +from __future__ import annotations + +import json +from dataclasses import asdict +from enum import Enum +from pathlib import Path + +from devops_learn.domain.security_models import SecurityReport +from devops_learn.security.redaction import redact_data + + +def render_summary(report: SecurityReport) -> str: + counts = {status: 0 for status in ("introduced", "resolved", "unchanged", "uncertain")} + severities = {severity: 0 for severity in ("critical", "high", "medium", "low")} + for finding in report.findings: + counts[finding.change_status.value] += 1 + if finding.severity.value in severities: + severities[finding.severity.value] += 1 + lines = [ + "DEVSECOPS GATE", + "", + f"Introduced: {counts['introduced']}", + f"Resolved: {counts['resolved']}", + f"Unchanged: {counts['unchanged']}", + f"Uncertain: {counts['uncertain']}", + "", + f"Critical: {severities['critical']}", + f"High: {severities['high']}", + f"Medium: {severities['medium']}", + f"Low: {severities['low']}", + "", + f"GATE: {report.policy.decision.name}", + ] + for finding in report.findings: + if finding.change_status.value == "introduced" and report.policy.decision.value in ( + "block", + "require_approval", + ): + location = f" {finding.file}:{finding.line}" if finding.file and finding.line else "" + lines.append( + f"- {finding.severity.value.upper()} {finding.title} ({finding.rule_id}){location}" + ) + return "\n".join(lines) + + +def write_report(report: SecurityReport, path: Path) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + document = { + "metadata": dict(report.metadata), + "scanner_versions": dict(report.scanner_versions), + "base_ref": report.base_ref, + "proposed_target": report.proposed_target, + "findings": [asdict(finding) for finding in report.findings], + "policy": asdict(report.policy), + } + path.write_text( + json.dumps( + redact_data(document), + indent=2, + sort_keys=True, + default=lambda value: value.value if isinstance(value, Enum) else str(value), + ) + + "\n" + ) diff --git a/src/devops_learn/tools/policy_tool.py b/src/devops_learn/tools/policy_tool.py new file mode 100644 index 0000000..d9a7bb1 --- /dev/null +++ b/src/devops_learn/tools/policy_tool.py @@ -0,0 +1,107 @@ +"""A narrow Conftest boundary for deterministic deployment policy.""" + +from __future__ import annotations + +import shutil +from pathlib import Path +from typing import Any, Mapping + +from devops_learn.tools import _subprocess_safety +from devops_learn.tools.approval import ApprovalRecord +from devops_learn.tools.base import RiskLevel, Tool, ToolOperationSpec, ToolResult + +_OPERATIONS = ( + ToolOperationSpec("version", RiskLevel.SAFE, False, False, False), + ToolOperationSpec("evaluate", RiskLevel.SAFE, False, False, False), +) + + +def _conftest_command() -> str: + command = shutil.which("conftest") + if command is None: + raise FileNotFoundError( + "Conftest CLI not found. Install it from https://www.conftest.dev/install/." + ) + return command + + +class PolicyTool(Tool): + @property + def name(self) -> str: + return "security_policy" + + @property + def operations(self) -> tuple[ToolOperationSpec, ...]: + return _OPERATIONS + + def execute( + self, + operation: str, + params: Mapping[str, Any], + *, + dry_run: bool, + approval: ApprovalRecord | None, + ) -> ToolResult: + spec = self.spec_for(operation) + if dry_run: + return ToolResult( + True, + f"Would run Conftest {operation} (dry run)", + {}, + spec.risk_level, + True, + approval, + ) + try: + conftest = _conftest_command() + if operation == "version": + result = _subprocess_safety.run_safely( + [conftest, "--version"], cwd=None, timeout=15 + ) + return ToolResult( + result.returncode == 0, + ("(real) " if result.returncode == 0 else "(real, failed) ") + + (result.stdout.strip() or result.stderr.strip()), + {"returncode": result.returncode}, + spec.risk_level, + False, + approval, + ) + input_path = Path(str(params["input_path"])).resolve() + policy_path = Path(str(params["policy_path"])).resolve() + if not input_path.is_file() or not policy_path.is_dir(): + raise ValueError("Policy input file or policy directory is unavailable.") + result = _subprocess_safety.run_safely( + [ + conftest, + "test", + "--policy", + str(policy_path), + "--output", + "json", + str(input_path), + ], + cwd=None, + timeout=int(params.get("timeout_seconds", 30)), + ) + # Conftest returns 1 for policy denials. That is a successful policy + # evaluation, not an execution error, when JSON is present. + return ToolResult( + result.returncode in (0, 1) and bool(result.stdout), + "(real) Conftest policy evaluated" + if result.returncode in (0, 1) and result.stdout + else "(real, failed) Conftest policy evaluation failed", + {"returncode": result.returncode, "output": result.stdout, "stderr": result.stderr}, + spec.risk_level, + False, + approval, + ) + except (FileNotFoundError, OSError, ValueError) as exc: + return ToolResult( + False, + f"(real, failed) {exc}", + {"error": str(exc)}, + spec.risk_level, + False, + approval, + ) diff --git a/src/devops_learn/tools/python_tool.py b/src/devops_learn/tools/python_tool.py index f0f2ef4..455e623 100644 --- a/src/devops_learn/tools/python_tool.py +++ b/src/devops_learn/tools/python_tool.py @@ -7,8 +7,11 @@ from __future__ import annotations +import os import shutil import subprocess +import sys +from pathlib import Path from typing import Any, Mapping from devops_learn.tools.approval import ApprovalRecord @@ -40,18 +43,24 @@ def _python_executable() -> str: - return shutil.which("python3") or shutil.which("python") or "python" + """Use the interpreter running the platform, not an unrelated system Python.""" + return sys.executable or shutil.which("python3") or shutil.which("python") or "python" def _run( command: list[str], *, cwd: str | None, check: bool = False ) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + # User-installed pytest plugins are outside a scanned project's dependency + # boundary and can make an otherwise valid local workflow non-deterministic. + environment.setdefault("PYTEST_DISABLE_PLUGIN_AUTOLOAD", "1") return subprocess.run( command, cwd=cwd, capture_output=True, text=True, check=check, + env=environment, ) @@ -130,18 +139,17 @@ def execute( cwd: str | None = params.get("path") try: if operation == "run_tests": - command = [_python_executable(), "-m", "pytest"] + tests_path = (Path(cwd or ".").resolve() / "tests") + command = [_python_executable(), "-m", "pytest", str(tests_path)] completed = _run(command, cwd=cwd) success = completed.returncode == 0 summary = ( - completed.stdout.strip().splitlines()[-1] - if completed.stdout - else "no output" + completed.stdout.strip().splitlines()[-1] if completed.stdout else "no output" ) details = {"returncode": completed.returncode, "stderr": completed.stderr} elif operation == "run_lint": lint_paths = params.get("paths", ["src", "tests"]) - command = ["flake8"] + lint_paths + command = ["flake8", "--jobs", "1"] + lint_paths completed = _run(command, cwd=cwd) success = completed.returncode == 0 summary = completed.stdout.strip() or "No lint issues found" diff --git a/src/devops_learn/tools/security_scanner_tool.py b/src/devops_learn/tools/security_scanner_tool.py new file mode 100644 index 0000000..7b2f1c7 --- /dev/null +++ b/src/devops_learn/tools/security_scanner_tool.py @@ -0,0 +1,266 @@ +"""A narrow, real Trivy tool boundary for read-only security scans.""" + +from __future__ import annotations + +import io +import json +import shutil +import subprocess +import tarfile +import tempfile +from pathlib import Path +from typing import Any, Mapping + +from devops_learn.security.redaction import redact_data +from devops_learn.tools import _subprocess_safety +from devops_learn.tools.approval import ApprovalRecord +from devops_learn.tools.base import RiskLevel, Tool, ToolOperationSpec, ToolResult + +_OPERATIONS = ( + ToolOperationSpec("version", RiskLevel.SAFE, False, False, False), + ToolOperationSpec("scan_filesystem", RiskLevel.SAFE, False, False, False), + ToolOperationSpec("scan_config", RiskLevel.SAFE, False, False, False), + ToolOperationSpec("scan_image", RiskLevel.SAFE, False, False, False), +) + + +def _trivy_command() -> str: + command = shutil.which("trivy") + if command is None: + raise FileNotFoundError( + "Trivy CLI not found. Install Trivy from " + "https://trivy.dev/latest/getting-started/installation/." + ) + return command + + +def _safe_archive_extract(archive: bytes, destination: Path) -> None: + """Extract only ordinary, relative Git archive members.""" + with tarfile.open(fileobj=io.BytesIO(archive), mode="r:") as tar: + for member in tar.getmembers(): + destination_path = (destination / member.name).resolve() + if destination_path != destination and destination not in destination_path.parents: + raise ValueError("Git archive contained an unsafe path") + if not (member.isdir() or member.isreg()): + raise ValueError("Git archive contained a non-regular file") + tar.extractall(destination, filter="data") + + +class SecurityScannerTool(Tool): + """Executes only allow-listed, non-destructive Trivy operations. + + A base ref is materialized as a temporary ``git archive`` rather than a + worktree, so comparison never changes the learner's working tree. + """ + + @property + def name(self) -> str: + return "security_scanner" + + @property + def operations(self) -> tuple[ToolOperationSpec, ...]: + return _OPERATIONS + + def execute( + self, + operation: str, + params: Mapping[str, Any], + *, + dry_run: bool, + approval: ApprovalRecord | None, + ) -> ToolResult: + spec = self.spec_for(operation) + if dry_run: + return ToolResult( + True, f"Would run Trivy {operation} (dry run)", {}, spec.risk_level, True, approval + ) + try: + trivy = _trivy_command() + if operation == "version": + result = _subprocess_safety.run_safely([trivy, "version"], cwd=None, timeout=15) + return ToolResult( + result.returncode == 0, + ("(real) " if result.returncode == 0 else "(real, failed) ") + + (result.stdout.strip() or result.stderr.strip()), + {"returncode": result.returncode}, + spec.risk_level, + False, + approval, + ) + target_value = str(params.get("target", ".")) + target = Path(target_value).resolve() + if operation != "scan_image" and not target.is_dir(): + raise ValueError(f"Scan target is not a directory: {target}") + timeout = int(params.get("timeout_seconds", 120)) + base_ref = params.get("base_ref") + if base_ref is not None and operation == "scan_image": + raise ValueError( + "Base comparison is supported for filesystem and config scans, not images." + ) + if base_ref is not None: + return self._scan_base( + trivy, operation, target, str(base_ref), timeout, spec, approval + ) + scan_target = target_value if operation == "scan_image" else str(target) + return self._scan(trivy, operation, scan_target, timeout, spec, approval) + except (FileNotFoundError, OSError, subprocess.TimeoutExpired, ValueError) as exc: + return ToolResult( + False, + f"(real, failed) {exc}", + {"error": str(exc)}, + spec.risk_level, + False, + approval, + ) + + def _scan_base( + self, + trivy: str, + operation: str, + target: Path, + base_ref: str, + timeout: int, + spec: ToolOperationSpec, + approval: ApprovalRecord | None, + ) -> ToolResult: + repository = subprocess.run( + ["git", "-C", str(target), "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + if repository.returncode != 0 or not repository.stdout.strip(): + return ToolResult( + False, + "(real, failed) Base comparison requires a Git repository target.", + {}, + spec.risk_level, + False, + approval, + ) + repository_root = Path(repository.stdout.strip()).resolve() + try: + relative_target = target.relative_to(repository_root) + except ValueError: + return ToolResult( + False, + "(real, failed) Scan target is outside its Git repository.", + {}, + spec.risk_level, + False, + approval, + ) + verified = subprocess.run( + [ + "git", + "-C", + str(repository_root), + "rev-parse", + "--verify", + "--end-of-options", + f"{base_ref}^{{commit}}", + ], + capture_output=True, + text=True, + check=False, + timeout=15, + ) + if verified.returncode != 0: + return ToolResult( + False, + "(real, failed) Base ref does not resolve to a commit.", + {"base_ref": base_ref}, + spec.risk_level, + False, + approval, + ) + archived = subprocess.run( + ["git", "-C", str(repository_root), "archive", "--format=tar", "--", base_ref], + capture_output=True, + check=False, + timeout=30, + ) + if archived.returncode != 0: + return ToolResult( + False, + "(real, failed) Could not archive the base ref.", + {"base_ref": base_ref}, + spec.risk_level, + False, + approval, + ) + with tempfile.TemporaryDirectory(prefix="devops-learn-security-") as directory: + base_dir = Path(directory) + _safe_archive_extract(archived.stdout, base_dir) + result = self._scan( + trivy, + operation, + str(base_dir / relative_target), + timeout, + spec, + approval, + ) + details = dict(result.details) + details["base_ref"] = base_ref + details["scanned_state"] = "base" + return ToolResult( + result.success, + result.summary, + details, + result.risk_level, + result.was_dry_run, + result.approval, + ) + + def _scan( + self, + trivy: str, + operation: str, + target: str, + timeout: int, + spec: ToolOperationSpec, + approval: ApprovalRecord | None, + ) -> ToolResult: + if operation == "scan_filesystem": + command = [ + trivy, + "fs", + "--format", + "json", + "--quiet", + "--scanners", + "vuln,secret", + ] + secret_config = Path(target) / "trivy-secret.yaml" + if secret_config.is_file(): + command.extend(["--secret-config", str(secret_config)]) + command.append(target) + elif operation == "scan_config": + command = [trivy, "config", "--format", "json", "--quiet", target] + else: + command = [trivy, "image", "--format", "json", "--quiet", target] + result = _subprocess_safety.run_safely(command, cwd=None, timeout=timeout) + try: + parsed = json.loads(result.stdout) if result.stdout else {} + except json.JSONDecodeError: + parsed = None + if result.returncode != 0 or not isinstance(parsed, dict): + reason = "Trivy returned malformed JSON" if parsed is None else "Trivy scan failed" + return ToolResult( + False, + f"(real, failed) {reason}", + {"returncode": result.returncode, "stderr": result.stderr}, + spec.risk_level, + False, + approval, + ) + safe_scan = redact_data(parsed) + return ToolResult( + True, + "(real) Trivy scan completed", + {"scan": safe_scan, "returncode": result.returncode}, + spec.risk_level, + False, + approval, + ) diff --git a/src/devops_learn/workflows/security_flow.py b/src/devops_learn/workflows/security_flow.py new file mode 100644 index 0000000..bc211b2 --- /dev/null +++ b/src/devops_learn/workflows/security_flow.py @@ -0,0 +1,297 @@ +"""The DevSecOps stage: scanner evidence -> policy -> eligibility evidence.""" + +from __future__ import annotations + +import json +import tempfile +from dataclasses import dataclass +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +from devops_learn.bootstrap import Platform +from devops_learn.domain.enums import ( + AuditEventType, + CloudProviderKind, + CostPriority, + EnvironmentKind, + ExecutionMode, + ExperienceState, + ExplanationDepth, +) +from devops_learn.domain.explanation_models import Explanation +from devops_learn.domain.project_models import Artifact +from devops_learn.domain.security_models import SecurityFinding, SecurityReport +from devops_learn.security.change_analysis import classify_changes +from devops_learn.security.normalization import normalize_trivy +from devops_learn.security.policy import parse_conftest_output, policy_input +from devops_learn.security.remediation import plan_remediation +from devops_learn.security.reporting import render_summary, write_report +from devops_learn.workflows.ui import Ui + + +@dataclass(frozen=True) +class SecurityOptions: + target: str + base_ref: str | None = None + image: str | None = None + artifact_path: str | None = None + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +def _audit( + platform: Platform, + session_id: int, + event_type: AuditEventType, + summary: str, + payload: dict[str, Any] | None = None, +) -> None: + platform.audit_service.record( + session_id=session_id, + event_type=event_type, + occurred_at=_now(), + summary=summary, + payload=payload or {}, + ) + + +def _scan( + platform: Platform, operation: str, target: str, base_ref: str | None +) -> tuple[bool, tuple[SecurityFinding, ...]]: + params: dict[str, Any] = {"target": target} + if base_ref: + params["base_ref"] = base_ref + result = platform.tool_service.invoke("security_scanner", operation, params) + if not result.success: + return False, () + scan = result.details.get("scan", {}) + return True, normalize_trivy(scan) if isinstance(scan, dict) else () + + +def run_security_scan( + platform: Platform, ui: Ui, options: SecurityOptions +) -> SecurityReport | None: + target = str(Path(options.target).resolve()) + session = platform.session_service.start( + project_root=target, + mode=ExecutionMode.COLLABORATIVE, + explanation_depth=ExplanationDepth.LEARNING, + cloud=CloudProviderKind.AZURE, + environment=EnvironmentKind.DEV, + cost_priority=CostPriority.BALANCED, + simulation_mode=False, + ) + assert session.id is not None + session_id = session.id + _audit( + platform, + session_id, + AuditEventType.SECURITY_SCAN_STARTED, + "Security scan started", + {"base_ref": options.base_ref} if options.base_ref else {}, + ) + ui.present( + platform.explanation_service.render( + Explanation( + action="Scanning the proposed change with Trivy.", + why=( + "Scanner evidence is collected before policy or AI explanation " + "can describe security risk." + ), + what_to_understand=( + "A scanner is evidence, not a deployment decision. Policy decides " + "eligibility from normalized findings." + ), + ), + mode=ExecutionMode.COLLABORATIVE, + depth=ExplanationDepth.LEARNING, + ) + ) + operations = ["scan_filesystem", "scan_config"] + proposed: list[SecurityFinding] = [] + base: list[SecurityFinding] = [] + for operation in operations: + ok, findings = _scan(platform, operation, target, None) + if not ok: + ui.present( + "SECURITY SCAN FAILED: Trivy could not produce usable evidence. " + "Run `devops-learn security doctor` for prerequisites." + ) + _audit( + platform, + session_id, + AuditEventType.SECURITY_SCAN_COMPLETED, + "Security scan failed", + {"success": False}, + ) + platform.session_service.complete(session) + return None + proposed.extend(findings) + if options.base_ref: + ok, findings = _scan(platform, operation, target, options.base_ref) + if not ok: + ui.present( + "SECURITY SCAN FAILED: the requested base ref could not be scanned safely." + ) + _audit( + platform, + session_id, + AuditEventType.SECURITY_SCAN_COMPLETED, + "Base security scan failed", + {"success": False}, + ) + platform.session_service.complete(session) + return None + base.extend(findings) + if options.image: + ok, findings = _scan(platform, "scan_image", options.image, None) + if ok: + proposed.extend(findings) + else: + ui.present( + "IMAGE SECURITY SCAN FAILED: filesystem/config evidence is retained, " + "but no image gate was evaluated." + ) + platform.session_service.complete(session) + return None + findings = ( + classify_changes(tuple(base), tuple(proposed)) if options.base_ref else tuple(proposed) + ) + if not options.base_ref: + # A single-state scan cannot assert historical novelty. + from dataclasses import replace + from devops_learn.domain.enums import FindingChangeStatus + + findings = tuple( + replace(finding, change_status=FindingChangeStatus.UNCERTAIN) for finding in findings + ) + with tempfile.NamedTemporaryFile( + mode="w", suffix=".json", prefix="devops-learn-policy-", delete=False + ) as temporary: + json.dump(policy_input(findings), temporary) + policy_input_path = Path(temporary.name) + try: + policy_dir = Path(__file__).resolve().parents[3] / "policy" / "security" + policy_result = platform.tool_service.invoke( + "security_policy", + "evaluate", + {"input_path": str(policy_input_path), "policy_path": str(policy_dir)}, + ) + finally: + policy_input_path.unlink(missing_ok=True) + if not policy_result.success: + ui.present( + "SECURITY POLICY FAILED: Conftest could not evaluate normalized evidence. " + "Run `devops-learn security doctor` for prerequisites." + ) + _audit( + platform, + session_id, + AuditEventType.SECURITY_GATE_EVALUATED, + "Security policy failed", + {"success": False}, + ) + platform.session_service.complete(session) + return None + policy = parse_conftest_output(str(policy_result.details.get("output", ""))) + report = SecurityReport( + findings=findings, + policy=policy, + scanner_versions={"trivy": "detected at runtime", "conftest": "detected at runtime"}, + base_ref=options.base_ref, + proposed_target=target, + metadata={"scanner": "real", "policy": "conftest", "raw_scanner_output_persisted": "false"}, + ) + artifact_path = ( + Path(options.artifact_path) + if options.artifact_path + else Path(target) / "artifacts" / "security" / "security-report.json" + ) + write_report(report, artifact_path) + platform.artifact_repository.create( + Artifact(session_id, "security_report", str(artifact_path), _now()) + ) + _audit( + platform, + session_id, + AuditEventType.SECURITY_SCAN_COMPLETED, + "Security scan completed", + {"finding_count": len(findings), "secret_values_logged": False}, + ) + _audit( + platform, + session_id, + AuditEventType.SECURITY_GATE_EVALUATED, + f"Security gate: {policy.decision.value}", + {"decision": policy.decision.value, "reason_count": len(policy.reasons)}, + ) + platform.experience_tracker.record( + session_id, "DevSecOps", "Reviewed normalized scanner evidence", ExperienceState.PRACTICED + ) + platform.experience_tracker.record( + session_id, + "Policy as code", + f"Observed {policy.decision.value} gate", + ExperienceState.INTRODUCED, + ) + ui.present(render_summary(report)) + for finding in findings: + if finding.change_status.value == "introduced" and policy.decision.value in ( + "block", + "require_approval", + ): + remediation = plan_remediation(finding) + ui.present( + platform.explanation_service.render( + Explanation( + action=f"Security finding: {finding.title}", + why=finding.description + or "The scanner reported this finding in the proposed state.", + decision=f"Policy gate is {policy.decision.value.upper()}.", + what_to_understand=( + "Least privilege and reduced attack surface reduce the impact " + "of a compromise." + ), + result=( + "Recommended remediation: " + f"{remediation.recommended_change} Risk: {remediation.risk.value}." + ), + ), + mode=ExecutionMode.COLLABORATIVE, + depth=ExplanationDepth.LEARNING, + ) + ) + _audit( + platform, + session_id, + AuditEventType.REMEDIATION_PROPOSED, + "Security remediation proposed", + {"finding": finding.fingerprint, "risk": remediation.risk.value}, + ) + ui.present(f"Security report: {artifact_path}") + platform.session_service.complete(session) + return report + + +def run_security_doctor(platform: Platform, ui: Ui) -> bool: + checks = ( + ("Git", ["git", "--version"]), + ("Docker", ["docker", "--version"]), + ("Terraform", ["terraform", "--version"]), + ) + import shutil + + for label, command in checks: + ui.present(f"{label:<10} {'available' if shutil.which(command[0]) else 'unavailable'}") + trivy = platform.tool_service.invoke("security_scanner", "version") + conftest = platform.tool_service.invoke("security_policy", "version") + ui.present(f"Trivy {'available' if trivy.success else 'unavailable'}") + ui.present(f"Conftest {'available' if conftest.success else 'unavailable'}") + if not trivy.success: + ui.present("Install Trivy: https://trivy.dev/latest/getting-started/installation/") + if not conftest.success: + ui.present("Install Conftest: https://www.conftest.dev/install/") + return trivy.success and conftest.success diff --git a/tests/security/__init__.py b/tests/security/__init__.py new file mode 100644 index 0000000..295ee2a --- /dev/null +++ b/tests/security/__init__.py @@ -0,0 +1 @@ +"""Tests for the DevSecOps control plane.""" diff --git a/tests/security/test_change_policy_eligibility.py b/tests/security/test_change_policy_eligibility.py new file mode 100644 index 0000000..3b7b51e --- /dev/null +++ b/tests/security/test_change_policy_eligibility.py @@ -0,0 +1,109 @@ +from devops_learn.domain.enums import ( + DeploymentEligibility, + FindingCategory, + FindingChangeStatus, + FindingSeverity, + SecurityGateDecision, +) +from devops_learn.domain.security_models import SecurityFinding +from devops_learn.security.change_analysis import classify_changes +from devops_learn.security.eligibility import evaluate_deployment_eligibility +from devops_learn.security.normalization import stable_fingerprint +from devops_learn.security.policy import parse_conftest_output + + +def _finding( + fingerprint: str = "same", + severity: FindingSeverity = FindingSeverity.HIGH, + category: FindingCategory = FindingCategory.DEPENDENCY, +) -> SecurityFinding: + return SecurityFinding(fingerprint, "trivy", "RULE", "Finding", category, severity, "target") + + +def test_change_classification_and_fingerprint_stability() -> None: + unchanged = _finding("unchanged") + introduced = _finding("introduced") + resolved = _finding("resolved") + result = classify_changes((unchanged, resolved), (unchanged, introduced)) + statuses = {finding.fingerprint: finding.change_status for finding in result} + assert statuses == { + "unchanged": FindingChangeStatus.UNCHANGED, + "introduced": FindingChangeStatus.INTRODUCED, + "resolved": FindingChangeStatus.RESOLVED, + } + first = stable_fingerprint( + category=FindingCategory.DEPENDENCY, + rule_id="CVE", + target="a", + file="a", + resource="pkg", + installed_version="1", + ) + second = stable_fingerprint( + category=FindingCategory.DEPENDENCY, + rule_id="CVE", + target="a", + file="a", + resource="pkg", + installed_version="1", + ) + assert first == second + + +def test_duplicate_fingerprint_is_uncertain() -> None: + result = classify_changes((_finding(),), (_finding(), _finding())) + assert all(finding.change_status is FindingChangeStatus.UNCERTAIN for finding in result) + + +def test_conftest_policy_output_maps_to_most_restrictive_gate() -> None: + output = ( + '[{"failures":[{"msg":"BLOCK: introduced secret"}],' + '"warnings":[{"msg":"WARN: introduced medium"}]}]' + ) + result = parse_conftest_output(output) + assert result.decision is SecurityGateDecision.BLOCK + assert ( + parse_conftest_output( + '[{"warnings":[{"msg":"REQUIRE_APPROVAL: high dependency"}]}]' + ).decision + is SecurityGateDecision.REQUIRE_APPROVAL + ) + assert ( + parse_conftest_output('[{"warnings":[{"msg":"WARN: uncertain high"}]}]').decision + is SecurityGateDecision.WARN + ) + + +def test_deployment_eligibility_obeys_validation_security_and_approval() -> None: + assert ( + evaluate_deployment_eligibility( + validation_passed=False, gate=SecurityGateDecision.ALLOW + ).eligibility + is DeploymentEligibility.INELIGIBLE + ) + assert ( + evaluate_deployment_eligibility( + validation_passed=True, gate=SecurityGateDecision.BLOCK + ).eligibility + is DeploymentEligibility.INELIGIBLE + ) + assert ( + evaluate_deployment_eligibility( + validation_passed=True, gate=SecurityGateDecision.REQUIRE_APPROVAL + ).eligibility + is DeploymentEligibility.PENDING_APPROVAL + ) + assert ( + evaluate_deployment_eligibility( + validation_passed=True, + gate=SecurityGateDecision.REQUIRE_APPROVAL, + approval_granted=True, + ).eligibility + is DeploymentEligibility.ELIGIBLE + ) + assert ( + evaluate_deployment_eligibility( + validation_passed=True, gate=SecurityGateDecision.ALLOW + ).eligibility + is DeploymentEligibility.ELIGIBLE + ) diff --git a/tests/security/test_normalization.py b/tests/security/test_normalization.py new file mode 100644 index 0000000..4ca0bd1 --- /dev/null +++ b/tests/security/test_normalization.py @@ -0,0 +1,79 @@ +from devops_learn.domain.enums import FindingCategory, FindingSeverity +from devops_learn.security.normalization import normalize_trivy +from devops_learn.security.reporting import write_report +from devops_learn.domain.security_models import PolicyResult, SecurityReport +from devops_learn.domain.enums import SecurityGateDecision + +FAKE_SECRET = "DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL_9f31b7" + + +def _document() -> dict: + return { + "Results": [ + { + "Target": "infra/main.tf", + "Type": "terraform", + "Vulnerabilities": [ + { + "VulnerabilityID": "CVE-2026-0001", + "PkgName": "demo", + "InstalledVersion": "1.0", + "FixedVersion": "1.1", + "Severity": "HIGH", + "Title": "demo vulnerability", + } + ], + "Misconfigurations": [ + { + "ID": "AVD-AZU-0001", + "Title": "Public SSH ingress", + "Description": "opens a port", + "Severity": "HIGH", + "CauseMetadata": {"StartLine": 42}, + } + ], + "Secrets": [ + { + "RuleID": "generic-api-key", + "Title": "Synthetic secret", + "Severity": "CRITICAL", + "Match": FAKE_SECRET, + "StartLine": 3, + } + ], + } + ] + } + + +def test_normalizes_vulnerability_iac_and_secret_without_match() -> None: + findings = normalize_trivy(_document()) + assert {finding.category for finding in findings} == { + FindingCategory.DEPENDENCY, + FindingCategory.NETWORK, + FindingCategory.SECRET, + } + secret = next(finding for finding in findings if finding.category is FindingCategory.SECRET) + assert secret.severity is FindingSeverity.CRITICAL + assert FAKE_SECRET not in str(secret) + assert "REDACTED" in (secret.evidence or "") + + +def test_incomplete_and_unknown_trivy_json_is_safe() -> None: + assert ( + normalize_trivy({"Results": [{"Target": "x", "Secrets": [{}]}]})[0].category + is FindingCategory.SECRET + ) + assert normalize_trivy({"Results": "not-a-list"}) == () + + +def test_secret_never_survives_report_serialization(tmp_path) -> None: + finding = next( + item for item in normalize_trivy(_document()) if item.category is FindingCategory.SECRET + ) + report = SecurityReport( + (finding,), PolicyResult(SecurityGateDecision.BLOCK, ("secret",)), {}, None, "fixture" + ) + path = tmp_path / "security-report.json" + write_report(report, path) + assert FAKE_SECRET not in path.read_text() diff --git a/tests/security/test_secret_safety.py b/tests/security/test_secret_safety.py new file mode 100644 index 0000000..3b71444 --- /dev/null +++ b/tests/security/test_secret_safety.py @@ -0,0 +1,65 @@ +import json +from datetime import datetime, timezone + +from devops_learn.domain.enums import AuditEventType, ExecutionMode, ExplanationDepth +from devops_learn.domain.explanation_models import Explanation +from devops_learn.domain.security_models import PolicyResult, SecurityReport +from devops_learn.domain.enums import SecurityGateDecision +from devops_learn.explanations.service import ExplanationService +from devops_learn.learning.persistence.repositories.audit_repository import AuditRepository +from devops_learn.audit.service import AuditService +from devops_learn.security.normalization import normalize_trivy +from devops_learn.security.reporting import write_report + +FAKE_SECRET = "demo-only-fixture-9f31b7-not-a-real-token" + + +def _secret_finding(): + return normalize_trivy( + { + "Results": [ + { + "Target": "fixture.py", + "Secrets": [ + { + "RuleID": "demo", + "Title": "Synthetic secret", + "Match": FAKE_SECRET, + "Severity": "CRITICAL", + } + ], + } + ] + } + )[0] + + +def test_fake_secret_cannot_reach_explanation_report_or_audit( + seeded_session, conn, tmp_path +) -> None: + finding = _secret_finding() + explanation = ExplanationService().render( + Explanation(action=finding.title, why=finding.evidence), + mode=ExecutionMode.COLLABORATIVE, + depth=ExplanationDepth.DEEP, + ) + assert FAKE_SECRET not in explanation + + report_path = tmp_path / "report.json" + write_report( + SecurityReport( + (finding,), PolicyResult(SecurityGateDecision.BLOCK, ("secret",)), {}, None, "fixture" + ), + report_path, + ) + assert FAKE_SECRET not in report_path.read_text() + + audit = AuditService(AuditRepository(conn)) + audit.record( + session_id=seeded_session, + event_type=AuditEventType.SECURITY_SCAN_COMPLETED, + occurred_at=datetime.now(timezone.utc), + summary="Synthetic scan", + payload={"Match": FAKE_SECRET}, + ) + assert FAKE_SECRET not in json.dumps(audit.history(seeded_session)[0].payload) diff --git a/tests/tools/test_security_tools.py b/tests/tools/test_security_tools.py new file mode 100644 index 0000000..fff580f --- /dev/null +++ b/tests/tools/test_security_tools.py @@ -0,0 +1,67 @@ +import os +from pathlib import Path + +from devops_learn.tools.policy_tool import PolicyTool +from devops_learn.tools.security_scanner_tool import SecurityScannerTool + + +def _write_binary(path: Path, body: str) -> None: + path.write_text("#!/bin/sh\n" + body) + path.chmod(0o755) + + +def test_scanner_reports_missing_binary(monkeypatch, tmp_path) -> None: + monkeypatch.setenv("PATH", str(tmp_path)) + result = SecurityScannerTool().execute( + "scan_filesystem", {"target": str(tmp_path)}, dry_run=False, approval=None + ) + assert not result.success + assert "Trivy CLI not found" in result.summary + + +def test_scanner_redacts_valid_json_and_rejects_malformed(monkeypatch, tmp_path) -> None: + _write_binary( + tmp_path / "trivy", + ( + 'if [ "$FAKE_BAD" = "1" ]; then echo nope; else echo ' + '\'{"Results":[{"Target":"x","Secrets":[{"RuleID":"x",' + '"Match":"DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL_9f31b7"}]}]}\'; fi' + ), + ) + monkeypatch.setenv("PATH", f"{tmp_path}{os.pathsep}{os.environ['PATH']}") + result = SecurityScannerTool().execute( + "scan_filesystem", {"target": str(tmp_path)}, dry_run=False, approval=None + ) + assert result.success + assert "DEVSECOPS_DEMO_ONLY" not in str(result.details) + monkeypatch.setenv("FAKE_BAD", "1") + malformed = SecurityScannerTool().execute( + "scan_filesystem", {"target": str(tmp_path)}, dry_run=False, approval=None + ) + assert not malformed.success + + +def test_scanner_reports_version_failure_and_timeout(monkeypatch, tmp_path) -> None: + _write_binary( + tmp_path / "trivy", + 'if [ "$1" = "version" ]; then echo "Version: test"; exit 0; fi; sleep 2; echo "{}"', + ) + monkeypatch.setenv("PATH", f"{tmp_path}{os.pathsep}{os.environ['PATH']}") + version = SecurityScannerTool().execute("version", {}, dry_run=False, approval=None) + assert version.success + assert "Version: test" in version.summary + timed_out = SecurityScannerTool().execute( + "scan_filesystem", + {"target": str(tmp_path), "timeout_seconds": 1}, + dry_run=False, + approval=None, + ) + assert not timed_out.success + assert "timed out" in str(timed_out.details).lower() + + +def test_policy_tool_reports_missing_conftest(monkeypatch, tmp_path) -> None: + monkeypatch.setenv("PATH", str(tmp_path)) + result = PolicyTool().execute("version", {}, dry_run=False, approval=None) + assert not result.success + assert "Conftest CLI not found" in result.summary From 547242934616ecc46514455c0f99e30107f13bbb Mon Sep 17 00:00:00 2001 From: howlcipher Date: Thu, 13 Aug 2026 12:41:01 -0400 Subject: [PATCH 8/8] docs(security): add gated DevSecOps demo --- projects/devsecops_demo/README.md | 12 ++++++++++++ projects/devsecops_demo/baseline/Dockerfile | 7 +++++++ projects/devsecops_demo/baseline/app.py | 1 + projects/devsecops_demo/baseline/infra/main.tf | 4 ++++ projects/devsecops_demo/baseline/trivy-secret.yaml | 10 ++++++++++ projects/devsecops_demo/proposed/Dockerfile | 5 +++++ projects/devsecops_demo/proposed/app.py | 3 +++ projects/devsecops_demo/proposed/infra/main.tf | 13 +++++++++++++ projects/devsecops_demo/proposed/trivy-secret.yaml | 10 ++++++++++ projects/devsecops_demo/remediated/Dockerfile | 7 +++++++ projects/devsecops_demo/remediated/app.py | 1 + projects/devsecops_demo/remediated/infra/main.tf | 13 +++++++++++++ .../devsecops_demo/remediated/trivy-secret.yaml | 10 ++++++++++ 13 files changed, 96 insertions(+) create mode 100644 projects/devsecops_demo/README.md create mode 100644 projects/devsecops_demo/baseline/Dockerfile create mode 100644 projects/devsecops_demo/baseline/app.py create mode 100644 projects/devsecops_demo/baseline/infra/main.tf create mode 100644 projects/devsecops_demo/baseline/trivy-secret.yaml create mode 100644 projects/devsecops_demo/proposed/Dockerfile create mode 100644 projects/devsecops_demo/proposed/app.py create mode 100644 projects/devsecops_demo/proposed/infra/main.tf create mode 100644 projects/devsecops_demo/proposed/trivy-secret.yaml create mode 100644 projects/devsecops_demo/remediated/Dockerfile create mode 100644 projects/devsecops_demo/remediated/app.py create mode 100644 projects/devsecops_demo/remediated/infra/main.tf create mode 100644 projects/devsecops_demo/remediated/trivy-secret.yaml diff --git a/projects/devsecops_demo/README.md b/projects/devsecops_demo/README.md new file mode 100644 index 0000000..382249e --- /dev/null +++ b/projects/devsecops_demo/README.md @@ -0,0 +1,12 @@ +# DevSecOps demo fixture + +This project is an isolated, synthetic demonstration of change-aware security +gating. It contains no live credential, cloud state, or deployable service. + +`baseline/` is the safe starting state. `proposed/` deliberately adds three +mistakes: a synthetic secret fixture, root container execution, and public SSH +ingress. `remediated/` removes those mistakes. Scan these directories only as +described in [docs/devsecops.md](../../docs/devsecops.md). + +The value named `DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL` is deliberately fake +and must never be replaced with a usable credential. diff --git a/projects/devsecops_demo/baseline/Dockerfile b/projects/devsecops_demo/baseline/Dockerfile new file mode 100644 index 0000000..b0b74e7 --- /dev/null +++ b/projects/devsecops_demo/baseline/Dockerfile @@ -0,0 +1,7 @@ +FROM python:3.11-slim + +WORKDIR /app +RUN useradd --create-home appuser +COPY app.py . +USER appuser +CMD ["python", "app.py"] diff --git a/projects/devsecops_demo/baseline/app.py b/projects/devsecops_demo/baseline/app.py new file mode 100644 index 0000000..b86b95f --- /dev/null +++ b/projects/devsecops_demo/baseline/app.py @@ -0,0 +1 @@ +print("secure demonstration") diff --git a/projects/devsecops_demo/baseline/infra/main.tf b/projects/devsecops_demo/baseline/infra/main.tf new file mode 100644 index 0000000..2e7c478 --- /dev/null +++ b/projects/devsecops_demo/baseline/infra/main.tf @@ -0,0 +1,4 @@ +resource "azurerm_resource_group" "demo" { + name = "devsecops-demo" + location = "eastus" +} diff --git a/projects/devsecops_demo/baseline/trivy-secret.yaml b/projects/devsecops_demo/baseline/trivy-secret.yaml new file mode 100644 index 0000000..f8a8654 --- /dev/null +++ b/projects/devsecops_demo/baseline/trivy-secret.yaml @@ -0,0 +1,10 @@ +rules: + - id: devsecops-demo-only-secret + category: devsecops-demo + title: Synthetic DevSecOps demo secret fixture + severity: CRITICAL + path: .*app\.py + keywords: + - DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL + regex: (?PDEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL)\s*=\s*["'](?P[a-z0-9-]{20,128})["'] + secret-group-name: secret diff --git a/projects/devsecops_demo/proposed/Dockerfile b/projects/devsecops_demo/proposed/Dockerfile new file mode 100644 index 0000000..13862e1 --- /dev/null +++ b/projects/devsecops_demo/proposed/Dockerfile @@ -0,0 +1,5 @@ +FROM python:3.11-slim + +WORKDIR /app +COPY app.py . +CMD ["python", "app.py"] diff --git a/projects/devsecops_demo/proposed/app.py b/projects/devsecops_demo/proposed/app.py new file mode 100644 index 0000000..5e6a799 --- /dev/null +++ b/projects/devsecops_demo/proposed/app.py @@ -0,0 +1,3 @@ +# Synthetic fixture only. It cannot authenticate to any service. +DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL = "demo-only-fixture-9f31b7-not-a-real-token" +print("insecure demonstration") diff --git a/projects/devsecops_demo/proposed/infra/main.tf b/projects/devsecops_demo/proposed/infra/main.tf new file mode 100644 index 0000000..d730761 --- /dev/null +++ b/projects/devsecops_demo/proposed/infra/main.tf @@ -0,0 +1,13 @@ +resource "azurerm_network_security_rule" "public_administrative_access" { + name = "demo-public-ssh" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "22" + source_address_prefix = "*" + destination_address_prefix = "*" + resource_group_name = "devsecops-demo" + network_security_group_name = "demo-nsg" +} diff --git a/projects/devsecops_demo/proposed/trivy-secret.yaml b/projects/devsecops_demo/proposed/trivy-secret.yaml new file mode 100644 index 0000000..f8a8654 --- /dev/null +++ b/projects/devsecops_demo/proposed/trivy-secret.yaml @@ -0,0 +1,10 @@ +rules: + - id: devsecops-demo-only-secret + category: devsecops-demo + title: Synthetic DevSecOps demo secret fixture + severity: CRITICAL + path: .*app\.py + keywords: + - DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL + regex: (?PDEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL)\s*=\s*["'](?P[a-z0-9-]{20,128})["'] + secret-group-name: secret diff --git a/projects/devsecops_demo/remediated/Dockerfile b/projects/devsecops_demo/remediated/Dockerfile new file mode 100644 index 0000000..b0b74e7 --- /dev/null +++ b/projects/devsecops_demo/remediated/Dockerfile @@ -0,0 +1,7 @@ +FROM python:3.11-slim + +WORKDIR /app +RUN useradd --create-home appuser +COPY app.py . +USER appuser +CMD ["python", "app.py"] diff --git a/projects/devsecops_demo/remediated/app.py b/projects/devsecops_demo/remediated/app.py new file mode 100644 index 0000000..b9cbf25 --- /dev/null +++ b/projects/devsecops_demo/remediated/app.py @@ -0,0 +1 @@ +print("remediated demonstration") diff --git a/projects/devsecops_demo/remediated/infra/main.tf b/projects/devsecops_demo/remediated/infra/main.tf new file mode 100644 index 0000000..10b02dd --- /dev/null +++ b/projects/devsecops_demo/remediated/infra/main.tf @@ -0,0 +1,13 @@ +resource "azurerm_network_security_rule" "administrative_access" { + name = "demo-private-ssh" + priority = 100 + direction = "Inbound" + access = "Allow" + protocol = "Tcp" + source_port_range = "*" + destination_port_range = "22" + source_address_prefix = "10.0.0.0/24" + destination_address_prefix = "10.0.0.4" + resource_group_name = "devsecops-demo" + network_security_group_name = "demo-nsg" +} diff --git a/projects/devsecops_demo/remediated/trivy-secret.yaml b/projects/devsecops_demo/remediated/trivy-secret.yaml new file mode 100644 index 0000000..f8a8654 --- /dev/null +++ b/projects/devsecops_demo/remediated/trivy-secret.yaml @@ -0,0 +1,10 @@ +rules: + - id: devsecops-demo-only-secret + category: devsecops-demo + title: Synthetic DevSecOps demo secret fixture + severity: CRITICAL + path: .*app\.py + keywords: + - DEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL + regex: (?PDEVSECOPS_DEMO_ONLY_NOT_A_CREDENTIAL)\s*=\s*["'](?P[a-z0-9-]{20,128})["'] + secret-group-name: secret