diff --git a/CHANGELOG.md b/CHANGELOG.md index 0150cb8bc..966235af9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Documentation + +- **Validation Gate Empty-Run Guard** (#9285): Documented `ApplyValidationSummary` + and its `all_required_passed` property in `docs/reference/validation_pipeline.md`, + `docs/reference/plan_apply.md`, and `docs/specification.md`. The property now + requires `required_count > 0` (at least one required validation was actually run) + AND `required_failed == 0` — previously it returned `True` trivially when no + validations ran (security bypass). Added property reference table, empty-run guard + warning admonition, truth table, and implementation snippet. Addresses spec gap + identified in [AUTO-SPEC-3] / issue #9285; relates to fix in issue #7508 + (commit `b84022795`). + ### Fixed - **Automation Profile Silent Fallback** (#8232): `_resolve_profile_for_plan` in diff --git a/docs/reference/plan_apply.md b/docs/reference/plan_apply.md index b53ea8045..a9d73fc16 100644 --- a/docs/reference/plan_apply.md +++ b/docs/reference/plan_apply.md @@ -128,12 +128,25 @@ file changes are committed, `apply_with_validation_gate()` checks the validation summary (either from the plan or supplied externally) and blocks apply if any **required** validations have failed. +#### Empty-Run Guard + +!!! warning "Apply is blocked when no validations ran" + `ApplyValidationSummary.all_required_passed` returns `False` when zero + validations were run (`is_empty == True`), even if `required_failed == 0`. + This prevents a security bypass where a plan with no attached validations + (or whose attachments were silently skipped) could proceed to apply without + any validation checks having been performed. + + If a plan legitimately requires no validations, the `allow_empty` flag on + `apply_with_validation_gate()` can be used to bypass the ChangeSet empty + guard, but the validation gate itself is governed by `all_required_passed`. + #### Outcomes | Outcome | Description | |---------|-------------| | `applied` | All required validations passed; changes committed | -| `constrained` | Required validations failed; apply blocked | +| `constrained` | Required validations failed **or** no validations ran; apply blocked | | `already_applied` | Plan was already in a terminal applied state | | `blocked_empty` | ChangeSet was empty and `allow_empty` not set | @@ -146,6 +159,8 @@ apply_with_validation_gate(plan_id) | +-- Empty ChangeSet? --> blocked_empty (unless allow_empty) | + +-- summary.is_empty? --> constrained ("no validations ran") + | +-- required_failed > 0? --> constrained (with actionable message) | +-- All passed --> persist_apply_summary() --> complete_apply() diff --git a/docs/reference/validation_pipeline.md b/docs/reference/validation_pipeline.md index eaacc5613..08c574c16 100644 --- a/docs/reference/validation_pipeline.md +++ b/docs/reference/validation_pipeline.md @@ -216,9 +216,50 @@ Per-validation execution result. Aggregated results with gating decision. -**Properties:** `total`, `required_passed`, `required_failed`, -`informational_passed`, `informational_failed`, `all_required_passed`, -`is_empty` +| Property | Type | Description | +|---------------------------|--------|------------------------------------------------------------------------------| +| `total` | `int` | Total number of validations run | +| `required_passed` | `int` | Number of required validations that passed | +| `required_total` | `int` | Total number of required validations evaluated | +| `required_failed` | `int` | Number of required validations that failed | +| `informational_passed` | `int` | Number of informational validations that passed | +| `informational_failed` | `int` | Number of informational validations that failed | +| `all_required_passed` | `bool` | `True` only when at least one validation ran **and** no required ones failed | +| `is_empty` | `bool` | `True` when zero validations were run (`total == 0`) | + +#### `all_required_passed` — Empty-Run Guard + +!!! warning "Security: Empty-Run Guard" + `all_required_passed` returns `False` whenever `is_empty` is `True` (i.e., zero + validations were run), **even if `required_failed == 0`**. + + **Why this matters**: Without this guard, a plan with no attached validations + (or whose attachments were silently skipped) would trivially pass the apply gate, + bypassing all validation checks. The empty-run guard ensures that apply is only + permitted when at least one validation was actually executed. + +The full evaluation logic is: + +```python +@property +def all_required_passed(self) -> bool: + if self.is_empty: # zero validations ran → block apply + return False + return self.required_failed == 0 +``` + +| Condition | `all_required_passed` | Outcome | +|-----------------------------------------------|-----------------------|-------------------| +| No validations ran (`total == 0`) | `False` | Apply **blocked** | +| Required validations ran, none failed | `True` | Apply **allowed** | +| Required validations ran, at least one failed | `False` | Apply **blocked** | +| Only informational validations ran | `True` | Apply **allowed** | + +> **Note**: A summary containing only informational validations (no required ones) with +> `total > 0` will have `required_failed == 0` and `is_empty == False`, so +> `all_required_passed` returns `True`. This is intentional — if no required validations +> are attached, there is nothing to block on. The empty-run guard specifically targets +> the case where *no validations at all* were run. **Methods:** - `to_plan_metadata()` — Dict for plan `validation_summary` field diff --git a/docs/specification.md b/docs/specification.md index e16c06ad1..7b645d162 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -22615,7 +22615,8 @@ Validation is the **final step** of the Execute phase. The execution actor's wor 2. **Collect applicable validations**: The system resolves all validations from the resource-direct, project, and plan scopes (as described in Attachment Resolution above). 3. **Execute validations**: Each applicable validation is invoked as a standard tool call. The validation reads the sandbox state and returns its structured result. Validations may be run in parallel since they are read-only and cannot interfere with each other. 4. **Process results**: - - **All required validations pass**: Execution is complete. The plan transitions to the review/Apply phase. + - **All required validations pass (and at least one ran)**: Execution is complete. The plan transitions to the review/Apply phase. + - **No validations ran** (`ApplyValidationSummary.is_empty == True`): Execution is **blocked** — the apply gate requires at least one validation to have actually executed. This prevents a silent bypass where a plan with no attached validations could proceed to apply unchecked. See [Empty-Run Guard](#empty-run-guard) below. - **Any required validation fails**: The execution actor enters the fix-then-revalidate loop (see Validation Failure Handling below). - **Informational validations fail**: Results are recorded in the plan's validation summary. No fix attempts are made. @@ -22628,6 +22629,42 @@ Validation is the **final step** of the Execute phase. The execution actor's wor **Rationale**: Introducing validation during Apply would create situations where committed changes might need to be rolled back, defeating the purpose of the sandbox model. +#### Empty-Run Guard + +!!! warning "Apply gate requires at least one validation to have run" + `ApplyValidationSummary.all_required_passed` returns `False` when the + validation result set is empty (`is_empty == True`), **regardless of + `required_failed`**. This is the **empty-run guard**. + +**Rationale**: Prior to this guard, `all_required_passed` returned `True` +trivially when zero validations ran (because `required_failed == 0` with an +empty result set). This created a security bypass: a plan with no attached +validations — or one whose attachments were silently dropped — could proceed +to apply without any validation checks having been performed. + +The guard was introduced in commit `b84022795` (fix: validation_apply.py +`all_required_passed` returns `True` when zero validations run, issue #7508). + +**Behaviour summary**: + +| Validation result set | `is_empty` | `required_failed` | `all_required_passed` | +|-----------------------|------------|-------------------|-----------------------| +| Empty (no validations ran) | `True` | 0 | `False` — apply **blocked** | +| Non-empty, no required failures | `False` | 0 | `True` — apply **allowed** | +| Non-empty, required failure(s) | `False` | > 0 | `False` — apply **blocked** | + +**Implementation** (`ApplyValidationSummary.all_required_passed`): + +```python +@property +def all_required_passed(self) -> bool: + if self.is_empty: # zero validations ran → block apply + return False + return self.required_failed == 0 +``` + +See also: [`ApplyValidationSummary` reference](../reference/validation_pipeline.md#applyvalidationsummary). + #### Validation Failure Handling Because validations are tools, their execution follows the standard tool invocation flow. Each validation returns a structured JSON result with `{ "passed": true/false, "message": "...", "data": {...} }`. The execution actor interprets these results based on the validation's `mode`.