From 212eaf80131dcb66e57d055a2930b8335011e05e Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 28 May 2025 15:37:00 -0400 Subject: [PATCH 1/2] Properly validate SDL directive arguments --- lib/graphql/schema/directive.rb | 11 +++++++ lib/graphql/schema/input_object.rb | 4 +++ spec/graphql/schema/directive_spec.rb | 42 ++++++++++++++++++++++++--- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/lib/graphql/schema/directive.rb b/lib/graphql/schema/directive.rb index 1bed986dd7..90f05b69ec 100644 --- a/lib/graphql/schema/directive.rb +++ b/lib/graphql/schema/directive.rb @@ -111,6 +111,9 @@ def inherited(subclass) # @return [GraphQL::Interpreter::Arguments] attr_reader :arguments + class InvalidArgumentError < GraphQL::Error + end + def initialize(owner, **arguments) @owner = owner assert_valid_owner @@ -119,6 +122,14 @@ def initialize(owner, **arguments) # - lazy resolution # Probably, those won't be needed here, since these are configuration arguments, # not runtime arguments. + self.class.all_argument_definitions.each do |arg_defn| + value = arguments[arg_defn.keyword] + arg_type = arg_defn.type + result = arg_defn.type.validate_input(value, Query::NullContext.instance) + if !result.valid? + raise InvalidArgumentError, "@#{graphql_name}.#{arg_defn.graphql_name} on #{owner.path} is invalid (#{value.inspect}): #{result.problems.first["explanation"]}" + end + end @arguments = self.class.coerce_arguments(nil, arguments, Query::NullContext.instance) end diff --git a/lib/graphql/schema/input_object.rb b/lib/graphql/schema/input_object.rb index 0e8e8631d1..05420201b4 100644 --- a/lib/graphql/schema/input_object.rb +++ b/lib/graphql/schema/input_object.rb @@ -182,6 +182,10 @@ def validate_non_null_input(input, ctx, max_errors: nil) input.each do |argument_name, value| argument = types.argument(self, argument_name) + if argument.nil? && ctx.is_a?(Query::NullContext) && argument_name.is_a?(Symbol) + # Validating definition directive arguments which come in as Symbols + argument = types.arguments(self).find { |arg| arg.keyword == argument_name } + end # Items in the input that are unexpected if argument.nil? result ||= Query::InputValidationResult.new diff --git a/spec/graphql/schema/directive_spec.rb b/spec/graphql/schema/directive_spec.rb index 0660ef5451..c57c34dfd5 100644 --- a/spec/graphql/schema/directive_spec.rb +++ b/spec/graphql/schema/directive_spec.rb @@ -55,7 +55,7 @@ class Thing < GraphQL::Schema::Object end it "validates arguments" do - err = assert_raises ArgumentError do + err = assert_raises GraphQL::Schema::Directive::InvalidArgumentError do GraphQL::Schema::Field.from_options( name: :something, type: String, @@ -65,9 +65,9 @@ class Thing < GraphQL::Schema::Object ) end - assert_equal "@secret.topSecret is required, but no value was given", err.message + assert_equal "@secret.topSecret on Thing.something is invalid (nil): Expected value to not be null", err.message - err2 = assert_raises ArgumentError do + err2 = assert_raises GraphQL::Schema::Directive::InvalidArgumentError do GraphQL::Schema::Field.from_options( name: :something, type: String, @@ -77,7 +77,7 @@ class Thing < GraphQL::Schema::Object ) end - assert_equal "@secret.topSecret is required, but no value was given", err2.message + assert_equal "@secret.topSecret on Thing.something is invalid (12.5): Could not coerce value 12.5 to Boolean", err2.message end describe 'repeatable directives' do @@ -400,4 +400,38 @@ def numbers enum_value = schema.get_type("Stuff").values["THING"] assert_equal [["tag", { name: "t7"}], ["tag", { name: "t8"}]], enum_value.directives.map { |dir| [dir.graphql_name, dir.arguments.to_h] } end + + describe "Validating schema directives" do + def build_sdl(size:) + <<~GRAPHQL + directive @tshirt(size: Size!) on INTERFACE | OBJECT + + type MyType @tshirt(size: #{size}) { + color: String + } + + type Query { + myType: MyType + } + + enum Size { + LARGE + MEDIUM + SMALL + } + GRAPHQL + end + + it "Raises a nice error for invalid enum values" do + valid_sdl = build_sdl(size: "MEDIUM") + assert_equal valid_sdl, GraphQL::Schema.from_definition(valid_sdl).to_definition + + typo_sdl = build_sdl(size: "BLAH") + err = assert_raises GraphQL::Schema::Directive::InvalidArgumentError do + GraphQL::Schema.from_definition(typo_sdl) + end + expected_msg = '@tshirt.size on MyType is invalid ("BLAH"): Expected "BLAH" to be one of: LARGE, MEDIUM, SMALL' + assert_equal expected_msg, err.message + end + end end From 4dd3ec7fe36f2dc9263abbe60150cd1c1b8e7c25 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 28 May 2025 16:04:38 -0400 Subject: [PATCH 2/2] Support custom validators on directives --- lib/graphql/execution/interpreter/runtime.rb | 7 +++ lib/graphql/schema/directive.rb | 15 ++++-- spec/graphql/schema/directive_spec.rb | 50 ++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 75429d5dc9..14d022f4b4 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -826,6 +826,13 @@ def run_directive(method_name, object, directives, idx, &block) else dir_defn = @schema_directives.fetch(dir_node.name) raw_dir_args = arguments(nil, dir_defn, dir_node) + if !raw_dir_args.is_a?(GraphQL::ExecutionError) + begin + dir_defn.validate!(raw_dir_args, context) + rescue GraphQL::ExecutionError => err + raw_dir_args = err + end + end dir_args = continue_value( raw_dir_args, # value nil, # field diff --git a/lib/graphql/schema/directive.rb b/lib/graphql/schema/directive.rb index 90f05b69ec..1cee5c4ee2 100644 --- a/lib/graphql/schema/directive.rb +++ b/lib/graphql/schema/directive.rb @@ -9,6 +9,7 @@ class Schema class Directive < GraphQL::Schema::Member extend GraphQL::Schema::Member::HasArguments extend GraphQL::Schema::Member::HasArguments::HasDirectiveArguments + extend GraphQL::Schema::Member::HasValidators class << self # Directives aren't types, they don't have kinds. @@ -75,6 +76,10 @@ def resolve_each(object, arguments, context) yield end + def validate!(arguments, context) + Schema::Validator.validate!(validators, self, context, arguments) + end + def on_field? locations.include?(FIELD) end @@ -122,15 +127,19 @@ def initialize(owner, **arguments) # - lazy resolution # Probably, those won't be needed here, since these are configuration arguments, # not runtime arguments. + context = Query::NullContext.instance self.class.all_argument_definitions.each do |arg_defn| value = arguments[arg_defn.keyword] - arg_type = arg_defn.type - result = arg_defn.type.validate_input(value, Query::NullContext.instance) + result = arg_defn.type.validate_input(value, context) if !result.valid? raise InvalidArgumentError, "@#{graphql_name}.#{arg_defn.graphql_name} on #{owner.path} is invalid (#{value.inspect}): #{result.problems.first["explanation"]}" end end - @arguments = self.class.coerce_arguments(nil, arguments, Query::NullContext.instance) + self.class.validate!(arguments, context) + @arguments = self.class.coerce_arguments(nil, arguments, context) + if @arguments.is_a?(GraphQL::ExecutionError) + raise @arguments + end end def graphql_name diff --git a/spec/graphql/schema/directive_spec.rb b/spec/graphql/schema/directive_spec.rb index c57c34dfd5..e5328ab3cf 100644 --- a/spec/graphql/schema/directive_spec.rb +++ b/spec/graphql/schema/directive_spec.rb @@ -401,6 +401,56 @@ def numbers assert_equal [["tag", { name: "t7"}], ["tag", { name: "t8"}]], enum_value.directives.map { |dir| [dir.graphql_name, dir.arguments.to_h] } end + describe "Custom validations on definition directives" do + class DirectiveValidationSchema < GraphQL::Schema + class ValidatedDirective < GraphQL::Schema::Directive + locations OBJECT, FIELD + argument :f, Float, required: false, validates: { numericality: { greater_than: 0 } } + argument :s, String, required: false, validates: { format: { with: /^[a-z]{3}$/ } } + validates required: { one_of: [:f, :s]} + end + + class Query < GraphQL::Schema::Object + field :i, Int, fallback_value: 100 + end + + query(Query) + directive(ValidatedDirective) + end + + it "runs custom validation during execution" do + f_err_res = DirectiveValidationSchema.execute("{ i @validatedDirective(f: -10) }") + assert_equal [{"message" => "f must be greater than 0", "locations" => [{"line" => 1, "column" => 5}], "path" => ["i"]}], f_err_res["errors"] + + s_err_res = DirectiveValidationSchema.execute("{ i @validatedDirective(s: \"wnrn\") }") + assert_equal [{"message" => "s is invalid", "locations" => [{"line" => 1, "column" => 5}], "path" => ["i"]}], s_err_res["errors"] + + f_s_err_res = DirectiveValidationSchema.execute("{ i @validatedDirective }") + assert_equal [{"message" => "validatedDirective must include exactly one of the following arguments: f, s.", "locations" => [{"line" => 1, "column" => 5}], "path" => ["i"]}], f_s_err_res["errors"] + end + + it "runs custom validation during definition" do + obj_type = Class.new(GraphQL::Schema::Object) + directive_defn = DirectiveValidationSchema::ValidatedDirective + obj_type.directive(directive_defn, f: 1) + f_err = assert_raises GraphQL::Schema::Validator::ValidationFailedError do + obj_type.directive(directive_defn, f: -1) + end + assert_equal "f must be greater than 0", f_err.message + + obj_type.directive(directive_defn, s: "abc") + s_err = assert_raises GraphQL::Schema::Validator::ValidationFailedError do + obj_type.directive(directive_defn, s: "defg") + end + assert_equal "s is invalid", s_err.message + + required_err = assert_raises GraphQL::Schema::Validator::ValidationFailedError do + obj_type.directive(directive_defn) + end + assert_equal "validatedDirective must include exactly one of the following arguments: f, s.", required_err.message + end + end + describe "Validating schema directives" do def build_sdl(size:) <<~GRAPHQL