From 6a1ae8d5d1462c3a259c50fad3f70e25c289229b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 10:21:21 +0000 Subject: [PATCH] Validate recipes before pipeline execution Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01H8NygEHhT3zGMnBmQF2F6Q --- doc/VERSIONS | 9 +-- lib/automatic/pipeline.rb | 18 ++++- lib/automatic/recipe.rb | 46 ++++++++++- spec/lib/automatic/pipeline_spec.rb | 84 ++++++++++++++++++++- spec/lib/automatic/recipe_safety_spec.rb | 69 ++++++++++++++++- spec/user_dir/plugins/filter/load_marker.rb | 27 +++++++ 6 files changed, 239 insertions(+), 14 deletions(-) create mode 100644 spec/user_dir/plugins/filter/load_marker.rb diff --git a/doc/VERSIONS b/doc/VERSIONS index 0be9656..bb9f7ef 100644 --- a/doc/VERSIONS +++ b/doc/VERSIONS @@ -3,13 +3,8 @@ automaticruby Repository Version History v26.09 (Release Date: TBD) -------------------------- -- Refresh the bundled LDRFullFeed siteinfo database from the newer upstream snapshot. -- Add FilterLimit, FilterBatch and FilterPresent for limiting, batching and required-field filtering. -- Add per-fetch interval handling to FilterFullFeed, FilterImageSource, and FilterDescriptionLink. -- Make doc/PLUGINS.md section 6 the single source of truth for the current plugin catalogue. -- Fix CLI contract drift so inspect parses the first discovered feed and scaffold restores missing bundled siteinfo and example configuration without overwriting existing user data. -- Distinguish missing optional dependencies from unrelated load failures so CLI diagnostics and plugin-spec skips do not hide broken loads. -- Preserve standard item metadata when FeedMaker rebuilds pipelines so filters do not discard source, enclosure or full content. +- Add FilterLimit, FilterBatch and FilterPresent; add per-fetch interval handling to FilterFullFeed, FilterImageSource and FilterDescriptionLink; refresh the bundled LDRFullFeed siteinfo database; make doc/PLUGINS.md section 6 the single source of truth for the plugin catalogue. +- Harden the framework's execution boundary: fix CLI inspect/scaffold contract drift, distinguish missing optional dependencies from unrelated load failures, preserve standard item metadata when FeedMaker rebuilds pipelines, and validate Recipe plugin entries with a discovery preflight before any plugin runs. v26.08 (2026-08-22) ------------------- diff --git a/lib/automatic/pipeline.rb b/lib/automatic/pipeline.rb index 26dec24..cc0bd35 100644 --- a/lib/automatic/pipeline.rb +++ b/lib/automatic/pipeline.rb @@ -5,12 +5,19 @@ # License:: The GPL version 3, or LGPL version 3 (Dual License). # Contact:: idnanashi@gmail.com # Created:: Feb 22, 2012 -# Updated:: Aug 14, 2026 +# Updated:: Sep 6, 2026 # Copyright:: Copyright (c) 2012-2026 Automatic Ruby Developers. # # The core: resolve a plugin's class name to a file, then run the Recipe's # plugins in order, each receiving the previous one's output. See # doc/BASIC_DESIGN.md section 4.6 and doc/PLUGINS.md section 3. +# +# Every module the Recipe names is discovered before any plugin runs, so that +# an unknown module later in the Recipe is refused before an earlier plugin's +# side effect, not after it. Discovery only registers an autoload; it does not +# read a plugin's own source, which stays lazy until that plugin's turn to +# run. See doc/PLUGINS.md on unknown plugin being refused before any plugin +# runs. require 'active_support/core_ext/string/inflections' @@ -51,10 +58,13 @@ def load_plugin(module_name) def run(recipe) raise NoRecipeError, 'no recipe given' if recipe.nil? + entries = [] + recipe.each_plugin { |plugin| entries << [plugin, plugin.module] } + + entries.each { |_plugin, mod| load_plugin(mod) } + pipeline = [] - recipe.each_plugin do |plugin| - mod = plugin.module - load_plugin(mod) + entries.each do |plugin, mod| klass = Automatic::Plugin.const_get(mod) pipeline = klass.new(plugin.config, pipeline).run end diff --git a/lib/automatic/recipe.rb b/lib/automatic/recipe.rb index 53ec4cd..0e9dfde 100644 --- a/lib/automatic/recipe.rb +++ b/lib/automatic/recipe.rb @@ -5,11 +5,16 @@ # License:: The GPL version 3, or LGPL version 3 (Dual License). # Contact:: idnanashi@gmail.com # Created:: Feb 18, 2012 -# Updated:: Aug 14, 2026 +# Updated:: Sep 6, 2026 # Copyright:: Copyright (c) 2012-2026 Automatic Ruby Developers. # # Turns a Recipe file into something the pipeline can iterate. The format is # specified in doc/PLUGINS.md section 2. +# +# Each plugin entry's documented shape -- a mapping naming a module, with an +# optional config mapping -- is validated here, at load time, rather than +# left to surface as whatever internal exception a malformed entry happens to +# raise once the pipeline reaches it. require 'date' require 'hashie' @@ -45,6 +50,8 @@ def load_recipe(path) "recipe #{resolved} has no plugins sequence" end + validate_plugins(resolved) + Automatic::Log.level(@procedure.global&.log&.level) Automatic::Log.puts('info', "Loading Recipe: #{resolved}") @procedure @@ -72,5 +79,42 @@ def parse(path) aliases: true ) end + + # Only the shape doc/PLUGINS.md documents is checked here: a mapping + # naming a module, with an optional config mapping. What a plugin does + # with its own config is that plugin's concern, not this one's. + def validate_plugins(resolved) + @procedure.plugins.each_with_index do |plugin, index| + validate_plugin_entry(resolved, plugin, index) + end + end + + def validate_plugin_entry(resolved, plugin, index) + unless plugin.is_a?(Hash) + raise InvalidRecipeError, + "recipe #{resolved} plugins[#{index}] is not a mapping" + end + + validate_module_name(resolved, plugin, index) + validate_plugin_config(resolved, plugin, index) + end + + def validate_module_name(resolved, plugin, index) + name = plugin['module'] + + return if name.is_a?(String) && !name.strip.empty? + + raise InvalidRecipeError, + "recipe #{resolved} plugins[#{index}] has no module name" + end + + def validate_plugin_config(resolved, plugin, index) + config = plugin['config'] + + return if config.nil? || config.is_a?(Hash) + + raise InvalidRecipeError, + "recipe #{resolved} plugins[#{index}] has a config that is not a mapping" + end end end diff --git a/spec/lib/automatic/pipeline_spec.rb b/spec/lib/automatic/pipeline_spec.rb index 0bde425..ed1b411 100644 --- a/spec/lib/automatic/pipeline_spec.rb +++ b/spec/lib/automatic/pipeline_spec.rb @@ -5,7 +5,7 @@ # License:: The GPL version 3, or LGPL version 3 (Dual License). # Contact:: idnanashi@gmail.com # Created:: Mar 10, 2012 -# Updated:: Feb 25, 2014 +# Updated:: Sep 6, 2026 # Copyright:: Copyright (c) 2012-2026 Automatic Ruby Developers. require File.expand_path(File.join(File.dirname(__FILE__) ,'../../spec_helper')) @@ -46,6 +46,49 @@ recipe.should_receive(:each_plugin).and_yield(plugin) Automatic::Pipeline.run(recipe).should == [] end + + # Every module a Recipe names is discovered before any plugin runs, so + # that an unknown module later on is refused before an earlier plugin's + # side effect, not after it; doc/PLUGINS.md documents unknown plugin as + # refused before any plugin runs, wherever in the Recipe it is. + it "refuses an unknown module before running a plugin that precedes it" do + good = double("plugin", :module => "FilterIgnore") + bad = double("plugin", :module => "NoSuchPluginAtAll") + recipe = double("recipe") + recipe.should_receive(:each_plugin).and_yield(good).and_yield(bad) + + expect(good).not_to receive(:config) + + lambda { + Automatic::Pipeline.run(recipe) + }.should raise_exception(Automatic::NoPluginError, + /unknown plugin named NoSuchPluginAtAll/) + end + + it "refuses an unknown module at the front before a plugin behind it runs" do + bad = double("plugin", :module => "NoSuchPluginAtAll") + good = double("plugin", :module => "FilterIgnore") + recipe = double("recipe") + recipe.should_receive(:each_plugin).and_yield(bad).and_yield(good) + + expect(good).not_to receive(:config) + + lambda { + Automatic::Pipeline.run(recipe) + }.should raise_exception(Automatic::NoPluginError, + /unknown plugin named NoSuchPluginAtAll/) + end + + it "runs every plugin once all named modules are discoverable" do + first = double("plugin", :module => "FilterIgnore") + second = double("plugin", :module => "FilterOne") + first.should_receive(:config) + second.should_receive(:config) + recipe = double("recipe") + recipe.should_receive(:each_plugin).and_yield(first).and_yield(second) + + Automatic::Pipeline.run(recipe) + end end end @@ -78,5 +121,44 @@ expect(pipeline.first.items.first.title).to eq "[News] Title" end end + + # FilterLoadMarker (spec/user_dir/plugins/filter/load_marker.rb) has no + # behaviour of its own; it exists only to be discoverable, so that its + # presence in $LOADED_FEATURES tells these two examples whether its file + # was actually read, as distinct from merely registered for autoload. + # Referencing Automatic::Plugin::FilterLoadMarker directly would load it + # and defeat that, so neither example does. + describe "#run and discovery preflight" do + let(:fixture_path) { + File.join(APP_ROOT, "spec", "user_dir", "plugins", "filter", "load_marker.rb") + } + + it "does not load a discoverable plugin's source during preflight, even when an unknown module follows it" do + expect($LOADED_FEATURES).not_to include(fixture_path) + + good = double("plugin", :module => "FilterLoadMarker") + bad = double("plugin", :module => "NoSuchPluginAtAll") + recipe = double("recipe") + recipe.should_receive(:each_plugin).and_yield(good).and_yield(bad) + + lambda { + Automatic::Pipeline.run(recipe) + }.should raise_exception(Automatic::NoPluginError, + /unknown plugin named NoSuchPluginAtAll/) + + expect($LOADED_FEATURES).not_to include(fixture_path) + end + + it "loads a discoverable plugin's source only once execution reaches it" do + plugin = double("plugin", :module => "FilterLoadMarker") + plugin.should_receive(:config) + recipe = double("recipe") + recipe.should_receive(:each_plugin).and_yield(plugin) + + Automatic::Pipeline.run(recipe) + + expect($LOADED_FEATURES).to include(fixture_path) + end + end end end diff --git a/spec/lib/automatic/recipe_safety_spec.rb b/spec/lib/automatic/recipe_safety_spec.rb index 1adf686..303bd5a 100644 --- a/spec/lib/automatic/recipe_safety_spec.rb +++ b/spec/lib/automatic/recipe_safety_spec.rb @@ -5,7 +5,7 @@ # License:: The GPL version 3, or LGPL version 3 (Dual License). # Contact:: idnanashi@gmail.com # Created:: Aug 14, 2026 -# Updated:: Aug 14, 2026 +# Updated:: Sep 6, 2026 # Copyright:: Copyright (c) 2012-2026 Automatic Ruby Developers. require File.expand_path(File.join(File.dirname(__FILE__), '../../spec_helper')) @@ -51,6 +51,62 @@ def recipe(body) body = "plugins:\n - module: PublishConsole\n config: !ruby/object:Struct {}\n" expect { recipe(body) }.to raise_error Psych::DisallowedClass end + + # doc/PLUGINS.md documents each plugins entry as a mapping naming a + # module, with an optional config mapping. A malformed entry is refused + # here, at load time, rather than left to surface as whatever internal + # exception it happens to raise once the pipeline reaches it. + it "refuses a plugin entry that is a scalar" do + expect { recipe("plugins:\n - nope\n") }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] is not a mapping/ + end + + it "refuses a plugin entry that is a sequence" do + expect { recipe("plugins:\n - [FilterOne]\n") }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] is not a mapping/ + end + + it "refuses a plugin entry with no module" do + body = "plugins:\n - config:\n foo: bar\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] has no module name/ + end + + it "refuses a plugin entry whose module is not a string" do + body = "plugins:\n - module: 42\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] has no module name/ + end + + it "refuses a plugin entry whose module is an empty string" do + body = "plugins:\n - module: ''\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] has no module name/ + end + + it "refuses a plugin entry whose module is whitespace only" do + body = "plugins:\n - module: \" \"\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] has no module name/ + end + + it "refuses a plugin entry whose config is a scalar" do + body = "plugins:\n - module: FilterOne\n config: nope\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] has a config that is not a mapping/ + end + + it "refuses a plugin entry whose config is a sequence" do + body = "plugins:\n - module: FilterOne\n config:\n - invalid\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[0\] has a config that is not a mapping/ + end + + it "identifies a malformed entry after valid ones by its own index" do + body = "plugins:\n - module: FilterOne\n - module: FilterClear\n - nope\n" + expect { recipe(body) }. + to raise_error Automatic::InvalidRecipeError, /plugins\[2\] is not a mapping/ + end end describe "what it accepts" do @@ -73,6 +129,17 @@ def recipe(body) expect(recipe(body).each_plugin.first.config).to be_nil end + it "accepts an entry with config explicitly set to null" do + body = "plugins:\n - module: FilterClear\n config: null\n" + expect(recipe(body).each_plugin.first.config).to be_nil + end + + it "accepts the same module named more than once" do + body = "plugins:\n - module: FilterOne\n - module: FilterOne\n" + expect(recipe(body).each_plugin.map { |plugin| plugin.module }). + to eq %w[FilterOne FilterOne] + end + # Aliases let a block of settings be shared between plugins, which is a # legitimate use and is documented in doc/PLUGINS.md section 2.6. it "resolves YAML aliases" do diff --git a/spec/user_dir/plugins/filter/load_marker.rb b/spec/user_dir/plugins/filter/load_marker.rb new file mode 100644 index 0000000..fd52ff0 --- /dev/null +++ b/spec/user_dir/plugins/filter/load_marker.rb @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +# Name:: Automatic::Plugin::FilterLoadMarker +# Author: id774 (More info: http://id774.net) +# Source Code:: https://github.com/id774/automaticruby +# License:: The GPL version 3, or LGPL version 3 (Dual License). +# Contact:: idnanashi@gmail.com +# Created:: Sep 6, 2026 +# Updated:: Sep 6, 2026 +# Copyright:: Copyright (c) 2012-2026 Automatic Ruby Developers. +# +# A plugin with no behaviour of its own: this file's only purpose is to be +# discoverable so that spec/lib/automatic/pipeline_spec.rb can tell "the +# module is registered for autoload" apart from "the file has actually been +# read", by checking $LOADED_FEATURES for this path rather than by touching +# the constant, which would load it and defeat the point. + +module Automatic::Plugin + class FilterLoadMarker + def initialize(config, pipeline = []) + @pipeline = pipeline + end + + def run + @pipeline + end + end +end