Skip to content

load_and_validate serves steps-less YAML as valid=True — entire validation pipeline silently bypassed #4063

Description

@Trecek

Summary

load_and_validate serves any recipe YAML that parses to a dict without a steps key as valid=True, errors=[], suggestions=[] with the raw, completely unvalidated file text as content. The entire validation pipeline — _parse_recipe, validate_recipe_structure, semantic rules, pruning, contract cards, staleness, compute_recipe_validity — runs only inside if isinstance(data, dict) and "steps" in data: (recipe/_api.py:332, installed v0.10.752; same at dev HEAD); valid is initialized True (:314) and the else branch (:458-459) records a timing metric and nothing else.

This is the fail-open mirror image of #4059 (which is a false rejection): here a garbage non-recipe is falsely accepted, defeating the #3995 fail-closed guard (valid=Truecontent non-empty both pass) and recreating the #3992 "orchestrator improvises" failure mode the guard was built to prevent.

Precise scope (empirical matrix, installed v0.10.752)

Input Outcome
dict, no steps key BYPASS — valid=True, unvalidated content served
truncated real recipe (cut before steps:) BYPASS — valid=True (1,066-byte truncation of implementation.yaml served in place of the 98,536-byte bundled recipe)
steps: mis-indented under another key BYPASS — valid=True
steps: null / steps: {} key present → pipeline runs → valid=False ("Recipe must have at least one step.") — correct
steps: list/string _parse_recipe AttributeError in _collect_recipes → file silently excluded (io.py:717-719) → RecipeNotFoundError — fail-closed but silent (adjacent gap)
top-level list / scalar / empty file _load_recipe_dict ValueError → exception handlers → valid=False — correct

Reachability and impact

Design intent (why the guard exists)

The guard originated in dd5747d35 (2026-02-23) inside the monolith's load_skill_script tool, where skipping validation of non-workflow script YAML was intentional and non-blocking. It was carried mechanically through the Feb 28 refactors (9d5f1131c, 4599e8b7f — where load_and_validate and its valid = True initialization were born) and has never been modified, documented, or tested since. load_skill_script/validate_script no longer exist. validate_from_path (the validate_recipe MCP tool, _api_listing.py:106-121) validates unconditionally and correctly rejects every bypass shape — two sibling APIs disagree about the same file.

Fix (validated; full analysis in the attached report)

Minimal diff: change _api.py:332 from if isinstance(data, dict) and "steps" in data: to if isinstance(data, dict): and delete the now-dead else (:458-459) — _load_recipe_dict already raises for non-dicts. Missing steps then flows through _parse_recipe (tolerates it via (data.get("steps") or {}).items(), io.py:505) → validate_recipe_structure error (validator.py:82-83) → compute_recipe_validityvalid=False → the #3995 guard fails closed with a named error. The post-fix pipeline was empirically executed end-to-end on a steps-less dict with zero exceptions at any stage.

Breakage: near zero, adversarially verified — no reachable bundled YAML lacks the key; campaign recipes unaffected; zero tests/fixtures depend on the else branch; identity.py:89 (same textual pattern, intentional hash-fallback semantics) must be left unchanged; migrate_recipe validates via a different path and is unaffected.

Companion tests (same change): the full malformed-input matrix through the REAL load_and_validate (bypass shapes assert valid is False + the steps error; already-fail-closed shapes pinned; one campaign-recipe case pinning the CAMPAIGN early-return); assert the cache holds valid=False and the staleness baseline is NOT refreshed post-fix.

Sequencing vs #4059 (important)

Both tickets edit load_and_validate. Implemented as the minimal condition-change diff above, the hunks do not overlap #4059's reordering (~:397-457) and merge cleanly. Do NOT implement as guard-removal-with-dedent — that would conflict across the entire block. This fix is the smaller, self-contained one and can land first.

Relationships

Investigation

Deep investigation completed interactively (5 exploration agents + adversarial challenge round + 2 post-report validators). Full report below.

Investigation: Steps-less Recipe Validation Bypass — load_and_validate Serves Unvalidated YAML as valid=True

Date: 2026-06-11
Scope: Silent fail-open in load_and_validate: a recipe YAML parsing to a dict WITHOUT a steps key skips the entire validation pipeline and is served as valid=True. Root cause, design intent, reachability/trigger surface, malformed-input behavior matrix, downstream-mitigation analysis, test gaps, and issue dedup.
Mode: Deep Analysis (1 exploration batch of 5 + adversarial challenge round + post-report validation; historical recurrence check)

Summary

load_and_validate (installed v0.10.752 recipe/_api.py; same code at dev HEAD) initializes valid = True (:314) and runs every validation stage — _parse_recipe, validate_recipe_structure, semantic rules, pruning, contract cards, staleness, compute_recipe_validity — only inside if isinstance(data, dict) and "steps" in data: (:332). The else branch (:458-459) records a timing metric and nothing else. Any YAML that parses to a dict missing the steps key is therefore served as valid=True, errors=[], suggestions=[] with the raw file text as content — fully unvalidated. The scope is precisely the missing-key case: steps: null and steps: {} have the key present, enter the pipeline, and correctly fail; non-dict YAML (list/scalar/empty file) raises ValueError into the fail-closed exception handlers. The bypass defeats the #3995 fail-closed guard (valid=True, content non-empty), is reachable through user-authored or truncated project-local recipes — including shadowing bundled recipe names (empirically demonstrated: a 1,066-byte truncation of implementation.yaml shadowed the 98,536-byte bundled recipe and was served valid=True) — flows through fleet dispatch, is written to _LOAD_CACHE, and refreshes the staleness baseline. The adversarial round additionally refuted the assumed downstream mitigation: the orchestrator prompt's open_kitchen FAILURE PREDICATE keys on the substring `--- INGREDIENTS TABLE ---` which does not literally match the formatter's actual marker (--- INGREDIENTS TABLE (display this verbatim to the user) ---; substring test = False), so no reliable catch exists on either backend. The guard is an artifact of Feb 2026 load_skill_script code (arbitrary script YAML, non-blocking semantics) carried mechanically through refactors into load_and_validate; nothing bundled, tested, or in production depends on the else branch, making the fix (validate unconditionally, mirroring validate_from_path) effectively zero-breakage.

Root Cause

  1. valid = True fail-open initialization (_api.py:314, alongside suggestions=[] :313, errors=[] :315) — established when load_and_validate was created (commit 9d5f1131c/4599e8b7f, Feb 28 2026) and never changed. [SUPPORTED]
  2. The guard if isinstance(data, dict) and "steps" in data: (_api.py:332) wraps the entire validation pipeline (_parse_recipe :333, hashes :337-349, _build_active_recipe :352-354, validate_recipe_structure :357, run_semantic_rules :397, _prune_skipped_steps :408, hidden-input interpolation :431, contract cards :438-449, staleness, diagram, compute_recipe_validity :457). The else (:458-459) is t0 = _t("yaml_parse", t0, name) — a no-op for correctness. [SUPPORTED]
  3. The guard is unnecessary for crash safety: _parse_recipe tolerates missing/null steps via (data.get("steps") or {}).items() (io.py:505); Recipe.__post_init__ does not require steps; validate_recipe_structure (validator.py:82-83) correctly emits "Recipe must have at least one step." (plus a _SKILL_HINT suffix pointing at /autoskillit:write-recipe) for empty steps. The pipeline could simply run — validated empirically end-to-end on a steps-less dict with zero exceptions at any stage. [SUPPORTED — empirically confirmed]
  4. Post-bypass, the result dict (:519-570) is {content: <raw>, errors: [], diagram, suggestions: [], valid: True, orchestration_rules: <generic>, stop_step_semantics: "", content_hash: "", composite_hash: "", recipe_version: None} with kitchen_rules, requires_packs, requires_features, ingredients_table, deferred_guards absent (their producers never ran). It is cached (_LOAD_CACHE.put :584) and, because valid=True, _refresh_staleness_baseline() runs (:586-587). [SUPPORTED]

Precise input-shape matrix (empirical, installed v0.10.752)

Input Path taken Outcome
dict, no steps key bypass (else branch) valid=True, errors=[], unvalidated content served
truncated real recipe (cut before steps:) bypass valid=True (1,066-byte truncation of implementation.yaml served)
steps: mis-indented (nested under another key) bypass valid=True
steps: null pipeline (key present) valid=False, "Recipe must have at least one step." — correct
steps: {} pipeline valid=False (standard recipes) / valid=True via RecipeKind.CAMPAIGN early-return (validator.py:77-80) for campaign recipes — correct
steps: list or string _parse_recipe AttributeError inside _collect_recipes → file silently excluded from registry (io.py:717-719, WARNING swallowed) → RecipeNotFoundError fail-closed but silent (adjacent gap)
top-level list / scalar / empty file _load_recipe_dict raises ValueError (io.py:141-144) → exception handlers (:461-493) valid=False + error-severity suggestion — correct fail-closed

Affected Components

  • src/autoskillit/recipe/_api.py (load_and_validate :313-315, :332, :458-459, :519-587): the bypass, cache write, staleness refresh [SUPPORTED]
  • src/autoskillit/server/tools/tools_kitchen.py: open_kitchen both paths (:577-585 deferred-recall, :648-656 normal) — Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995 guard (:624/:711) passes the bypass shape; ingredients_table normalized to null (:721-722); get_recipe resource (:386-414) serves raw content with no validity check [SUPPORTED]
  • src/autoskillit/server/tools/tools_recipe.py (load_recipe tool :225-239): returns valid=True result verbatim [SUPPORTED]
  • src/autoskillit/fleet/_api.py (:349-388): the valid gate passes the bypass; dispatch proceeds with a steps-less recipe [SUPPORTED]
  • src/autoskillit/recipe/io.py: _collect_recipes (:679-719) registers steps-less dicts (name truthy ⇒ registered, project-first ⇒ shadowing); swallows _parse_recipe crashes at WARNING (:717-719) [SUPPORTED]
  • src/autoskillit/cli/_prompts_orchestrator.py:159 and src/autoskillit/fleet/_prompts.py:205: FAILURE PREDICATE substring --- INGREDIENTS TABLE --- vs actual formatter marker --- INGREDIENTS TABLE (display this verbatim to the user) --- (hooks/formatters/_fmt_recipe.py:107) — literal substring match is False; the assumed mitigation is unreliable (and is itself a defect feeding T5-P1-A2-WP1 Ensure the open_kitchen failure predicate keys on a stable, backend-agnostic JSON field ("ingredients_table": null) instead of a content-formatting marker (--- INGREDIENTS TABLE ---) that is in... #3999) [SUPPORTED — verified firsthand]
  • src/autoskillit/recipe/_api_listing.py (validate_from_path :71-151): the correct sibling — validates unconditionally, rejects all bypass shapes with valid=False [SUPPORTED]
  • src/autoskillit/recipe/identity.py:89 (_load_sub_recipe_for_hash): same textual pattern, intentionally different semantics (hash-only fallback) — NOT the same defect; do not change [SUPPORTED]

Data Flow

.autoskillit/recipes/X.yaml  (name: X present, steps key ABSENT — truncation/hand-authoring/LLM generation)
  → _collect_recipes: _parse_recipe OK (steps default {}) → registered; project-first ⇒ shadows bundled name
  → load_and_validate(X):
      :310 raw = file text   :314 valid = True
      :332 if dict and "steps" in data → FALSE → else :459 (timing only)
      :519 result = {content: raw, valid: True, errors: [], suggestions: [], content_hash: "", ...}
      :584 cached            :586 staleness baseline refreshed
  → open_kitchen: #3995 guard passes (valid ∧ content) → success=true, kitchen=open, ingredients_table=null
  → orchestrator: predicate substring never matches marker text on ANY response → behavior model-dependent
  → fleet/_api.py:374 valid gate passes → dispatch proceeds with steps={}

Test Gap Analysis

  1. No test feeds any malformed YAML through load_and_validate. Its only non-happy-path tests are RecipeNotFoundError and ProcessStaleError — both exit before the guard. The exception handlers (:461-493) are never exercised via real files. [SUPPORTED]
  2. The right assertion exists at the wrong layer. test_recipe_requires_steps (tests/recipe/test_validator_structural.py:45-50) proves validate_recipe_structure catches steps-less recipes — but routes through load_recipe + validate_recipe_structure directly, never through load_and_validate, so the guard that skips that check is untested. No test asserts "validation actually ran." [SUPPORTED]
  3. All load_and_validate fixtures contain steps: (test_api.py _RECIPE_WITH_RULES, MINIMAL_RECIPE_YAML, etc.); no fixture or test depends on the else branch — a fail-closed fix breaks zero existing tests. [SUPPORTED]
  4. Rectify: Surface LoadRecipeResult Errors and Fail-Closed on Invalid Content #3995's envelope mocks never model valid=True + unvalidated content. [SUPPORTED]
  5. Path filtering is not implicated: a recipe/_api.py diff selects tests/recipe/ — the tests would run; they just don't exist. [SUPPORTED]

Similar Patterns

  • Positive model: validate_from_path (_api_listing.py:106-121) — rejects non-dicts with valid=False, then calls _parse_recipe + validate_recipe_structure unconditionally. The validate_recipe MCP tool therefore correctly returns valid=False for every bypass shape that load_and_validate accepts — two sibling APIs giving opposite answers for identical input.
  • Strict-raise idiom: migration/loader.py:114-122 raises ValueError for missing required keys; io.py:_load_recipe_dict raises for non-dict YAML. The recipe-pipeline idiom, however, is error-list → compute_recipe_validityvalid=False; the fix should follow that (per validate_from_path).
  • Same textual pattern, different intent: identity.py:89 returns None for steps-less sub-recipe YAML with an explicit content-hash fallback — correct in its context.

Design Intent Findings

  • The "steps" in data guard: introduced dd5747d35 (2026-02-23) in the monolith's load_skill_script tool, where skipping semantic validation for non-workflow script YAML was intentional and non-blocking ("parse failures don't affect load"). Carried mechanically through 9d700462a9d5f1131c4599e8b7f (2026-02-28, creation of recipe/_api.py) into load_and_validate. git log -S '"steps" in data' -- recipe/_api.py shows only the carry commit — never modified since, never documented (no ADR, no AGENTS.md entry, no docstring mention), and load_skill_script/validate_script no longer exist in the installed package. The else branch was never more than a no-op. [SUPPORTED]
  • valid = True initialization: born with load_and_validate (9d5f1131c, Feb 28 2026); the precursor returned no valid key at all. Never fail-closed. [SUPPORTED]
  • Bundled steps-less YAML: 43 files lack steps: (contracts/, methodology-traditions/, experiment-types/, _disambiguation, _ml_sub_area_folding) — ALL outside RECIPE_SCAN_DIRS (io.py:41-57), loaded by dedicated loaders, unreachable via load_and_validate. Campaign recipes use steps: {} (key present) + RecipeKind.CAMPAIGN early-return — verified empirically to remain valid through the full pipeline. No reachable production YAML depends on the bypass. [SUPPORTED]

Historical Context

External Research

No external libraries implicated. All verification was local/empirical (installed v0.10.752 interpreter probes; substring semantics of the predicate confirmed with a direct Python check: "--- INGREDIENTS TABLE ---" in "--- INGREDIENTS TABLE (display this verbatim to the user) ---"False).

Downstream Mitigation Audit (what would catch it today: nothing reliable)

Layer Catches the bypass? Why
#3995 open_kitchen guard (tools_kitchen.py:624/:711) No valid=True, content non-empty
Orchestrator/fleet FAILURE PREDICATE (substring) Unreliable predicate substring never literally matches the marker on ANY response (mismatch verified); literal compliance would halt every session, fuzzy compliance is model-dependent — not a dependable control on either backend
#3999 (proposed JSON-field predicate, open WP) Yes, prompt-soft only open_kitchen explicitly sets ingredients_table: null for this shape (:721-722); still LLM-instruction-level, and incidentally fixes the substring mismatch
#4036 _validate_result (open WP) No required keys present, success=True, content non-empty — all pass
#4000 degraded-payload halt (open WP) No targets success:false + empty content
#4008 recipe×backend matrix (open WP) No parametrizes bundled recipes only — all have steps
record_pipeline_step(init) Partial hard-fails only if active_recipe_steps is None and only if called
run_skill / lock_ingredients No no step gating without steps; ingredient lock inert when declarations absent
fleet valid gate (fleet/_api.py:374) No valid=True passes; dispatch proceeds

Scope Boundary

Investigated: the guard's full history and intent; every load_and_validate caller; recipe registry/resolution incl. project-local shadowing (empirical); 11-case malformed-input matrix (empirical); cache and staleness side effects; sub-recipe loading; schema-layer behavior; all proposed/open mitigation WPs; test inventory; issue-tracker dedup; the predicate/marker mismatch.

Not yet explored: whether any historical production session actually triggered the bypass (log mining of real sessions); LLM-generation frequency of steps-less YAML (note: write-recipe's SKILL.md does instruct a validate_recipe call — the correct validate_from_path path — so generated recipes are defended when the skill is followed; the unguarded flows are manual edits, truncated writes, and any generation path that skips the validate step); the _collect_recipes WARNING-swallow UX (steps-list/string files vanish into RecipeNotFoundError with no user-visible reason) — adjacent, smaller gap.

Recommendations

Single converged recommendation: remove the "steps" in data condition — validate unconditionally. Implement as the minimal diff: change _api.py:332 from if isinstance(data, dict) and "steps" in data: to if isinstance(data, dict): (and delete the now-dead else at :458-459 — _load_recipe_dict already raises ValueError for non-dicts, so the branch becomes unreachable). Preserving the block's indentation keeps the textual overlap with #4059 (which reorders ~:397-457 of the same function) to non-overlapping hunks; do NOT implement as guard-removal-with-dedent, which would conflict across the entire block. Missing steps then flows: _parse_recipeRecipe(steps={})validate_recipe_structure"Recipe must have at least one step." in errorscompute_recipe_validityvalid=False → the #3995 guard fails closed with a named structural error (and after #4060, a fully rendered envelope). This mirrors the established validate_from_path idiom exactly, eliminating the divergence where two sibling APIs disagree about the same file. Leave identity.py:89 unchanged (intentional hash-fallback semantics).

Companion work in the same change:

  1. Regression tests through the REAL load_and_validate: dict-without-steps, truncated-before-steps, mis-indented steps (assert valid is False + the steps error), plus the already-fail-closed shapes (steps: null/{}/list/scalar/empty) to pin the whole matrix, and one campaign-recipe case to pin the RecipeKind.CAMPAIGN early-return.
  2. Assert the cache is not poisoned: post-fix, the steps-less result cached is valid=False and _refresh_staleness_baseline is NOT called.
  3. Out-of-scope notes to carry on the ticket: the predicate/marker substring mismatch (hand to T5-P1-A2-WP1 Ensure the open_kitchen failure predicate keys on a stable, backend-agnostic JSON field ("ingredients_table": null) instead of a content-formatting marker (--- INGREDIENTS TABLE ---) that is in... #3999 — it makes the current prompt predicate unreliable for ALL failures, not just this one); the _collect_recipes WARNING-swallow (io.py:717-719) leaving users with bare RecipeNotFoundError for steps-list/string files; setup-project-generated recipes that skip write-recipe's explicit validate_recipe step have no post-generation validation gate.

Killed alternatives:

Breakage Analysis

  • Recommendation: remove the "steps" in data guard in load_and_validate (changes an existing mechanism)
    • Breakage surface: near zero, adversarially verified. No bundled YAML reachable via load_and_validate lacks the steps key (43 steps-less bundled files are all outside RECIPE_SCAN_DIRS); campaign recipes (steps: {}) verified empirically to pass the full pipeline via the RecipeKind.CAMPAIGN early-return; load_skill_script/validate_script (the guard's original beneficiaries) no longer exist; zero tests or fixtures depend on the else branch; identity.py:89 is a separate function left untouched. Behavioral change is confined to inputs that are today served unvalidated: they will start returning valid=False with a named error — the intended outcome.
    • Prior reverts: none — git log -S '"steps" in data' shows only the refactor carry commit (4599e8b7f); the mechanism has never been fixed or reverted.
    • Downstream contract violations: none — LoadRecipeResult shape unchanged; consumers already handle valid=False (and Recipe validation envelope hides semantic errors behind 'unknown structural error' #4060 improves its rendering). Adjacent paths confirmed unaffected: migrate_recipe validates via load_recipe + validate_recipe_structure directly (never load_and_validate); the _prompts_orchestrator.py:44-53 ingredients_table consumer receives None both pre- and post-fix.
    • Sequencing vs Pre-prune validity computation breaks Codex open_kitchen and fleet dispatch #4059: both tickets edit load_and_validate. Implemented as the minimal condition-change diff (above), the hunks do not overlap Pre-prune validity computation breaks Codex open_kitchen and fleet dispatch #4059's reordering (~:397-457) and merge cleanly; a dedent-style implementation would instead produce a substantial manual conflict — avoid it. This fix is the smaller, self-contained one and can land first.
    • Risk level: LOW.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugExisting behavior is brokenrecipe:remediationRoute: investigate/decompose before implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions