From 72a419216280f42e7ce1bb690882355e174ae4d9 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:22:12 +0100 Subject: [PATCH 01/24] fix: stabilize CI and isolate ExternalId verification - fix all current CLI Ruff findings so the CLI test job can execute pytest - pin CLI CI pytest/ruff versions to the repository-verified toolchain - make the wrong-ExternalId STS probe use the same role session name and duration as the successful AssumeRole path - assert the negative-control request shape in tests so unrelated trust-policy conditions cannot masquerade as ExternalId enforcement This is the safety baseline before replacing the legacy drift detector with the evidence-core architecture. --- .github/workflows/ci.yml | 2 +- backend/integrations/aws_auth.py | 43 +++++++++++++++++--------------- backend/tests/test_aws_auth.py | 14 ++++++++--- cli/driftguard_cli/config.py | 4 +-- cli/driftguard_cli/main.py | 2 +- cli/tests/test_client.py | 1 - 6 files changed, 38 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 39f57f4..e059132 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: python-version: "3.12" cache: pip - run: pip install -e ./cli - - run: pip install pytest ruff + - run: pip install pytest==9.1.1 ruff==0.16.5 - run: ruff check cli/driftguard_cli/ cli/tests/ - run: pytest cli/tests/ -v --tb=short diff --git a/backend/integrations/aws_auth.py b/backend/integrations/aws_auth.py index 8afac85..0a2557a 100644 --- a/backend/integrations/aws_auth.py +++ b/backend/integrations/aws_auth.py @@ -8,7 +8,7 @@ 1. Customer creates an IAM role in their account with a trust policy that allows DriftGuard's own AWS account to assume it, conditioned on a per-workspace external ID (confused-deputy - protection — see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html). + protection — see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_common-scenarios_third-party.html). 2. DriftGuard stores only the role ARN + external ID (both non-sensitive; the external ID is not a secret, it's a correlation token). @@ -55,39 +55,42 @@ class AssumeRoleResult: def check_role_misconfigured(role_arn: str, region: str) -> bool: """ - Validates that a customer's IAM role actually enforces the external ID - condition, rather than trusting our external_id blindly. - - Attempts sts:AssumeRole with a deliberately wrong external ID. If AWS - rejects it (AccessDenied), the role's trust policy correctly conditions - on sts:ExternalId. If it unexpectedly succeeds, the customer's trust - policy has no external ID condition at all — the role is open to any - party that knows the account ID, and DriftGuard should refuse to use it. - - This mirrors the validation pattern documented by Datadog Security Labs - for multi-tenant SaaS integrations assuming customer-owned roles. + Negative-control check for sts:ExternalId enforcement. + + This function MUST be called only after the same principal has already + proved it can assume ``role_arn`` with the workspace's correct external + ID. The negative control then repeats the same AssumeRole request shape + (same role ARN, session name, duration, caller and region) while changing + only ExternalId to a deliberately wrong value. + + Holding the other request parameters constant is important. IAM trust + policies can independently constrain values such as sts:RoleSessionName; + changing those values as part of the negative control would make an + AccessDenied response ambiguous and could falsely "prove" that + sts:ExternalId was enforced. + + Returns True if the wrong external ID is unexpectedly accepted (unsafe), + False when AWS rejects the otherwise-equivalent request with AccessDenied, + and re-raises non-authorization errors because they are not evidence either + way about the external-ID condition. """ probe_external_id = f"probe-{secrets.token_urlsafe(16)}" try: sts = boto3.client("sts", region_name=region) sts.assume_role( RoleArn=role_arn, - RoleSessionName="driftguard-misconfig-check", + RoleSessionName=ASSUME_ROLE_SESSION_NAME, ExternalId=probe_external_id, - DurationSeconds=900, # minimum allowed; this session is discarded immediately + DurationSeconds=ASSUME_ROLE_DURATION_SECONDS, ) - # If assume_role succeeded with a made-up external ID, the trust - # policy isn't enforcing sts:ExternalId at all. Vulnerable. log.warning( - "Role has no enforced external ID condition — confused deputy risk", + "Role accepted a deliberately wrong external ID — confused deputy protection is not enforced", role_arn=role_arn, ) return True except ClientError as e: if e.response.get("Error", {}).get("Code") == "AccessDenied": - return False # correctly rejected the wrong external ID — properly configured - # Any other error (bad ARN, role doesn't exist, etc.) isn't a - # confused-deputy signal — surface it separately, don't claim "safe". + return False log.error("Could not evaluate role trust policy", role_arn=role_arn, error=str(e)) raise diff --git a/backend/tests/test_aws_auth.py b/backend/tests/test_aws_auth.py index 81cb70e..ca8e914 100644 --- a/backend/tests/test_aws_auth.py +++ b/backend/tests/test_aws_auth.py @@ -11,6 +11,8 @@ from moto import mock_aws from backend.integrations.aws_auth import ( + ASSUME_ROLE_DURATION_SECONDS, + ASSUME_ROLE_SESSION_NAME, assume_workspace_role, check_role_misconfigured, generate_external_id, @@ -127,7 +129,7 @@ def test_resolve_scan_session_ambient_chain_succeeds_when_credentials_present(mo # both real AWS behaviors are exercised deliberately. def test_check_role_misconfigured_returns_false_when_access_denied(): - """AWS correctly rejecting a wrong external ID = properly configured role.""" + """Rejecting the otherwise-equivalent wrong-ExternalId probe is the required negative control.""" mock_client = MagicMock() mock_client.assume_role.side_effect = _client_error("AccessDenied") @@ -135,6 +137,11 @@ def test_check_role_misconfigured_returns_false_when_access_denied(): result = check_role_misconfigured(TEST_ROLE_ARN, "us-east-1") assert result is False + call = mock_client.assume_role.call_args.kwargs + assert call["RoleArn"] == TEST_ROLE_ARN + assert call["RoleSessionName"] == ASSUME_ROLE_SESSION_NAME + assert call["DurationSeconds"] == ASSUME_ROLE_DURATION_SECONDS + assert call["ExternalId"].startswith("probe-") def test_check_role_misconfigured_returns_true_when_assume_unexpectedly_succeeds(): @@ -153,6 +160,9 @@ def test_check_role_misconfigured_returns_true_when_assume_unexpectedly_succeeds result = check_role_misconfigured(TEST_ROLE_ARN, "us-east-1") assert result is True + call = mock_client.assume_role.call_args.kwargs + assert call["RoleSessionName"] == ASSUME_ROLE_SESSION_NAME + assert call["DurationSeconds"] == ASSUME_ROLE_DURATION_SECONDS def test_check_role_misconfigured_reraises_non_access_denied_errors(): @@ -165,8 +175,6 @@ def test_check_role_misconfigured_reraises_non_access_denied_errors(): def _client_error(code: str): - from botocore.exceptions import ClientError - return ClientError( error_response={"Error": {"Code": code, "Message": f"Simulated {code}"}}, operation_name="AssumeRole", diff --git a/cli/driftguard_cli/config.py b/cli/driftguard_cli/config.py index f5405b8..d2debb0 100644 --- a/cli/driftguard_cli/config.py +++ b/cli/driftguard_cli/config.py @@ -10,7 +10,7 @@ import json import os -from dataclasses import dataclass, asdict +from dataclasses import asdict, dataclass from pathlib import Path CONFIG_DIR = Path.home() / ".driftguard" @@ -24,7 +24,7 @@ class Config: api_key: str | None = None @classmethod - def load(cls) -> "Config": + def load(cls) -> Config: cfg = cls() if CONFIG_FILE.exists(): try: diff --git a/cli/driftguard_cli/main.py b/cli/driftguard_cli/main.py index 242413c..f55edbd 100644 --- a/cli/driftguard_cli/main.py +++ b/cli/driftguard_cli/main.py @@ -11,8 +11,8 @@ import typer from rich.console import Console -from rich.table import Table from rich.panel import Panel +from rich.table import Table from .client import DriftGuardAPIError, DriftGuardClient from .config import Config diff --git a/cli/tests/test_client.py b/cli/tests/test_client.py index 554ccb7..2018616 100644 --- a/cli/tests/test_client.py +++ b/cli/tests/test_client.py @@ -7,7 +7,6 @@ import httpx import pytest - from driftguard_cli.client import DriftGuardAPIError, DriftGuardClient From 6f8e84f80f50bfbdfa6d179bad6d565d6fda6445 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:24:43 +0100 Subject: [PATCH 02/24] feat: introduce provider-native drift evidence core Add Evidence Bundle v1 and a fail-closed Terraform/OpenTofu plan analyzer built around provider-native resource_drift semantics. Key invariants: - preserve absolute Terraform/OpenTofu resource addresses verbatim - omit all raw before/after values from evidence output - retain only changed JSON-pointer paths and sensitivity paths - reject errored plans and unsupported major JSON formats - skip non-managed entries explicitly rather than inventing drift semantics Document the architecture decision, rejected alternatives, security boundary, migration sequence and release gate in ADR 0001. This does not replace the legacy scan path yet; replacement is gated on real provider-generated adversarial fixtures. --- backend/evidence/__init__.py | 16 ++ backend/evidence/models.py | 44 ++++ backend/evidence/terraform_plan.py | 200 ++++++++++++++++++ backend/tests/test_plan_evidence.py | 133 ++++++++++++ .../0001-provider-native-drift-evidence.md | 168 +++++++++++++++ 5 files changed, 561 insertions(+) create mode 100644 backend/evidence/__init__.py create mode 100644 backend/evidence/models.py create mode 100644 backend/evidence/terraform_plan.py create mode 100644 backend/tests/test_plan_evidence.py create mode 100644 docs/adr/0001-provider-native-drift-evidence.md diff --git a/backend/evidence/__init__.py b/backend/evidence/__init__.py new file mode 100644 index 0000000..5642332 --- /dev/null +++ b/backend/evidence/__init__.py @@ -0,0 +1,16 @@ +"""Provider-native drift evidence primitives. + +The evidence core is intentionally independent from the legacy AWS collector +engine. Terraform/OpenTofu plan JSON is the source of drift semantics; cloud +APIs are enrichment sources, not truth substitutes. +""" + +from .models import DriftEvidence, EvidenceBundle +from .terraform_plan import PlanEvidenceError, analyze_plan_json + +__all__ = [ + "DriftEvidence", + "EvidenceBundle", + "PlanEvidenceError", + "analyze_plan_json", +] diff --git a/backend/evidence/models.py b/backend/evidence/models.py new file mode 100644 index 0000000..ed0107c --- /dev/null +++ b/backend/evidence/models.py @@ -0,0 +1,44 @@ +from __future__ import annotations + +from typing import Literal + +from pydantic import BaseModel, ConfigDict, Field + + +IaCEngine = Literal["terraform", "opentofu", "unknown"] + + +class DriftEvidence(BaseModel): + """One provider-native drift observation with all raw values omitted.""" + + model_config = ConfigDict(extra="forbid") + + source: Literal["resource_drift"] = "resource_drift" + resource_address: str = Field(min_length=1) + module_address: str | None = None + resource_type: str = Field(min_length=1) + resource_name: str = Field(min_length=1) + resource_index: str | int | None = None + provider_name: str | None = None + actions: list[str] + changed_paths: list[str] + sensitive_paths: list[str] + + +class EvidenceBundle(BaseModel): + """Versioned, redacted output of DriftGuard plan adjudication.""" + + model_config = ConfigDict(extra="forbid") + + schema_version: Literal["1.0"] = "1.0" + iac_engine: IaCEngine = "unknown" + iac_engine_version: str | None = None + source_format_version: str = Field(min_length=1) + plan_timestamp: str | None = None + redaction_policy: Literal["omit_change_values"] = "omit_change_values" + findings: list[DriftEvidence] + skipped_nonmanaged: int = Field(default=0, ge=0) + + @property + def finding_count(self) -> int: + return len(self.findings) diff --git a/backend/evidence/terraform_plan.py b/backend/evidence/terraform_plan.py new file mode 100644 index 0000000..7d18ff1 --- /dev/null +++ b/backend/evidence/terraform_plan.py @@ -0,0 +1,200 @@ +from __future__ import annotations + +from collections.abc import Mapping +from typing import Any + +from .models import DriftEvidence, EvidenceBundle, IaCEngine + + +class PlanEvidenceError(ValueError): + """Raised when plan JSON cannot be safely interpreted as evidence.""" + + +_MISSING = object() + + +def analyze_plan_json( + plan: Mapping[str, Any], + *, + iac_engine: IaCEngine = "unknown", +) -> EvidenceBundle: + """Convert Terraform/OpenTofu plan JSON into a redacted evidence bundle. + + Drift semantics come exclusively from the plan's ``resource_drift`` + collection. DriftGuard does not infer remote drift by diffing arbitrary + Terraform state attributes against partial cloud-API observations. + + Raw ``before`` and ``after`` values are used only in-process to identify + changed JSON-pointer paths. They are never copied into the returned model. + This is a hard boundary because ``terraform show -json`` can contain + sensitive values in plaintext. + """ + if not isinstance(plan, Mapping): + raise PlanEvidenceError("Plan JSON must be an object.") + + format_version = plan.get("format_version") + if not isinstance(format_version, str) or not format_version: + raise PlanEvidenceError("Plan JSON is missing a valid format_version.") + if format_version.split(".", 1)[0] != "1": + raise PlanEvidenceError( + f"Unsupported plan JSON major format version: {format_version}." + ) + + if plan.get("errored") is True: + raise PlanEvidenceError( + "Refusing to produce drift evidence from an errored plan because the observation may be incomplete." + ) + + resource_drift = plan.get("resource_drift", []) + if not isinstance(resource_drift, list): + raise PlanEvidenceError("resource_drift must be an array when present.") + + findings: list[DriftEvidence] = [] + skipped_nonmanaged = 0 + + for position, raw in enumerate(resource_drift): + if not isinstance(raw, Mapping): + raise PlanEvidenceError(f"resource_drift[{position}] must be an object.") + + mode = raw.get("mode") + if not isinstance(mode, str): + raise PlanEvidenceError(f"resource_drift[{position}] is missing mode.") + if mode != "managed": + skipped_nonmanaged += 1 + continue + + address = _required_string(raw, "address", position) + resource_type = _required_string(raw, "type", position) + resource_name = _required_string(raw, "name", position) + + module_address = raw.get("module_address") + if module_address is not None and not isinstance(module_address, str): + raise PlanEvidenceError( + f"resource_drift[{position}].module_address must be a string when present." + ) + + provider_name = raw.get("provider_name") + if provider_name is not None and not isinstance(provider_name, str): + raise PlanEvidenceError( + f"resource_drift[{position}].provider_name must be a string when present." + ) + + resource_index = raw.get("index") + if resource_index is not None and not isinstance(resource_index, (str, int)): + raise PlanEvidenceError( + f"resource_drift[{position}].index must be a string or integer when present." + ) + + change = raw.get("change") + if not isinstance(change, Mapping): + raise PlanEvidenceError(f"resource_drift[{position}] is missing change.") + + actions = change.get("actions") + if ( + not isinstance(actions, list) + or not actions + or any(not isinstance(action, str) for action in actions) + ): + raise PlanEvidenceError( + f"resource_drift[{position}].change.actions must be a non-empty string array." + ) + + changed_paths = sorted(_diff_paths(change.get("before"), change.get("after"))) + sensitive_paths = sorted( + _sensitive_paths(change.get("before_sensitive")) + | _sensitive_paths(change.get("after_sensitive")) + ) + + findings.append( + DriftEvidence( + resource_address=address, + module_address=module_address, + resource_type=resource_type, + resource_name=resource_name, + resource_index=resource_index, + provider_name=provider_name, + actions=actions, + changed_paths=changed_paths, + sensitive_paths=sensitive_paths, + ) + ) + + engine_version = plan.get("terraform_version") + if not isinstance(engine_version, str): + engine_version = None + + plan_timestamp = plan.get("timestamp") + if not isinstance(plan_timestamp, str): + plan_timestamp = None + + return EvidenceBundle( + iac_engine=iac_engine, + iac_engine_version=engine_version, + source_format_version=format_version, + plan_timestamp=plan_timestamp, + findings=findings, + skipped_nonmanaged=skipped_nonmanaged, + ) + + +def _required_string(raw: Mapping[str, Any], key: str, position: int) -> str: + value = raw.get(key) + if not isinstance(value, str) or not value: + raise PlanEvidenceError(f"resource_drift[{position}].{key} must be a non-empty string.") + return value + + +def _pointer_child(pointer: str, token: str | int) -> str: + escaped = str(token).replace("~", "~0").replace("/", "~1") + return f"{pointer}/{escaped}" + + +def _diff_paths(before: Any, after: Any, pointer: str = "") -> set[str]: + """Return changed paths as RFC 6901 JSON pointers without retaining values.""" + if isinstance(before, Mapping) and isinstance(after, Mapping): + changed: set[str] = set() + for key in set(before) | set(after): + child = _pointer_child(pointer, key) + left = before.get(key, _MISSING) + right = after.get(key, _MISSING) + if left is _MISSING or right is _MISSING: + changed.add(child) + else: + changed |= _diff_paths(left, right, child) + return changed + + if isinstance(before, list) and isinstance(after, list): + changed = set() + for index in range(max(len(before), len(after))): + child = _pointer_child(pointer, index) + if index >= len(before) or index >= len(after): + changed.add(child) + else: + changed |= _diff_paths(before[index], after[index], child) + return changed + + if before != after: + return {pointer} + return set() + + +def _sensitive_paths(mask: Any, pointer: str = "") -> set[str]: + """Collect sensitive locations from Terraform/OpenTofu sensitivity masks.""" + if mask is True: + return {pointer} + if mask in (False, None): + return set() + + if isinstance(mask, Mapping): + paths: set[str] = set() + for key, value in mask.items(): + paths |= _sensitive_paths(value, _pointer_child(pointer, key)) + return paths + + if isinstance(mask, list): + paths = set() + for index, value in enumerate(mask): + paths |= _sensitive_paths(value, _pointer_child(pointer, index)) + return paths + + raise PlanEvidenceError("Sensitive-value mask contains an unsupported shape.") diff --git a/backend/tests/test_plan_evidence.py b/backend/tests/test_plan_evidence.py new file mode 100644 index 0000000..bb22b2a --- /dev/null +++ b/backend/tests/test_plan_evidence.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import pytest + +from backend.evidence import PlanEvidenceError, analyze_plan_json + + +def _plan(*drift: dict, **overrides) -> dict: + plan = { + "format_version": "1.2", + "terraform_version": "1.14.0", + "timestamp": "2026-09-14T10:00:00Z", + "errored": False, + "resource_drift": list(drift), + } + plan.update(overrides) + return plan + + +def _drift( + *, + address: str = 'module.compute.aws_instance.web["blue"]', + mode: str = "managed", + before: object | None = None, + after: object | None = None, + before_sensitive: object | None = None, + after_sensitive: object | None = None, + actions: list[str] | None = None, +) -> dict: + return { + "address": address, + "module_address": "module.compute", + "mode": mode, + "type": "aws_instance", + "name": "web", + "index": "blue", + "provider_name": "registry.terraform.io/hashicorp/aws", + "change": { + "actions": actions or ["update"], + "before": before if before is not None else {"instance_type": "t3.micro"}, + "after": after if after is not None else {"instance_type": "t3.large"}, + "before_sensitive": before_sensitive if before_sensitive is not None else {}, + "after_sensitive": after_sensitive if after_sensitive is not None else {}, + }, + } + + +def test_preserves_absolute_resource_identity_from_plan_json(): + bundle = analyze_plan_json(_plan(_drift()), iac_engine="terraform") + + assert bundle.finding_count == 1 + finding = bundle.findings[0] + assert finding.resource_address == 'module.compute.aws_instance.web["blue"]' + assert finding.module_address == "module.compute" + assert finding.resource_index == "blue" + assert finding.provider_name == "registry.terraform.io/hashicorp/aws" + + +def test_emits_changed_paths_without_persisting_raw_values(): + secret_before = "super-secret-before" + secret_after = "super-secret-after" + plan = _plan( + _drift( + before={"instance_type": "t3.micro", "password": secret_before}, + after={"instance_type": "t3.large", "password": secret_after}, + before_sensitive={"password": True}, + after_sensitive={"password": True}, + ) + ) + + bundle = analyze_plan_json(plan) + finding = bundle.findings[0] + + assert finding.changed_paths == ["/instance_type", "/password"] + assert finding.sensitive_paths == ["/password"] + + serialized = bundle.model_dump_json() + assert secret_before not in serialized + assert secret_after not in serialized + assert "t3.micro" not in serialized + assert "t3.large" not in serialized + + +def test_changed_paths_use_json_pointer_escaping(): + plan = _plan( + _drift( + before={"tags": {"team/name~legacy": "platform"}}, + after={"tags": {"team/name~legacy": "security"}}, + ) + ) + + finding = analyze_plan_json(plan).findings[0] + assert finding.changed_paths == ["/tags/team~1name~0legacy"] + + +def test_whole_resource_deletion_is_root_pointer_change(): + drift = _drift( + before={"id": "i-123", "instance_type": "t3.micro"}, + after=None, + actions=["delete"], + ) + # Explicitly override the helper's None fallback. + drift["change"]["after"] = None + + finding = analyze_plan_json(_plan(drift)).findings[0] + + assert finding.actions == ["delete"] + assert finding.changed_paths == [""] + + +def test_nonmanaged_entries_are_not_adjudicated_as_managed_drift(): + bundle = analyze_plan_json(_plan(_drift(mode="data"))) + + assert bundle.findings == [] + assert bundle.skipped_nonmanaged == 1 + + +def test_refuses_errored_plan_as_incomplete_evidence(): + with pytest.raises(PlanEvidenceError, match="errored plan"): + analyze_plan_json(_plan(_drift(), errored=True)) + + +def test_refuses_unknown_major_plan_format(): + with pytest.raises(PlanEvidenceError, match="Unsupported plan JSON major"): + analyze_plan_json(_plan(_drift(), format_version="2.0")) + + +def test_empty_resource_drift_is_valid_clean_evidence_bundle(): + bundle = analyze_plan_json(_plan(), iac_engine="opentofu") + + assert bundle.iac_engine == "opentofu" + assert bundle.finding_count == 0 + assert bundle.redaction_policy == "omit_change_values" diff --git a/docs/adr/0001-provider-native-drift-evidence.md b/docs/adr/0001-provider-native-drift-evidence.md new file mode 100644 index 0000000..4accf63 --- /dev/null +++ b/docs/adr/0001-provider-native-drift-evidence.md @@ -0,0 +1,168 @@ +# ADR 0001: Provider-native drift evidence core + +- Status: Accepted +- Date: 2026-09-14 +- Scope: Drift detection semantics, evidence boundary, remediation preconditions + +## Context + +DriftGuard's legacy detector parses `terraform.tfstate`, collects a selected +subset of AWS attributes with handwritten boto3 collectors, then performs a +generic dictionary comparison. That design has two correctness failures that +make it unsuitable as the long-term production truth engine: + +1. A Terraform-managed resource type that is not collected can be interpreted + as deleted because absence from the live collector map is treated as proof + of remote absence. +2. For supported resource types, Terraform state can contain many attributes + that a partial collector never observes. Generic union-of-keys comparison + can therefore interpret an unobserved attribute as `actual=None` drift. + +The legacy parser also does not preserve Terraform's full absolute resource +identity across module paths and `count`/`for_each` instances. + +Terraform and OpenTofu already perform provider-native refresh and expose +external changes in machine-readable plan JSON as `resource_drift`. Their JSON +formats preserve absolute resource addresses, module addresses, instance +indexes, provider identity, change actions and sensitivity metadata. + +Authoritative references: + +- Terraform JSON output format: https://developer.hashicorp.com/terraform/internals/json-format +- Terraform `show -json`: https://developer.hashicorp.com/terraform/cli/commands/show +- OpenTofu JSON output format: https://opentofu.org/docs/internals/json-format/ + +## Decision + +DriftGuard will treat Terraform/OpenTofu plan JSON `resource_drift` as the +source of drift semantics. + +Handwritten AWS collectors will no longer determine whether Terraform-managed +resources drifted. Cloud APIs remain useful as optional enrichment sources for +attribution, security context, cost context and independent evidence, but an +enrichment failure cannot manufacture a drift finding or a deletion. + +The first production-facing core primitive is a versioned **Evidence Bundle**. +It contains resource identity, change actions, changed paths, sensitivity +paths, plan-format metadata and IaC-engine metadata. It deliberately omits raw +`before` and `after` values. + +## Security boundary + +`terraform show -json` may expose sensitive state/plan values in plaintext. +Therefore raw plan JSON is treated as sensitive execution-boundary data. + +Evidence-core v1 follows these invariants: + +1. Raw `before` and `after` values are never copied into the Evidence Bundle. +2. Changed locations are represented only as RFC 6901 JSON-pointer paths. +3. Terraform/OpenTofu sensitivity masks are preserved as path metadata, never + as raw sensitive values. +4. An errored plan is rejected rather than converted into apparently complete + evidence. +5. Unknown major JSON-format versions are rejected. Unknown minor fields are + ignored for forward compatibility within major format v1. +6. Absolute resource addresses are opaque identifiers and are never rebuilt + from `type`, `name`, module or index components. + +## Initial Evidence Bundle v1 contract + +Top level: + +- `schema_version`: `1.0` +- `iac_engine`: `terraform`, `opentofu`, or `unknown` +- `iac_engine_version`: version reported by the input plan when available +- `source_format_version`: Terraform/OpenTofu JSON format version +- `plan_timestamp`: observation timestamp when present +- `redaction_policy`: `omit_change_values` +- `findings`: zero or more drift-evidence records +- `skipped_nonmanaged`: number of non-managed entries intentionally skipped + +Each finding contains: + +- exact `resource_address` +- optional exact `module_address` +- `resource_type`, `resource_name`, optional `resource_index` +- optional `provider_name` +- provider-native change `actions` +- `changed_paths` as JSON pointers +- `sensitive_paths` as JSON pointers + +No raw infrastructure values are part of this schema. + +## Failure semantics + +Evidence generation fails closed when: + +- the input is not plan JSON, +- `format_version` is missing or has an unsupported major version, +- the plan reports `errored=true`, +- a `resource_drift` entry is structurally malformed, +- a sensitivity mask has an unsupported shape. + +A failure to adjudicate is not converted to "no drift". + +## Remediation contract + +No remediation engine may treat an Evidence Bundle as permission to mutate +infrastructure. + +Future remediation paths are intentionally separate: + +- **REVERT_CLOUD_TO_CODE**: prove with a fresh provider-native plan that the + existing configuration would remove the target drift without unrelated + changes. +- **ACCEPT_CLOUD_INTO_CODE**: edit source only when source mapping is + unambiguous; then require format, validate and a fresh plan proving that the + target drift disappears without unintended changes. + +Ambiguous cases must return an explicit non-automatable state rather than a +guessed patch. + +## Rejected alternatives + +### Expand handwritten AWS collectors + +Rejected. It creates an endless parity race against provider schemas and still +requires DriftGuard to recreate Terraform semantics for computed values, +provider defaults, modules, aliases, nested blocks and lifecycle behavior. + +### Continue comparing raw Terraform state to cloud dictionaries + +Rejected for the correctness failures described above. State is an input to +Terraform's reconciliation model, not a sufficient standalone desired-state +specification for a generic external diff engine. + +### Persist complete plan JSON in DriftGuard SaaS + +Rejected as the default architecture because plan/state JSON may contain +plaintext secrets. Local-first redaction is the required direction. + +### Auto-apply remediation + +Rejected for the initial production architecture. DriftGuard will produce +reviewable and independently validated evidence/remediation artifacts; human +approval remains the execution boundary. + +## Migration sequence + +1. Stabilize current CI and security P0s. +2. Introduce Evidence Bundle v1 and plan-JSON analyzer behind tests. +3. Add real Terraform/OpenTofu-generated fixture plans for modules, `count`, + `for_each`, deletions, updates, sensitive values and partial failures. +4. Add a local CLI entry point that analyzes an existing plan JSON without + uploading raw state/plan values. +5. Introduce finding lifecycle identity and deduplication on evidence records. +6. Add optional CloudTrail/security/cost enrichers that cannot change the + provider-native drift verdict. +7. Replace the legacy collector-based scan path only after equivalence and + adversarial tests pass. +8. Build remediation validation after detection correctness is proven. + +## Release gate + +The legacy detector must not be presented as production-grade while it remains +the authoritative scan path. The evidence core becomes eligible to replace it +only after real provider-generated fixtures prove correct behavior for modules, +`count`, `for_each`, sensitive paths, resource deletion, provider failure and +unsupported/unknown input states. From 4cb068d5ea952cf0689d56aad1e84a00acc84161 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:29:23 +0100 Subject: [PATCH 03/24] fix: enforce tenant ownership on scan retrieval - scope scan lookup by authenticated organization at the SQL query boundary - scope serialized findings to both the authorized scan and its workspace - add an API regression test covering owner access, cross-tenant 404, unauthenticated 401, and deliberately inconsistent cross-workspace finding data - fix the evidence-model Ruff import-format failure found by CI - tighten verify-role wording so it states exactly what the negative ExternalId control proves This closes the scan-read isolation P0 without relying on UUID secrecy as an authorization boundary. --- backend/api/main.py | 28 ++++-- backend/evidence/models.py | 1 - backend/tests/test_api_tenant_isolation.py | 111 +++++++++++++++++++++ 3 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 backend/tests/test_api_tenant_isolation.py diff --git a/backend/api/main.py b/backend/api/main.py index be69442..d0b3897 100644 --- a/backend/api/main.py +++ b/backend/api/main.py @@ -271,12 +271,11 @@ async def verify_role( """ Called after the customer has created the IAM role in AWS. Confirms: 1. The role is actually assumable with the correct external ID. - 2. The trust policy enforces the external ID condition — i.e. it - does NOT also accept a wrong/blank external ID. + 2. The trust policy rejects an otherwise-equivalent AssumeRole call + when only the external ID is changed to an invalid value. A workspace with a role_arn is not usable for scanning until this - passes; failing closed here is deliberate — an unverified role could - mean either a broken setup (scans fail) or a misconfigured trust - policy open to any AWS account (a real, exploitable hole). + passes. Failing closed avoids treating an ambiguous authorization + failure as proof that the required sts:ExternalId isolation exists. """ ws_result = await db.execute( select(Workspace).where(Workspace.id == workspace_id, Workspace.org_id == org.id) @@ -306,9 +305,9 @@ async def verify_role( raise HTTPException( status_code=422, detail=( - "Role is assumable WITHOUT the correct external ID — the trust policy " - "does not enforce sts:ExternalId. This role is exploitable by any AWS " - "account that guesses the ARN. Fix the trust policy Condition block before retrying." + "Role accepted an otherwise-equivalent AssumeRole request with the wrong external ID. " + "DriftGuard cannot verify the required sts:ExternalId isolation. " + "Fix the trust policy Condition block before retrying." ), ) @@ -391,12 +390,21 @@ async def get_scan( org: Organization = Depends(verify_api_key), db: AsyncSession = Depends(get_db), ): - result = await db.execute(select(DriftScan).where(DriftScan.id == scan_id)) + result = await db.execute( + select(DriftScan) + .join(Workspace, DriftScan.workspace_id == Workspace.id) + .where(DriftScan.id == scan_id, Workspace.org_id == org.id) + ) scan = result.scalar_one_or_none() if not scan: raise HTTPException(status_code=404, detail="Scan not found.") - findings_result = await db.execute(select(DriftFinding).where(DriftFinding.scan_id == scan_id)) + findings_result = await db.execute( + select(DriftFinding).where( + DriftFinding.scan_id == scan.id, + DriftFinding.workspace_id == scan.workspace_id, + ) + ) findings = findings_result.scalars().all() return { diff --git a/backend/evidence/models.py b/backend/evidence/models.py index ed0107c..65d39d6 100644 --- a/backend/evidence/models.py +++ b/backend/evidence/models.py @@ -4,7 +4,6 @@ from pydantic import BaseModel, ConfigDict, Field - IaCEngine = Literal["terraform", "opentofu", "unknown"] diff --git a/backend/tests/test_api_tenant_isolation.py b/backend/tests/test_api_tenant_isolation.py new file mode 100644 index 0000000..29a1eb3 --- /dev/null +++ b/backend/tests/test_api_tenant_isolation.py @@ -0,0 +1,111 @@ +from __future__ import annotations + +import httpx +import pytest +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine +from sqlalchemy.pool import StaticPool + +from backend.api.main import create_app +from backend.core.auth import verify_api_key +from backend.database import get_db +from backend.models.base import Base +from backend.models.models import ( + CloudProvider, + DriftFinding, + DriftScan, + Organization, + ScanStatus, + Workspace, +) + + +@pytest.mark.asyncio +async def test_scan_endpoint_enforces_tenant_ownership_and_finding_scope(): + engine = create_async_engine( + "sqlite+aiosqlite:///:memory:", + poolclass=StaticPool, + ) + Session = async_sessionmaker(engine, expire_on_commit=False) + + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + + async with Session() as db: + org_a = Organization(name="Org A", slug="org-a") + org_b = Organization(name="Org B", slug="org-b") + db.add_all([org_a, org_b]) + await db.flush() + + workspace_a = Workspace( + org_id=org_a.id, + name="prod-a", + slug="prod-a", + provider=CloudProvider.AWS, + region="us-east-1", + ) + workspace_b = Workspace( + org_id=org_b.id, + name="prod-b", + slug="prod-b", + provider=CloudProvider.AWS, + region="us-east-1", + ) + db.add_all([workspace_a, workspace_b]) + await db.flush() + + scan_a = DriftScan( + workspace_id=workspace_a.id, + status=ScanStatus.COMPLETED, + ) + db.add(scan_a) + await db.flush() + + owned_finding = DriftFinding( + workspace_id=workspace_a.id, + scan_id=scan_a.id, + resource_type="aws_instance", + resource_id="i-owned", + ) + inconsistent_finding = DriftFinding( + workspace_id=workspace_b.id, + scan_id=scan_a.id, + resource_type="aws_instance", + resource_id="i-cross-tenant", + ) + db.add_all([owned_finding, inconsistent_finding]) + await db.commit() + + app = create_app() + + async def override_db(): + async with Session() as db: + yield db + + async def authenticate_org_a(): + return org_a + + async def authenticate_org_b(): + return org_b + + app.dependency_overrides[get_db] = override_db + transport = httpx.ASGITransport(app=app) + + try: + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + app.dependency_overrides[verify_api_key] = authenticate_org_a + own_response = await client.get(f"/scans/{scan_a.id}") + assert own_response.status_code == 200 + own_payload = own_response.json() + assert [finding["resource_id"] for finding in own_payload["findings"]] == ["i-owned"] + + app.dependency_overrides[verify_api_key] = authenticate_org_b + cross_tenant_response = await client.get(f"/scans/{scan_a.id}") + assert cross_tenant_response.status_code == 404 + assert cross_tenant_response.json() == {"detail": "Scan not found."} + + app.dependency_overrides.pop(verify_api_key) + unauthenticated_response = await client.get(f"/scans/{scan_a.id}") + assert unauthenticated_response.status_code == 401 + finally: + app.dependency_overrides.clear() + await engine.dispose() From 73dbb4c396eacf30d2f593a902062df67dddee5b Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:34:11 +0100 Subject: [PATCH 04/24] test: prove evidence core against real Terraform and OpenTofu drift Add a provider-backed contract harness that applies hashicorp/local 2.9.0 resources in a nested module, including count, for_each and sensitive data, mutates three resources outside IaC, then generates a real refreshed plan and feeds its resource_drift to DriftGuard. The verifier requires: - exact preservation of provider-native absolute addresses - correct count and for_each indexes - no false drift on untouched sibling instances - retention of sensitive-path metadata - no sentinel raw values in serialized Evidence Bundle output CI runs the contract independently against Terraform 1.16.2 and OpenTofu 1.12.6. Raw show -json output remains in an ephemeral temp directory and is never printed or uploaded. GitHub Actions are pinned to immutable commit SHAs and workflow permissions are explicitly read-only. --- .github/workflows/ci.yml | 52 +++++++-- scripts/run_provider_drift_contract.sh | 71 +++++++++++ scripts/verify_provider_drift_contract.py | 110 ++++++++++++++++++ tests/iac/provider-drift/main.tf | 24 ++++ .../iac/provider-drift/modules/files/main.tf | 27 +++++ 5 files changed, 276 insertions(+), 8 deletions(-) create mode 100755 scripts/run_provider_drift_contract.sh create mode 100755 scripts/verify_provider_drift_contract.py create mode 100644 tests/iac/provider-drift/main.tf create mode 100644 tests/iac/provider-drift/modules/files/main.tf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e059132..038d6ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,12 +6,15 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: "3.12" cache: pip @@ -22,8 +25,8 @@ jobs: cli-test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 with: python-version: "3.12" cache: pip @@ -32,11 +35,44 @@ jobs: - run: ruff check cli/driftguard_cli/ cli/tests/ - run: pytest cli/tests/ -v --tb=short + provider-drift-contract: + name: provider-drift-contract (${{ matrix.engine }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + engine: [terraform, opentofu] + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + - name: Install evidence-core Python dependency + run: python -m pip install --disable-pip-version-check pydantic==2.13.5 + - name: Set up Terraform 1.16.2 + if: matrix.engine == 'terraform' + uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1 + with: + terraform_version: "1.16.2" + terraform_wrapper: false + - name: Set up OpenTofu 1.12.6 + if: matrix.engine == 'opentofu' + uses: opentofu/setup-opentofu@a1320f892987e89d278cc92dc5adc984fb93aca4 # v2.0.2 + with: + tofu_version: "1.12.6" + tofu_wrapper: false + - name: Verify provider-native drift contract with Terraform + if: matrix.engine == 'terraform' + run: ./scripts/run_provider_drift_contract.sh terraform terraform + - name: Verify provider-native drift contract with OpenTofu + if: matrix.engine == 'opentofu' + run: ./scripts/run_provider_drift_contract.sh opentofu tofu + vscode-extension-build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "22" - working-directory: vscode-extension @@ -47,8 +83,8 @@ jobs: frontend-build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "22" - working-directory: frontend diff --git a/scripts/run_provider_drift_contract.sh b/scripts/run_provider_drift_contract.sh new file mode 100755 index 0000000..98c0162 --- /dev/null +++ b/scripts/run_provider_drift_contract.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [[ $# -ne 2 ]]; then + echo "usage: $0 " >&2 + exit 64 +fi + +engine_name="$1" +iac_bin="$2" + +if [[ "$engine_name" != "terraform" && "$engine_name" != "opentofu" ]]; then + echo "unsupported engine name: $engine_name" >&2 + exit 64 +fi + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +fixture_root="$repo_root/tests/iac/provider-drift" +work_dir="$(mktemp -d)" + +cleanup() { + rm -rf "$work_dir" +} +trap cleanup EXIT + +cp -R "$fixture_root/." "$work_dir/" +mkdir -p "$work_dir/runtime" + +cd "$work_dir" + +"$iac_bin" init -input=false -no-color >init.stdout 2>init.stderr || { + cat init.stderr >&2 + exit 1 +} + +"$iac_bin" apply -auto-approve -input=false -no-color >apply.stdout 2>apply.stderr || { + cat apply.stderr >&2 + exit 1 +} + +# Create three independent out-of-band changes while leaving two sibling +# instances untouched. This exercises module, count, for_each and sensitive +# resource identity using a real provider refresh. +printf '%s\n' 'tampered-count-1' >runtime/counted-1.txt +rm runtime/keyed-green.txt +printf '%s\n' 'driftguard-fixture-secret-tampered' >runtime/secret.txt + +set +e +"$iac_bin" plan \ + -input=false \ + -no-color \ + -detailed-exitcode \ + -out=drift.plan \ + >plan.stdout 2>plan.stderr +plan_rc=$? +set -e + +if [[ $plan_rc -ne 2 ]]; then + echo "expected provider-native drift plan exit code 2, got $plan_rc" >&2 + cat plan.stderr >&2 + exit 1 +fi + +# `show -json` may contain sensitive values in plaintext. Keep it only in the +# ephemeral work directory; never print it or upload it as a CI artifact. +"$iac_bin" show -json drift.plan >drift.json + +cd "$repo_root" +PYTHONPATH="$repo_root" python scripts/verify_provider_drift_contract.py \ + "$work_dir/drift.json" \ + --engine "$engine_name" diff --git a/scripts/verify_provider_drift_contract.py b/scripts/verify_provider_drift_contract.py new file mode 100755 index 0000000..2ba5602 --- /dev/null +++ b/scripts/verify_provider_drift_contract.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +"""Verify DriftGuard against a provider-generated Terraform/OpenTofu drift plan. + +This script intentionally inspects only structural expectations and the +redacted Evidence Bundle. Raw plan values stay inside the ephemeral CI worker. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT)) + +from backend.evidence import analyze_plan_json # noqa: E402 + +EXPECTED_DRIFT = { + 'module.files.local_file.counted[1]', + 'module.files.local_file.keyed["green"]', + "module.files.local_sensitive_file.secret", +} + +EXPECTED_UNCHANGED = { + "module.files.local_file.counted[0]", + 'module.files.local_file.keyed["blue"]', +} + +SENTINEL_VALUES = { + "driftguard-fixture-secret-v1", + "driftguard-fixture-secret-tampered", + "tampered-count-1", +} + + +def _parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser() + parser.add_argument("plan_json", type=Path) + parser.add_argument("--engine", choices=("terraform", "opentofu"), required=True) + return parser.parse_args() + + +def main() -> int: + args = _parse_args() + plan = json.loads(args.plan_json.read_text()) + bundle = analyze_plan_json(plan, iac_engine=args.engine) + + raw_managed_addresses = { + entry["address"] + for entry in plan.get("resource_drift", []) + if entry.get("mode") == "managed" + } + evidence_addresses = {finding.resource_address for finding in bundle.findings} + + if evidence_addresses != raw_managed_addresses: + raise SystemExit( + "Evidence addresses diverged from provider-native resource_drift addresses: " + f"raw={sorted(raw_managed_addresses)!r} evidence={sorted(evidence_addresses)!r}" + ) + + missing = EXPECTED_DRIFT - evidence_addresses + if missing: + raise SystemExit(f"Provider fixture did not produce the required drift addresses: {sorted(missing)!r}") + + unexpected = EXPECTED_UNCHANGED & evidence_addresses + if unexpected: + raise SystemExit(f"Unmodified resources were reported as drift: {sorted(unexpected)!r}") + + by_address = {finding.resource_address: finding for finding in bundle.findings} + + counted = by_address['module.files.local_file.counted[1]'] + if counted.module_address != "module.files" or counted.resource_index != 1: + raise SystemExit("Counted-resource identity was not preserved exactly.") + + keyed = by_address['module.files.local_file.keyed["green"]'] + if keyed.module_address != "module.files" or keyed.resource_index != "green": + raise SystemExit("for_each resource identity was not preserved exactly.") + + secret = by_address["module.files.local_sensitive_file.secret"] + if not secret.sensitive_paths: + raise SystemExit("Provider-generated sensitive-value metadata was lost from the evidence bundle.") + + serialized = bundle.model_dump_json() + leaked = sorted(value for value in SENTINEL_VALUES if value in serialized) + if leaked: + raise SystemExit(f"Raw fixture values leaked into redacted evidence: {leaked!r}") + + summary = { + "engine": bundle.iac_engine, + "engine_version": bundle.iac_engine_version, + "source_format_version": bundle.source_format_version, + "finding_count": bundle.finding_count, + "findings": [ + { + "address": finding.resource_address, + "actions": finding.actions, + "changed_paths": finding.changed_paths, + "sensitive_paths": finding.sensitive_paths, + } + for finding in bundle.findings + ], + } + print(json.dumps(summary, indent=2, sort_keys=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/iac/provider-drift/main.tf b/tests/iac/provider-drift/main.tf new file mode 100644 index 0000000..3e8701b --- /dev/null +++ b/tests/iac/provider-drift/main.tf @@ -0,0 +1,24 @@ +terraform { + required_version = ">= 1.6.0" + + required_providers { + local = { + source = "hashicorp/local" + version = "2.9.0" + } + } +} + +variable "secret_content" { + description = "Sentinel value used only to prove that DriftGuard evidence never persists sensitive plan values." + type = string + sensitive = true + default = "driftguard-fixture-secret-v1" +} + +module "files" { + source = "./modules/files" + + base_dir = abspath("${path.root}/runtime") + secret_content = var.secret_content +} diff --git a/tests/iac/provider-drift/modules/files/main.tf b/tests/iac/provider-drift/modules/files/main.tf new file mode 100644 index 0000000..cc81116 --- /dev/null +++ b/tests/iac/provider-drift/modules/files/main.tf @@ -0,0 +1,27 @@ +variable "base_dir" { + type = string +} + +variable "secret_content" { + type = string + sensitive = true +} + +resource "local_file" "counted" { + count = 2 + + filename = "${var.base_dir}/counted-${count.index}.txt" + content = "declared-count-${count.index}\n" +} + +resource "local_file" "keyed" { + for_each = toset(["blue", "green"]) + + filename = "${var.base_dir}/keyed-${each.key}.txt" + content = "declared-key-${each.key}\n" +} + +resource "local_sensitive_file" "secret" { + filename = "${var.base_dir}/secret.txt" + content = var.secret_content +} From 1385e577104ded54af158aa1a1a84e7b21a7007c Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:37:13 +0100 Subject: [PATCH 05/24] feat: add local redacted evidence analysis interface Add `python -m backend.evidence ` as the first local-first analysis surface. It reads raw Terraform/OpenTofu plan JSON only on the local machine and emits or writes the redacted Evidence Bundle; tests assert sensitive sentinel values never appear in stdout, output files, or adjudication errors. Document the real provider-native validation from CI run 34838882930, including exact Terraform/OpenTofu versions, fixture topology, observed resource_drift semantics, redaction boundary, proven invariants and explicit non-claims. The packaged `driftguard analyze` command is intentionally deferred until the evidence contract stabilizes, avoiding premature shared-package restructuring. --- backend/evidence/__main__.py | 3 + backend/evidence/cli.py | 78 ++++++++++ backend/tests/test_evidence_cli.py | 87 +++++++++++ .../provider-native-contract-2026-09-14.md | 141 ++++++++++++++++++ 4 files changed, 309 insertions(+) create mode 100644 backend/evidence/__main__.py create mode 100644 backend/evidence/cli.py create mode 100644 backend/tests/test_evidence_cli.py create mode 100644 docs/validation/provider-native-contract-2026-09-14.md diff --git a/backend/evidence/__main__.py b/backend/evidence/__main__.py new file mode 100644 index 0000000..eb53e2f --- /dev/null +++ b/backend/evidence/__main__.py @@ -0,0 +1,3 @@ +from .cli import main + +raise SystemExit(main()) diff --git a/backend/evidence/cli.py b/backend/evidence/cli.py new file mode 100644 index 0000000..caa597f --- /dev/null +++ b/backend/evidence/cli.py @@ -0,0 +1,78 @@ +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from typing import Sequence + +from pydantic import ValidationError + +from .models import IaCEngine +from .terraform_plan import PlanEvidenceError, analyze_plan_json + + +def _parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog="python -m backend.evidence", + description=( + "Analyze Terraform/OpenTofu plan JSON locally and emit a redacted " + "DriftGuard Evidence Bundle. Raw before/after values are never emitted." + ), + ) + parser.add_argument("plan_json", type=Path, help="Path produced by `terraform show -json` or `tofu show -json`.") + parser.add_argument( + "--engine", + choices=("terraform", "opentofu", "unknown"), + default="unknown", + help="IaC engine that produced the plan JSON.", + ) + parser.add_argument( + "--output", + type=Path, + help="Write the redacted Evidence Bundle to this file instead of stdout.", + ) + return parser + + +def main(argv: Sequence[str] | None = None) -> int: + args = _parser().parse_args(argv) + + try: + raw = args.plan_json.read_text(encoding="utf-8") + except OSError as exc: + print(f"error: could not read plan JSON: {exc}", file=sys.stderr) + return 2 + + try: + plan = json.loads(raw) + except json.JSONDecodeError as exc: + print( + f"error: invalid JSON at line {exc.lineno}, column {exc.colno}", + file=sys.stderr, + ) + return 2 + + try: + bundle = analyze_plan_json(plan, iac_engine=args.engine) + except (PlanEvidenceError, ValidationError) as exc: + print(f"error: plan cannot be adjudicated: {exc}", file=sys.stderr) + return 2 + + serialized = bundle.model_dump_json(indent=2) + "\n" + + if args.output is None: + sys.stdout.write(serialized) + return 0 + + try: + args.output.write_text(serialized, encoding="utf-8") + except OSError as exc: + print(f"error: could not write evidence output: {exc}", file=sys.stderr) + return 2 + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/backend/tests/test_evidence_cli.py b/backend/tests/test_evidence_cli.py new file mode 100644 index 0000000..974a48a --- /dev/null +++ b/backend/tests/test_evidence_cli.py @@ -0,0 +1,87 @@ +from __future__ import annotations + +import json + +from backend.evidence.cli import main + + +def _plan_with_secret(secret_before: str, secret_after: str) -> dict: + return { + "format_version": "1.2", + "terraform_version": "1.16.2", + "errored": False, + "resource_drift": [ + { + "address": "aws_ssm_parameter.example", + "mode": "managed", + "type": "aws_ssm_parameter", + "name": "example", + "provider_name": "registry.terraform.io/hashicorp/aws", + "change": { + "actions": ["update"], + "before": {"value": secret_before}, + "after": {"value": secret_after}, + "before_sensitive": {"value": True}, + "after_sensitive": {"value": True}, + }, + } + ], + } + + +def test_local_cli_emits_redacted_evidence_to_stdout(tmp_path, capsys): + before = "secret-before-value" + after = "secret-after-value" + plan_path = tmp_path / "plan.json" + plan_path.write_text(json.dumps(_plan_with_secret(before, after))) + + rc = main([str(plan_path), "--engine", "terraform"]) + + assert rc == 0 + captured = capsys.readouterr() + payload = json.loads(captured.out) + assert payload["iac_engine"] == "terraform" + assert payload["findings"][0]["changed_paths"] == ["/value"] + assert payload["findings"][0]["sensitive_paths"] == ["/value"] + assert before not in captured.out + assert after not in captured.out + assert captured.err == "" + + +def test_local_cli_writes_only_redacted_evidence_to_file(tmp_path, capsys): + before = "never-persist-before" + after = "never-persist-after" + plan_path = tmp_path / "plan.json" + output_path = tmp_path / "evidence.json" + plan_path.write_text(json.dumps(_plan_with_secret(before, after))) + + rc = main([ + str(plan_path), + "--engine", + "opentofu", + "--output", + str(output_path), + ]) + + assert rc == 0 + assert capsys.readouterr().out == "" + output = output_path.read_text() + assert '"iac_engine": "opentofu"' in output + assert before not in output + assert after not in output + + +def test_local_cli_rejects_errored_plan_without_echoing_values(tmp_path, capsys): + secret = "must-not-appear-in-error" + plan = _plan_with_secret(secret, "other") + plan["errored"] = True + plan_path = tmp_path / "errored.json" + plan_path.write_text(json.dumps(plan)) + + rc = main([str(plan_path)]) + + assert rc == 2 + captured = capsys.readouterr() + assert "errored plan" in captured.err + assert secret not in captured.err + assert secret not in captured.out diff --git a/docs/validation/provider-native-contract-2026-09-14.md b/docs/validation/provider-native-contract-2026-09-14.md new file mode 100644 index 0000000..bfcc156 --- /dev/null +++ b/docs/validation/provider-native-contract-2026-09-14.md @@ -0,0 +1,141 @@ +# Provider-native drift contract validation — 2026-09-14 + +## Purpose + +This validation establishes whether DriftGuard Evidence Core v1 can consume +**real provider-refreshed drift output** from both Terraform and OpenTofu +without reconstructing resource identity, manufacturing drift, or persisting +raw infrastructure values. + +This is a semantic contract test, not an AWS integration test. + +## Tested revision + +- DriftGuard branch: `feat/evidence-core-v1` +- DriftGuard commit: `73dbb4c396eacf30d2f593a902062df67dddee5b` +- GitHub Actions run: `34838882930` +- Result: all six CI jobs passed + +## Toolchain + +The contract jobs use pinned production releases rather than prereleases: + +- Terraform `1.16.2` +- OpenTofu `1.12.6` +- `hashicorp/local` provider `2.9.0` +- Python `3.12` + +GitHub Actions are referenced by immutable commit SHA and the workflow token is +restricted to `contents: read`. + +## Fixture topology + +The fixture uses a nested module so DriftGuard must preserve absolute resource +identity rather than derive it from type/name fields. + +```text +module.files +├── local_file.counted[0] +├── local_file.counted[1] +├── local_file.keyed["blue"] +├── local_file.keyed["green"] +└── local_sensitive_file.secret +``` + +After `apply`, the test performs three changes outside IaC: + +1. overwrites `counted-1.txt`, +2. deletes `keyed-green.txt`, +3. overwrites `secret.txt` with a sensitive sentinel value. + +The sibling instances `counted[0]` and `keyed["blue"]` are deliberately left +untouched and act as negative controls. + +## Execution boundary + +For each engine the harness performs: + +```text +init + → apply + → out-of-band mutation + → plan -detailed-exitcode -out=drift.plan + → show -json drift.plan + → DriftGuard analyze_plan_json(...) +``` + +The raw `show -json` document remains only in an ephemeral CI temp directory. +It is never printed, committed, cached as a DriftGuard artifact, or uploaded to +the DriftGuard API. The temp directory is removed on exit. + +This boundary is intentional because plan JSON may contain sensitive values in +plaintext. + +## Observed Terraform result + +Terraform `1.16.2` produced `format_version = 1.2` and exactly three managed +`resource_drift` entries: + +| Address | Action | DriftGuard changed path | Sensitive paths retained | +|---|---|---|---| +| `module.files.local_file.counted[1]` | `delete` | root (`""`) | `/sensitive_content` | +| `module.files.local_file.keyed["green"]` | `delete` | root (`""`) | `/sensitive_content` | +| `module.files.local_sensitive_file.secret` | `delete` | root (`""`) | `/content`, `/content_base64` | + +The local provider reports an externally modified/deleted managed file as a +remote deletion and a subsequent configuration-driven recreation. DriftGuard +preserves the provider-native `resource_drift` action instead of inventing its +own cloud-state classification. + +## Observed OpenTofu result + +OpenTofu `1.12.6` also produced `format_version = 1.2` and the same three +managed drift addresses, actions, changed-path roots, and sensitivity paths. + +For this fixture, Terraform and OpenTofu therefore exposed equivalent +`resource_drift` semantics to DriftGuard. + +This is evidence for compatibility of this contract surface, not a claim that +all future Terraform/OpenTofu plan formats or providers are identical. + +## Assertions proven by CI + +The verifier fails unless all of the following hold: + +1. Evidence Bundle addresses exactly equal the managed addresses emitted by + the engine's own `resource_drift` collection. +2. `module.files.local_file.counted[1]` preserves integer index `1`. +3. `module.files.local_file.keyed["green"]` preserves string index `green`. +4. Both preserve module address `module.files`. +5. Untouched `counted[0]` and `keyed["blue"]` are absent from drift evidence. +6. Sensitive-path metadata survives conversion into Evidence Bundle v1. +7. Known raw sentinel values are absent from serialized DriftGuard evidence. +8. The plan command must return detailed-exitcode `2`; a clean or failed plan + cannot silently pass this contract test. + +## What this validation does NOT prove + +This run does not prove: + +- AWS provider drift behavior, +- AWS API/STS behavior in a real account, +- CloudTrail attribution, +- source-code mapping for remediation, +- safe cloud-to-code rewriting, +- cost enrichment, +- finding lifecycle/deduplication, +- durable worker execution, +- multi-account scale or load behavior. + +Those remain separate gates. Passing this contract is necessary but not +sufficient for production readiness. + +## Result + +**PASS.** Evidence Core v1 has now been validated against provider-generated +Terraform and OpenTofu drift, including nested-module identity, `count`, +`for_each`, negative-control sibling instances, and sensitivity metadata. + +The next product gate is a local, redacted analysis interface. The legacy +state-vs-partial-AWS-collector path remains active and must not be described as +the production-grade detector until a controlled migration is complete. From 5c31cac7cb80b5828a834d2df691c3a7b7a444c1 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:38:35 +0100 Subject: [PATCH 06/24] fix: satisfy evidence CLI lint gate Move Sequence to collections.abc and remove an unused IaCEngine import. These are the two exact Ruff failures from CI run 34839136526; no lint rules are weakened or suppressed. --- backend/evidence/cli.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/evidence/cli.py b/backend/evidence/cli.py index caa597f..3fffb99 100644 --- a/backend/evidence/cli.py +++ b/backend/evidence/cli.py @@ -3,12 +3,11 @@ import argparse import json import sys +from collections.abc import Sequence from pathlib import Path -from typing import Sequence from pydantic import ValidationError -from .models import IaCEngine from .terraform_plan import PlanEvidenceError, analyze_plan_json From 99bd27d382eb0ee08d944f64fbc51e0c3bc0e08a Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 12:43:13 +0100 Subject: [PATCH 07/24] fix: remove false precision from evidence paths Design review against the official Terraform/OpenTofu JSON contract found two evidence-model hazards: - lists, sets and tuples all lower to JSON arrays, so element indexes are not generically trustworthy without provider schema; array changes now collapse to the parent JSON pointer - deposed change objects require address + deposed key for unique identity; Evidence v1 now preserves deposed and previous-address metadata Also reject state JSON as the wrong input type, preserve provider-native applyable/complete flags and after_unknown paths, and add regression tests for each invariant. ADR 0001 is updated with the exact rationale and failure semantics. --- backend/evidence/models.py | 5 + backend/evidence/terraform_plan.py | 98 +++++++++++++------ backend/tests/test_plan_evidence.py | 69 ++++++++++++- .../0001-provider-native-drift-evidence.md | 48 ++++++--- 4 files changed, 173 insertions(+), 47 deletions(-) diff --git a/backend/evidence/models.py b/backend/evidence/models.py index 65d39d6..b73a91b 100644 --- a/backend/evidence/models.py +++ b/backend/evidence/models.py @@ -14,7 +14,9 @@ class DriftEvidence(BaseModel): source: Literal["resource_drift"] = "resource_drift" resource_address: str = Field(min_length=1) + previous_resource_address: str | None = None module_address: str | None = None + deposed_key: str | None = None resource_type: str = Field(min_length=1) resource_name: str = Field(min_length=1) resource_index: str | int | None = None @@ -22,6 +24,7 @@ class DriftEvidence(BaseModel): actions: list[str] changed_paths: list[str] sensitive_paths: list[str] + unknown_paths: list[str] class EvidenceBundle(BaseModel): @@ -34,6 +37,8 @@ class EvidenceBundle(BaseModel): iac_engine_version: str | None = None source_format_version: str = Field(min_length=1) plan_timestamp: str | None = None + plan_applyable: bool | None = None + plan_complete: bool | None = None redaction_policy: Literal["omit_change_values"] = "omit_change_values" findings: list[DriftEvidence] skipped_nonmanaged: int = Field(default=0, ge=0) diff --git a/backend/evidence/terraform_plan.py b/backend/evidence/terraform_plan.py index 7d18ff1..1924110 100644 --- a/backend/evidence/terraform_plan.py +++ b/backend/evidence/terraform_plan.py @@ -40,11 +40,23 @@ def analyze_plan_json( f"Unsupported plan JSON major format version: {format_version}." ) - if plan.get("errored") is True: + # A Terraform/OpenTofu state JSON document also has format_version and + # terraform_version, but plan representations include an explicit boolean + # `errored` field. Requiring it prevents a state file from being silently + # interpreted as a clean plan with no resource_drift entries. + errored = plan.get("errored") + if not isinstance(errored, bool): + raise PlanEvidenceError( + "Input is not a plan representation: expected boolean plan field 'errored'." + ) + if errored: raise PlanEvidenceError( "Refusing to produce drift evidence from an errored plan because the observation may be incomplete." ) + applyable = _optional_bool(plan, "applyable") + complete = _optional_bool(plan, "complete") + resource_drift = plan.get("resource_drift", []) if not isinstance(resource_drift, list): raise PlanEvidenceError("resource_drift must be an array when present.") @@ -67,23 +79,17 @@ def analyze_plan_json( resource_type = _required_string(raw, "type", position) resource_name = _required_string(raw, "name", position) - module_address = raw.get("module_address") - if module_address is not None and not isinstance(module_address, str): - raise PlanEvidenceError( - f"resource_drift[{position}].module_address must be a string when present." - ) - - provider_name = raw.get("provider_name") - if provider_name is not None and not isinstance(provider_name, str): - raise PlanEvidenceError( - f"resource_drift[{position}].provider_name must be a string when present." - ) + previous_address = _optional_string(raw, "previous_address", position) + module_address = _optional_string(raw, "module_address", position) + deposed_key = _optional_string(raw, "deposed", position) + provider_name = _optional_string(raw, "provider_name", position) resource_index = raw.get("index") - if resource_index is not None and not isinstance(resource_index, (str, int)): - raise PlanEvidenceError( - f"resource_drift[{position}].index must be a string or integer when present." - ) + if resource_index is not None: + if isinstance(resource_index, bool) or not isinstance(resource_index, (str, int)): + raise PlanEvidenceError( + f"resource_drift[{position}].index must be a string or integer when present." + ) change = raw.get("change") if not isinstance(change, Mapping): @@ -101,14 +107,19 @@ def analyze_plan_json( changed_paths = sorted(_diff_paths(change.get("before"), change.get("after"))) sensitive_paths = sorted( - _sensitive_paths(change.get("before_sensitive")) - | _sensitive_paths(change.get("after_sensitive")) + _mask_paths(change.get("before_sensitive"), "sensitive-value") + | _mask_paths(change.get("after_sensitive"), "sensitive-value") + ) + unknown_paths = sorted( + _mask_paths(change.get("after_unknown"), "unknown-value") ) findings.append( DriftEvidence( resource_address=address, + previous_resource_address=previous_address, module_address=module_address, + deposed_key=deposed_key, resource_type=resource_type, resource_name=resource_name, resource_index=resource_index, @@ -116,6 +127,7 @@ def analyze_plan_json( actions=actions, changed_paths=changed_paths, sensitive_paths=sensitive_paths, + unknown_paths=unknown_paths, ) ) @@ -132,11 +144,22 @@ def analyze_plan_json( iac_engine_version=engine_version, source_format_version=format_version, plan_timestamp=plan_timestamp, + plan_applyable=applyable, + plan_complete=complete, findings=findings, skipped_nonmanaged=skipped_nonmanaged, ) +def _optional_bool(raw: Mapping[str, Any], key: str) -> bool | None: + value = raw.get(key) + if value is None: + return None + if not isinstance(value, bool): + raise PlanEvidenceError(f"Plan field '{key}' must be boolean when present.") + return value + + def _required_string(raw: Mapping[str, Any], key: str, position: int) -> str: value = raw.get(key) if not isinstance(value, str) or not value: @@ -144,13 +167,31 @@ def _required_string(raw: Mapping[str, Any], key: str, position: int) -> str: return value +def _optional_string(raw: Mapping[str, Any], key: str, position: int) -> str | None: + value = raw.get(key) + if value is None: + return None + if not isinstance(value, str) or not value: + raise PlanEvidenceError( + f"resource_drift[{position}].{key} must be a non-empty string when present." + ) + return value + + def _pointer_child(pointer: str, token: str | int) -> str: escaped = str(token).replace("~", "~0").replace("/", "~1") return f"{pointer}/{escaped}" def _diff_paths(before: Any, after: Any, pointer: str = "") -> set[str]: - """Return changed paths as RFC 6901 JSON pointers without retaining values.""" + """Return changed paths as RFC 6901 JSON pointers without retaining values. + + JSON plan output loses the distinction between Terraform/OpenTofu lists, + sets, and tuples: all three lower to JSON arrays. Without provider schema, + numeric array indexes can therefore create false precision (especially for + sets whose ordering is not semantic). If an array changes, v1 reports its + parent path rather than inventing element-level identity. + """ if isinstance(before, Mapping) and isinstance(after, Mapping): changed: set[str] = set() for key in set(before) | set(after): @@ -164,22 +205,15 @@ def _diff_paths(before: Any, after: Any, pointer: str = "") -> set[str]: return changed if isinstance(before, list) and isinstance(after, list): - changed = set() - for index in range(max(len(before), len(after))): - child = _pointer_child(pointer, index) - if index >= len(before) or index >= len(after): - changed.add(child) - else: - changed |= _diff_paths(before[index], after[index], child) - return changed + return {pointer} if before != after else set() if before != after: return {pointer} return set() -def _sensitive_paths(mask: Any, pointer: str = "") -> set[str]: - """Collect sensitive locations from Terraform/OpenTofu sensitivity masks.""" +def _mask_paths(mask: Any, label: str, pointer: str = "") -> set[str]: + """Collect true locations from Terraform/OpenTofu boolean-shape masks.""" if mask is True: return {pointer} if mask in (False, None): @@ -188,13 +222,13 @@ def _sensitive_paths(mask: Any, pointer: str = "") -> set[str]: if isinstance(mask, Mapping): paths: set[str] = set() for key, value in mask.items(): - paths |= _sensitive_paths(value, _pointer_child(pointer, key)) + paths |= _mask_paths(value, label, _pointer_child(pointer, key)) return paths if isinstance(mask, list): paths = set() for index, value in enumerate(mask): - paths |= _sensitive_paths(value, _pointer_child(pointer, index)) + paths |= _mask_paths(value, label, _pointer_child(pointer, index)) return paths - raise PlanEvidenceError("Sensitive-value mask contains an unsupported shape.") + raise PlanEvidenceError(f"{label.capitalize()} mask contains an unsupported shape.") diff --git a/backend/tests/test_plan_evidence.py b/backend/tests/test_plan_evidence.py index bb22b2a..9d35ac8 100644 --- a/backend/tests/test_plan_evidence.py +++ b/backend/tests/test_plan_evidence.py @@ -8,8 +8,10 @@ def _plan(*drift: dict, **overrides) -> dict: plan = { "format_version": "1.2", - "terraform_version": "1.14.0", + "terraform_version": "1.16.2", "timestamp": "2026-09-14T10:00:00Z", + "applyable": True, + "complete": True, "errored": False, "resource_drift": list(drift), } @@ -25,9 +27,12 @@ def _drift( after: object | None = None, before_sensitive: object | None = None, after_sensitive: object | None = None, + after_unknown: object | None = None, actions: list[str] | None = None, + previous_address: str | None = None, + deposed: str | None = None, ) -> dict: - return { + raw = { "address": address, "module_address": "module.compute", "mode": mode, @@ -41,8 +46,14 @@ def _drift( "after": after if after is not None else {"instance_type": "t3.large"}, "before_sensitive": before_sensitive if before_sensitive is not None else {}, "after_sensitive": after_sensitive if after_sensitive is not None else {}, + "after_unknown": after_unknown if after_unknown is not None else {}, }, } + if previous_address is not None: + raw["previous_address"] = previous_address + if deposed is not None: + raw["deposed"] = deposed + return raw def test_preserves_absolute_resource_identity_from_plan_json(): @@ -54,6 +65,20 @@ def test_preserves_absolute_resource_identity_from_plan_json(): assert finding.module_address == "module.compute" assert finding.resource_index == "blue" assert finding.provider_name == "registry.terraform.io/hashicorp/aws" + assert bundle.plan_applyable is True + assert bundle.plan_complete is True + + +def test_preserves_previous_address_and_deposed_object_identity(): + drift = _drift( + previous_address='module.old.aws_instance.web["blue"]', + deposed="deadbeef", + ) + + finding = analyze_plan_json(_plan(drift)).findings[0] + + assert finding.previous_resource_address == 'module.old.aws_instance.web["blue"]' + assert finding.deposed_key == "deadbeef" def test_emits_changed_paths_without_persisting_raw_values(): @@ -93,6 +118,35 @@ def test_changed_paths_use_json_pointer_escaping(): assert finding.changed_paths == ["/tags/team~1name~0legacy"] +def test_array_changes_collapse_to_parent_path_without_provider_schema(): + plan = _plan( + _drift( + before={"rules": [{"name": "a"}, {"name": "b"}]}, + after={"rules": [{"name": "b"}, {"name": "a"}]}, + ) + ) + + finding = analyze_plan_json(plan).findings[0] + + assert finding.changed_paths == ["/rules"] + assert "/rules/0/name" not in finding.changed_paths + + +def test_unknown_value_mask_is_preserved_without_exposing_values(): + plan = _plan( + _drift( + before={"endpoint": "old.example"}, + after={"endpoint": None}, + after_unknown={"endpoint": True}, + ) + ) + + finding = analyze_plan_json(plan).findings[0] + + assert finding.changed_paths == ["/endpoint"] + assert finding.unknown_paths == ["/endpoint"] + + def test_whole_resource_deletion_is_root_pointer_change(): drift = _drift( before={"id": "i-123", "instance_type": "t3.micro"}, @@ -115,6 +169,17 @@ def test_nonmanaged_entries_are_not_adjudicated_as_managed_drift(): assert bundle.skipped_nonmanaged == 1 +def test_refuses_state_json_instead_of_reporting_false_clean_plan(): + state = { + "format_version": "1.0", + "terraform_version": "1.16.2", + "values": {"root_module": {"resources": []}}, + } + + with pytest.raises(PlanEvidenceError, match="not a plan representation"): + analyze_plan_json(state) + + def test_refuses_errored_plan_as_incomplete_evidence(): with pytest.raises(PlanEvidenceError, match="errored plan"): analyze_plan_json(_plan(_drift(), errored=True)) diff --git a/docs/adr/0001-provider-native-drift-evidence.md b/docs/adr/0001-provider-native-drift-evidence.md index 4accf63..c3f9e14 100644 --- a/docs/adr/0001-provider-native-drift-evidence.md +++ b/docs/adr/0001-provider-native-drift-evidence.md @@ -43,11 +43,11 @@ attribution, security context, cost context and independent evidence, but an enrichment failure cannot manufacture a drift finding or a deletion. The first production-facing core primitive is a versioned **Evidence Bundle**. -It contains resource identity, change actions, changed paths, sensitivity -paths, plan-format metadata and IaC-engine metadata. It deliberately omits raw -`before` and `after` values. +It contains resource identity, change actions, changed paths, sensitivity and +unknown-value paths, plan-format/completeness metadata and IaC-engine metadata. +It deliberately omits raw `before` and `after` values. -## Security boundary +## Security and correctness boundary `terraform show -json` may expose sensitive state/plan values in plaintext. Therefore raw plan JSON is treated as sensitive execution-boundary data. @@ -58,12 +58,23 @@ Evidence-core v1 follows these invariants: 2. Changed locations are represented only as RFC 6901 JSON-pointer paths. 3. Terraform/OpenTofu sensitivity masks are preserved as path metadata, never as raw sensitive values. -4. An errored plan is rejected rather than converted into apparently complete +4. `after_unknown` locations are preserved as `unknown_paths`; an unknown value + is never silently represented as a known null. +5. An errored plan is rejected rather than converted into apparently complete evidence. -5. Unknown major JSON-format versions are rejected. Unknown minor fields are +6. State JSON is rejected as the wrong input type rather than being interpreted + as a clean plan merely because it has no `resource_drift` collection. +7. Unknown major JSON-format versions are rejected. Unknown minor fields are ignored for forward compatibility within major format v1. -6. Absolute resource addresses are opaque identifiers and are never rebuilt +8. Absolute resource addresses are opaque identifiers and are never rebuilt from `type`, `name`, module or index components. +9. `previous_address` and the opaque `deposed` key are preserved when present; + Terraform/OpenTofu document `address + deposed` as the unique identity of a + deposed change object. +10. JSON arrays are not interpreted as stable element-addressable collections + without provider schema. Terraform/OpenTofu lower lists, sets and tuples to + the same JSON array representation, so an array-level change is reported at + its parent pointer instead of manufacturing numeric element precision. ## Initial Evidence Bundle v1 contract @@ -74,6 +85,8 @@ Top level: - `iac_engine_version`: version reported by the input plan when available - `source_format_version`: Terraform/OpenTofu JSON format version - `plan_timestamp`: observation timestamp when present +- `plan_applyable`: provider-native plan flag when present +- `plan_complete`: provider-native plan completeness flag when present - `redaction_policy`: `omit_change_values` - `findings`: zero or more drift-evidence records - `skipped_nonmanaged`: number of non-managed entries intentionally skipped @@ -81,12 +94,15 @@ Top level: Each finding contains: - exact `resource_address` +- optional exact `previous_resource_address` - optional exact `module_address` +- optional opaque `deposed_key` - `resource_type`, `resource_name`, optional `resource_index` - optional `provider_name` - provider-native change `actions` -- `changed_paths` as JSON pointers +- conservative `changed_paths` as JSON pointers - `sensitive_paths` as JSON pointers +- `unknown_paths` as JSON pointers No raw infrastructure values are part of this schema. @@ -94,11 +110,11 @@ No raw infrastructure values are part of this schema. Evidence generation fails closed when: -- the input is not plan JSON, +- the input is not a plan representation, - `format_version` is missing or has an unsupported major version, - the plan reports `errored=true`, - a `resource_drift` entry is structurally malformed, -- a sensitivity mask has an unsupported shape. +- a sensitivity or unknown-value mask has an unsupported shape. A failure to adjudicate is not converted to "no drift". @@ -138,6 +154,12 @@ specification for a generic external diff engine. Rejected as the default architecture because plan/state JSON may contain plaintext secrets. Local-first redaction is the required direction. +### Infer collection-element paths from JSON array indexes + +Rejected without provider schema. The JSON format intentionally loses the +list/set/tuple distinction, so positional interpretation can invent identity +that does not exist for set-valued attributes or nested blocks. + ### Auto-apply remediation Rejected for the initial production architecture. DriftGuard will produce @@ -150,7 +172,7 @@ approval remains the execution boundary. 2. Introduce Evidence Bundle v1 and plan-JSON analyzer behind tests. 3. Add real Terraform/OpenTofu-generated fixture plans for modules, `count`, `for_each`, deletions, updates, sensitive values and partial failures. -4. Add a local CLI entry point that analyzes an existing plan JSON without +4. Add a local analysis entry point that analyzes an existing plan JSON without uploading raw state/plan values. 5. Introduce finding lifecycle identity and deduplication on evidence records. 6. Add optional CloudTrail/security/cost enrichers that cannot change the @@ -164,5 +186,5 @@ approval remains the execution boundary. The legacy detector must not be presented as production-grade while it remains the authoritative scan path. The evidence core becomes eligible to replace it only after real provider-generated fixtures prove correct behavior for modules, -`count`, `for_each`, sensitive paths, resource deletion, provider failure and -unsupported/unknown input states. +`count`, `for_each`, sensitive/unknown paths, resource deletion, provider +failure, deposed identity and unsupported/unknown input states. From dea4c9796a81f00ce626107fc99d2fb803c86c1b Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 14:36:34 +0100 Subject: [PATCH 08/24] fix: satisfy reviewed evidence lint gate --- backend/evidence/terraform_plan.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/backend/evidence/terraform_plan.py b/backend/evidence/terraform_plan.py index 1924110..cb994e9 100644 --- a/backend/evidence/terraform_plan.py +++ b/backend/evidence/terraform_plan.py @@ -85,11 +85,13 @@ def analyze_plan_json( provider_name = _optional_string(raw, "provider_name", position) resource_index = raw.get("index") - if resource_index is not None: - if isinstance(resource_index, bool) or not isinstance(resource_index, (str, int)): - raise PlanEvidenceError( - f"resource_drift[{position}].index must be a string or integer when present." - ) + if resource_index is not None and ( + isinstance(resource_index, bool) + or not isinstance(resource_index, (str, int)) + ): + raise PlanEvidenceError( + f"resource_drift[{position}].index must be a string or integer when present." + ) change = raw.get("change") if not isinstance(change, Mapping): From 829d868b45267d03c99941191ff61038adf4ba19 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 14:46:02 +0100 Subject: [PATCH 09/24] feat: add evidence incident lifecycle and replay-safe identity - add workspace-scoped deterministic SHA-256 incident identity for Evidence Core - add incident, occurrence, and exactly-once reconciliation tables without mutating the legacy drift_findings schema - resolve incidents only from complete plans; incomplete/deferred plans cannot manufacture recovery - make scan replay idempotent and fail closed when replayed evidence differs - preserve redaction: lifecycle stores identity and path metadata, never raw before/after values - reserve a stable full-fingerprint remediation branch for future PR deduplication - register ORM models deterministically for create_all - add lifecycle regression tests and ADR 0002 The legacy collector-based scan path and its GitHub PR behavior remain unchanged in this commit. --- backend/database.py | 7 + backend/evidence/lifecycle.py | 243 +++++++++++++++ backend/models/incidents.py | 123 ++++++++ backend/tests/test_evidence_lifecycle.py | 322 ++++++++++++++++++++ docs/adr/0002-evidence-finding-lifecycle.md | 196 ++++++++++++ 5 files changed, 891 insertions(+) create mode 100644 backend/evidence/lifecycle.py create mode 100644 backend/models/incidents.py create mode 100644 backend/tests/test_evidence_lifecycle.py create mode 100644 docs/adr/0002-evidence-finding-lifecycle.md diff --git a/backend/database.py b/backend/database.py index ada41bb..f302e5d 100644 --- a/backend/database.py +++ b/backend/database.py @@ -25,8 +25,15 @@ create_async_engine, ) +from .models import incidents as incident_models +from .models import models as core_models from .models.base import Base +# Register every ORM table with Base.metadata even when this module is used +# outside backend.api.main. create_all() otherwise depends on unrelated import +# order and can silently omit tables in scripts/tests. +_REGISTERED_MODEL_MODULES = (core_models, incident_models) + DATABASE_URL = os.getenv( "DATABASE_URL", "sqlite+aiosqlite:///./driftguard.db", diff --git a/backend/evidence/lifecycle.py b/backend/evidence/lifecycle.py new file mode 100644 index 0000000..7da362b --- /dev/null +++ b/backend/evidence/lifecycle.py @@ -0,0 +1,243 @@ +from __future__ import annotations + +import hashlib +import json +import re +from dataclasses import dataclass +from datetime import UTC, datetime + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from ..models.incidents import ( + EvidenceReconciliation, + FindingIncident, + FindingOccurrence, +) +from ..models.models import DriftScan, Workspace +from .models import DriftEvidence, EvidenceBundle + +IDENTITY_VERSION = "1" +INCIDENT_OPEN = "open" +INCIDENT_RESOLVED = "resolved" +_FINGERPRINT_RE = re.compile(r"^[0-9a-f]{64}$") + + +class LifecycleReconcileError(ValueError): + """Raised when evidence cannot be reconciled without ambiguity.""" + + +@dataclass(frozen=True, slots=True) +class LifecycleResult: + created: int = 0 + observed_existing: int = 0 + reopened: int = 0 + resolved: int = 0 + already_reconciled: bool = False + resolution_performed: bool = False + + +def finding_fingerprint(finding: DriftEvidence) -> str: + """Return the deterministic v1 incident identity for one evidence record. + + Identity deliberately includes the ordered provider-native action sequence + and the normalized changed-path surface. Sensitive/unknown masks describe + observation quality, not incident identity, so they do not split one drift + incident when only masking/knownness changes between scans. + """ + payload = { + "identity_version": IDENTITY_VERSION, + "resource_address": finding.resource_address, + "deposed_key": finding.deposed_key, + "resource_type": finding.resource_type, + "provider_name": finding.provider_name, + "actions": list(finding.actions), + "changed_paths": sorted(set(finding.changed_paths)), + } + canonical = json.dumps( + payload, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf-8") + return hashlib.sha256(canonical).hexdigest() + + +def remediation_branch_for_fingerprint(fingerprint: str) -> str: + """Return the stable GitHub branch reserved for one incident identity.""" + if not _FINGERPRINT_RE.fullmatch(fingerprint): + raise LifecycleReconcileError("Incident fingerprint must be a 64-character lowercase SHA-256 hex digest.") + return f"driftguard/fix-{fingerprint}" + + +def _observation_set_digest(fingerprints: set[str]) -> str: + canonical = "\n".join(sorted(fingerprints)).encode("ascii") + return hashlib.sha256(canonical).hexdigest() + + +async def reconcile_evidence_bundle( + db: AsyncSession, + *, + workspace_id: str, + scan_id: str, + bundle: EvidenceBundle, + observed_at: datetime | None = None, +) -> LifecycleResult: + """Reconcile one redacted Evidence Bundle into workspace incident state. + + The caller owns the surrounding transaction. Reconciliation is exactly-once + per scan ID: replays with byte-equivalent incident identity are no-ops, and + a replay whose evidence set or completeness differs fails closed. + + Missing incidents are resolved only when ``bundle.plan_complete is True``. + Terraform/OpenTofu may emit incomplete/deferred plans; absence from such a + plan is not proof that previously observed drift disappeared. + """ + observed_at = observed_at or datetime.now(UTC) + if observed_at.tzinfo is None or observed_at.utcoffset() is None: + raise LifecycleReconcileError("observed_at must be timezone-aware.") + + observations: dict[str, DriftEvidence] = {} + for finding in bundle.findings: + fingerprint = finding_fingerprint(finding) + if fingerprint in observations: + raise LifecycleReconcileError( + f"Evidence bundle contains duplicate incident identity {fingerprint}." + ) + observations[fingerprint] = finding + + observation_digest = _observation_set_digest(set(observations)) + + # Serialize lifecycle mutation per workspace on databases that support row + # locks. SQLite ignores FOR UPDATE, which is sufficient for deterministic + # single-process tests; PostgreSQL production receives the real lock. + workspace_result = await db.execute( + select(Workspace.id).where(Workspace.id == workspace_id).with_for_update() + ) + if workspace_result.scalar_one_or_none() is None: + raise LifecycleReconcileError(f"Workspace {workspace_id} does not exist.") + + scan_result = await db.execute( + select(DriftScan.id).where( + DriftScan.id == scan_id, + DriftScan.workspace_id == workspace_id, + ) + ) + if scan_result.scalar_one_or_none() is None: + raise LifecycleReconcileError( + f"Scan {scan_id} does not belong to workspace {workspace_id}." + ) + + marker_result = await db.execute( + select(EvidenceReconciliation).where(EvidenceReconciliation.scan_id == scan_id) + ) + marker = marker_result.scalar_one_or_none() + if marker is not None: + if ( + marker.workspace_id != workspace_id + or marker.observation_set_digest != observation_digest + or marker.finding_count != len(observations) + or marker.plan_complete is not bundle.plan_complete + ): + raise LifecycleReconcileError( + "Scan replay differs from the evidence already reconciled for this scan ID." + ) + return LifecycleResult(already_reconciled=True) + + incidents_result = await db.execute( + select(FindingIncident).where( + FindingIncident.workspace_id == workspace_id, + FindingIncident.identity_version == IDENTITY_VERSION, + ) + ) + incidents = {incident.fingerprint: incident for incident in incidents_result.scalars().all()} + + created = 0 + observed_existing = 0 + reopened = 0 + + for fingerprint, finding in observations.items(): + incident = incidents.get(fingerprint) + if incident is None: + incident = FindingIncident( + workspace_id=workspace_id, + fingerprint=fingerprint, + identity_version=IDENTITY_VERSION, + resource_address=finding.resource_address, + deposed_key=finding.deposed_key, + resource_type=finding.resource_type, + provider_name=finding.provider_name, + actions=list(finding.actions), + changed_paths=sorted(set(finding.changed_paths)), + status=INCIDENT_OPEN, + first_seen_at=observed_at, + last_seen_at=observed_at, + last_scan_id=scan_id, + occurrence_count=1, + reopen_count=0, + remediation_branch=remediation_branch_for_fingerprint(fingerprint), + ) + db.add(incident) + await db.flush() + incidents[fingerprint] = incident + created += 1 + else: + incident.last_seen_at = observed_at + incident.last_scan_id = scan_id + incident.occurrence_count += 1 + if incident.status == INCIDENT_RESOLVED: + incident.status = INCIDENT_OPEN + incident.resolved_at = None + incident.reopened_at = observed_at + incident.reopen_count += 1 + reopened += 1 + elif incident.status == INCIDENT_OPEN: + observed_existing += 1 + else: + raise LifecycleReconcileError( + f"Incident {incident.id} has unsupported lifecycle status {incident.status!r}." + ) + + db.add( + FindingOccurrence( + incident_id=incident.id, + workspace_id=workspace_id, + scan_id=scan_id, + observed_at=observed_at, + evidence_schema_version=bundle.schema_version, + sensitive_paths=sorted(set(finding.sensitive_paths)), + unknown_paths=sorted(set(finding.unknown_paths)), + ) + ) + + resolved = 0 + resolution_performed = bundle.plan_complete is True + if resolution_performed: + observed_fingerprints = set(observations) + for fingerprint, incident in incidents.items(): + if fingerprint in observed_fingerprints: + continue + if incident.status == INCIDENT_OPEN: + incident.status = INCIDENT_RESOLVED + incident.resolved_at = observed_at + resolved += 1 + + db.add( + EvidenceReconciliation( + scan_id=scan_id, + workspace_id=workspace_id, + observation_set_digest=observation_digest, + finding_count=len(observations), + plan_complete=bundle.plan_complete, + reconciled_at=observed_at, + ) + ) + await db.flush() + + return LifecycleResult( + created=created, + observed_existing=observed_existing, + reopened=reopened, + resolved=resolved, + resolution_performed=resolution_performed, + ) diff --git a/backend/models/incidents.py b/backend/models/incidents.py new file mode 100644 index 0000000..327cc87 --- /dev/null +++ b/backend/models/incidents.py @@ -0,0 +1,123 @@ +from __future__ import annotations + +from datetime import datetime + +from sqlalchemy import ( + JSON, + Boolean, + CheckConstraint, + DateTime, + ForeignKey, + Integer, + String, + UniqueConstraint, +) +from sqlalchemy.orm import Mapped, mapped_column + +from .base import Base, TimestampMixin, generate_id + + +class FindingIncident(Base, TimestampMixin): + """Workspace-scoped lifecycle record for one deterministic drift identity.""" + + __tablename__ = "finding_incidents" + __table_args__ = ( + UniqueConstraint( + "workspace_id", + "fingerprint", + name="uq_finding_incidents_workspace_fingerprint", + ), + CheckConstraint( + "status IN ('open', 'resolved')", + name="ck_finding_incidents_status", + ), + CheckConstraint( + "occurrence_count >= 1", + name="ck_finding_incidents_occurrence_count", + ), + CheckConstraint( + "reopen_count >= 0", + name="ck_finding_incidents_reopen_count", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=generate_id) + workspace_id: Mapped[str] = mapped_column( + ForeignKey("workspaces.id"), nullable=False, index=True + ) + fingerprint: Mapped[str] = mapped_column(String(64), nullable=False, index=True) + identity_version: Mapped[str] = mapped_column(String(16), nullable=False, default="1") + + resource_address: Mapped[str] = mapped_column(String(1000), nullable=False) + deposed_key: Mapped[str | None] = mapped_column(String(255)) + resource_type: Mapped[str] = mapped_column(String(255), nullable=False) + provider_name: Mapped[str | None] = mapped_column(String(500)) + actions: Mapped[list[str]] = mapped_column(JSON, nullable=False) + changed_paths: Mapped[list[str]] = mapped_column(JSON, nullable=False) + + status: Mapped[str] = mapped_column(String(20), nullable=False, default="open", index=True) + first_seen_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + last_seen_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + last_scan_id: Mapped[str] = mapped_column(ForeignKey("drift_scans.id"), nullable=False) + occurrence_count: Mapped[int] = mapped_column(Integer, nullable=False, default=1) + reopen_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + reopened_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + resolved_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) + + # Stable future GitHub idempotency key. The evidence path does not open PRs + # yet, but every recurrence of the same incident receives the same branch. + remediation_branch: Mapped[str] = mapped_column(String(255), nullable=False) + remediation_pr_url: Mapped[str | None] = mapped_column(String(500)) + remediation_pr_number: Mapped[int | None] = mapped_column(Integer) + + +class FindingOccurrence(Base, TimestampMixin): + """Redacted audit record that an incident was observed in a specific scan.""" + + __tablename__ = "finding_occurrences" + __table_args__ = ( + UniqueConstraint( + "incident_id", + "scan_id", + name="uq_finding_occurrences_incident_scan", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=generate_id) + incident_id: Mapped[str] = mapped_column( + ForeignKey("finding_incidents.id"), nullable=False, index=True + ) + workspace_id: Mapped[str] = mapped_column( + ForeignKey("workspaces.id"), nullable=False, index=True + ) + scan_id: Mapped[str] = mapped_column( + ForeignKey("drift_scans.id"), nullable=False, index=True + ) + observed_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + evidence_schema_version: Mapped[str] = mapped_column(String(16), nullable=False) + sensitive_paths: Mapped[list[str]] = mapped_column(JSON, nullable=False) + unknown_paths: Mapped[list[str]] = mapped_column(JSON, nullable=False) + + +class EvidenceReconciliation(Base, TimestampMixin): + """Exactly-once marker for lifecycle reconciliation of one scan.""" + + __tablename__ = "evidence_reconciliations" + __table_args__ = ( + CheckConstraint( + "finding_count >= 0", + name="ck_evidence_reconciliations_finding_count", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=generate_id) + scan_id: Mapped[str] = mapped_column( + ForeignKey("drift_scans.id"), nullable=False, unique=True, index=True + ) + workspace_id: Mapped[str] = mapped_column( + ForeignKey("workspaces.id"), nullable=False, index=True + ) + observation_set_digest: Mapped[str] = mapped_column(String(64), nullable=False) + finding_count: Mapped[int] = mapped_column(Integer, nullable=False) + plan_complete: Mapped[bool | None] = mapped_column(Boolean) + reconciled_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) diff --git a/backend/tests/test_evidence_lifecycle.py b/backend/tests/test_evidence_lifecycle.py new file mode 100644 index 0000000..7fca42c --- /dev/null +++ b/backend/tests/test_evidence_lifecycle.py @@ -0,0 +1,322 @@ +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +import pytest +from sqlalchemy import func, select +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine +from sqlalchemy.pool import StaticPool + +from backend.evidence.lifecycle import ( + INCIDENT_OPEN, + LifecycleReconcileError, + finding_fingerprint, + reconcile_evidence_bundle, + remediation_branch_for_fingerprint, +) +from backend.evidence.models import DriftEvidence, EvidenceBundle +from backend.models.base import Base +from backend.models.incidents import ( + EvidenceReconciliation, + FindingIncident, + FindingOccurrence, +) +from backend.models.models import CloudProvider, DriftScan, Organization, ScanStatus, Workspace + + +def _finding( + *, + address: str = 'module.compute.aws_instance.web["blue"]', + deposed_key: str | None = None, + actions: list[str] | None = None, + changed_paths: list[str] | None = None, + sensitive_paths: list[str] | None = None, + unknown_paths: list[str] | None = None, +) -> DriftEvidence: + return DriftEvidence( + resource_address=address, + previous_resource_address=None, + module_address="module.compute", + deposed_key=deposed_key, + resource_type="aws_instance", + resource_name="web", + resource_index="blue", + provider_name="registry.terraform.io/hashicorp/aws", + actions=actions or ["update"], + changed_paths=changed_paths or ["/instance_type"], + sensitive_paths=sensitive_paths or [], + unknown_paths=unknown_paths or [], + ) + + +def _bundle(*findings: DriftEvidence, complete: bool | None = True) -> EvidenceBundle: + return EvidenceBundle( + iac_engine="terraform", + iac_engine_version="1.16.2", + source_format_version="1.2", + plan_timestamp="2026-09-14T13:00:00Z", + plan_applyable=True, + plan_complete=complete, + findings=list(findings), + ) + + +async def _new_database(): + engine = create_async_engine( + "sqlite+aiosqlite:///:memory:", + poolclass=StaticPool, + ) + Session = async_sessionmaker(engine, expire_on_commit=False) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + return engine, Session + + +async def _seed_workspace_and_scans(Session, scan_count: int = 4): + async with Session() as db: + org = Organization(name="Lifecycle Org", slug=f"lifecycle-org-{scan_count}") + db.add(org) + await db.flush() + workspace = Workspace( + org_id=org.id, + name="prod", + slug="prod", + provider=CloudProvider.AWS, + region="us-east-1", + ) + db.add(workspace) + await db.flush() + scans = [ + DriftScan(workspace_id=workspace.id, status=ScanStatus.COMPLETED) + for _ in range(scan_count) + ] + db.add_all(scans) + await db.commit() + return workspace.id, [scan.id for scan in scans] + + +def test_fingerprint_is_deterministic_but_preserves_semantic_identity_boundaries(): + base = _finding(changed_paths=["/tags", "/instance_type", "/tags"]) + reordered = _finding(changed_paths=["/instance_type", "/tags"]) + mask_only_change = _finding( + changed_paths=["/instance_type", "/tags"], + sensitive_paths=["/tags"], + unknown_paths=["/instance_type"], + ) + + fingerprint = finding_fingerprint(base) + assert fingerprint == finding_fingerprint(reordered) + assert fingerprint == finding_fingerprint(mask_only_change) + assert len(fingerprint) == 64 + + assert fingerprint != finding_fingerprint(_finding(address="aws_instance.other")) + assert fingerprint != finding_fingerprint(_finding(deposed_key="deadbeef")) + assert fingerprint != finding_fingerprint(_finding(actions=["delete", "create"])) + assert fingerprint != finding_fingerprint(_finding(changed_paths=["/ami"])) + assert remediation_branch_for_fingerprint(fingerprint) == f"driftguard/fix-{fingerprint}" + + +@pytest.mark.asyncio +async def test_lifecycle_create_repeat_resolve_and_reopen(): + engine, Session = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 4) + finding = _finding() + fingerprint = finding_fingerprint(finding) + t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) + + try: + async with Session() as db: + first = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(finding), + observed_at=t0, + ) + await db.commit() + assert first.created == 1 + assert first.resolution_performed is True + + async with Session() as db: + second = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[1], + bundle=_bundle(finding), + observed_at=t0 + timedelta(hours=1), + ) + await db.commit() + assert second.observed_existing == 1 + + async with Session() as db: + third = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[2], + bundle=_bundle(), + observed_at=t0 + timedelta(hours=2), + ) + await db.commit() + assert third.resolved == 1 + + async with Session() as db: + fourth = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[3], + bundle=_bundle(finding), + observed_at=t0 + timedelta(hours=3), + ) + await db.commit() + assert fourth.reopened == 1 + + async with Session() as db: + incident = ( + await db.execute( + select(FindingIncident).where( + FindingIncident.workspace_id == workspace_id, + FindingIncident.fingerprint == fingerprint, + ) + ) + ).scalar_one() + assert incident.status == INCIDENT_OPEN + assert incident.occurrence_count == 3 + assert incident.reopen_count == 1 + assert incident.resolved_at is None + assert incident.remediation_branch == f"driftguard/fix-{fingerprint}" + + occurrence_count = await db.scalar( + select(func.count()).select_from(FindingOccurrence) + ) + reconciliation_count = await db.scalar( + select(func.count()).select_from(EvidenceReconciliation) + ) + assert occurrence_count == 3 + assert reconciliation_count == 4 + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_incomplete_plan_cannot_resolve_missing_incident(): + engine, Session = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 2) + finding = _finding() + t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) + + try: + async with Session() as db: + await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(finding), + observed_at=t0, + ) + await db.commit() + + async with Session() as db: + result = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[1], + bundle=_bundle(complete=False), + observed_at=t0 + timedelta(hours=1), + ) + await db.commit() + assert result.resolution_performed is False + assert result.resolved == 0 + + async with Session() as db: + incident = ( + await db.execute( + select(FindingIncident).where(FindingIncident.workspace_id == workspace_id) + ) + ).scalar_one() + assert incident.status == INCIDENT_OPEN + assert incident.resolved_at is None + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_clean_scan_replay_is_exactly_once_and_inconsistent_replay_fails_closed(): + engine, Session = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 1) + t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) + + try: + async with Session() as db: + first = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(), + observed_at=t0, + ) + await db.commit() + assert first.already_reconciled is False + + async with Session() as db: + replay = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(), + observed_at=t0 + timedelta(minutes=1), + ) + await db.commit() + assert replay.already_reconciled is True + + async with Session() as db: + with pytest.raises(LifecycleReconcileError, match="Scan replay differs"): + await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(_finding()), + observed_at=t0 + timedelta(minutes=2), + ) + await db.rollback() + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_scan_workspace_mismatch_fails_before_lifecycle_mutation(): + engine, Session = await _new_database() + _, scan_ids = await _seed_workspace_and_scans(Session, 1) + + async with Session() as db: + org = Organization(name="Other Org", slug="other-lifecycle-org") + db.add(org) + await db.flush() + workspace_b = Workspace( + org_id=org.id, + name="other", + slug="other", + provider=CloudProvider.AWS, + region="us-east-1", + ) + db.add(workspace_b) + await db.commit() + workspace_b_id = workspace_b.id + + try: + async with Session() as db: + with pytest.raises(LifecycleReconcileError, match="does not belong"): + await reconcile_evidence_bundle( + db, + workspace_id=workspace_b_id, + scan_id=scan_ids[0], + bundle=_bundle(_finding()), + ) + await db.rollback() + + async with Session() as db: + incident_count = await db.scalar( + select(func.count()).select_from(FindingIncident) + ) + assert incident_count == 0 + finally: + await engine.dispose() diff --git a/docs/adr/0002-evidence-finding-lifecycle.md b/docs/adr/0002-evidence-finding-lifecycle.md new file mode 100644 index 0000000..5e96115 --- /dev/null +++ b/docs/adr/0002-evidence-finding-lifecycle.md @@ -0,0 +1,196 @@ +# ADR 0002: Evidence finding identity and lifecycle + +- Status: Accepted +- Date: 2026-09-14 +- Scope: incident identity, recurrence, resolution, replay safety, remediation deduplication + +## Context + +The legacy `drift_findings` table stores one row per finding per scan. A new UUID +is generated every time the same drift is observed. GitHub remediation branches +are derived from that UUID, so recurrence can create a new branch and a new PR +for an unchanged underlying incident. + +That behavior is unsuitable for Evidence Core. A provider-native observation +must have a stable identity across scans so DriftGuard can distinguish: + +- a newly observed drift incident, +- the same unresolved incident seen again, +- an incident that disappeared from a complete observation, +- and a previously resolved incident that later reappeared. + +The repository currently has no schema-migration system. `Base.metadata.create_all()` +creates missing tables but does not add columns to an already-existing table. +Therefore lifecycle data must not be introduced by silently adding fields to +`drift_findings`; that would work on a fresh database and fail on an existing +production database. + +## Decision + +Evidence Core uses separate lifecycle tables: + +- `finding_incidents`: one workspace-scoped row per deterministic incident identity, +- `finding_occurrences`: one redacted observation of an incident in a scan, +- `evidence_reconciliations`: one exactly-once reconciliation marker per scan. + +The legacy `drift_findings` table remains unchanged. The new lifecycle is not +wired to the legacy collector-based scan path. + +## Identity v1 + +The incident fingerprint is the full lowercase SHA-256 digest of canonical JSON +containing: + +- identity version, +- exact provider-native `resource_address`, +- optional Terraform/OpenTofu `deposed` key, +- resource type, +- provider source name, +- provider-native action sequence in its original order, +- sorted unique `changed_paths`. + +The database uniqueness boundary is `(workspace_id, fingerprint)`. Workspace ID +is intentionally not embedded in the digest; tenancy is enforced by the unique +constraint and all lifecycle queries. + +Sensitivity masks and `after_unknown` paths are excluded from identity. They +represent what can safely be known or disclosed about an observation, not the +underlying drift surface. A secret becoming masked or an unknown value becoming +known must not create a second incident when resource identity, actions and +changed paths are unchanged. + +Raw infrastructure values are never inputs to lifecycle identity and are never +persisted by these tables. + +## Resource moves + +`previous_resource_address` is not part of identity v1. The current absolute +resource address is authoritative. + +This means an explicit Terraform/OpenTofu resource move can produce a new +incident identity after the move. Preserving lifecycle across moves requires a +separate, explicit address-migration mapping. v1 does not guess that mapping, +because using `previous_resource_address` as a permanent canonical address would +make identity unstable once that field disappears from later plans. + +## State machine + +Lifecycle v1 has only two machine states: + +- `open` +- `resolved` + +Transitions: + +1. unseen fingerprint -> `open`, `occurrence_count = 1` +2. `open` fingerprint observed again -> remain `open`, increment occurrence count +3. `open` fingerprint absent from a **complete** plan -> `resolved` +4. `resolved` fingerprint observed again -> `open`, increment `reopen_count` + +Manual dispositions such as ignored/false-positive are intentionally not mixed +into this state machine yet. They are policy/user-decision states and need a +separate contract rather than overloading machine observation state. + +## Completeness gate + +Absence is evidence only when the IaC engine says the plan is complete. + +Therefore DriftGuard resolves missing incidents only when: + +```text +EvidenceBundle.plan_complete is True +``` + +If `plan_complete` is `False` or unavailable, observed incidents may be created +or refreshed, but missing incidents cannot be resolved. This prevents deferred +or partial planning from creating false recovery events. + +## Exactly-once reconciliation + +A background task or worker can be retried. Reprocessing the same scan must not: + +- increment occurrence counts twice, +- reopen an incident twice, +- resolve an incident twice, +- or create a second remediation side effect. + +`evidence_reconciliations` records one row per `scan_id` containing: + +- workspace ID, +- SHA-256 digest of the sorted observed incident fingerprints, +- finding count, +- plan completeness, +- reconciliation timestamp. + +A replay with the same scan ID and the same observation set/completeness is a +no-op. A replay with different evidence or different completeness fails closed. +This marker is required even for a clean scan with zero findings; occurrence rows +alone cannot make a zero-finding scan idempotent. + +PostgreSQL reconciliation acquires a row lock on the workspace before mutation, +serializing lifecycle changes for concurrent scans of the same workspace. The +unique constraints remain the final integrity boundary. + +## Occurrence audit trail + +`finding_occurrences` is append-only redacted metadata. It stores: + +- incident ID, +- workspace ID, +- scan ID, +- observation time, +- Evidence Bundle schema version, +- sensitive paths, +- unknown paths. + +It never stores Terraform/OpenTofu `before` or `after` values. + +The unique `(incident_id, scan_id)` constraint prevents one scan from recording +the same incident twice. + +## Remediation deduplication key + +Every incident receives one stable branch name: + +```text +driftguard/fix- +``` + +The branch is persisted on the incident. Future Evidence Core GitHub automation +must use this branch and the incident-level PR metadata instead of a per-scan +finding UUID. This commit defines and tests that idempotency key but deliberately +does not attach new PR side effects to the legacy detector. + +## Failure semantics + +Reconciliation fails without partial commit when: + +- the workspace does not exist, +- the scan does not belong to the workspace, +- the bundle contains duplicate incident identities, +- a replay disagrees with the already-reconciled evidence set/completeness, +- an incident contains an unsupported lifecycle status, +- or the supplied observation timestamp is timezone-naive. + +The caller owns the surrounding transaction. A failure is not converted into a +clean scan or a resolved incident set. + +## Migration sequence + +1. Land lifecycle schema and reconciliation contract behind tests. +2. Keep the legacy scan pipeline unchanged. +3. Add redacted Evidence Bundle persistence from the provider-native scan path. +4. Reconcile Evidence Bundle findings into incidents transactionally. +5. Expose incident lifecycle through API/UI without treating scan snapshots as incidents. +6. Move GitHub remediation automation to incident-level idempotency. +7. Only then retire per-scan legacy finding behavior when the Evidence Core cutover gates pass. + +## Explicit non-claims + +This ADR does not claim: + +- that the legacy collector detector now has lifecycle correctness, +- that GitHub remediation is already incident-aware, +- that resource moves preserve incident identity, +- that manual ignore/false-positive policy is implemented, +- or that Evidence Core has replaced the production scan path. From 932c19c4aba6a5f0f82847c85867ce6ae11e3ad1 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 14:49:48 +0100 Subject: [PATCH 10/24] fix: make evidence lifecycle monotonic under out-of-order scans - add one workspace-scoped EvidenceCursor watermark - record stale/ambiguous scans without applying lifecycle transitions - prevent late workers from falsely resolving or reopening newer incident state - require an explicit timezone-aware observation timestamp - validate all loaded incident states before mutation - preserve exactly-once replay semantics including observation time - fix the lifecycle-test Ruff import-format failure - document ordering and stale-scan semantics in ADR 0002 --- backend/evidence/lifecycle.py | 85 ++++++-- backend/models/incidents.py | 20 +- backend/tests/test_evidence_lifecycle.py | 115 ++++++++++- docs/adr/0002-evidence-finding-lifecycle.md | 215 ++++++++++---------- 4 files changed, 309 insertions(+), 126 deletions(-) diff --git a/backend/evidence/lifecycle.py b/backend/evidence/lifecycle.py index 7da362b..b6cb5de 100644 --- a/backend/evidence/lifecycle.py +++ b/backend/evidence/lifecycle.py @@ -10,6 +10,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from ..models.incidents import ( + EvidenceCursor, EvidenceReconciliation, FindingIncident, FindingOccurrence, @@ -34,6 +35,7 @@ class LifecycleResult: reopened: int = 0 resolved: int = 0 already_reconciled: bool = False + stale_ignored: bool = False resolution_performed: bool = False @@ -66,7 +68,9 @@ def finding_fingerprint(finding: DriftEvidence) -> str: def remediation_branch_for_fingerprint(fingerprint: str) -> str: """Return the stable GitHub branch reserved for one incident identity.""" if not _FINGERPRINT_RE.fullmatch(fingerprint): - raise LifecycleReconcileError("Incident fingerprint must be a 64-character lowercase SHA-256 hex digest.") + raise LifecycleReconcileError( + "Incident fingerprint must be a 64-character lowercase SHA-256 hex digest." + ) return f"driftguard/fix-{fingerprint}" @@ -75,27 +79,39 @@ def _observation_set_digest(fingerprints: set[str]) -> str: return hashlib.sha256(canonical).hexdigest() +def _as_utc(value: datetime) -> datetime: + """Normalize persisted datetimes; SQLite may discard timezone metadata.""" + if value.tzinfo is None or value.utcoffset() is None: + return value.replace(tzinfo=UTC) + return value.astimezone(UTC) + + async def reconcile_evidence_bundle( db: AsyncSession, *, workspace_id: str, scan_id: str, bundle: EvidenceBundle, - observed_at: datetime | None = None, + observed_at: datetime, ) -> LifecycleResult: """Reconcile one redacted Evidence Bundle into workspace incident state. The caller owns the surrounding transaction. Reconciliation is exactly-once - per scan ID: replays with byte-equivalent incident identity are no-ops, and - a replay whose evidence set or completeness differs fails closed. + per scan ID: replays with identical evidence are no-ops, while a replay with + different evidence, completeness, or observation time fails closed. + + Lifecycle mutations are monotonic by ``observed_at``. A scan that arrives + after a newer workspace observation is recorded as stale and performs no + incident transitions. This prevents late workers from resolving or reopening + incidents based on superseded infrastructure state. Missing incidents are resolved only when ``bundle.plan_complete is True``. Terraform/OpenTofu may emit incomplete/deferred plans; absence from such a plan is not proof that previously observed drift disappeared. """ - observed_at = observed_at or datetime.now(UTC) if observed_at.tzinfo is None or observed_at.utcoffset() is None: raise LifecycleReconcileError("observed_at must be timezone-aware.") + observed_at = observed_at.astimezone(UTC) observations: dict[str, DriftEvidence] = {} for finding in bundle.findings: @@ -138,11 +154,37 @@ async def reconcile_evidence_bundle( or marker.observation_set_digest != observation_digest or marker.finding_count != len(observations) or marker.plan_complete is not bundle.plan_complete + or _as_utc(marker.observed_at) != observed_at ): raise LifecycleReconcileError( "Scan replay differs from the evidence already reconciled for this scan ID." ) - return LifecycleResult(already_reconciled=True) + return LifecycleResult( + already_reconciled=True, + stale_ignored=not marker.applied_to_lifecycle, + ) + + cursor_result = await db.execute( + select(EvidenceCursor) + .where(EvidenceCursor.workspace_id == workspace_id) + .with_for_update() + ) + cursor = cursor_result.scalar_one_or_none() + if cursor is not None and observed_at <= _as_utc(cursor.latest_observed_at): + db.add( + EvidenceReconciliation( + scan_id=scan_id, + workspace_id=workspace_id, + observation_set_digest=observation_digest, + finding_count=len(observations), + plan_complete=bundle.plan_complete, + observed_at=observed_at, + applied_to_lifecycle=False, + reconciled_at=datetime.now(UTC), + ) + ) + await db.flush() + return LifecycleResult(stale_ignored=True) incidents_result = await db.execute( select(FindingIncident).where( @@ -150,7 +192,14 @@ async def reconcile_evidence_bundle( FindingIncident.identity_version == IDENTITY_VERSION, ) ) - incidents = {incident.fingerprint: incident for incident in incidents_result.scalars().all()} + incidents = { + incident.fingerprint: incident for incident in incidents_result.scalars().all() + } + for incident in incidents.values(): + if incident.status not in {INCIDENT_OPEN, INCIDENT_RESOLVED}: + raise LifecycleReconcileError( + f"Incident {incident.id} has unsupported lifecycle status {incident.status!r}." + ) created = 0 observed_existing = 0 @@ -191,12 +240,8 @@ async def reconcile_evidence_bundle( incident.reopened_at = observed_at incident.reopen_count += 1 reopened += 1 - elif incident.status == INCIDENT_OPEN: - observed_existing += 1 else: - raise LifecycleReconcileError( - f"Incident {incident.id} has unsupported lifecycle status {incident.status!r}." - ) + observed_existing += 1 db.add( FindingOccurrence( @@ -229,9 +274,23 @@ async def reconcile_evidence_bundle( observation_set_digest=observation_digest, finding_count=len(observations), plan_complete=bundle.plan_complete, - reconciled_at=observed_at, + observed_at=observed_at, + applied_to_lifecycle=True, + reconciled_at=datetime.now(UTC), ) ) + if cursor is None: + db.add( + EvidenceCursor( + workspace_id=workspace_id, + latest_observed_at=observed_at, + latest_scan_id=scan_id, + ) + ) + else: + cursor.latest_observed_at = observed_at + cursor.latest_scan_id = scan_id + await db.flush() return LifecycleResult( diff --git a/backend/models/incidents.py b/backend/models/incidents.py index 327cc87..4c9e263 100644 --- a/backend/models/incidents.py +++ b/backend/models/incidents.py @@ -72,7 +72,7 @@ class FindingIncident(Base, TimestampMixin): class FindingOccurrence(Base, TimestampMixin): - """Redacted audit record that an incident was observed in a specific scan.""" + """Redacted audit row for an incident in a lifecycle-applied scan.""" __tablename__ = "finding_occurrences" __table_args__ = ( @@ -99,6 +99,22 @@ class FindingOccurrence(Base, TimestampMixin): unknown_paths: Mapped[list[str]] = mapped_column(JSON, nullable=False) +class EvidenceCursor(Base, TimestampMixin): + """Monotonic lifecycle watermark for one workspace.""" + + __tablename__ = "evidence_cursors" + + workspace_id: Mapped[str] = mapped_column( + ForeignKey("workspaces.id"), primary_key=True + ) + latest_observed_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False + ) + latest_scan_id: Mapped[str] = mapped_column( + ForeignKey("drift_scans.id"), nullable=False + ) + + class EvidenceReconciliation(Base, TimestampMixin): """Exactly-once marker for lifecycle reconciliation of one scan.""" @@ -120,4 +136,6 @@ class EvidenceReconciliation(Base, TimestampMixin): observation_set_digest: Mapped[str] = mapped_column(String(64), nullable=False) finding_count: Mapped[int] = mapped_column(Integer, nullable=False) plan_complete: Mapped[bool | None] = mapped_column(Boolean) + observed_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + applied_to_lifecycle: Mapped[bool] = mapped_column(Boolean, nullable=False) reconciled_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) diff --git a/backend/tests/test_evidence_lifecycle.py b/backend/tests/test_evidence_lifecycle.py index 7fca42c..cf38517 100644 --- a/backend/tests/test_evidence_lifecycle.py +++ b/backend/tests/test_evidence_lifecycle.py @@ -9,6 +9,7 @@ from backend.evidence.lifecycle import ( INCIDENT_OPEN, + INCIDENT_RESOLVED, LifecycleReconcileError, finding_fingerprint, reconcile_evidence_bundle, @@ -17,11 +18,18 @@ from backend.evidence.models import DriftEvidence, EvidenceBundle from backend.models.base import Base from backend.models.incidents import ( + EvidenceCursor, EvidenceReconciliation, FindingIncident, FindingOccurrence, ) -from backend.models.models import CloudProvider, DriftScan, Organization, ScanStatus, Workspace +from backend.models.models import ( + CloudProvider, + DriftScan, + Organization, + ScanStatus, + Workspace, +) def _finding( @@ -263,7 +271,7 @@ async def test_clean_scan_replay_is_exactly_once_and_inconsistent_replay_fails_c workspace_id=workspace_id, scan_id=scan_ids[0], bundle=_bundle(), - observed_at=t0 + timedelta(minutes=1), + observed_at=t0, ) await db.commit() assert replay.already_reconciled is True @@ -310,6 +318,7 @@ async def test_scan_workspace_mismatch_fails_before_lifecycle_mutation(): workspace_id=workspace_b_id, scan_id=scan_ids[0], bundle=_bundle(_finding()), + observed_at=datetime(2026, 9, 14, 13, 0, tzinfo=UTC), ) await db.rollback() @@ -320,3 +329,105 @@ async def test_scan_workspace_mismatch_fails_before_lifecycle_mutation(): assert incident_count == 0 finally: await engine.dispose() + + +@pytest.mark.asyncio +async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_state(): + engine, Session = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 3) + finding = _finding() + t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) + + try: + async with Session() as db: + await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(finding), + observed_at=t0, + ) + await db.commit() + + async with Session() as db: + resolved = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[2], + bundle=_bundle(), + observed_at=t0 + timedelta(hours=2), + ) + await db.commit() + assert resolved.resolved == 1 + + async with Session() as db: + stale = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[1], + bundle=_bundle(finding), + observed_at=t0 + timedelta(hours=1), + ) + await db.commit() + assert stale.stale_ignored is True + assert stale.reopened == 0 + + async with Session() as db: + incident = ( + await db.execute( + select(FindingIncident).where(FindingIncident.workspace_id == workspace_id) + ) + ).scalar_one() + assert incident.status == INCIDENT_RESOLVED + assert incident.occurrence_count == 1 + assert incident.reopen_count == 0 + + stale_marker = ( + await db.execute( + select(EvidenceReconciliation).where( + EvidenceReconciliation.scan_id == scan_ids[1] + ) + ) + ).scalar_one() + assert stale_marker.applied_to_lifecycle is False + + cursor = ( + await db.execute( + select(EvidenceCursor).where(EvidenceCursor.workspace_id == workspace_id) + ) + ).scalar_one() + assert cursor.latest_scan_id == scan_ids[2] + + async with Session() as db: + replay = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[1], + bundle=_bundle(finding), + observed_at=t0 + timedelta(hours=1), + ) + await db.commit() + assert replay.already_reconciled is True + assert replay.stale_ignored is True + finally: + await engine.dispose() + + +@pytest.mark.asyncio +async def test_timezone_naive_observation_is_rejected(): + engine, Session = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 1) + + try: + async with Session() as db: + with pytest.raises(LifecycleReconcileError, match="timezone-aware"): + await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan_ids[0], + bundle=_bundle(_finding()), + observed_at=datetime(2026, 9, 14, 13, 0), + ) + await db.rollback() + finally: + await engine.dispose() diff --git a/docs/adr/0002-evidence-finding-lifecycle.md b/docs/adr/0002-evidence-finding-lifecycle.md index 5e96115..b190a99 100644 --- a/docs/adr/0002-evidence-finding-lifecycle.md +++ b/docs/adr/0002-evidence-finding-lifecycle.md @@ -2,7 +2,7 @@ - Status: Accepted - Date: 2026-09-14 -- Scope: incident identity, recurrence, resolution, replay safety, remediation deduplication +- Scope: incident identity, recurrence, resolution, replay safety, observation ordering, remediation deduplication ## Context @@ -11,142 +11,139 @@ is generated every time the same drift is observed. GitHub remediation branches are derived from that UUID, so recurrence can create a new branch and a new PR for an unchanged underlying incident. -That behavior is unsuitable for Evidence Core. A provider-native observation -must have a stable identity across scans so DriftGuard can distinguish: - -- a newly observed drift incident, -- the same unresolved incident seen again, -- an incident that disappeared from a complete observation, -- and a previously resolved incident that later reappeared. +Evidence Core needs stable identity across scans so DriftGuard can distinguish a +new incident, a recurrence, a verified resolution, and a later reappearance. +It must also remain correct when workers finish out of order: serialization alone +does not prevent an older observation from incorrectly resolving or reopening +state established by a newer observation. The repository currently has no schema-migration system. `Base.metadata.create_all()` -creates missing tables but does not add columns to an already-existing table. -Therefore lifecycle data must not be introduced by silently adding fields to -`drift_findings`; that would work on a fresh database and fail on an existing -production database. +creates missing tables but does not add columns to an existing table. Lifecycle +data therefore must not be introduced by silently adding fields to the existing +`drift_findings` table. ## Decision Evidence Core uses separate lifecycle tables: -- `finding_incidents`: one workspace-scoped row per deterministic incident identity, -- `finding_occurrences`: one redacted observation of an incident in a scan, -- `evidence_reconciliations`: one exactly-once reconciliation marker per scan. +- `finding_incidents`: one workspace-scoped row per deterministic incident identity; +- `finding_occurrences`: one redacted observation for each lifecycle-applied scan; +- `evidence_reconciliations`: one exactly-once reconciliation marker per scan; +- `evidence_cursors`: one monotonic lifecycle watermark per workspace. -The legacy `drift_findings` table remains unchanged. The new lifecycle is not -wired to the legacy collector-based scan path. +The legacy `drift_findings` table and legacy collector scan path remain unchanged. ## Identity v1 The incident fingerprint is the full lowercase SHA-256 digest of canonical JSON containing: -- identity version, -- exact provider-native `resource_address`, -- optional Terraform/OpenTofu `deposed` key, -- resource type, -- provider source name, -- provider-native action sequence in its original order, +- identity version; +- exact provider-native `resource_address`; +- optional Terraform/OpenTofu `deposed` key; +- resource type; +- provider source name; +- provider-native action sequence in its original order; - sorted unique `changed_paths`. The database uniqueness boundary is `(workspace_id, fingerprint)`. Workspace ID is intentionally not embedded in the digest; tenancy is enforced by the unique -constraint and all lifecycle queries. - -Sensitivity masks and `after_unknown` paths are excluded from identity. They -represent what can safely be known or disclosed about an observation, not the -underlying drift surface. A secret becoming masked or an unknown value becoming -known must not create a second incident when resource identity, actions and -changed paths are unchanged. - -Raw infrastructure values are never inputs to lifecycle identity and are never -persisted by these tables. - -## Resource moves +constraint and workspace-scoped queries. -`previous_resource_address` is not part of identity v1. The current absolute -resource address is authoritative. +Sensitivity masks and `after_unknown` paths are excluded from identity because +they describe observation/disclosure quality rather than the underlying drift +surface. Raw infrastructure values are never inputs to identity and are never +persisted by lifecycle tables. -This means an explicit Terraform/OpenTofu resource move can produce a new -incident identity after the move. Preserving lifecycle across moves requires a -separate, explicit address-migration mapping. v1 does not guess that mapping, -because using `previous_resource_address` as a permanent canonical address would -make identity unstable once that field disappears from later plans. +`previous_resource_address` is also excluded. A resource move can therefore +produce a new v1 incident. Preserving lifecycle across explicit moves requires a +future address-migration contract; v1 does not guess one. ## State machine -Lifecycle v1 has only two machine states: +Lifecycle v1 has two machine states: - `open` - `resolved` -Transitions: +Transitions for a lifecycle-applied observation are: -1. unseen fingerprint -> `open`, `occurrence_count = 1` -2. `open` fingerprint observed again -> remain `open`, increment occurrence count -3. `open` fingerprint absent from a **complete** plan -> `resolved` -4. `resolved` fingerprint observed again -> `open`, increment `reopen_count` +1. unseen fingerprint -> `open`, occurrence count 1; +2. `open` fingerprint observed again -> remain `open`, increment occurrence count; +3. `open` fingerprint absent from a complete plan -> `resolved`; +4. `resolved` fingerprint observed again -> `open`, increment reopen count. -Manual dispositions such as ignored/false-positive are intentionally not mixed -into this state machine yet. They are policy/user-decision states and need a -separate contract rather than overloading machine observation state. +Manual dispositions such as ignored or false-positive are policy decisions and +are intentionally not overloaded onto this machine-observation state. ## Completeness gate -Absence is evidence only when the IaC engine says the plan is complete. - -Therefore DriftGuard resolves missing incidents only when: +Absence is evidence only when the IaC engine reports a complete plan. Missing +incidents are therefore resolved only when: ```text EvidenceBundle.plan_complete is True ``` -If `plan_complete` is `False` or unavailable, observed incidents may be created -or refreshed, but missing incidents cannot be resolved. This prevents deferred -or partial planning from creating false recovery events. +When `plan_complete` is `False` or unavailable, observed incidents may be +created/refreshed, but absence cannot resolve anything. -## Exactly-once reconciliation +## Monotonic observation ordering + +Each workspace has an `evidence_cursors` row containing the latest observation +time and scan ID that was allowed to mutate lifecycle state. -A background task or worker can be retried. Reprocessing the same scan must not: +Reconciliation acquires a workspace row lock on PostgreSQL before reading the +cursor. If a different scan arrives with: -- increment occurrence counts twice, -- reopen an incident twice, -- resolve an incident twice, -- or create a second remediation side effect. +```text +observed_at <= latest_observed_at +``` -`evidence_reconciliations` records one row per `scan_id` containing: +it is stale or order-ambiguous. DriftGuard records an +`evidence_reconciliations` row with `applied_to_lifecycle = false` and performs +no incident creation, recurrence increment, resolution, reopen, occurrence +insert, or cursor movement. -- workspace ID, -- SHA-256 digest of the sorted observed incident fingerprints, -- finding count, -- plan completeness, -- reconciliation timestamp. +This deliberately prefers a missed historical lifecycle transition over a +false current-state transition. A later Evidence Bundle persistence layer may +retain the stale bundle for historical analysis independently of lifecycle. -A replay with the same scan ID and the same observation set/completeness is a -no-op. A replay with different evidence or different completeness fails closed. -This marker is required even for a clean scan with zero findings; occurrence rows -alone cannot make a zero-finding scan idempotent. +Callers must supply a timezone-aware observation time. Processing/arrival time +must not be substituted silently for infrastructure observation order. -PostgreSQL reconciliation acquires a row lock on the workspace before mutation, -serializing lifecycle changes for concurrent scans of the same workspace. The -unique constraints remain the final integrity boundary. +## Exactly-once reconciliation -## Occurrence audit trail +A worker retry must not increment counts or repeat transitions. Every scan gets +at most one `evidence_reconciliations` row containing workspace ID, the digest +of its sorted incident fingerprints, finding count, completeness, observation +time, whether it was applied to lifecycle, and reconciliation time. -`finding_occurrences` is append-only redacted metadata. It stores: +Replaying the same scan with the same evidence/completeness/observation time is +a no-op. Replaying the same scan ID with different evidence, completeness, or +observation time fails closed. This marker is required even for zero-finding +scans; occurrence rows alone cannot make a clean scan idempotent. -- incident ID, -- workspace ID, -- scan ID, -- observation time, -- Evidence Bundle schema version, -- sensitive paths, -- unknown paths. +## Occurrence semantics +`finding_occurrences` is append-only redacted metadata for lifecycle-applied +observations only. It stores incident ID, workspace ID, scan ID, observation +time, Evidence Bundle schema version, sensitive paths, and unknown paths. It never stores Terraform/OpenTofu `before` or `after` values. -The unique `(incident_id, scan_id)` constraint prevents one scan from recording -the same incident twice. +The unique `(incident_id, scan_id)` constraint prevents duplicate occurrence +records. Stale scans intentionally do not increment occurrence counts or create +occurrence rows because they were not allowed to affect the lifecycle timeline. + +## Concurrency and integrity boundaries + +PostgreSQL reconciliation locks the workspace row before lifecycle mutation, +serializing concurrent scans for one workspace. The monotonic cursor then +ensures serialized-but-out-of-order scans cannot roll state backward. Database +unique/check constraints remain the final integrity boundary. + +All loaded incident states are validated before mutation; unknown lifecycle +states fail closed even if that incident is absent from the current bundle. ## Remediation deduplication key @@ -156,41 +153,39 @@ Every incident receives one stable branch name: driftguard/fix- ``` -The branch is persisted on the incident. Future Evidence Core GitHub automation -must use this branch and the incident-level PR metadata instead of a per-scan -finding UUID. This commit defines and tests that idempotency key but deliberately -does not attach new PR side effects to the legacy detector. +Future Evidence Core GitHub automation must use this persisted incident-level +key rather than a per-scan finding UUID. This ADR defines and tests the key but +does not attach new GitHub side effects to the legacy detector. ## Failure semantics Reconciliation fails without partial commit when: -- the workspace does not exist, -- the scan does not belong to the workspace, -- the bundle contains duplicate incident identities, -- a replay disagrees with the already-reconciled evidence set/completeness, -- an incident contains an unsupported lifecycle status, -- or the supplied observation timestamp is timezone-naive. +- workspace does not exist; +- scan does not belong to workspace; +- bundle contains duplicate incident identities; +- same scan ID is replayed with different evidence/completeness/time; +- an incident contains an unsupported lifecycle status; +- observation time is timezone-naive. -The caller owns the surrounding transaction. A failure is not converted into a +The caller owns the surrounding transaction. Failure is never converted into a clean scan or a resolved incident set. ## Migration sequence 1. Land lifecycle schema and reconciliation contract behind tests. 2. Keep the legacy scan pipeline unchanged. -3. Add redacted Evidence Bundle persistence from the provider-native scan path. -4. Reconcile Evidence Bundle findings into incidents transactionally. -5. Expose incident lifecycle through API/UI without treating scan snapshots as incidents. -6. Move GitHub remediation automation to incident-level idempotency. -7. Only then retire per-scan legacy finding behavior when the Evidence Core cutover gates pass. +3. Introduce a real database migration mechanism before lifecycle tables are a production dependency. +4. Persist redacted Evidence Bundles from the provider-native scan path. +5. Reconcile persisted Evidence Bundle findings transactionally into incidents. +6. Expose incidents through API/UI without treating scan snapshots as incidents. +7. Move GitHub remediation automation to incident-level idempotency. +8. Retire per-scan legacy finding behavior only after the Evidence Core cutover gates pass. ## Explicit non-claims -This ADR does not claim: - -- that the legacy collector detector now has lifecycle correctness, -- that GitHub remediation is already incident-aware, -- that resource moves preserve incident identity, -- that manual ignore/false-positive policy is implemented, -- or that Evidence Core has replaced the production scan path. +This ADR does not claim that the legacy detector has lifecycle correctness, +GitHub remediation is incident-aware, resource moves preserve incident identity, +manual ignore/false-positive policy is implemented, stale bundles are already +persisted for historical analytics, or Evidence Core has replaced production +scan execution. From 88c886173958b1ed911faa77885fd10cac945d71 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Mon, 14 Sep 2026 14:52:18 +0100 Subject: [PATCH 11/24] fix: keep naive-time regression without weakening datetime lint --- backend/tests/test_evidence_lifecycle.py | 149 +++++++++++------------ 1 file changed, 69 insertions(+), 80 deletions(-) diff --git a/backend/tests/test_evidence_lifecycle.py b/backend/tests/test_evidence_lifecycle.py index cf38517..647a832 100644 --- a/backend/tests/test_evidence_lifecycle.py +++ b/backend/tests/test_evidence_lifecycle.py @@ -74,14 +74,14 @@ async def _new_database(): "sqlite+aiosqlite:///:memory:", poolclass=StaticPool, ) - Session = async_sessionmaker(engine, expire_on_commit=False) + session_factory = async_sessionmaker(engine, expire_on_commit=False) async with engine.begin() as conn: await conn.run_sync(Base.metadata.create_all) - return engine, Session + return engine, session_factory -async def _seed_workspace_and_scans(Session, scan_count: int = 4): - async with Session() as db: +async def _seed_workspace_and_scans(session_factory, scan_count: int): + async with session_factory() as db: org = Organization(name="Lifecycle Org", slug=f"lifecycle-org-{scan_count}") db.add(org) await db.flush() @@ -103,20 +103,19 @@ async def _seed_workspace_and_scans(Session, scan_count: int = 4): return workspace.id, [scan.id for scan in scans] -def test_fingerprint_is_deterministic_but_preserves_semantic_identity_boundaries(): +def test_fingerprint_normalizes_paths_but_preserves_incident_boundaries(): base = _finding(changed_paths=["/tags", "/instance_type", "/tags"]) - reordered = _finding(changed_paths=["/instance_type", "/tags"]) - mask_only_change = _finding( + normalized = _finding(changed_paths=["/instance_type", "/tags"]) + masks_changed = _finding( changed_paths=["/instance_type", "/tags"], sensitive_paths=["/tags"], unknown_paths=["/instance_type"], ) fingerprint = finding_fingerprint(base) - assert fingerprint == finding_fingerprint(reordered) - assert fingerprint == finding_fingerprint(mask_only_change) + assert fingerprint == finding_fingerprint(normalized) + assert fingerprint == finding_fingerprint(masks_changed) assert len(fingerprint) == 64 - assert fingerprint != finding_fingerprint(_finding(address="aws_instance.other")) assert fingerprint != finding_fingerprint(_finding(deposed_key="deadbeef")) assert fingerprint != finding_fingerprint(_finding(actions=["delete", "create"])) @@ -125,15 +124,15 @@ def test_fingerprint_is_deterministic_but_preserves_semantic_identity_boundaries @pytest.mark.asyncio -async def test_lifecycle_create_repeat_resolve_and_reopen(): - engine, Session = await _new_database() - workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 4) +async def test_create_repeat_resolve_and_reopen_lifecycle(): + engine, sessions = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(sessions, 4) finding = _finding() fingerprint = finding_fingerprint(finding) t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) try: - async with Session() as db: + async with sessions() as db: first = await reconcile_evidence_bundle( db, workspace_id=workspace_id, @@ -145,8 +144,8 @@ async def test_lifecycle_create_repeat_resolve_and_reopen(): assert first.created == 1 assert first.resolution_performed is True - async with Session() as db: - second = await reconcile_evidence_bundle( + async with sessions() as db: + repeated = await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[1], @@ -154,10 +153,10 @@ async def test_lifecycle_create_repeat_resolve_and_reopen(): observed_at=t0 + timedelta(hours=1), ) await db.commit() - assert second.observed_existing == 1 + assert repeated.observed_existing == 1 - async with Session() as db: - third = await reconcile_evidence_bundle( + async with sessions() as db: + resolved = await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[2], @@ -165,10 +164,10 @@ async def test_lifecycle_create_repeat_resolve_and_reopen(): observed_at=t0 + timedelta(hours=2), ) await db.commit() - assert third.resolved == 1 + assert resolved.resolved == 1 - async with Session() as db: - fourth = await reconcile_evidence_bundle( + async with sessions() as db: + reopened = await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[3], @@ -176,9 +175,9 @@ async def test_lifecycle_create_repeat_resolve_and_reopen(): observed_at=t0 + timedelta(hours=3), ) await db.commit() - assert fourth.reopened == 1 + assert reopened.reopened == 1 - async with Session() as db: + async with sessions() as db: incident = ( await db.execute( select(FindingIncident).where( @@ -192,28 +191,21 @@ async def test_lifecycle_create_repeat_resolve_and_reopen(): assert incident.reopen_count == 1 assert incident.resolved_at is None assert incident.remediation_branch == f"driftguard/fix-{fingerprint}" - - occurrence_count = await db.scalar( - select(func.count()).select_from(FindingOccurrence) - ) - reconciliation_count = await db.scalar( - select(func.count()).select_from(EvidenceReconciliation) - ) - assert occurrence_count == 3 - assert reconciliation_count == 4 + assert await db.scalar(select(func.count()).select_from(FindingOccurrence)) == 3 + assert await db.scalar(select(func.count()).select_from(EvidenceReconciliation)) == 4 finally: await engine.dispose() @pytest.mark.asyncio async def test_incomplete_plan_cannot_resolve_missing_incident(): - engine, Session = await _new_database() - workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 2) + engine, sessions = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(sessions, 2) finding = _finding() t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) try: - async with Session() as db: + async with sessions() as db: await reconcile_evidence_bundle( db, workspace_id=workspace_id, @@ -223,7 +215,7 @@ async def test_incomplete_plan_cannot_resolve_missing_incident(): ) await db.commit() - async with Session() as db: + async with sessions() as db: result = await reconcile_evidence_bundle( db, workspace_id=workspace_id, @@ -235,7 +227,7 @@ async def test_incomplete_plan_cannot_resolve_missing_incident(): assert result.resolution_performed is False assert result.resolved == 0 - async with Session() as db: + async with sessions() as db: incident = ( await db.execute( select(FindingIncident).where(FindingIncident.workspace_id == workspace_id) @@ -248,42 +240,41 @@ async def test_incomplete_plan_cannot_resolve_missing_incident(): @pytest.mark.asyncio -async def test_clean_scan_replay_is_exactly_once_and_inconsistent_replay_fails_closed(): - engine, Session = await _new_database() - workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 1) - t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) +async def test_scan_replay_is_exactly_once_and_inconsistent_replay_fails_closed(): + engine, sessions = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(sessions, 1) + observed_at = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) try: - async with Session() as db: - first = await reconcile_evidence_bundle( + async with sessions() as db: + await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[0], bundle=_bundle(), - observed_at=t0, + observed_at=observed_at, ) await db.commit() - assert first.already_reconciled is False - async with Session() as db: + async with sessions() as db: replay = await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[0], bundle=_bundle(), - observed_at=t0, + observed_at=observed_at, ) await db.commit() assert replay.already_reconciled is True - async with Session() as db: + async with sessions() as db: with pytest.raises(LifecycleReconcileError, match="Scan replay differs"): await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[0], bundle=_bundle(_finding()), - observed_at=t0 + timedelta(minutes=2), + observed_at=observed_at + timedelta(minutes=1), ) await db.rollback() finally: @@ -291,55 +282,52 @@ async def test_clean_scan_replay_is_exactly_once_and_inconsistent_replay_fails_c @pytest.mark.asyncio -async def test_scan_workspace_mismatch_fails_before_lifecycle_mutation(): - engine, Session = await _new_database() - _, scan_ids = await _seed_workspace_and_scans(Session, 1) +async def test_scan_workspace_mismatch_fails_before_mutation(): + engine, sessions = await _new_database() + _, scan_ids = await _seed_workspace_and_scans(sessions, 1) - async with Session() as db: + async with sessions() as db: org = Organization(name="Other Org", slug="other-lifecycle-org") db.add(org) await db.flush() - workspace_b = Workspace( + workspace = Workspace( org_id=org.id, name="other", slug="other", provider=CloudProvider.AWS, region="us-east-1", ) - db.add(workspace_b) + db.add(workspace) await db.commit() - workspace_b_id = workspace_b.id + wrong_workspace_id = workspace.id try: - async with Session() as db: + async with sessions() as db: with pytest.raises(LifecycleReconcileError, match="does not belong"): await reconcile_evidence_bundle( db, - workspace_id=workspace_b_id, + workspace_id=wrong_workspace_id, scan_id=scan_ids[0], bundle=_bundle(_finding()), observed_at=datetime(2026, 9, 14, 13, 0, tzinfo=UTC), ) await db.rollback() - async with Session() as db: - incident_count = await db.scalar( - select(func.count()).select_from(FindingIncident) - ) - assert incident_count == 0 + async with sessions() as db: + assert await db.scalar(select(func.count()).select_from(FindingIncident)) == 0 finally: await engine.dispose() @pytest.mark.asyncio -async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_state(): - engine, Session = await _new_database() - workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 3) +async def test_out_of_order_scan_cannot_roll_back_newer_lifecycle_state(): + engine, sessions = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(sessions, 3) finding = _finding() t0 = datetime(2026, 9, 14, 13, 0, tzinfo=UTC) try: - async with Session() as db: + async with sessions() as db: await reconcile_evidence_bundle( db, workspace_id=workspace_id, @@ -349,8 +337,8 @@ async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_st ) await db.commit() - async with Session() as db: - resolved = await reconcile_evidence_bundle( + async with sessions() as db: + result = await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[2], @@ -358,9 +346,9 @@ async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_st observed_at=t0 + timedelta(hours=2), ) await db.commit() - assert resolved.resolved == 1 + assert result.resolved == 1 - async with Session() as db: + async with sessions() as db: stale = await reconcile_evidence_bundle( db, workspace_id=workspace_id, @@ -372,7 +360,7 @@ async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_st assert stale.stale_ignored is True assert stale.reopened == 0 - async with Session() as db: + async with sessions() as db: incident = ( await db.execute( select(FindingIncident).where(FindingIncident.workspace_id == workspace_id) @@ -382,14 +370,14 @@ async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_st assert incident.occurrence_count == 1 assert incident.reopen_count == 0 - stale_marker = ( + marker = ( await db.execute( select(EvidenceReconciliation).where( EvidenceReconciliation.scan_id == scan_ids[1] ) ) ).scalar_one() - assert stale_marker.applied_to_lifecycle is False + assert marker.applied_to_lifecycle is False cursor = ( await db.execute( @@ -398,7 +386,7 @@ async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_st ).scalar_one() assert cursor.latest_scan_id == scan_ids[2] - async with Session() as db: + async with sessions() as db: replay = await reconcile_evidence_bundle( db, workspace_id=workspace_id, @@ -415,18 +403,19 @@ async def test_out_of_order_scan_is_audited_but_cannot_mutate_newer_lifecycle_st @pytest.mark.asyncio async def test_timezone_naive_observation_is_rejected(): - engine, Session = await _new_database() - workspace_id, scan_ids = await _seed_workspace_and_scans(Session, 1) + engine, sessions = await _new_database() + workspace_id, scan_ids = await _seed_workspace_and_scans(sessions, 1) + naive_observed_at = datetime(2026, 9, 14, 13, 0, tzinfo=UTC).replace(tzinfo=None) try: - async with Session() as db: + async with sessions() as db: with pytest.raises(LifecycleReconcileError, match="timezone-aware"): await reconcile_evidence_bundle( db, workspace_id=workspace_id, scan_id=scan_ids[0], bundle=_bundle(_finding()), - observed_at=datetime(2026, 9, 14, 13, 0), + observed_at=naive_observed_at, ) await db.rollback() finally: From 847c40052413fb28e5fa2718f669650b020eab9d Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Thu, 17 Sep 2026 17:00:15 +0100 Subject: [PATCH 12/24] feat: establish fail-closed Alembic migration authority - add Alembic 1.20 migration history with legacy baseline and Evidence lifecycle revision - normalize database URLs through one shared function - add strict bootstrap classification for fresh, recognized legacy, versioned, and ambiguous schemas - preserve legacy data during migration and reject unknown unversioned layouts - add migration regression tests including alembic metadata parity checks - document production cutover boundary in ADR 0003 Runtime production scan behavior remains unchanged; API startup cutover is intentionally deferred until migration CI is green. --- alembic.ini | 38 +++++ backend/database.py | 31 ++-- backend/db_url.py | 11 ++ backend/migrations/__init__.py | 1 + backend/migrations/bootstrap.py | 133 ++++++++++++++++++ backend/migrations/env.py | 77 ++++++++++ backend/migrations/script.py.mako | 25 ++++ .../versions/0001_legacy_baseline.py | 20 +++ .../versions/0002_evidence_lifecycle.py | 127 +++++++++++++++++ backend/migrations/versions/__init__.py | 0 backend/tests/test_migrations.py | 131 +++++++++++++++++ docs/adr/0003-database-migration-authority.md | 70 +++++++++ requirements.txt | 1 + 13 files changed, 643 insertions(+), 22 deletions(-) create mode 100644 alembic.ini create mode 100644 backend/db_url.py create mode 100644 backend/migrations/__init__.py create mode 100644 backend/migrations/bootstrap.py create mode 100644 backend/migrations/env.py create mode 100644 backend/migrations/script.py.mako create mode 100644 backend/migrations/versions/0001_legacy_baseline.py create mode 100644 backend/migrations/versions/0002_evidence_lifecycle.py create mode 100644 backend/migrations/versions/__init__.py create mode 100644 backend/tests/test_migrations.py create mode 100644 docs/adr/0003-database-migration-authority.md diff --git a/alembic.ini b/alembic.ini new file mode 100644 index 0000000..a7ec574 --- /dev/null +++ b/alembic.ini @@ -0,0 +1,38 @@ +[alembic] +script_location = backend/migrations +prepend_sys_path = . +path_separator = os + +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/backend/database.py b/backend/database.py index f302e5d..2e12b05 100644 --- a/backend/database.py +++ b/backend/database.py @@ -4,13 +4,8 @@ Async SQLAlchemy 2.0 engine and session management. Works with PostgreSQL (production) and SQLite (local dev/testing). -Production: set DATABASE_URL to a Postgres connection string. -Recommended free provider: Neon.tech (serverless Postgres, no -expiry on free tier, unlike Render's 90-day free Postgres). - - postgresql+asyncpg://user:pass@host/dbname - -Local dev / CI: falls back to SQLite if DATABASE_URL is unset. +Production schema changes are managed by Alembic. ``init_db()`` remains a +local-development/test bootstrap only; it is not a migration mechanism. """ from __future__ import annotations @@ -25,28 +20,20 @@ create_async_engine, ) +from .db_url import normalize_database_url from .models import incidents as incident_models from .models import models as core_models from .models.base import Base -# Register every ORM table with Base.metadata even when this module is used -# outside backend.api.main. create_all() otherwise depends on unrelated import -# order and can silently omit tables in scripts/tests. _REGISTERED_MODEL_MODULES = (core_models, incident_models) -DATABASE_URL = os.getenv( - "DATABASE_URL", - "sqlite+aiosqlite:///./driftguard.db", +DATABASE_URL = normalize_database_url( + os.getenv( + "DATABASE_URL", + "sqlite+aiosqlite:///./driftguard.db", + ) ) -# Neon/Postgres URLs from providers often come as postgres:// — normalize. -if DATABASE_URL.startswith("postgres://"): - DATABASE_URL = DATABASE_URL.replace("postgres://", "postgresql+asyncpg://", 1) -elif DATABASE_URL.startswith("postgresql://") and "+asyncpg" not in DATABASE_URL: - DATABASE_URL = DATABASE_URL.replace("postgresql://", "postgresql+asyncpg://", 1) - -# SQLite needs check_same_thread=False equivalent handled by aiosqlite driver; -# no special connect_args required for asyncpg or aiosqlite. engine = create_async_engine( DATABASE_URL, echo=False, @@ -63,7 +50,7 @@ async def init_db() -> None: - """Create all tables. Call once on startup. Safe to call repeatedly (no-op if tables exist).""" + """Local dev/test bootstrap. Production deployments must run migrations.""" async with engine.begin() as conn: await conn.run_sync(Base.metadata.create_all) diff --git a/backend/db_url.py b/backend/db_url.py new file mode 100644 index 0000000..2230e0b --- /dev/null +++ b/backend/db_url.py @@ -0,0 +1,11 @@ +from __future__ import annotations + + +def normalize_database_url(url: str) -> str: + """Normalize provider-style database URLs for SQLAlchemy async engines.""" + url = url.strip() + if url.startswith("postgres://"): + return url.replace("postgres://", "postgresql+asyncpg://", 1) + if url.startswith("postgresql://") and "+asyncpg" not in url: + return url.replace("postgresql://", "postgresql+asyncpg://", 1) + return url diff --git a/backend/migrations/__init__.py b/backend/migrations/__init__.py new file mode 100644 index 0000000..0351b72 --- /dev/null +++ b/backend/migrations/__init__.py @@ -0,0 +1 @@ +"""Alembic migration package for DriftGuard.""" diff --git a/backend/migrations/bootstrap.py b/backend/migrations/bootstrap.py new file mode 100644 index 0000000..93869fc --- /dev/null +++ b/backend/migrations/bootstrap.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import argparse +import asyncio +import os +from collections.abc import Mapping +from pathlib import Path + +from alembic import command +from alembic.config import Config +from sqlalchemy import inspect +from sqlalchemy.ext.asyncio import create_async_engine + +from backend.db_url import normalize_database_url +from backend.models import incidents as incident_models +from backend.models import models as core_models +from backend.models.base import Base + +_REGISTERED_MODEL_MODULES = (core_models, incident_models) + +LEGACY_REVISION = "0001_legacy_baseline" +HEAD_REVISION = "0002_evidence_lifecycle" + +LEGACY_COLUMNS: Mapping[str, frozenset[str]] = { + "organizations": frozenset({"id", "name", "slug", "plan", "github_org", "stripe_customer_id", "is_active", "max_workspaces", "max_resources", "created_at", "updated_at"}), + "api_keys": frozenset({"id", "org_id", "name", "key_hash", "key_prefix", "is_active", "last_used_at", "expires_at", "created_at", "updated_at"}), + "workspaces": frozenset({"id", "org_id", "name", "slug", "provider", "region", "state_backend", "s3_bucket", "s3_key", "s3_region", "github_repo", "github_branch", "terraform_dir", "github_app_installation_id", "aws_role_arn", "aws_external_id", "scan_interval_minutes", "auto_pr_enabled", "notifications_slack_webhook", "notifications_email", "is_active", "last_scanned_at", "created_at", "updated_at"}), + "drift_scans": frozenset({"id", "workspace_id", "status", "triggered_by", "total_resources_checked", "drift_count", "security_findings_count", "cost_delta_monthly", "posture_score", "started_at", "completed_at", "error_message", "created_at", "updated_at"}), + "drift_findings": frozenset({"id", "workspace_id", "scan_id", "resource_type", "resource_id", "resource_name", "region", "status", "severity", "drift_type", "expected_state", "actual_state", "diff_summary", "security_impact", "compliance_violations", "cost_delta_monthly", "terraform_patch", "github_pr_url", "github_pr_number", "resolved_at", "resolved_by", "created_at", "updated_at"}), +} + +LIFECYCLE_TABLES = frozenset({"finding_incidents", "finding_occurrences", "evidence_cursors", "evidence_reconciliations"}) + + +class MigrationBootstrapError(RuntimeError): + """Raised when an existing schema cannot be identified safely.""" + + +def _alembic_config(database_url: str) -> Config: + repo_root = Path(__file__).resolve().parents[2] + config = Config(str(repo_root / "alembic.ini")) + config.attributes["connection_url"] = normalize_database_url(database_url) + return config + + +async def _schema_snapshot(database_url: str) -> dict[str, frozenset[str]]: + engine = create_async_engine(normalize_database_url(database_url), pool_pre_ping=True) + try: + async with engine.connect() as connection: + def inspect_schema(sync_connection) -> dict[str, frozenset[str]]: + inspector = inspect(sync_connection) + return { + table: frozenset(column["name"] for column in inspector.get_columns(table)) + for table in inspector.get_table_names() + } + + return await connection.run_sync(inspect_schema) + finally: + await engine.dispose() + + +async def _create_current_schema(database_url: str) -> None: + engine = create_async_engine(normalize_database_url(database_url), pool_pre_ping=True) + try: + async with engine.begin() as connection: + await connection.run_sync(Base.metadata.create_all) + finally: + await engine.dispose() + + +def _validate_legacy_schema(snapshot: Mapping[str, frozenset[str]]) -> None: + tables = frozenset(snapshot) + expected = frozenset(LEGACY_COLUMNS) + if tables & LIFECYCLE_TABLES: + raise MigrationBootstrapError( + "Unversioned database already contains Evidence lifecycle tables; refusing to guess migration history." + ) + if tables != expected: + missing = sorted(expected - tables) + extra = sorted(tables - expected) + raise MigrationBootstrapError( + f"Unversioned schema is not the recognized DriftGuard legacy schema (missing_tables={missing}, extra_tables={extra})." + ) + + mismatches: list[str] = [] + for table, expected_columns in LEGACY_COLUMNS.items(): + actual_columns = snapshot[table] + if actual_columns != expected_columns: + missing = sorted(expected_columns - actual_columns) + extra = sorted(actual_columns - expected_columns) + mismatches.append(f"{table}: missing={missing}, extra={extra}") + if mismatches: + raise MigrationBootstrapError( + "Unversioned DriftGuard schema has unexpected columns: " + "; ".join(mismatches) + ) + + +def bootstrap_database(database_url: str) -> str: + """Bring a DriftGuard database under Alembic without guessing its history.""" + database_url = normalize_database_url(database_url) + snapshot = asyncio.run(_schema_snapshot(database_url)) + config = _alembic_config(database_url) + + if "alembic_version" in snapshot: + command.upgrade(config, "head") + return "upgraded-versioned" + if not snapshot: + asyncio.run(_create_current_schema(database_url)) + command.stamp(config, "head") + return "initialized-fresh" + + _validate_legacy_schema(snapshot) + command.stamp(config, LEGACY_REVISION) + command.upgrade(config, "head") + return "upgraded-legacy" + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Safely initialize or upgrade the DriftGuard database schema." + ) + parser.add_argument( + "--database-url", + default=os.getenv("DATABASE_URL", "sqlite+aiosqlite:///./driftguard.db"), + ) + args = parser.parse_args() + result = bootstrap_database(args.database_url) + print(result) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/backend/migrations/env.py b/backend/migrations/env.py new file mode 100644 index 0000000..d729d36 --- /dev/null +++ b/backend/migrations/env.py @@ -0,0 +1,77 @@ +from __future__ import annotations + +import asyncio +import os +from logging.config import fileConfig + +from alembic import context +from sqlalchemy import pool +from sqlalchemy.ext.asyncio import async_engine_from_config + +from backend.db_url import normalize_database_url +from backend.models import incidents as incident_models +from backend.models import models as core_models +from backend.models.base import Base + +config = context.config +if config.config_file_name is not None: + fileConfig(config.config_file_name) + +_REGISTERED_MODEL_MODULES = (core_models, incident_models) +target_metadata = Base.metadata + + +def _database_url() -> str: + explicit = config.attributes.get("connection_url") + if explicit: + return normalize_database_url(str(explicit)) + return normalize_database_url( + os.getenv("DATABASE_URL", "sqlite+aiosqlite:///./driftguard.db") + ) + + +def run_migrations_offline() -> None: + context.configure( + url=_database_url(), + target_metadata=target_metadata, + literal_binds=True, + compare_type=True, + compare_server_default=True, + dialect_opts={"paramstyle": "named"}, + ) + with context.begin_transaction(): + context.run_migrations() + + +def _do_run_migrations(connection) -> None: + context.configure( + connection=connection, + target_metadata=target_metadata, + compare_type=True, + compare_server_default=True, + ) + with context.begin_transaction(): + context.run_migrations() + + +async def _run_async_migrations() -> None: + configuration = config.get_section(config.config_ini_section) or {} + configuration["sqlalchemy.url"] = _database_url() + connectable = async_engine_from_config( + configuration, + prefix="sqlalchemy.", + poolclass=pool.NullPool, + ) + async with connectable.connect() as connection: + await connection.run_sync(_do_run_migrations) + await connectable.dispose() + + +def run_migrations_online() -> None: + asyncio.run(_run_async_migrations()) + + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/backend/migrations/script.py.mako b/backend/migrations/script.py.mako new file mode 100644 index 0000000..cb52045 --- /dev/null +++ b/backend/migrations/script.py.mako @@ -0,0 +1,25 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +revision: str = ${repr(up_revision)} +down_revision: Union[str, Sequence[str], None] = ${repr(down_revision)} +branch_labels: Union[str, Sequence[str], None] = ${repr(branch_labels)} +depends_on: Union[str, Sequence[str], None] = ${repr(depends_on)} + + +def upgrade() -> None: + ${upgrades if upgrades else "pass"} + + +def downgrade() -> None: + ${downgrades if downgrades else "pass"} diff --git a/backend/migrations/versions/0001_legacy_baseline.py b/backend/migrations/versions/0001_legacy_baseline.py new file mode 100644 index 0000000..b0c3b79 --- /dev/null +++ b/backend/migrations/versions/0001_legacy_baseline.py @@ -0,0 +1,20 @@ +"""Mark the schema that predates Alembic. + +Revision ID: 0001_legacy_baseline +Revises: +""" + +from typing import Sequence, Union + +revision: str = "0001_legacy_baseline" +down_revision: Union[str, Sequence[str], None] = None +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + pass + + +def downgrade() -> None: + pass diff --git a/backend/migrations/versions/0002_evidence_lifecycle.py b/backend/migrations/versions/0002_evidence_lifecycle.py new file mode 100644 index 0000000..526a438 --- /dev/null +++ b/backend/migrations/versions/0002_evidence_lifecycle.py @@ -0,0 +1,127 @@ +"""Add Evidence Core incident lifecycle persistence. + +Revision ID: 0002_evidence_lifecycle +Revises: 0001_legacy_baseline +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +revision: str = "0002_evidence_lifecycle" +down_revision: Union[str, Sequence[str], None] = "0001_legacy_baseline" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def _timestamps() -> list[sa.Column]: + return [ + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + ] + + +def upgrade() -> None: + op.create_table( + "finding_incidents", + sa.Column("id", sa.String(length=36), nullable=False), + sa.Column("workspace_id", sa.String(length=36), nullable=False), + sa.Column("fingerprint", sa.String(length=64), nullable=False), + sa.Column("identity_version", sa.String(length=16), nullable=False), + sa.Column("resource_address", sa.String(length=1000), nullable=False), + sa.Column("deposed_key", sa.String(length=255), nullable=True), + sa.Column("resource_type", sa.String(length=255), nullable=False), + sa.Column("provider_name", sa.String(length=500), nullable=True), + sa.Column("actions", sa.JSON(), nullable=False), + sa.Column("changed_paths", sa.JSON(), nullable=False), + sa.Column("status", sa.String(length=20), nullable=False), + sa.Column("first_seen_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("last_seen_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("last_scan_id", sa.String(length=36), nullable=False), + sa.Column("occurrence_count", sa.Integer(), nullable=False), + sa.Column("reopen_count", sa.Integer(), nullable=False), + sa.Column("reopened_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("resolved_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("remediation_branch", sa.String(length=255), nullable=False), + sa.Column("remediation_pr_url", sa.String(length=500), nullable=True), + sa.Column("remediation_pr_number", sa.Integer(), nullable=True), + *_timestamps(), + sa.CheckConstraint("occurrence_count >= 1", name="ck_finding_incidents_occurrence_count"), + sa.CheckConstraint("reopen_count >= 0", name="ck_finding_incidents_reopen_count"), + sa.CheckConstraint("status IN ('open', 'resolved')", name="ck_finding_incidents_status"), + sa.ForeignKeyConstraint(["last_scan_id"], ["drift_scans.id"]), + sa.ForeignKeyConstraint(["workspace_id"], ["workspaces.id"]), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("workspace_id", "fingerprint", name="uq_finding_incidents_workspace_fingerprint"), + ) + op.create_index(op.f("ix_finding_incidents_fingerprint"), "finding_incidents", ["fingerprint"], unique=False) + op.create_index(op.f("ix_finding_incidents_status"), "finding_incidents", ["status"], unique=False) + op.create_index(op.f("ix_finding_incidents_workspace_id"), "finding_incidents", ["workspace_id"], unique=False) + + op.create_table( + "finding_occurrences", + sa.Column("id", sa.String(length=36), nullable=False), + sa.Column("incident_id", sa.String(length=36), nullable=False), + sa.Column("workspace_id", sa.String(length=36), nullable=False), + sa.Column("scan_id", sa.String(length=36), nullable=False), + sa.Column("observed_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("evidence_schema_version", sa.String(length=16), nullable=False), + sa.Column("sensitive_paths", sa.JSON(), nullable=False), + sa.Column("unknown_paths", sa.JSON(), nullable=False), + *_timestamps(), + sa.ForeignKeyConstraint(["incident_id"], ["finding_incidents.id"]), + sa.ForeignKeyConstraint(["scan_id"], ["drift_scans.id"]), + sa.ForeignKeyConstraint(["workspace_id"], ["workspaces.id"]), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("incident_id", "scan_id", name="uq_finding_occurrences_incident_scan"), + ) + op.create_index(op.f("ix_finding_occurrences_incident_id"), "finding_occurrences", ["incident_id"], unique=False) + op.create_index(op.f("ix_finding_occurrences_scan_id"), "finding_occurrences", ["scan_id"], unique=False) + op.create_index(op.f("ix_finding_occurrences_workspace_id"), "finding_occurrences", ["workspace_id"], unique=False) + + op.create_table( + "evidence_cursors", + sa.Column("workspace_id", sa.String(length=36), nullable=False), + sa.Column("latest_observed_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("latest_scan_id", sa.String(length=36), nullable=False), + *_timestamps(), + sa.ForeignKeyConstraint(["latest_scan_id"], ["drift_scans.id"]), + sa.ForeignKeyConstraint(["workspace_id"], ["workspaces.id"]), + sa.PrimaryKeyConstraint("workspace_id"), + ) + + op.create_table( + "evidence_reconciliations", + sa.Column("id", sa.String(length=36), nullable=False), + sa.Column("scan_id", sa.String(length=36), nullable=False), + sa.Column("workspace_id", sa.String(length=36), nullable=False), + sa.Column("observation_set_digest", sa.String(length=64), nullable=False), + sa.Column("finding_count", sa.Integer(), nullable=False), + sa.Column("plan_complete", sa.Boolean(), nullable=True), + sa.Column("observed_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("applied_to_lifecycle", sa.Boolean(), nullable=False), + sa.Column("reconciled_at", sa.DateTime(timezone=True), nullable=False), + *_timestamps(), + sa.CheckConstraint("finding_count >= 0", name="ck_evidence_reconciliations_finding_count"), + sa.ForeignKeyConstraint(["scan_id"], ["drift_scans.id"]), + sa.ForeignKeyConstraint(["workspace_id"], ["workspaces.id"]), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index(op.f("ix_evidence_reconciliations_scan_id"), "evidence_reconciliations", ["scan_id"], unique=True) + op.create_index(op.f("ix_evidence_reconciliations_workspace_id"), "evidence_reconciliations", ["workspace_id"], unique=False) + + +def downgrade() -> None: + op.drop_index(op.f("ix_evidence_reconciliations_workspace_id"), table_name="evidence_reconciliations") + op.drop_index(op.f("ix_evidence_reconciliations_scan_id"), table_name="evidence_reconciliations") + op.drop_table("evidence_reconciliations") + op.drop_table("evidence_cursors") + op.drop_index(op.f("ix_finding_occurrences_workspace_id"), table_name="finding_occurrences") + op.drop_index(op.f("ix_finding_occurrences_scan_id"), table_name="finding_occurrences") + op.drop_index(op.f("ix_finding_occurrences_incident_id"), table_name="finding_occurrences") + op.drop_table("finding_occurrences") + op.drop_index(op.f("ix_finding_incidents_workspace_id"), table_name="finding_incidents") + op.drop_index(op.f("ix_finding_incidents_status"), table_name="finding_incidents") + op.drop_index(op.f("ix_finding_incidents_fingerprint"), table_name="finding_incidents") + op.drop_table("finding_incidents") diff --git a/backend/migrations/versions/__init__.py b/backend/migrations/versions/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/backend/tests/test_migrations.py b/backend/tests/test_migrations.py new file mode 100644 index 0000000..92e6d5a --- /dev/null +++ b/backend/tests/test_migrations.py @@ -0,0 +1,131 @@ +from __future__ import annotations + +import sqlite3 +from pathlib import Path + +import pytest +from alembic import command +from sqlalchemy import create_engine, inspect, text + +from backend.migrations.bootstrap import ( + HEAD_REVISION, + MigrationBootstrapError, + _alembic_config, + bootstrap_database, +) +from backend.models.base import Base + + +def _url(path: Path) -> str: + return f"sqlite+aiosqlite:///{path}" + + +def _sync_url(path: Path) -> str: + return f"sqlite:///{path}" + + +def _current_revision(path: Path) -> str: + engine = create_engine(_sync_url(path)) + try: + with engine.connect() as connection: + return connection.execute( + text("SELECT version_num FROM alembic_version") + ).scalar_one() + finally: + engine.dispose() + + +def _tables(path: Path) -> set[str]: + engine = create_engine(_sync_url(path)) + try: + return set(inspect(engine).get_table_names()) + finally: + engine.dispose() + + +def _create_legacy_schema(path: Path) -> None: + legacy_tables = [ + Base.metadata.tables["organizations"], + Base.metadata.tables["api_keys"], + Base.metadata.tables["workspaces"], + Base.metadata.tables["drift_scans"], + Base.metadata.tables["drift_findings"], + ] + engine = create_engine(_sync_url(path)) + try: + Base.metadata.create_all(engine, tables=legacy_tables) + with engine.begin() as connection: + connection.execute( + Base.metadata.tables["organizations"].insert().values( + id="org-1", + name="Migration Test", + slug="migration-test", + plan="FREE", + is_active=True, + max_workspaces=1, + max_resources=50, + ) + ) + finally: + engine.dispose() + + +def test_fresh_database_bootstrap_is_current_and_idempotent(tmp_path: Path): + db_path = tmp_path / "fresh.db" + assert bootstrap_database(_url(db_path)) == "initialized-fresh" + assert _current_revision(db_path) == HEAD_REVISION + assert { + "finding_incidents", + "finding_occurrences", + "evidence_cursors", + "evidence_reconciliations", + }.issubset(_tables(db_path)) + + assert bootstrap_database(_url(db_path)) == "upgraded-versioned" + assert _current_revision(db_path) == HEAD_REVISION + + +def test_legacy_database_upgrades_without_destroying_existing_data(tmp_path: Path): + db_path = tmp_path / "legacy.db" + _create_legacy_schema(db_path) + + assert bootstrap_database(_url(db_path)) == "upgraded-legacy" + assert _current_revision(db_path) == HEAD_REVISION + + engine = create_engine(_sync_url(db_path)) + try: + with engine.connect() as connection: + assert ( + connection.execute( + text("SELECT name FROM organizations WHERE id = 'org-1'") + ).scalar_one() + == "Migration Test" + ) + finally: + engine.dispose() + + +def test_partial_unversioned_schema_fails_closed_without_stamp(tmp_path: Path): + db_path = tmp_path / "partial.db" + with sqlite3.connect(db_path) as connection: + connection.execute( + "CREATE TABLE organizations (id VARCHAR(36) PRIMARY KEY, name VARCHAR(255))" + ) + + with pytest.raises(MigrationBootstrapError, match="not the recognized"): + bootstrap_database(_url(db_path)) + + assert "alembic_version" not in _tables(db_path) + + +def test_alembic_metadata_check_passes_after_fresh_bootstrap(tmp_path: Path): + db_path = tmp_path / "check.db" + bootstrap_database(_url(db_path)) + command.check(_alembic_config(_url(db_path))) + + +def test_alembic_metadata_check_passes_after_legacy_upgrade(tmp_path: Path): + db_path = tmp_path / "legacy-check.db" + _create_legacy_schema(db_path) + bootstrap_database(_url(db_path)) + command.check(_alembic_config(_url(db_path))) diff --git a/docs/adr/0003-database-migration-authority.md b/docs/adr/0003-database-migration-authority.md new file mode 100644 index 0000000..f2dfe12 --- /dev/null +++ b/docs/adr/0003-database-migration-authority.md @@ -0,0 +1,70 @@ +# ADR 0003: Make Alembic the schema authority + +Status: Accepted for Evidence Core branch; production cutover remains gated. + +## Context + +DriftGuard historically called `Base.metadata.create_all()` at API startup. That is safe for creating missing tables in local development, but it is not a schema migration system: it does not alter existing tables, record migration history, or provide a deterministic upgrade path for deployed databases. + +Evidence Core introduces persistent lifecycle tables. Depending on startup `create_all()` would make deployment order determine schema correctness and would make rollback/audit behavior impossible to reason about. + +## Decision + +Alembic is the authoritative production schema history. + +Two revisions establish the transition: + +- `0001_legacy_baseline` describes the schema that existed before Alembic. +- `0002_evidence_lifecycle` adds Evidence Core incident lifecycle persistence. + +The bootstrap command classifies the database before taking action: + +1. Empty database: create the current ORM schema, then `alembic stamp head`. +2. Recognized unversioned legacy DriftGuard database: require the exact five legacy tables and exact legacy column sets, stamp `0001_legacy_baseline`, then upgrade to head. +3. Versioned database: run `alembic upgrade head`. +4. Any partial, extra, or ambiguous unversioned schema: abort without stamping. + +The classifier is intentionally strict. A failed deployment is preferable to writing a false migration history onto an unknown database. + +## Deployment boundary + +The existing Render Blueprint uses a free web service. Render's pre-deploy migration hook is not available on that tier. Therefore this ADR does **not** wire migrations into the free Render startup path and does not make the new lifecycle tables a production dependency yet. + +Production cutover requires a deployment target that can run one migration command before the API version depending on that schema becomes active, or an equivalent explicitly controlled release step. + +## Invariants + +- No migration may infer that a partial schema is safe. +- Existing legacy rows must survive bootstrap unchanged. +- Re-running bootstrap at head is idempotent. +- ORM metadata and Alembic head must have zero pending schema operations under `alembic check`. +- `create_all()` may remain for local tests/development, but it is not the production migration authority. +- Evidence Core production code must not depend on a migration until the deployment path can execute that migration deterministically. + +## Rejected alternatives + +### Continue using `create_all()` + +Rejected because it cannot evolve existing schemas and records no history. + +### Stamp every unversioned database as legacy + +Rejected because a malformed or partially upgraded database would be declared valid without evidence. + +### Run migrations in FastAPI startup + +Rejected because concurrent application instances can race schema changes and because application availability would be coupled to migration execution. + +### Add Render `preDeployCommand` immediately + +Rejected for the current free service because the deployment tier does not provide that capability. + +## Verification gate + +Before runtime cutover, CI must prove: + +- fresh bootstrap reaches Alembic head; +- recognized legacy bootstrap preserves existing data; +- a second bootstrap is a no-op upgrade; +- ambiguous schemas fail without creating `alembic_version`; +- `alembic check` passes after both fresh and legacy paths. diff --git a/requirements.txt b/requirements.txt index 0706535..c4b1288 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,7 @@ fastapi==0.141.1 uvicorn[standard]==0.52.4 sqlalchemy[asyncio]==2.0.52 +alembic==1.20.0 asyncpg==0.31.0 aiosqlite==0.22.1 boto3==1.43.86 From 6f20780682c3e7e120bc3a3b1ecb63d3a8d965f8 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Thu, 17 Sep 2026 17:02:03 +0100 Subject: [PATCH 13/24] fix: satisfy migration revision lint without changing schema behavior --- backend/migrations/versions/0001_legacy_baseline.py | 8 ++++---- backend/migrations/versions/0002_evidence_lifecycle.py | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/backend/migrations/versions/0001_legacy_baseline.py b/backend/migrations/versions/0001_legacy_baseline.py index b0c3b79..e2f1003 100644 --- a/backend/migrations/versions/0001_legacy_baseline.py +++ b/backend/migrations/versions/0001_legacy_baseline.py @@ -4,12 +4,12 @@ Revises: """ -from typing import Sequence, Union +from collections.abc import Sequence revision: str = "0001_legacy_baseline" -down_revision: Union[str, Sequence[str], None] = None -branch_labels: Union[str, Sequence[str], None] = None -depends_on: Union[str, Sequence[str], None] = None +down_revision: str | Sequence[str] | None = None +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None def upgrade() -> None: diff --git a/backend/migrations/versions/0002_evidence_lifecycle.py b/backend/migrations/versions/0002_evidence_lifecycle.py index 526a438..22184fd 100644 --- a/backend/migrations/versions/0002_evidence_lifecycle.py +++ b/backend/migrations/versions/0002_evidence_lifecycle.py @@ -4,15 +4,15 @@ Revises: 0001_legacy_baseline """ -from typing import Sequence, Union +from collections.abc import Sequence -from alembic import op import sqlalchemy as sa +from alembic import op revision: str = "0002_evidence_lifecycle" -down_revision: Union[str, Sequence[str], None] = "0001_legacy_baseline" -branch_labels: Union[str, Sequence[str], None] = None -depends_on: Union[str, Sequence[str], None] = None +down_revision: str | Sequence[str] | None = "0001_legacy_baseline" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None def _timestamps() -> list[sa.Column]: From a781e958351f09c51e1e7589ae939ce17835df75 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Thu, 17 Sep 2026 17:11:18 +0100 Subject: [PATCH 14/24] feat: add redacted provider-native evidence ingestion - persist sanitized submission provenance and source-execution idempotency - add migration 0003 for evidence submission records - reject conflicting retries and preserve one scan per submission key - expose tenant-scoped evidence ingestion behind an explicit feature flag - reject raw/unknown plan change-value fields at validation boundary - reconcile lifecycle atomically with submission persistence - add HTTP regressions for disabled mode, tenant isolation, replay safety, conflict handling, and redaction - document ingestion trust boundary in ADR 0004 Legacy collector-based scanning remains unchanged and provider-native ingestion is disabled by default. --- .env.example | 3 + backend/api/evidence.py | 111 +++++++++ backend/api/main.py | 2 + backend/evidence/ingest.py | 153 ++++++++++++ backend/migrations/bootstrap.py | 10 +- .../versions/0003_evidence_submissions.py | 53 +++++ backend/models/incidents.py | 87 ++++--- backend/tests/test_evidence_ingest.py | 218 ++++++++++++++++++ docs/adr/0004-evidence-ingestion-boundary.md | 88 +++++++ 9 files changed, 693 insertions(+), 32 deletions(-) create mode 100644 backend/api/evidence.py create mode 100644 backend/evidence/ingest.py create mode 100644 backend/migrations/versions/0003_evidence_submissions.py create mode 100644 backend/tests/test_evidence_ingest.py create mode 100644 docs/adr/0004-evidence-ingestion-boundary.md diff --git a/.env.example b/.env.example index eca6048..23ce005 100644 --- a/.env.example +++ b/.env.example @@ -12,4 +12,7 @@ GITHUB_APP_PRIVATE_KEY= # Only needed for the multi-tenant SaaS flow — self-hosted single-account # deployments using the ambient credential chain can leave this unset. DRIFTGUARD_AWS_ACCOUNT_ID= +# Provider-native Evidence Bundle ingestion remains opt-in until the deployed +# database is explicitly migrated to the Evidence schema. +DRIFTGUARD_EVIDENCE_INGEST_ENABLED=false ALLOWED_ORIGINS=http://localhost:3000 diff --git a/backend/api/evidence.py b/backend/api/evidence.py new file mode 100644 index 0000000..8a4fe4c --- /dev/null +++ b/backend/api/evidence.py @@ -0,0 +1,111 @@ +from __future__ import annotations + +import os + +from fastapi import APIRouter, Depends, HTTPException +from pydantic import BaseModel, ConfigDict, Field +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from ..core.auth import verify_api_key +from ..database import get_db +from ..evidence.ingest import EvidenceIngestConflictError, ingest_evidence_bundle +from ..evidence.lifecycle import LifecycleReconcileError +from ..evidence.models import EvidenceBundle +from ..models.models import Organization, Workspace + +router = APIRouter(tags=["evidence"]) + + +class EvidenceIngestRequest(BaseModel): + model_config = ConfigDict(extra="forbid") + + submission_id: str = Field(min_length=1, max_length=255) + bundle: EvidenceBundle + + +class EvidenceLifecycleResponse(BaseModel): + created: int + observed_existing: int + reopened: int + resolved: int + already_reconciled: bool + stale_ignored: bool + resolution_performed: bool + + +class EvidenceIngestResponse(BaseModel): + workspace_id: str + scan_id: str + submission_id: str + bundle_digest: str + replayed: bool + finding_count: int + skipped_nonmanaged: int + lifecycle: EvidenceLifecycleResponse + + +def evidence_ingest_enabled() -> bool: + return os.getenv("DRIFTGUARD_EVIDENCE_INGEST_ENABLED", "").strip().lower() in { + "1", + "true", + "yes", + "on", + } + + +@router.post( + "/workspaces/{workspace_id}/evidence", + response_model=EvidenceIngestResponse, +) +async def ingest_workspace_evidence( + workspace_id: str, + body: EvidenceIngestRequest, + org: Organization = Depends(verify_api_key), + db: AsyncSession = Depends(get_db), +): + if not evidence_ingest_enabled(): + raise HTTPException( + status_code=503, + detail="Provider-native evidence ingestion is disabled.", + ) + + workspace_result = await db.execute( + select(Workspace.id).where( + Workspace.id == workspace_id, + Workspace.org_id == org.id, + ) + ) + if workspace_result.scalar_one_or_none() is None: + raise HTTPException(status_code=404, detail="Workspace not found.") + + try: + result = await ingest_evidence_bundle( + db, + workspace_id=workspace_id, + submission_id=body.submission_id, + bundle=body.bundle, + ) + except EvidenceIngestConflictError as exc: + raise HTTPException(status_code=409, detail=str(exc)) from exc + except (LifecycleReconcileError, ValueError) as exc: + raise HTTPException(status_code=422, detail=str(exc)) from exc + + return EvidenceIngestResponse( + workspace_id=workspace_id, + scan_id=result.scan_id, + submission_id=result.submission_id, + bundle_digest=result.bundle_digest, + replayed=result.replayed, + finding_count=body.bundle.finding_count, + skipped_nonmanaged=body.bundle.skipped_nonmanaged, + lifecycle=EvidenceLifecycleResponse( + created=result.lifecycle.created, + observed_existing=result.lifecycle.observed_existing, + reopened=result.lifecycle.reopened, + resolved=result.lifecycle.resolved, + already_reconciled=result.lifecycle.already_reconciled, + stale_ignored=result.lifecycle.stale_ignored, + resolution_performed=result.lifecycle.resolution_performed, + ), + ) diff --git a/backend/api/main.py b/backend/api/main.py index d0b3897..2e77109 100644 --- a/backend/api/main.py +++ b/backend/api/main.py @@ -48,6 +48,7 @@ StateBackend, Workspace, ) +from .evidence import router as evidence_router log = structlog.get_logger(__name__) @@ -156,6 +157,7 @@ def create_app() -> FastAPI: ) register_routes(app) + app.include_router(evidence_router) return app diff --git a/backend/evidence/ingest.py b/backend/evidence/ingest.py new file mode 100644 index 0000000..9a6d111 --- /dev/null +++ b/backend/evidence/ingest.py @@ -0,0 +1,153 @@ +from __future__ import annotations + +import hashlib +import json +from dataclasses import dataclass +from datetime import UTC, datetime + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from ..models.incidents import EvidenceSubmission +from ..models.models import DriftScan, ScanStatus, Workspace +from .lifecycle import LifecycleResult, reconcile_evidence_bundle +from .models import EvidenceBundle + + +class EvidenceIngestConflictError(ValueError): + """Raised when one idempotency key is reused for different evidence.""" + + +@dataclass(frozen=True, slots=True) +class EvidenceIngestResult: + scan_id: str + submission_id: str + bundle_digest: str + replayed: bool + lifecycle: LifecycleResult + + +def evidence_bundle_digest(bundle: EvidenceBundle) -> str: + """Canonical SHA-256 over the already-redacted Evidence Bundle.""" + payload = bundle.model_dump(mode="json") + canonical = json.dumps( + payload, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf-8") + return hashlib.sha256(canonical).hexdigest() + + +def _as_utc(value: datetime) -> datetime: + if value.tzinfo is None or value.utcoffset() is None: + return value.replace(tzinfo=UTC) + return value.astimezone(UTC) + + +async def ingest_evidence_bundle( + db: AsyncSession, + *, + workspace_id: str, + submission_id: str, + bundle: EvidenceBundle, + received_at: datetime | None = None, +) -> EvidenceIngestResult: + """Persist redacted provenance and reconcile lifecycle atomically. + + The surrounding transaction belongs to the caller. A workspace row lock + serializes same-workspace submissions on PostgreSQL. Idempotency is scoped + to ``(workspace_id, submission_id)``: an exact replay is safe, while reuse + of the same key for different evidence fails closed. + """ + if not submission_id or not submission_id.strip(): + raise ValueError("submission_id must contain a non-whitespace character.") + if len(submission_id) > 255: + raise ValueError("submission_id must not exceed 255 characters.") + + received_at = (received_at or datetime.now(UTC)).astimezone(UTC) + digest = evidence_bundle_digest(bundle) + + workspace_result = await db.execute( + select(Workspace.id).where(Workspace.id == workspace_id).with_for_update() + ) + if workspace_result.scalar_one_or_none() is None: + raise ValueError(f"Workspace {workspace_id} does not exist.") + + existing_result = await db.execute( + select(EvidenceSubmission).where( + EvidenceSubmission.workspace_id == workspace_id, + EvidenceSubmission.submission_id == submission_id, + ) + ) + existing = existing_result.scalar_one_or_none() + if existing is not None: + if existing.bundle_digest != digest: + raise EvidenceIngestConflictError( + "submission_id was already used with different evidence." + ) + lifecycle = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=existing.scan_id, + bundle=bundle, + observed_at=_as_utc(existing.received_at), + ) + return EvidenceIngestResult( + scan_id=existing.scan_id, + submission_id=submission_id, + bundle_digest=digest, + replayed=True, + lifecycle=lifecycle, + ) + + scan = DriftScan( + workspace_id=workspace_id, + status=ScanStatus.RUNNING, + triggered_by="evidence", + total_resources_checked=0, + drift_count=bundle.finding_count, + started_at=received_at, + ) + db.add(scan) + await db.flush() + + submission = EvidenceSubmission( + workspace_id=workspace_id, + scan_id=scan.id, + submission_id=submission_id, + bundle_digest=digest, + schema_version=bundle.schema_version, + iac_engine=bundle.iac_engine, + iac_engine_version=bundle.iac_engine_version, + source_format_version=bundle.source_format_version, + plan_timestamp=bundle.plan_timestamp, + plan_applyable=bundle.plan_applyable, + plan_complete=bundle.plan_complete, + redaction_policy=bundle.redaction_policy, + finding_count=bundle.finding_count, + skipped_nonmanaged=bundle.skipped_nonmanaged, + received_at=received_at, + ) + db.add(submission) + await db.flush() + + lifecycle = await reconcile_evidence_bundle( + db, + workspace_id=workspace_id, + scan_id=scan.id, + bundle=bundle, + observed_at=received_at, + ) + + scan.status = ScanStatus.COMPLETED + scan.completed_at = datetime.now(UTC) + await db.flush() + + return EvidenceIngestResult( + scan_id=scan.id, + submission_id=submission_id, + bundle_digest=digest, + replayed=False, + lifecycle=lifecycle, + ) diff --git a/backend/migrations/bootstrap.py b/backend/migrations/bootstrap.py index 93869fc..d58bdfe 100644 --- a/backend/migrations/bootstrap.py +++ b/backend/migrations/bootstrap.py @@ -19,7 +19,7 @@ _REGISTERED_MODEL_MODULES = (core_models, incident_models) LEGACY_REVISION = "0001_legacy_baseline" -HEAD_REVISION = "0002_evidence_lifecycle" +HEAD_REVISION = "0003_evidence_submissions" LEGACY_COLUMNS: Mapping[str, frozenset[str]] = { "organizations": frozenset({"id", "name", "slug", "plan", "github_org", "stripe_customer_id", "is_active", "max_workspaces", "max_resources", "created_at", "updated_at"}), @@ -29,7 +29,13 @@ "drift_findings": frozenset({"id", "workspace_id", "scan_id", "resource_type", "resource_id", "resource_name", "region", "status", "severity", "drift_type", "expected_state", "actual_state", "diff_summary", "security_impact", "compliance_violations", "cost_delta_monthly", "terraform_patch", "github_pr_url", "github_pr_number", "resolved_at", "resolved_by", "created_at", "updated_at"}), } -LIFECYCLE_TABLES = frozenset({"finding_incidents", "finding_occurrences", "evidence_cursors", "evidence_reconciliations"}) +LIFECYCLE_TABLES = frozenset({ + "finding_incidents", + "finding_occurrences", + "evidence_cursors", + "evidence_reconciliations", + "evidence_submissions", +}) class MigrationBootstrapError(RuntimeError): diff --git a/backend/migrations/versions/0003_evidence_submissions.py b/backend/migrations/versions/0003_evidence_submissions.py new file mode 100644 index 0000000..eb9fffb --- /dev/null +++ b/backend/migrations/versions/0003_evidence_submissions.py @@ -0,0 +1,53 @@ +"""Add redacted provider-native evidence submission provenance. + +Revision ID: 0003_evidence_submissions +Revises: 0002_evidence_lifecycle +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +revision: str = "0003_evidence_submissions" +down_revision: str | Sequence[str] | None = "0002_evidence_lifecycle" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.create_table( + "evidence_submissions", + sa.Column("id", sa.String(length=36), nullable=False), + sa.Column("workspace_id", sa.String(length=36), nullable=False), + sa.Column("scan_id", sa.String(length=36), nullable=False), + sa.Column("submission_id", sa.String(length=255), nullable=False), + sa.Column("bundle_digest", sa.String(length=64), nullable=False), + sa.Column("schema_version", sa.String(length=16), nullable=False), + sa.Column("iac_engine", sa.String(length=20), nullable=False), + sa.Column("iac_engine_version", sa.String(length=64), nullable=True), + sa.Column("source_format_version", sa.String(length=32), nullable=False), + sa.Column("plan_timestamp", sa.String(length=64), nullable=True), + sa.Column("plan_applyable", sa.Boolean(), nullable=True), + sa.Column("plan_complete", sa.Boolean(), nullable=True), + sa.Column("redaction_policy", sa.String(length=64), nullable=False), + sa.Column("finding_count", sa.Integer(), nullable=False), + sa.Column("skipped_nonmanaged", sa.Integer(), nullable=False), + sa.Column("received_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("created_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.Column("updated_at", sa.DateTime(timezone=True), server_default=sa.func.now(), nullable=False), + sa.CheckConstraint("finding_count >= 0", name="ck_evidence_submissions_finding_count"), + sa.CheckConstraint("skipped_nonmanaged >= 0", name="ck_evidence_submissions_skipped_nonmanaged"), + sa.ForeignKeyConstraint(["scan_id"], ["drift_scans.id"]), + sa.ForeignKeyConstraint(["workspace_id"], ["workspaces.id"]), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("workspace_id", "submission_id", name="uq_evidence_submissions_workspace_submission"), + ) + op.create_index(op.f("ix_evidence_submissions_scan_id"), "evidence_submissions", ["scan_id"], unique=True) + op.create_index(op.f("ix_evidence_submissions_workspace_id"), "evidence_submissions", ["workspace_id"], unique=False) + + +def downgrade() -> None: + op.drop_index(op.f("ix_evidence_submissions_workspace_id"), table_name="evidence_submissions") + op.drop_index(op.f("ix_evidence_submissions_scan_id"), table_name="evidence_submissions") + op.drop_table("evidence_submissions") diff --git a/backend/models/incidents.py b/backend/models/incidents.py index 4c9e263..7212306 100644 --- a/backend/models/incidents.py +++ b/backend/models/incidents.py @@ -2,16 +2,7 @@ from datetime import datetime -from sqlalchemy import ( - JSON, - Boolean, - CheckConstraint, - DateTime, - ForeignKey, - Integer, - String, - UniqueConstraint, -) +from sqlalchemy import Boolean, CheckConstraint, DateTime, ForeignKey, Integer, JSON, String, UniqueConstraint from sqlalchemy.orm import Mapped, mapped_column from .base import Base, TimestampMixin, generate_id @@ -64,15 +55,13 @@ class FindingIncident(Base, TimestampMixin): reopened_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) resolved_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) - # Stable future GitHub idempotency key. The evidence path does not open PRs - # yet, but every recurrence of the same incident receives the same branch. remediation_branch: Mapped[str] = mapped_column(String(255), nullable=False) remediation_pr_url: Mapped[str | None] = mapped_column(String(500)) remediation_pr_number: Mapped[int | None] = mapped_column(Integer) class FindingOccurrence(Base, TimestampMixin): - """Redacted audit row for an incident in a lifecycle-applied scan.""" + """Redacted audit record that an incident was observed in a specific scan.""" __tablename__ = "finding_occurrences" __table_args__ = ( @@ -99,22 +88,6 @@ class FindingOccurrence(Base, TimestampMixin): unknown_paths: Mapped[list[str]] = mapped_column(JSON, nullable=False) -class EvidenceCursor(Base, TimestampMixin): - """Monotonic lifecycle watermark for one workspace.""" - - __tablename__ = "evidence_cursors" - - workspace_id: Mapped[str] = mapped_column( - ForeignKey("workspaces.id"), primary_key=True - ) - latest_observed_at: Mapped[datetime] = mapped_column( - DateTime(timezone=True), nullable=False - ) - latest_scan_id: Mapped[str] = mapped_column( - ForeignKey("drift_scans.id"), nullable=False - ) - - class EvidenceReconciliation(Base, TimestampMixin): """Exactly-once marker for lifecycle reconciliation of one scan.""" @@ -137,5 +110,59 @@ class EvidenceReconciliation(Base, TimestampMixin): finding_count: Mapped[int] = mapped_column(Integer, nullable=False) plan_complete: Mapped[bool | None] = mapped_column(Boolean) observed_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) - applied_to_lifecycle: Mapped[bool] = mapped_column(Boolean, nullable=False) + applied_to_lifecycle: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) reconciled_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + + +class EvidenceCursor(Base, TimestampMixin): + """Monotonic per-workspace lifecycle observation cursor.""" + + __tablename__ = "evidence_cursors" + + workspace_id: Mapped[str] = mapped_column( + ForeignKey("workspaces.id"), primary_key=True + ) + latest_observed_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) + latest_scan_id: Mapped[str] = mapped_column(ForeignKey("drift_scans.id"), nullable=False) + + +class EvidenceSubmission(Base, TimestampMixin): + """Redacted provider-native submission provenance and idempotency record.""" + + __tablename__ = "evidence_submissions" + __table_args__ = ( + UniqueConstraint( + "workspace_id", + "submission_id", + name="uq_evidence_submissions_workspace_submission", + ), + CheckConstraint( + "finding_count >= 0", + name="ck_evidence_submissions_finding_count", + ), + CheckConstraint( + "skipped_nonmanaged >= 0", + name="ck_evidence_submissions_skipped_nonmanaged", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True, default=generate_id) + workspace_id: Mapped[str] = mapped_column( + ForeignKey("workspaces.id"), nullable=False, index=True + ) + scan_id: Mapped[str] = mapped_column( + ForeignKey("drift_scans.id"), nullable=False, unique=True, index=True + ) + submission_id: Mapped[str] = mapped_column(String(255), nullable=False) + bundle_digest: Mapped[str] = mapped_column(String(64), nullable=False) + schema_version: Mapped[str] = mapped_column(String(16), nullable=False) + iac_engine: Mapped[str] = mapped_column(String(20), nullable=False) + iac_engine_version: Mapped[str | None] = mapped_column(String(64)) + source_format_version: Mapped[str] = mapped_column(String(32), nullable=False) + plan_timestamp: Mapped[str | None] = mapped_column(String(64)) + plan_applyable: Mapped[bool | None] = mapped_column(Boolean) + plan_complete: Mapped[bool | None] = mapped_column(Boolean) + redaction_policy: Mapped[str] = mapped_column(String(64), nullable=False) + finding_count: Mapped[int] = mapped_column(Integer, nullable=False) + skipped_nonmanaged: Mapped[int] = mapped_column(Integer, nullable=False) + received_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), nullable=False) diff --git a/backend/tests/test_evidence_ingest.py b/backend/tests/test_evidence_ingest.py new file mode 100644 index 0000000..a98fd7a --- /dev/null +++ b/backend/tests/test_evidence_ingest.py @@ -0,0 +1,218 @@ +from __future__ import annotations + +import httpx +import pytest +from sqlalchemy import func, select +from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine +from sqlalchemy.pool import StaticPool + +from backend.api.main import create_app +from backend.core.auth import verify_api_key +from backend.database import get_db +from backend.models.base import Base +from backend.models.incidents import EvidenceSubmission, FindingIncident, FindingOccurrence +from backend.models.models import CloudProvider, DriftScan, Organization, Workspace + + +def _request_payload() -> dict: + return { + "submission_id": "gha:run-123:attempt-1", + "bundle": { + "schema_version": "1.0", + "iac_engine": "terraform", + "iac_engine_version": "1.16.2", + "source_format_version": "1.2", + "plan_timestamp": "2026-09-17T16:00:00Z", + "plan_applyable": True, + "plan_complete": True, + "redaction_policy": "omit_change_values", + "findings": [ + { + "source": "resource_drift", + "resource_address": 'module.web.aws_instance.app["blue"]', + "previous_resource_address": None, + "module_address": "module.web", + "deposed_key": None, + "resource_type": "aws_instance", + "resource_name": "app", + "resource_index": "blue", + "provider_name": "registry.terraform.io/hashicorp/aws", + "actions": ["update"], + "changed_paths": ["/instance_type"], + "sensitive_paths": [], + "unknown_paths": [], + } + ], + "skipped_nonmanaged": 0, + }, + } + + +@pytest.mark.asyncio +async def test_evidence_ingest_is_disabled_by_default_and_rejects_raw_change_values(monkeypatch): + engine = create_async_engine( + "sqlite+aiosqlite:///:memory:", + poolclass=StaticPool, + ) + sessions = async_sessionmaker(engine, expire_on_commit=False) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + + async with sessions() as db: + org = Organization(name="Evidence Org", slug="evidence-org") + db.add(org) + await db.flush() + workspace = Workspace( + org_id=org.id, + name="prod", + slug="prod", + provider=CloudProvider.AWS, + region="us-east-1", + ) + db.add(workspace) + await db.commit() + + app = create_app() + + async def override_db(): + async with sessions() as db: + try: + yield db + await db.commit() + except Exception: + await db.rollback() + raise + + async def authenticate(): + return org + + app.dependency_overrides[get_db] = override_db + app.dependency_overrides[verify_api_key] = authenticate + transport = httpx.ASGITransport(app=app) + + try: + monkeypatch.delenv("DRIFTGUARD_EVIDENCE_INGEST_ENABLED", raising=False) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + disabled = await client.post( + f"/workspaces/{workspace.id}/evidence", + json=_request_payload(), + ) + assert disabled.status_code == 503 + + monkeypatch.setenv("DRIFTGUARD_EVIDENCE_INGEST_ENABLED", "true") + raw_payload = _request_payload() + raw_payload["bundle"]["findings"][0]["before"] = "DO_NOT_STORE_ME" + rejected = await client.post( + f"/workspaces/{workspace.id}/evidence", + json=raw_payload, + ) + assert rejected.status_code == 422 + + async with sessions() as db: + assert await db.scalar(select(func.count()).select_from(EvidenceSubmission)) == 0 + finally: + app.dependency_overrides.clear() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_evidence_ingest_is_tenant_scoped_replay_safe_and_persists_only_redacted_provenance(monkeypatch): + monkeypatch.setenv("DRIFTGUARD_EVIDENCE_INGEST_ENABLED", "1") + engine = create_async_engine( + "sqlite+aiosqlite:///:memory:", + poolclass=StaticPool, + ) + sessions = async_sessionmaker(engine, expire_on_commit=False) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + + async with sessions() as db: + org_a = Organization(name="Org A", slug="evidence-org-a") + org_b = Organization(name="Org B", slug="evidence-org-b") + db.add_all([org_a, org_b]) + await db.flush() + workspace = Workspace( + org_id=org_a.id, + name="prod", + slug="prod", + provider=CloudProvider.AWS, + region="us-east-1", + ) + db.add(workspace) + await db.commit() + + app = create_app() + + async def override_db(): + async with sessions() as db: + try: + yield db + await db.commit() + except Exception: + await db.rollback() + raise + + async def authenticate_a(): + return org_a + + async def authenticate_b(): + return org_b + + app.dependency_overrides[get_db] = override_db + transport = httpx.ASGITransport(app=app) + + try: + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + app.dependency_overrides[verify_api_key] = authenticate_a + first = await client.post( + f"/workspaces/{workspace.id}/evidence", + json=_request_payload(), + ) + assert first.status_code == 200 + first_payload = first.json() + assert first_payload["replayed"] is False + assert first_payload["lifecycle"]["created"] == 1 + scan_id = first_payload["scan_id"] + + replay = await client.post( + f"/workspaces/{workspace.id}/evidence", + json=_request_payload(), + ) + assert replay.status_code == 200 + assert replay.json()["replayed"] is True + assert replay.json()["scan_id"] == scan_id + assert replay.json()["lifecycle"]["already_reconciled"] is True + + changed = _request_payload() + changed["bundle"]["findings"][0]["changed_paths"] = ["/ami"] + conflict = await client.post( + f"/workspaces/{workspace.id}/evidence", + json=changed, + ) + assert conflict.status_code == 409 + + app.dependency_overrides[verify_api_key] = authenticate_b + cross_tenant = await client.post( + f"/workspaces/{workspace.id}/evidence", + json={**_request_payload(), "submission_id": "other-org-attempt"}, + ) + assert cross_tenant.status_code == 404 + + async with sessions() as db: + assert await db.scalar(select(func.count()).select_from(DriftScan)) == 1 + assert await db.scalar(select(func.count()).select_from(EvidenceSubmission)) == 1 + assert await db.scalar(select(func.count()).select_from(FindingIncident)) == 1 + assert await db.scalar(select(func.count()).select_from(FindingOccurrence)) == 1 + + submission = (await db.execute(select(EvidenceSubmission))).scalar_one() + assert submission.submission_id == "gha:run-123:attempt-1" + assert submission.iac_engine == "terraform" + assert submission.finding_count == 1 + assert submission.plan_complete is True + + incident = (await db.execute(select(FindingIncident))).scalar_one() + assert incident.resource_address == 'module.web.aws_instance.app["blue"]' + assert incident.changed_paths == ["/instance_type"] + finally: + app.dependency_overrides.clear() + await engine.dispose() diff --git a/docs/adr/0004-evidence-ingestion-boundary.md b/docs/adr/0004-evidence-ingestion-boundary.md new file mode 100644 index 0000000..81c259b --- /dev/null +++ b/docs/adr/0004-evidence-ingestion-boundary.md @@ -0,0 +1,88 @@ +# ADR 0004: Provider-native evidence ingestion boundary + +Status: Accepted on Evidence Core branch; disabled by default. + +## Context + +Evidence Core can now derive a redacted `EvidenceBundle` locally from Terraform/OpenTofu plan JSON and reconcile deterministic incident lifecycle state. The next boundary is transporting that sanitized evidence into DriftGuard without turning the service into a raw plan/state custodian or creating duplicate scans when CI retries a request. + +Raw Terraform plan/state JSON is intentionally excluded from this API. Provider plan JSON may contain plaintext sensitive values even when Terraform marks them sensitive. + +## Decision + +Add an opt-in endpoint: + +`POST /workspaces/{workspace_id}/evidence` + +The request contains only: + +- a caller-generated `submission_id` identifying one source execution/attempt; +- an `EvidenceBundle` whose Pydantic contract forbids unknown fields and contains no `before`/`after` values. + +The endpoint is disabled unless `DRIFTGUARD_EVIDENCE_INGEST_ENABLED` is explicitly true. + +### Idempotency + +Every accepted submission persists a redacted provenance row keyed uniquely by `(workspace_id, submission_id)` and stores a canonical SHA-256 digest of the sanitized bundle. + +- same submission ID + identical bundle: replay the existing scan safely; +- same submission ID + different bundle: return conflict and perform no lifecycle mutation. + +A workspace row lock serializes same-workspace ingestion on PostgreSQL. The provenance row, scan row, lifecycle reconciliation, and incident transitions live in one request transaction. + +### Observation ordering + +Lifecycle ordering uses server receipt time, not the client-supplied plan timestamp. The plan timestamp is retained only as provenance. This prevents a client from poisoning the lifecycle cursor with an arbitrary future timestamp. + +### Tenant isolation + +The API resolves the workspace using both workspace ID and authenticated organization ID before any ingestion work starts. A workspace owned by another tenant is returned as not found. + +### Persisted data + +DriftGuard persists: + +- IaC engine and version; +- source JSON format version; +- plan timestamp/applyable/complete metadata; +- redaction policy; +- finding and skipped-nonmanaged counts; +- deterministic bundle digest; +- resource identity, action and changed-path metadata through the incident lifecycle tables; +- sensitive/unknown *paths*, never their values. + +It does not persist raw plan JSON or raw before/after values through this path. + +## Deployment boundary + +The endpoint is disabled by default because the current free Render service cannot run a deterministic pre-deploy migration hook. Enabling ingestion requires the target database to have been explicitly upgraded to Alembic head first. + +## Rejected alternatives + +### Upload raw Terraform plan/state to the API + +Rejected because it expands secret custody and makes the service responsible for protecting values it does not need. + +### Deduplicate by bundle digest globally + +Rejected because two legitimate scans may observe identical drift. Idempotency belongs to a source execution key, not to the evidence contents alone. + +### Generate a new scan on every HTTP retry + +Rejected because retries would inflate occurrence counts and could later fan out into duplicate remediation work. + +### Order lifecycle by client plan timestamp + +Rejected because a malformed or malicious future timestamp could make subsequent valid observations appear stale. + +## Verification gates + +CI must prove: + +- ingestion is disabled by default; +- raw/unknown change-value fields are rejected by validation; +- cross-tenant workspace ingestion returns not found; +- exact HTTP replay reuses one scan and one occurrence; +- conflicting reuse of a submission ID returns 409; +- migration head matches ORM metadata with the provenance table included; +- legacy Terraform/OpenTofu provider contract jobs remain green. From 4e551c8a7bcec03fbb4b9bd55850ca49c3ae8715 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:45:19 +0100 Subject: [PATCH 15/24] fix: satisfy evidence ingestion lint gate --- backend/models/incidents.py | 11 ++++++++++- backend/tests/test_evidence_ingest.py | 6 +++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/backend/models/incidents.py b/backend/models/incidents.py index 7212306..bcb1044 100644 --- a/backend/models/incidents.py +++ b/backend/models/incidents.py @@ -2,7 +2,16 @@ from datetime import datetime -from sqlalchemy import Boolean, CheckConstraint, DateTime, ForeignKey, Integer, JSON, String, UniqueConstraint +from sqlalchemy import ( + JSON, + Boolean, + CheckConstraint, + DateTime, + ForeignKey, + Integer, + String, + UniqueConstraint, +) from sqlalchemy.orm import Mapped, mapped_column from .base import Base, TimestampMixin, generate_id diff --git a/backend/tests/test_evidence_ingest.py b/backend/tests/test_evidence_ingest.py index a98fd7a..994cdaa 100644 --- a/backend/tests/test_evidence_ingest.py +++ b/backend/tests/test_evidence_ingest.py @@ -10,7 +10,11 @@ from backend.core.auth import verify_api_key from backend.database import get_db from backend.models.base import Base -from backend.models.incidents import EvidenceSubmission, FindingIncident, FindingOccurrence +from backend.models.incidents import ( + EvidenceSubmission, + FindingIncident, + FindingOccurrence, +) from backend.models.models import CloudProvider, DriftScan, Organization, Workspace From b164a047082c4946226625d862e88f57084e998f Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:47:28 +0100 Subject: [PATCH 16/24] feat: make Alembic bootstrap the deployment schema gate --- backend/api/main.py | 8 +++++--- docs/adr/0003-database-migration-authority.md | 20 ++++++++++++++----- render.yaml | 3 +++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/backend/api/main.py b/backend/api/main.py index 2e77109..e95929a 100644 --- a/backend/api/main.py +++ b/backend/api/main.py @@ -22,7 +22,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from ..core.auth import generate_api_key, verify_api_key -from ..database import db_session, get_db, init_db +from ..database import db_session, get_db from ..engines.drift import ( AWSStateCollector, DriftAnalyzer, @@ -117,8 +117,10 @@ class ScanTriggerRequest(BaseModel): @asynccontextmanager async def lifespan(app: FastAPI): - await init_db() - log.info("DriftGuard API started", db_ready=True) + # Schema authority lives outside the application process. Deployments must + # run backend.migrations.bootstrap (or alembic upgrade head on an already + # versioned database) before Uvicorn starts. + log.info("DriftGuard API started", schema_authority="alembic") yield log.info("DriftGuard API shutting down") diff --git a/docs/adr/0003-database-migration-authority.md b/docs/adr/0003-database-migration-authority.md index f2dfe12..c0f0588 100644 --- a/docs/adr/0003-database-migration-authority.md +++ b/docs/adr/0003-database-migration-authority.md @@ -1,6 +1,6 @@ # ADR 0003: Make Alembic the schema authority -Status: Accepted for Evidence Core branch; production cutover remains gated. +Status: Accepted; schema-authority cutover implemented for the current single-instance Render deployment. ## Context @@ -16,6 +16,7 @@ Two revisions establish the transition: - `0001_legacy_baseline` describes the schema that existed before Alembic. - `0002_evidence_lifecycle` adds Evidence Core incident lifecycle persistence. +- `0003_evidence_submissions` adds redacted provider-native submission provenance and idempotency. The bootstrap command classifies the database before taking action: @@ -28,9 +29,13 @@ The classifier is intentionally strict. A failed deployment is preferable to wri ## Deployment boundary -The existing Render Blueprint uses a free web service. Render's pre-deploy migration hook is not available on that tier. Therefore this ADR does **not** wire migrations into the free Render startup path and does not make the new lifecycle tables a production dependency yet. +The current Render Blueprint uses a free, single-instance web service. Render's dedicated pre-deploy migration hook is unavailable on that tier, so the Blueprint performs the migration bootstrap as the first command in the service start sequence: -Production cutover requires a deployment target that can run one migration command before the API version depending on that schema becomes active, or an equivalent explicitly controlled release step. +`python -m backend.migrations.bootstrap && uvicorn ...` + +Uvicorn is therefore never started if schema bootstrap fails. FastAPI startup no longer calls `Base.metadata.create_all()`; the application process is not a schema authority. + +This is intentionally scoped to the current single-instance deployment. Before DriftGuard is scaled to multiple API instances, the migration command must move to a one-shot release/pre-deploy job so multiple instances cannot race schema changes. ## Invariants @@ -39,7 +44,8 @@ Production cutover requires a deployment target that can run one migration comma - Re-running bootstrap at head is idempotent. - ORM metadata and Alembic head must have zero pending schema operations under `alembic check`. - `create_all()` may remain for local tests/development, but it is not the production migration authority. -- Evidence Core production code must not depend on a migration until the deployment path can execute that migration deterministically. +- Uvicorn must not start if migration bootstrap fails. +- Evidence ingestion remains opt-in even after schema bootstrap; `DRIFTGUARD_EVIDENCE_INGEST_ENABLED` defaults to false. ## Rejected alternatives @@ -59,9 +65,13 @@ Rejected because concurrent application instances can race schema changes and be Rejected for the current free service because the deployment tier does not provide that capability. +### Keep migrations inside FastAPI lifespan + +Rejected. Schema mutation happens before the application process starts, not inside application startup. + ## Verification gate -Before runtime cutover, CI must prove: +CI must continue proving: - fresh bootstrap reaches Alembic head; - recognized legacy bootstrap preserves existing data; diff --git a/render.yaml b/render.yaml index b522c01..18706b1 100644 --- a/render.yaml +++ b/render.yaml @@ -29,6 +29,7 @@ services: region: oregon buildCommand: pip install -r requirements.txt startCommand: >- + python -m backend.migrations.bootstrap && ALLOWED_ORIGINS="https://${FRONTEND_HOST}" uvicorn backend.api.main:app --host 0.0.0.0 --port $PORT healthCheckPath: /health @@ -48,6 +49,8 @@ services: sync: false - key: DRIFTGUARD_AWS_ACCOUNT_ID sync: false + - key: DRIFTGUARD_EVIDENCE_INGEST_ENABLED + value: "false" - type: web name: driftguard-frontend From 5a0764d677f0facfd6ae917872201ec5e5ccae76 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:53:18 +0100 Subject: [PATCH 17/24] test: add disposable real AWS provider drift proof --- .github/workflows/aws-real-provider-proof.yml | 182 ++++++++++++++++++ tests/integration/aws_real/main.tf | 42 ++++ 2 files changed, 224 insertions(+) create mode 100644 .github/workflows/aws-real-provider-proof.yml create mode 100644 tests/integration/aws_real/main.tf diff --git a/.github/workflows/aws-real-provider-proof.yml b/.github/workflows/aws-real-provider-proof.yml new file mode 100644 index 0000000..75aba82 --- /dev/null +++ b/.github/workflows/aws-real-provider-proof.yml @@ -0,0 +1,182 @@ +name: Real AWS Provider Drift Proof + +on: + push: + branches: + - feat/evidence-core-v1 + paths: + - ".github/workflows/aws-real-provider-proof.yml" + - "tests/integration/aws_real/**" + - "backend/evidence/**" + workflow_dispatch: + +permissions: + contents: read + id-token: write + +concurrency: + group: driftguard-real-aws-provider-proof + cancel-in-progress: false + +jobs: + aws-provider-proof: + runs-on: ubuntu-latest + timeout-minutes: 15 + env: + AWS_REGION: us-east-1 + TF_IN_AUTOMATION: "true" + PARAMETER_NAME: /driftguard/proof/${{ github.run_id }}-${{ github.run_attempt }} + + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + + - name: Install exact Python dependencies used by the proof + run: python -m pip install --disable-pip-version-check pydantic==2.13.5 boto3==1.43.86 + + - name: Configure short-lived AWS credentials + uses: aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502 + with: + role-to-assume: arn:aws:iam::018724217910:role/DriftGuardGitHubProofRole + role-session-name: DriftGuardProof-${{ github.run_id }} + aws-region: ${{ env.AWS_REGION }} + role-duration-seconds: 900 + + - name: Set up Terraform 1.16.2 + uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4.0.1 + with: + terraform_version: "1.16.2" + terraform_wrapper: false + + - name: Verify AWS identity + run: | + python - <<'PY' + import boto3 + identity = boto3.client("sts").get_caller_identity() + assert identity["Account"] == "018724217910" + assert ":assumed-role/DriftGuardGitHubProofRole/" in identity["Arn"] + print({"Account": identity["Account"], "Arn": identity["Arn"]}) + PY + + - name: Initialize exact AWS provider + working-directory: tests/integration/aws_real + run: terraform init -input=false + + - name: Create disposable Terraform-managed parameter + working-directory: tests/integration/aws_real + run: terraform apply -input=false -auto-approve -var="parameter_name=${PARAMETER_NAME}" + + - name: Verify baseline then mutate outside Terraform + run: | + python - <<'PY' + import os + import boto3 + + name = os.environ["PARAMETER_NAME"] + ssm = boto3.client("ssm", region_name=os.environ["AWS_REGION"]) + before = ssm.get_parameter(Name=name)["Parameter"]["Value"] + assert before == "driftguard-baseline-value" + ssm.put_parameter( + Name=name, + Type="String", + Value="driftguard-out-of-band-value", + Overwrite=True, + ) + after = ssm.get_parameter(Name=name)["Parameter"]["Value"] + assert after == "driftguard-out-of-band-value" + print({"parameter": name, "out_of_band_mutation_verified": True}) + PY + + - name: Produce provider-native refresh-only drift plan + working-directory: tests/integration/aws_real + run: | + terraform plan -refresh-only -input=false -out=drift.tfplan -var="parameter_name=${PARAMETER_NAME}" + terraform show -json drift.tfplan > raw-plan.json + + - name: Produce redacted DriftGuard Evidence Bundle + working-directory: tests/integration/aws_real + run: | + python -m backend.evidence raw-plan.json --engine terraform --output evidence.json + + - name: Verify drift semantics and redaction + working-directory: tests/integration/aws_real + run: | + python - <<'PY' + import hashlib + import json + from pathlib import Path + + raw_bytes = Path("raw-plan.json").read_bytes() + evidence_bytes = Path("evidence.json").read_bytes() + raw = json.loads(raw_bytes) + evidence = json.loads(evidence_bytes) + + assert b"driftguard-baseline-value" in raw_bytes + assert b"driftguard-out-of-band-value" in raw_bytes + assert b"driftguard-baseline-value" not in evidence_bytes + assert b"driftguard-out-of-band-value" not in evidence_bytes + + drift = raw.get("resource_drift") + assert isinstance(drift, list) and len(drift) == 1 + + findings = evidence.get("findings") + assert isinstance(findings, list) and len(findings) == 1 + finding = findings[0] + assert finding["resource_address"] == "aws_ssm_parameter.proof" + assert finding["resource_type"] == "aws_ssm_parameter" + assert "update" in finding["actions"] + assert "/value" in finding["changed_paths"] + assert evidence["iac_engine"] == "terraform" + assert evidence["iac_engine_version"] == "1.16.2" + assert evidence["redaction_policy"] == "omit_change_values" + + print(json.dumps({ + "resource_address": finding["resource_address"], + "actions": finding["actions"], + "changed_paths": finding["changed_paths"], + "sensitive_paths": finding["sensitive_paths"], + "unknown_paths": finding["unknown_paths"], + "evidence_sha256": hashlib.sha256(evidence_bytes).hexdigest(), + "raw_plan_sha256": hashlib.sha256(raw_bytes).hexdigest(), + "redaction_verified": True, + }, sort_keys=True)) + PY + + - name: Destroy Terraform-managed proof resource + if: always() + working-directory: tests/integration/aws_real + run: | + terraform destroy -input=false -auto-approve -var="parameter_name=${PARAMETER_NAME}" || true + + - name: Force cleanup and verify absence + if: always() + run: | + python - <<'PY' + import os + import boto3 + from botocore.exceptions import ClientError + + name = os.environ["PARAMETER_NAME"] + ssm = boto3.client("ssm", region_name=os.environ["AWS_REGION"]) + try: + ssm.delete_parameter(Name=name) + except ClientError as exc: + if exc.response["Error"]["Code"] != "ParameterNotFound": + raise + + try: + ssm.get_parameter(Name=name) + except ClientError as exc: + assert exc.response["Error"]["Code"] == "ParameterNotFound" + print({"parameter": name, "cleanup_verified": True}) + else: + raise AssertionError("Disposable DriftGuard proof parameter still exists.") + PY + + - name: Remove raw plan material from runner + if: always() + working-directory: tests/integration/aws_real + run: rm -f raw-plan.json drift.tfplan terraform.tfstate terraform.tfstate.backup evidence.json diff --git a/tests/integration/aws_real/main.tf b/tests/integration/aws_real/main.tf new file mode 100644 index 0000000..bc881f1 --- /dev/null +++ b/tests/integration/aws_real/main.tf @@ -0,0 +1,42 @@ +terraform { + required_version = "= 1.16.2" + + required_providers { + aws = { + source = "hashicorp/aws" + version = "= 6.65.0" + } + } +} + +provider "aws" { + region = var.region +} + +variable "region" { + description = "AWS region used by the disposable provider-native drift proof." + type = string + default = "us-east-1" +} + +variable "parameter_name" { + description = "Unique SSM parameter path for one proof run." + type = string + + validation { + condition = startswith(var.parameter_name, "/driftguard/proof/") + error_message = "parameter_name must stay under /driftguard/proof/." + } +} + +resource "aws_ssm_parameter" "proof" { + name = var.parameter_name + type = "String" + value = "driftguard-baseline-value" + tier = "Standard" + + tags = { + Project = "DriftGuard" + Purpose = "ProviderNativeProof" + } +} From 705e5e83cc8dfc8034b5117c78c40b7357d056d4 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:54:30 +0100 Subject: [PATCH 18/24] fix: run evidence analyzer from repository import path From b2b99001fb94459067b7b8461cd44ba3de8e61d8 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:55:46 +0100 Subject: [PATCH 19/24] docs: define real AWS drift proof contract --- tests/integration/aws_real/README.md | 30 ++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 tests/integration/aws_real/README.md diff --git a/tests/integration/aws_real/README.md b/tests/integration/aws_real/README.md new file mode 100644 index 0000000..be647ca --- /dev/null +++ b/tests/integration/aws_real/README.md @@ -0,0 +1,30 @@ +# Real AWS provider drift proof + +This fixture is an intentionally small end-to-end validation of DriftGuard's +provider-native evidence path against a real AWS API. + +The GitHub Actions workflow creates one **SSM Standard String parameter** under +`/driftguard/proof/`, mutates its value directly through AWS (outside +Terraform), runs a Terraform 1.16.2 refresh-only plan with AWS provider 6.65.0, +and converts the resulting `resource_drift` into DriftGuard Evidence Bundle +v1. + +The proof is successful only when all of the following hold: + +- GitHub authenticates to AWS through OIDC; no long-lived AWS access key is used. +- The assumed role is scoped to the proof parameter namespace. +- Terraform observes exactly one managed-resource drift record. +- DriftGuard preserves the Terraform resource identity and update action. +- The changed-value path is represented in the Evidence Bundle. +- Raw baseline and mutated parameter values occur in the ephemeral plan JSON + but do **not** occur in the Evidence Bundle. +- Terraform destroy and a direct SSM fallback delete both run under `always()`. +- A final AWS API read proves the disposable parameter no longer exists. + +The fixture intentionally uses local Terraform state because the resource exists +only for one workflow run. Raw plan JSON, plan files, state, and evidence output +are deleted from the runner and are not uploaded as artifacts. + +This is an integration proof, not a production deployment topology. A failed +proof must be investigated; the test must not be weakened to accommodate a +provider or parser discrepancy. From d7a00acffa18c43b75ce9c04575b6836f84c6a18 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:57:15 +0100 Subject: [PATCH 20/24] fix: expose repository package path to AWS proof analyzer --- .github/workflows/aws-real-provider-proof.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/aws-real-provider-proof.yml b/.github/workflows/aws-real-provider-proof.yml index 75aba82..13e97d3 100644 --- a/.github/workflows/aws-real-provider-proof.yml +++ b/.github/workflows/aws-real-provider-proof.yml @@ -99,7 +99,7 @@ jobs: - name: Produce redacted DriftGuard Evidence Bundle working-directory: tests/integration/aws_real run: | - python -m backend.evidence raw-plan.json --engine terraform --output evidence.json + PYTHONPATH="${GITHUB_WORKSPACE}" python -m backend.evidence raw-plan.json --engine terraform --output evidence.json - name: Verify drift semantics and redaction working-directory: tests/integration/aws_real From 33758e39f7a3139ca579fcfd3412edb646da4e96 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 12:59:24 +0100 Subject: [PATCH 21/24] docs: record verified real AWS drift proof --- .../real-aws-provider-proof-2026-09-18.md | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 docs/validation/real-aws-provider-proof-2026-09-18.md diff --git a/docs/validation/real-aws-provider-proof-2026-09-18.md b/docs/validation/real-aws-provider-proof-2026-09-18.md new file mode 100644 index 0000000..91bb03c --- /dev/null +++ b/docs/validation/real-aws-provider-proof-2026-09-18.md @@ -0,0 +1,108 @@ +# Real AWS provider-native drift proof — 2026-09-18 + +## Scope + +This record captures the first DriftGuard Evidence Core validation against a +real AWS API and a real HashiCorp AWS provider refresh. + +It is evidence for one controlled resource type and one mutation path. It is +**not** evidence that every AWS provider resource or every drift shape is +correctly adjudicated. + +## Source revision + +- DriftGuard commit: `d7a00acffa18c43b75ce9c04575b6836f84c6a18` +- GitHub Actions run: `35342171422` +- Workflow: `Real AWS Provider Drift Proof` +- Region: `us-east-1` +- Terraform: `1.16.2` +- HashiCorp AWS provider: `6.65.0` + +Authentication used GitHub Actions OIDC and the scoped +`DriftGuardGitHubProofRole`. No long-lived AWS access key was stored in the +repository or workflow. + +## Experiment + +The workflow created one Terraform-managed SSM Standard String parameter under +`/driftguard/proof/`. + +The sequence was: + +1. Terraform applied the baseline parameter. +2. A direct AWS API read verified the baseline state. +3. The parameter value was changed directly through AWS, outside Terraform. +4. A second AWS API read proved the out-of-band mutation occurred. +5. Terraform generated a refresh-only plan and detected the remote change. +6. `terraform show -json` produced ephemeral plan JSON. +7. DriftGuard converted `resource_drift` into Evidence Bundle v1. +8. The workflow asserted semantic identity and redaction. +9. Terraform destroy ran under `always()`. +10. A direct SSM fallback deletion ran under `always()`. +11. A final AWS read proved the parameter no longer existed. +12. A separate post-run AWS query found no parameters remaining under + `/driftguard/proof/`. + +## Observed provider-native evidence + +DriftGuard emitted: + +- resource address: `aws_ssm_parameter.proof` +- resource type: `aws_ssm_parameter` +- provider action: `update` +- changed paths: + - `/value` + - `/version` +- sensitive paths: + - `/value` + - `/value_wo` +- unknown paths: none + +Evidence Bundle SHA-256: + +`c0081f296855c3b42c882f8bc9bc17842dffacb68d362f958e19082f28d60d13` + +Ephemeral raw-plan SHA-256: + +`ee9689d70563f0b80fe662d7a5dd07ce678c7c971058d45ef30d234567f0a8f5` + +The raw plan contained both the prior and out-of-band parameter values. The +Evidence Bundle contained neither value. The workflow asserted this directly +without printing the values. + +## What this proves + +This run proves that, for this fixture: + +- GitHub OIDC can obtain short-lived AWS credentials with the scoped proof role. +- Terraform's AWS provider can observe the out-of-band mutation. +- Terraform emits the change through `resource_drift`. +- DriftGuard preserves the provider-native resource identity and action. +- DriftGuard derives changed-path metadata from the real provider plan. +- DriftGuard carries sensitive-path metadata without persisting the sensitive + values. +- The proof resource is removed after the run. + +## What this does not prove + +This run does not prove: + +- correctness for every AWS resource type; +- correctness for modules, `count`, or `for_each` on AWS specifically + (those identity semantics are covered by the separate local-provider + Terraform/OpenTofu contract); +- CloudTrail attribution; +- code remediation safety; +- cloud-to-code source rewriting; +- ingestion into a deployed DriftGuard API; +- multi-account role behavior; +- concurrent worker behavior; +- production readiness. + +Those remain separate gates and must not be inferred from this result. + +## Operational note + +The AWS account was queried after the workflow completed. No SSM parameters +remained under the proof namespace. The workflow's own cleanup verification also +passed. From 50b2b1b5b904845d18e68aea44f564086ee9ec26 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 13:03:28 +0100 Subject: [PATCH 22/24] feat: add conservative AWS cloud locators to evidence v1.1 --- .github/workflows/aws-real-provider-proof.yml | 8 +++ backend/evidence/models.py | 23 ++++++- backend/evidence/terraform_plan.py | 55 ++++++++++++++++- backend/tests/test_plan_evidence.py | 60 +++++++++++++++++++ docs/adr/0005-cloud-resource-locators.md | 60 +++++++++++++++++++ 5 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 docs/adr/0005-cloud-resource-locators.md diff --git a/.github/workflows/aws-real-provider-proof.yml b/.github/workflows/aws-real-provider-proof.yml index 13e97d3..d8eb6ad 100644 --- a/.github/workflows/aws-real-provider-proof.yml +++ b/.github/workflows/aws-real-provider-proof.yml @@ -129,10 +129,18 @@ jobs: assert finding["resource_type"] == "aws_ssm_parameter" assert "update" in finding["actions"] assert "/value" in finding["changed_paths"] + assert evidence["schema_version"] == "1.1" assert evidence["iac_engine"] == "terraform" assert evidence["iac_engine_version"] == "1.16.2" assert evidence["redaction_policy"] == "omit_change_values" + locator = finding["cloud_locator"] + assert locator["provider"] == "aws" + assert locator["name"] == os.environ["PARAMETER_NAME"] + assert locator["id"] == os.environ["PARAMETER_NAME"] + assert locator["region"] == os.environ["AWS_REGION"] + assert locator["arn"].endswith(os.environ["PARAMETER_NAME"].removeprefix("/")) + print(json.dumps({ "resource_address": finding["resource_address"], "actions": finding["actions"], diff --git a/backend/evidence/models.py b/backend/evidence/models.py index b73a91b..421d30b 100644 --- a/backend/evidence/models.py +++ b/backend/evidence/models.py @@ -2,11 +2,29 @@ from typing import Literal -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator IaCEngine = Literal["terraform", "opentofu", "unknown"] +class CloudResourceLocator(BaseModel): + """Non-sensitive cloud identity metadata used for optional enrichment.""" + + model_config = ConfigDict(extra="forbid") + + provider: Literal["aws"] + arn: str | None = None + id: str | None = None + name: str | None = None + region: str | None = None + + @model_validator(mode="after") + def require_identifier(self): + if not any((self.arn, self.id, self.name)): + raise ValueError("Cloud resource locator requires arn, id, or name.") + return self + + class DriftEvidence(BaseModel): """One provider-native drift observation with all raw values omitted.""" @@ -21,6 +39,7 @@ class DriftEvidence(BaseModel): resource_name: str = Field(min_length=1) resource_index: str | int | None = None provider_name: str | None = None + cloud_locator: CloudResourceLocator | None = None actions: list[str] changed_paths: list[str] sensitive_paths: list[str] @@ -32,7 +51,7 @@ class EvidenceBundle(BaseModel): model_config = ConfigDict(extra="forbid") - schema_version: Literal["1.0"] = "1.0" + schema_version: Literal["1.0", "1.1"] = "1.1" iac_engine: IaCEngine = "unknown" iac_engine_version: str | None = None source_format_version: str = Field(min_length=1) diff --git a/backend/evidence/terraform_plan.py b/backend/evidence/terraform_plan.py index cb994e9..7be7bcf 100644 --- a/backend/evidence/terraform_plan.py +++ b/backend/evidence/terraform_plan.py @@ -3,7 +3,7 @@ from collections.abc import Mapping from typing import Any -from .models import DriftEvidence, EvidenceBundle, IaCEngine +from .models import CloudResourceLocator, DriftEvidence, EvidenceBundle, IaCEngine class PlanEvidenceError(ValueError): @@ -115,6 +115,11 @@ def analyze_plan_json( unknown_paths = sorted( _mask_paths(change.get("after_unknown"), "unknown-value") ) + cloud_locator = _extract_cloud_locator( + provider_name=provider_name, + change=change, + blocked_paths=set(sensitive_paths) | set(unknown_paths), + ) findings.append( DriftEvidence( @@ -126,6 +131,7 @@ def analyze_plan_json( resource_name=resource_name, resource_index=resource_index, provider_name=provider_name, + cloud_locator=cloud_locator, actions=actions, changed_paths=changed_paths, sensitive_paths=sensitive_paths, @@ -234,3 +240,50 @@ def _mask_paths(mask: Any, label: str, pointer: str = "") -> set[str]: return paths raise PlanEvidenceError(f"{label.capitalize()} mask contains an unsupported shape.") + + +def _extract_cloud_locator( + *, + provider_name: str | None, + change: Mapping[str, Any], + blocked_paths: set[str], +) -> CloudResourceLocator | None: + """Extract only well-known, non-sensitive AWS identity metadata. + + This function is intentionally conservative. It does not copy arbitrary + provider state into evidence and it refuses any candidate field covered by + Terraform/OpenTofu sensitive or unknown masks. + """ + if provider_name != "registry.terraform.io/hashicorp/aws": + return None + if "" in blocked_paths: + return None + + before = change.get("before") + after = change.get("after") + + def pick(field: str) -> str | None: + if _pointer_child("", field) in blocked_paths: + return None + for candidate in (after, before): + if not isinstance(candidate, Mapping): + continue + value = candidate.get(field) + if isinstance(value, str) and value: + return value + return None + + arn = pick("arn") + resource_id = pick("id") + name = pick("name") + region = pick("region") + if not any((arn, resource_id, name)): + return None + + return CloudResourceLocator( + provider="aws", + arn=arn, + id=resource_id, + name=name, + region=region, + ) diff --git a/backend/tests/test_plan_evidence.py b/backend/tests/test_plan_evidence.py index 9d35ac8..47f6724 100644 --- a/backend/tests/test_plan_evidence.py +++ b/backend/tests/test_plan_evidence.py @@ -106,6 +106,66 @@ def test_emits_changed_paths_without_persisting_raw_values(): assert "t3.large" not in serialized +def test_extracts_only_non_sensitive_aws_cloud_locator_metadata(): + plan = _plan( + _drift( + before={ + "arn": "arn:aws:ec2:us-east-1:123456789012:instance/i-123", + "id": "i-123", + "name": "web-prod", + "region": "us-east-1", + "password": "secret-before", + }, + after={ + "arn": "arn:aws:ec2:us-east-1:123456789012:instance/i-123", + "id": "i-123", + "name": "web-prod", + "region": "us-east-1", + "password": "secret-after", + }, + before_sensitive={"password": True}, + after_sensitive={"password": True}, + ) + ) + + bundle = analyze_plan_json(plan) + locator = bundle.findings[0].cloud_locator + + assert bundle.schema_version == "1.1" + assert locator is not None + assert locator.provider == "aws" + assert locator.arn == "arn:aws:ec2:us-east-1:123456789012:instance/i-123" + assert locator.id == "i-123" + assert locator.name == "web-prod" + assert locator.region == "us-east-1" + serialized = bundle.model_dump_json() + assert "secret-before" not in serialized + assert "secret-after" not in serialized + + +def test_cloud_locator_refuses_identity_fields_masked_sensitive_or_unknown(): + plan = _plan( + _drift( + before={ + "arn": "arn:aws:ssm:us-east-1:123456789012:parameter/private", + "id": "/private", + "name": "/private", + "region": "us-east-1", + }, + after={ + "arn": "arn:aws:ssm:us-east-1:123456789012:parameter/private", + "id": "/private", + "name": "/private", + "region": "us-east-1", + }, + before_sensitive={"arn": True, "id": True, "name": True}, + after_sensitive={"arn": True, "id": True, "name": True}, + ) + ) + + assert analyze_plan_json(plan).findings[0].cloud_locator is None + + def test_changed_paths_use_json_pointer_escaping(): plan = _plan( _drift( diff --git a/docs/adr/0005-cloud-resource-locators.md b/docs/adr/0005-cloud-resource-locators.md new file mode 100644 index 0000000..80b631a --- /dev/null +++ b/docs/adr/0005-cloud-resource-locators.md @@ -0,0 +1,60 @@ +# ADR 0005: Carry only conservative cloud resource locators in evidence + +Status: Accepted for Evidence Bundle v1.1. + +## Context + +Provider-native drift identity and CloudTrail attribution solve different +identity problems. + +Terraform/OpenTofu absolute resource addresses identify the IaC object. AWS +audit APIs identify cloud resources using ARNs, provider IDs, names, regions and +service-specific request fields. Correlating the two without a cloud locator +would require guessing from Terraform block names or re-uploading raw plan +state, both of which are unacceptable. + +Raw provider state can contain secrets and arbitrary customer data, so copying +whole resource objects into Evidence Bundles is also unacceptable. + +## Decision + +Evidence Bundle v1.1 adds an optional `cloud_locator` to each drift finding. + +For the AWS provider the local analyzer may extract only these top-level fields: + +- `arn` +- `id` +- `name` +- `region` + +The locator is emitted only when at least one of ARN, ID or name is available. +Any candidate path marked sensitive or unknown by Terraform/OpenTofu is omitted. +If the whole resource is masked sensitive/unknown, no locator is emitted. + +The extractor recognizes only the canonical HashiCorp AWS provider name. Other +providers remain unsupported until they receive an explicit locator contract. + +This is infrastructure identity metadata, not arbitrary resource state. +`before`/`after` objects are still excluded from the Evidence Bundle. + +## Compatibility + +The ingestion model accepts Evidence Bundle schema versions `1.0` and `1.1`. +The local analyzer emits `1.1`. Existing v1.0 submissions remain valid. + +## Non-goals + +This ADR does not define CloudTrail event attribution itself. It only provides +the minimum non-sensitive locator required for a later attribution stage. + +It does not claim that ARN/ID/name semantics are sufficient for every AWS +resource type. Service-specific attribution remains an explicit adapter +responsibility and unsupported resources must remain unsupported rather than +using heuristic matching. + +## Verification + +Unit tests prove that safe identity fields are retained while sensitive identity +fields are omitted. The real AWS SSM proof additionally requires the emitted +locator to match the actual parameter name, ID, region and ARN while raw +parameter values remain absent from evidence. From 737de6767b2b063c307f58c2b2018c098df5af44 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 13:05:52 +0100 Subject: [PATCH 23/24] fix: import environment module in AWS locator proof --- .github/workflows/aws-real-provider-proof.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/aws-real-provider-proof.yml b/.github/workflows/aws-real-provider-proof.yml index d8eb6ad..ae3711f 100644 --- a/.github/workflows/aws-real-provider-proof.yml +++ b/.github/workflows/aws-real-provider-proof.yml @@ -107,6 +107,7 @@ jobs: python - <<'PY' import hashlib import json + import os from pathlib import Path raw_bytes = Path("raw-plan.json").read_bytes() From df8cd946fb38696cbc7dff8b09b604a6c03256a9 Mon Sep 17 00:00:00 2001 From: Edwin Jonathan Date: Fri, 18 Sep 2026 13:10:28 +0100 Subject: [PATCH 24/24] feat: add fail-closed SSM CloudTrail attribution contract --- backend/evidence/__init__.py | 11 +- backend/evidence/attribution.py | 287 +++++++++++++++++++++ backend/tests/test_evidence_attribution.py | 280 ++++++++++++++++++++ docs/adr/0006-cloudtrail-attribution.md | 81 ++++++ 4 files changed, 658 insertions(+), 1 deletion(-) create mode 100644 backend/evidence/attribution.py create mode 100644 backend/tests/test_evidence_attribution.py create mode 100644 docs/adr/0006-cloudtrail-attribution.md diff --git a/backend/evidence/__init__.py b/backend/evidence/__init__.py index 5642332..52797f1 100644 --- a/backend/evidence/__init__.py +++ b/backend/evidence/__init__.py @@ -5,12 +5,21 @@ APIs are enrichment sources, not truth substitutes. """ -from .models import DriftEvidence, EvidenceBundle +from .attribution import ( + AttributionResult, + AuditEventMatch, + attribute_aws_drift, +) +from .models import CloudResourceLocator, DriftEvidence, EvidenceBundle from .terraform_plan import PlanEvidenceError, analyze_plan_json __all__ = [ + "AttributionResult", + "AuditEventMatch", + "CloudResourceLocator", "DriftEvidence", "EvidenceBundle", "PlanEvidenceError", "analyze_plan_json", + "attribute_aws_drift", ] diff --git a/backend/evidence/attribution.py b/backend/evidence/attribution.py new file mode 100644 index 0000000..2aecad6 --- /dev/null +++ b/backend/evidence/attribution.py @@ -0,0 +1,287 @@ +from __future__ import annotations + +import json +from dataclasses import dataclass +from datetime import UTC, datetime +from typing import Any, Literal, Protocol + +from .models import DriftEvidence + +AttributionStatus = Literal[ + "attributed", + "not_found", + "ambiguous", + "unsupported", + "error", +] + +_SSM_PROVIDER = "registry.terraform.io/hashicorp/aws" +_SSM_VALUE_PATH = "/value" +_SSM_EVENT_NAME = "PutParameter" +_SSM_EVENT_SOURCE = "ssm.amazonaws.com" + + +class CloudTrailLookupClient(Protocol): + def lookup_events(self, **kwargs: Any) -> dict[str, Any]: ... + + +@dataclass(frozen=True, slots=True) +class AuditEventMatch: + event_id: str + event_name: str + event_time: datetime + event_source: str + actor_session: str | None + + +@dataclass(frozen=True, slots=True) +class AttributionResult: + status: AttributionStatus + resource_address: str + adapter: str + candidate_count: int + candidate_event_ids: tuple[str, ...] + event: AuditEventMatch | None + reason: str + + +def attribute_aws_drift( + finding: DriftEvidence, + *, + cloudtrail: CloudTrailLookupClient, + start_time: datetime, + end_time: datetime, +) -> AttributionResult: + """Find a bounded CloudTrail audit candidate for one AWS drift finding. + + A successful result means exactly one event satisfied a service-specific, + deterministic matching contract. It does not claim to prove human intent or + business causality. + + Raw CloudTrailEvent payloads are parsed only in memory. This function never + returns request parameters or event payloads because those can contain + sensitive values. + """ + _validate_window(start_time, end_time) + + if finding.provider_name != _SSM_PROVIDER: + return _unsupported( + finding, + "CloudTrail attribution currently supports only the canonical HashiCorp AWS provider.", + ) + if finding.resource_type != "aws_ssm_parameter": + return _unsupported( + finding, + f"No CloudTrail attribution adapter exists for {finding.resource_type!r}.", + ) + if _SSM_VALUE_PATH not in finding.changed_paths: + return _unsupported( + finding, + "The SSM adapter currently attributes only drift that includes /value.", + ) + + locator = finding.cloud_locator + if locator is None or locator.provider != "aws": + return _unsupported( + finding, + "SSM attribution requires a non-sensitive AWS cloud locator.", + ) + + resource_name = locator.name or locator.id + if not resource_name: + return _unsupported( + finding, + "SSM attribution requires a parameter name or provider ID.", + ) + + try: + events = _lookup_put_parameter_events( + cloudtrail, + start_time=start_time, + end_time=end_time, + ) + except Exception as exc: + return AttributionResult( + status="error", + resource_address=finding.resource_address, + adapter="aws_ssm_parameter", + candidate_count=0, + candidate_event_ids=(), + event=None, + reason=f"CloudTrail lookup failed safely: {type(exc).__name__}.", + ) + + candidates: list[AuditEventMatch] = [] + for raw_event in events: + try: + candidate = _match_ssm_value_mutation( + raw_event, + resource_name=resource_name, + start_time=start_time, + end_time=end_time, + ) + except (TypeError, ValueError, json.JSONDecodeError) as exc: + return AttributionResult( + status="error", + resource_address=finding.resource_address, + adapter="aws_ssm_parameter", + candidate_count=0, + candidate_event_ids=(), + event=None, + reason=f"CloudTrail event could not be interpreted safely: {type(exc).__name__}.", + ) + if candidate is not None: + candidates.append(candidate) + + candidate_ids = tuple(sorted(candidate.event_id for candidate in candidates)) + if not candidates: + return AttributionResult( + status="not_found", + resource_address=finding.resource_address, + adapter="aws_ssm_parameter", + candidate_count=0, + candidate_event_ids=(), + event=None, + reason=( + "No PutParameter event matched the exact parameter name with " + "overwrite=true inside the requested time window." + ), + ) + if len(candidates) > 1: + return AttributionResult( + status="ambiguous", + resource_address=finding.resource_address, + adapter="aws_ssm_parameter", + candidate_count=len(candidates), + candidate_event_ids=candidate_ids, + event=None, + reason=( + "Multiple PutParameter mutation events matched; DriftGuard refuses " + "to choose one as the drift source." + ), + ) + + return AttributionResult( + status="attributed", + resource_address=finding.resource_address, + adapter="aws_ssm_parameter", + candidate_count=1, + candidate_event_ids=candidate_ids, + event=candidates[0], + reason=( + "Exactly one CloudTrail PutParameter event matched the exact parameter " + "name with overwrite=true inside the bounded time window." + ), + ) + + +def _validate_window(start_time: datetime, end_time: datetime) -> None: + for label, value in (("start_time", start_time), ("end_time", end_time)): + if value.tzinfo is None or value.utcoffset() is None: + raise ValueError(f"{label} must be timezone-aware.") + if start_time.astimezone(UTC) >= end_time.astimezone(UTC): + raise ValueError("start_time must be earlier than end_time.") + + +def _unsupported(finding: DriftEvidence, reason: str) -> AttributionResult: + return AttributionResult( + status="unsupported", + resource_address=finding.resource_address, + adapter="none", + candidate_count=0, + candidate_event_ids=(), + event=None, + reason=reason, + ) + + +def _lookup_put_parameter_events( + cloudtrail: CloudTrailLookupClient, + *, + start_time: datetime, + end_time: datetime, +) -> list[dict[str, Any]]: + params: dict[str, Any] = { + "LookupAttributes": [ + {"AttributeKey": "EventName", "AttributeValue": _SSM_EVENT_NAME} + ], + "StartTime": start_time, + "EndTime": end_time, + "MaxResults": 50, + } + events: list[dict[str, Any]] = [] + seen_tokens: set[str] = set() + + while True: + response = cloudtrail.lookup_events(**params) + page = response.get("Events", []) + if not isinstance(page, list): + raise TypeError("CloudTrail Events must be a list.") + if any(not isinstance(event, dict) for event in page): + raise TypeError("CloudTrail event entries must be objects.") + events.extend(page) + + token = response.get("NextToken") + if token is None: + break + if not isinstance(token, str) or not token: + raise TypeError("CloudTrail NextToken must be a non-empty string.") + if token in seen_tokens: + raise ValueError("CloudTrail pagination repeated a NextToken.") + seen_tokens.add(token) + params["NextToken"] = token + + return events + + +def _match_ssm_value_mutation( + event: dict[str, Any], + *, + resource_name: str, + start_time: datetime, + end_time: datetime, +) -> AuditEventMatch | None: + if event.get("EventName") != _SSM_EVENT_NAME: + return None + + raw_payload = event.get("CloudTrailEvent") + if not isinstance(raw_payload, str): + raise TypeError("CloudTrailEvent must be a JSON string.") + payload = json.loads(raw_payload) + if not isinstance(payload, dict): + raise TypeError("CloudTrailEvent JSON must decode to an object.") + if payload.get("eventSource") != _SSM_EVENT_SOURCE: + return None + + request = payload.get("requestParameters") + if not isinstance(request, dict): + raise TypeError("PutParameter requestParameters must be an object.") + if request.get("name") != resource_name: + return None + # overwrite=false is the create path in this contract. It is explicitly not + # accepted as evidence for a drift mutation. + if request.get("overwrite") is not True: + return None + + event_id = event.get("EventId") + if not isinstance(event_id, str) or not event_id: + raise TypeError("Matched CloudTrail event is missing EventId.") + + event_time = event.get("EventTime") + if not isinstance(event_time, datetime): + raise TypeError("Matched CloudTrail event is missing datetime EventTime.") + if event_time.tzinfo is None or event_time.utcoffset() is None: + raise ValueError("Matched CloudTrail EventTime must be timezone-aware.") + event_time = event_time.astimezone(UTC) + if not (start_time.astimezone(UTC) <= event_time <= end_time.astimezone(UTC)): + return None + + username = event.get("Username") + actor_session = username if isinstance(username, str) and username else None + return AuditEventMatch( + event_id=event_id, + event_name=_SSM_EVENT_NAME, + event_time=event_time, + event_source=_SSM_EVENT_SOURCE, + actor_session=actor_session, + ) diff --git a/backend/tests/test_evidence_attribution.py b/backend/tests/test_evidence_attribution.py new file mode 100644 index 0000000..cfc4d41 --- /dev/null +++ b/backend/tests/test_evidence_attribution.py @@ -0,0 +1,280 @@ +from __future__ import annotations + +import json +from datetime import UTC, datetime + +import pytest + +from backend.evidence.attribution import attribute_aws_drift +from backend.evidence.models import CloudResourceLocator, DriftEvidence + + +class FakeCloudTrail: + def __init__(self, pages: list[dict] | None = None, error: Exception | None = None): + self.pages = pages or [{"Events": []}] + self.error = error + self.calls: list[dict] = [] + + def lookup_events(self, **kwargs): + self.calls.append(kwargs) + if self.error is not None: + raise self.error + index = len(self.calls) - 1 + return self.pages[index] + + +def _finding( + *, + resource_type: str = "aws_ssm_parameter", + changed_paths: list[str] | None = None, + locator: CloudResourceLocator | None = None, +) -> DriftEvidence: + return DriftEvidence( + resource_address="aws_ssm_parameter.proof", + resource_type=resource_type, + resource_name="proof", + provider_name="registry.terraform.io/hashicorp/aws", + cloud_locator=locator + or CloudResourceLocator( + provider="aws", + arn="arn:aws:ssm:us-east-1:123456789012:parameter/driftguard/proof/test", + id="/driftguard/proof/test", + name="/driftguard/proof/test", + region="us-east-1", + ), + actions=["update"], + changed_paths=changed_paths or ["/value", "/version"], + sensitive_paths=["/value"], + unknown_paths=[], + ) + + +def _event( + *, + event_id: str, + overwrite: bool, + name: str = "/driftguard/proof/test", + when: datetime | None = None, + secret: str = "DO_NOT_PERSIST", +) -> dict: + when = when or datetime(2026, 9, 18, 12, 0, tzinfo=UTC) + return { + "EventId": event_id, + "EventName": "PutParameter", + "EventTime": when, + "Username": "proof-session", + "CloudTrailEvent": json.dumps( + { + "eventSource": "ssm.amazonaws.com", + "requestParameters": { + "name": name, + "overwrite": overwrite, + "value": secret, + }, + } + ), + } + + +START = datetime(2026, 9, 18, 11, 55, tzinfo=UTC) +END = datetime(2026, 9, 18, 12, 5, tzinfo=UTC) + + +def test_unique_overwrite_event_is_attributed_without_returning_raw_payload(): + client = FakeCloudTrail( + pages=[ + { + "Events": [ + _event(event_id="create", overwrite=False), + _event(event_id="mutation", overwrite=True), + ] + } + ] + ) + + result = attribute_aws_drift( + _finding(), + cloudtrail=client, + start_time=START, + end_time=END, + ) + + assert result.status == "attributed" + assert result.candidate_count == 1 + assert result.candidate_event_ids == ("mutation",) + assert result.event is not None + assert result.event.event_id == "mutation" + assert result.event.actor_session == "proof-session" + assert "DO_NOT_PERSIST" not in repr(result) + + call = client.calls[0] + assert call["LookupAttributes"] == [ + {"AttributeKey": "EventName", "AttributeValue": "PutParameter"} + ] + assert call["StartTime"] == START + assert call["EndTime"] == END + assert call["MaxResults"] == 50 + + +def test_multiple_exact_mutations_are_ambiguous_not_latest_wins(): + client = FakeCloudTrail( + pages=[ + { + "Events": [ + _event(event_id="older", overwrite=True), + _event( + event_id="newer", + overwrite=True, + when=datetime(2026, 9, 18, 12, 1, tzinfo=UTC), + ), + ] + } + ] + ) + + result = attribute_aws_drift( + _finding(), + cloudtrail=client, + start_time=START, + end_time=END, + ) + + assert result.status == "ambiguous" + assert result.candidate_count == 2 + assert result.candidate_event_ids == ("newer", "older") + assert result.event is None + + +def test_exact_resource_name_is_required(): + client = FakeCloudTrail( + pages=[ + { + "Events": [ + _event( + event_id="other", + overwrite=True, + name="/driftguard/proof/other", + ) + ] + } + ] + ) + + result = attribute_aws_drift( + _finding(), + cloudtrail=client, + start_time=START, + end_time=END, + ) + + assert result.status == "not_found" + + +def test_pagination_is_exhaustive_and_duplicate_token_fails_closed(): + client = FakeCloudTrail( + pages=[ + {"Events": [], "NextToken": "page-2"}, + {"Events": [_event(event_id="mutation", overwrite=True)]}, + ] + ) + + result = attribute_aws_drift( + _finding(), + cloudtrail=client, + start_time=START, + end_time=END, + ) + + assert result.status == "attributed" + assert len(client.calls) == 2 + assert client.calls[1]["NextToken"] == "page-2" + + repeated = FakeCloudTrail( + pages=[ + {"Events": [], "NextToken": "repeat"}, + {"Events": [], "NextToken": "repeat"}, + ] + ) + failed = attribute_aws_drift( + _finding(), + cloudtrail=repeated, + start_time=START, + end_time=END, + ) + assert failed.status == "error" + + +def test_unsupported_resource_or_non_value_drift_never_calls_cloudtrail(): + client = FakeCloudTrail() + + unsupported_type = attribute_aws_drift( + _finding(resource_type="aws_instance"), + cloudtrail=client, + start_time=START, + end_time=END, + ) + unsupported_path = attribute_aws_drift( + _finding(changed_paths=["/tags"]), + cloudtrail=client, + start_time=START, + end_time=END, + ) + + assert unsupported_type.status == "unsupported" + assert unsupported_path.status == "unsupported" + assert client.calls == [] + + +def test_cloudtrail_failure_and_malformed_event_fail_closed(): + failed_client = FakeCloudTrail(error=RuntimeError("simulated")) + failed = attribute_aws_drift( + _finding(), + cloudtrail=failed_client, + start_time=START, + end_time=END, + ) + assert failed.status == "error" + assert "simulated" not in failed.reason + + malformed = FakeCloudTrail( + pages=[ + { + "Events": [ + { + "EventId": "broken", + "EventName": "PutParameter", + "EventTime": datetime(2026, 9, 18, 12, 0, tzinfo=UTC), + "CloudTrailEvent": "{not-json}", + } + ] + } + ] + ) + result = attribute_aws_drift( + _finding(), + cloudtrail=malformed, + start_time=START, + end_time=END, + ) + assert result.status == "error" + + +def test_attribution_window_must_be_timezone_aware_and_ordered(): + client = FakeCloudTrail() + naive = datetime(2026, 9, 18, 12, 0) + + with pytest.raises(ValueError, match="timezone-aware"): + attribute_aws_drift( + _finding(), + cloudtrail=client, + start_time=naive, + end_time=END, + ) + + with pytest.raises(ValueError, match="earlier"): + attribute_aws_drift( + _finding(), + cloudtrail=client, + start_time=END, + end_time=START, + ) diff --git a/docs/adr/0006-cloudtrail-attribution.md b/docs/adr/0006-cloudtrail-attribution.md new file mode 100644 index 0000000..4a39967 --- /dev/null +++ b/docs/adr/0006-cloudtrail-attribution.md @@ -0,0 +1,81 @@ +# ADR 0006: Service-specific CloudTrail attribution with refusal states + +Status: Accepted for the first attribution adapter. + +## Context + +CloudTrail Event History can be queried with `LookupEvents` for management +events in one region from the previous 90 days. AWS currently permits one +lookup attribute per request, returns at most 50 events per page, and limits +lookup requests to two per second per account per region. + +The real SSM proof showed why a generic "latest event wins" rule is unsafe: +creating a parameter and mutating it both emit `PutParameter`. Earlier runs +contained two events for the same parameter within seconds. + +Terraform resource addresses also cannot be matched directly to CloudTrail. +Evidence Bundle v1.1 therefore carries a conservative cloud locator before +attribution is attempted. + +## Decision + +Attribution is an optional enrichment stage. It does not participate in drift +truth and it does not run inside lifecycle database transactions. + +The first adapter supports only: + +- provider: canonical HashiCorp AWS provider; +- resource: `aws_ssm_parameter`; +- drift surface: findings that include `/value`. + +The adapter queries CloudTrail by `EventName=PutParameter` inside a caller- +supplied, timezone-aware window, exhausts pagination, and then filters locally. + +A candidate must satisfy all of: + +- event source is `ssm.amazonaws.com`; +- event name is `PutParameter`; +- `requestParameters.name` exactly equals the evidence locator name/ID; +- `requestParameters.overwrite` is exactly `true`; +- event time is inside the bounded window. + +`overwrite=false` is treated as creation and is not accepted as the mutation +source for this contract. + +## Outcome states + +- `attributed`: exactly one audit event matches the contract. +- `not_found`: no event matches. +- `ambiguous`: multiple events match; DriftGuard refuses to choose. +- `unsupported`: no explicit adapter/locator/changed-path contract exists. +- `error`: lookup or event interpretation could not be completed safely. + +An `attributed` result means a unique matching audit event exists. It does not +claim to prove human intent or broader business causality. + +## Data minimization + +CloudTrail event JSON is parsed only in memory because request parameters may +contain sensitive resource values. The result retains only event ID, event name, +time, event source and actor session label. Raw event JSON and request +parameters are never returned or persisted by this adapter. + +## Window ownership + +The adapter requires explicit `start_time` and `end_time`; it does not invent +a global lookback. The future orchestrator should derive the narrowest justified +window from observation history (ideally the last known clean complete +observation through the current observation) and may widen only deliberately. + +## Non-goals + +This ADR does not generalize attribution to other AWS resource types. EC2, IAM, +RDS, S3 and other services require explicit service-specific event contracts. +It also does not persist attribution yet. + +## Verification + +Unit tests must prove exact-name matching, create-vs-mutate separation, +ambiguity refusal, pagination, malformed-event failure, data minimization and +unsupported-path refusal. A separate real-AWS proof is required before the SSM +adapter can be called verified.