Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix building of GraphQL queries arguments hash #3887

Merged
merged 19 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions lib/datadog/appsec/contrib/graphql/gateway/multiplex.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ def build_arguments_hash

def arguments_from_selections(selections, query_variables, args_hash)
selections.each do |selection|
next unless selection.is_a?(::GraphQL::Language::Nodes::Field)
# rubocop:disable Style/ClassEqualityComparison
next unless selection.class.to_s == Integration::AST_NODE_CLASS_NAMES[:field]
p-datadog marked this conversation as resolved.
Show resolved Hide resolved
# rubocop:enable Style/ClassEqualityComparison

selection_name = selection.alias || selection.name

Expand All @@ -73,7 +75,9 @@ def arguments_from_selections(selections, query_variables, args_hash)

def arguments_from_directives(directives, query_variables)
directives.each_with_object({}) do |directive, args_hash|
next unless directive.is_a?(::GraphQL::Language::Nodes::Directive)
# rubocop:disable Style/ClassEqualityComparison
next unless directive.class.to_s == Integration::AST_NODE_CLASS_NAMES[:directive]
# rubocop:enable Style/ClassEqualityComparison

args_hash[directive.name] = arguments_hash(directive.arguments, query_variables)
end
Expand All @@ -86,12 +90,12 @@ def arguments_hash(arguments, query_variables)
end

def argument_value(argument, query_variables)
case argument.value
when ::GraphQL::Language::Nodes::VariableIdentifier
case argument.value.class.to_s
when Integration::AST_NODE_CLASS_NAMES[:variable_identifier]
# we need to pass query.variables here instead of query.provided_variables,
# since #provided_variables don't know anything about variable default value
query_variables[argument.value.name]
when ::GraphQL::Language::Nodes::InputObject
when Integration::AST_NODE_CLASS_NAMES[:input_object]
arguments_hash(argument.value.arguments, query_variables)
else
argument.value
Expand Down
15 changes: 14 additions & 1 deletion lib/datadog/appsec/contrib/graphql/integration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ class Integration

MINIMUM_VERSION = Gem::Version.new('2.0.19')

AST_NODE_CLASS_NAMES = {
field: 'GraphQL::Language::Nodes::Field',
directive: 'GraphQL::Language::Nodes::Directive',
variable_identifier: 'GraphQL::Language::Nodes::VariableIdentifier',
input_object: 'GraphQL::Language::Nodes::InputObject',
}.freeze

register_as :graphql, auto_patch: false

def self.version
Expand All @@ -24,13 +31,19 @@ def self.loaded?
end

def self.compatible?
super && version >= MINIMUM_VERSION
super && version >= MINIMUM_VERSION && ast_node_classes_defined?
end

def self.auto_instrument?
true
end

def self.ast_node_classes_defined?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a couple of test cases

  1. Test positive with our graphql appraisal groups.
  2. Test negative with hide_const

Copy link
Member Author

@y9v y9v Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added here: 20a00b8

AST_NODE_CLASS_NAMES.all? do |_, class_name|
Object.const_defined?(class_name)
end
end

def patcher
Patcher
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
require 'datadog/appsec/contrib/graphql/gateway/multiplex'

RSpec.describe Datadog::AppSec::Contrib::GraphQL::Gateway::Multiplex do
subject(:dd_multiplex) do
described_class.new(multiplex)
end
subject(:dd_multiplex) { described_class.new(multiplex) }

let(:schema) do
# we are only testing how arguments are extracted from the queries,
Expand Down