From 8068a76e7115e31ddc4ea53562ce7dc6b10d5410 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 25 Feb 2026 10:15:26 +0000 Subject: [PATCH] Validate constant directive values on variable definitions (#3927) Per the GraphQL spec, directives applied to variable definitions accept only constant values. Variable references like $v are not allowed in directive arguments on variable definitions. For example, the query `query ($v:Int @dir(arg:$v)) { ... }` should be rejected. Add a new validation rule VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS that detects variable references inside directive arguments on variable definitions and reports a VariableNotAllowed error. https://claude.ai/code/session_01QH4Ce5Fs9zfEjktZV4j1i2 --- .../validation/OperationValidationRule.java | 4 + .../validation/OperationValidator.java | 39 +++++- src/main/resources/i18n/Validation.properties | 2 + ...sNotAllowedInConstantDirectivesTest.groovy | 130 ++++++++++++++++++ 4 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 src/test/groovy/graphql/validation/VariablesNotAllowedInConstantDirectivesTest.groovy diff --git a/src/main/java/graphql/validation/OperationValidationRule.java b/src/main/java/graphql/validation/OperationValidationRule.java index d5645aa5af..e56d96c557 100644 --- a/src/main/java/graphql/validation/OperationValidationRule.java +++ b/src/main/java/graphql/validation/OperationValidationRule.java @@ -69,6 +69,7 @@ *
  • {@link #UNIQUE_OBJECT_FIELD_NAME} - input object fields are unique
    • *
  • {@link #DEFER_DIRECTIVE_LABEL} - defer labels are unique strings
    • *
  • {@link #KNOWN_OPERATION_TYPES} - schema supports the operation type
    • + *
  • {@link #VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS} - variable references not allowed in constant directive positions
    • * * *

    Operation-Scoped Rules

    @@ -184,4 +185,7 @@ public enum OperationValidationRule { /** Defer directive must not be used in subscription operations. Requires operation context. */ DEFER_DIRECTIVE_ON_VALID_OPERATION, + + /** Variable references must not appear in directive arguments on variable definitions (constant context). */ + VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS, } diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index fea8df24aa..ebdfb52944 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -123,6 +123,7 @@ import static graphql.validation.ValidationErrorType.UnknownType; import static graphql.validation.ValidationErrorType.UnusedFragment; import static graphql.validation.ValidationErrorType.UnusedVariable; +import static graphql.validation.ValidationErrorType.VariableNotAllowed; import static graphql.validation.ValidationErrorType.VariableTypeMismatch; import static graphql.validation.ValidationErrorType.WrongType; import static java.lang.System.arraycopy; @@ -413,7 +414,7 @@ public void enter(Node node, List ancestors) { } else if (node instanceof OperationDefinition) { checkOperationDefinition((OperationDefinition) node); } else if (node instanceof VariableReference) { - checkVariable((VariableReference) node); + checkVariable((VariableReference) node, ancestors); } else if (node instanceof SelectionSet) { checkSelectionSet(); } else if (node instanceof ObjectValue) { @@ -685,7 +686,12 @@ private void checkOperationDefinition(OperationDefinition operationDefinition) { } } - private void checkVariable(VariableReference variableReference) { + private void checkVariable(VariableReference variableReference, List ancestors) { + if (shouldRunDocumentLevelRules()) { + if (isRuleEnabled(OperationValidationRule.VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS)) { + validateVariableNotAllowedInConstantDirective(variableReference, ancestors); + } + } if (shouldRunOperationScopedRules()) { if (isRuleEnabled(OperationValidationRule.NO_UNDEFINED_VARIABLES)) { validateNoUndefinedVariables(variableReference); @@ -980,6 +986,35 @@ private void validateNoUndefinedVariables(VariableReference variableReference) { } } + // --- VariablesNotAllowedInDirectivesOnVariableDefinitions --- + /** + * Per the GraphQL spec, directives applied to variable definitions accept only constant values. + * Variable references like {@code $v} are not allowed in directive arguments on variable definitions. + * + *

    For example, {@code query ($v:Int @dir(arg:$v)) { ... }} is invalid because {@code $v} + * is used in a directive argument on a variable definition. + */ + private void validateVariableNotAllowedInConstantDirective(VariableReference variableReference, List ancestors) { + // Walk the ancestor list to check if this variable reference is inside a directive + // that is applied to a variable definition. + // The ancestor path would be: ... > VariableDefinition > Directive > Argument > [ObjectValue > ObjectField >]* VariableReference + boolean inDirective = false; + for (int i = ancestors.size() - 1; i >= 0; i--) { + Node ancestor = ancestors.get(i); + if (ancestor instanceof Directive) { + inDirective = true; + } else if (ancestor instanceof VariableDefinition) { + if (inDirective) { + String message = i18n(VariableNotAllowed, "VariableNotAllowedInConstantDirective.variableNotAllowed", variableReference.getName()); + addError(VariableNotAllowed, variableReference.getSourceLocation(), message); + } + return; + } else if (ancestor instanceof OperationDefinition || ancestor instanceof Document) { + return; + } + } + } + // --- NoUnusedFragments --- private void validateNoUnusedFragments() { Set allUsedFragments = new HashSet<>(); diff --git a/src/main/resources/i18n/Validation.properties b/src/main/resources/i18n/Validation.properties index a9403bea5b..ffefbf8a68 100644 --- a/src/main/resources/i18n/Validation.properties +++ b/src/main/resources/i18n/Validation.properties @@ -86,6 +86,8 @@ VariableDefaultValuesOfCorrectType.badDefault=Validation error ({0}) : Bad defau # VariablesAreInputTypes.wrongType=Validation error ({0}) : Input variable ''{1}'' type ''{2}'' is not an input type # +VariableNotAllowedInConstantDirective.variableNotAllowed=Validation error ({0}) : Variable ''{1}'' is not allowed in directive arguments on variable definitions. Only constant values are allowed here +# VariableTypesMatchRule.unexpectedType=Validation error ({0}) : Variable ''{1}'' of type ''{2}'' used in position expecting type ''{3}'' # UniqueObjectFieldName.duplicateFieldName=Validation Error ({0}) : There can be only one field named ''{1}'' diff --git a/src/test/groovy/graphql/validation/VariablesNotAllowedInConstantDirectivesTest.groovy b/src/test/groovy/graphql/validation/VariablesNotAllowedInConstantDirectivesTest.groovy new file mode 100644 index 0000000000..bba68decba --- /dev/null +++ b/src/test/groovy/graphql/validation/VariablesNotAllowedInConstantDirectivesTest.groovy @@ -0,0 +1,130 @@ +package graphql.validation + +import graphql.TestUtil +import graphql.i18n.I18n +import graphql.language.Document +import graphql.parser.Parser +import spock.lang.Specification + +class VariablesNotAllowedInConstantDirectivesTest extends Specification { + + ValidationErrorCollector errorCollector = new ValidationErrorCollector() + + def schema = TestUtil.schema(''' + directive @dir(arg: Int) on VARIABLE_DEFINITION + directive @strDir(arg: String) on VARIABLE_DEFINITION | FIELD + type Query { + field(arg: Int): String + x: Int + } + ''') + + def traverse(String query) { + Document document = new Parser().parseDocument(query) + I18n i18n = I18n.i18n(I18n.BundleType.Validation, Locale.ENGLISH) + ValidationContext validationContext = new ValidationContext(schema, document, i18n) + OperationValidator operationValidator = new OperationValidator(validationContext, errorCollector, + { r -> r == OperationValidationRule.VARIABLES_NOT_ALLOWED_IN_DIRECTIVES_ON_VARIABLE_DEFINITIONS }) + LanguageTraversal languageTraversal = new LanguageTraversal() + languageTraversal.traverse(document, operationValidator) + } + + def "variable reference in directive argument on variable definition is rejected"() { + given: + def query = ''' + query ($v: Int @dir(arg: $v)) { x } + ''' + + when: + traverse(query) + + then: + !errorCollector.errors.isEmpty() + errorCollector.containsValidationError(ValidationErrorType.VariableNotAllowed) + errorCollector.errors[0].message.contains("Variable 'v' is not allowed in directive arguments on variable definitions") + } + + def "constant value in directive argument on variable definition is accepted"() { + given: + def query = ''' + query ($v: Int @dir(arg: 42)) { x } + ''' + + when: + traverse(query) + + then: + errorCollector.errors.isEmpty() + } + + def "variable reference in field directive argument is accepted"() { + given: + def query = ''' + query ($v: Int) { x @strDir(arg: "hello") } + ''' + + when: + traverse(query) + + then: + errorCollector.errors.isEmpty() + } + + def "no directive on variable definition is accepted"() { + given: + def query = ''' + query ($v: Int) { field(arg: $v) } + ''' + + when: + traverse(query) + + then: + errorCollector.errors.isEmpty() + } + + def "variable reference from another variable in directive is rejected"() { + given: + def query = ''' + query ($v: Int, $w: Int @dir(arg: $v)) { x } + ''' + + when: + traverse(query) + + then: + !errorCollector.errors.isEmpty() + errorCollector.containsValidationError(ValidationErrorType.VariableNotAllowed) + errorCollector.errors[0].message.contains("Variable 'v' is not allowed in directive arguments on variable definitions") + } + + def "full validation rejects variable in directive on variable definition"() { + given: + def query = ''' + query ($v: Int @dir(arg: $v)) { x } + ''' + def document = TestUtil.parseQuery(query) + def validator = new Validator() + + when: + def validationErrors = validator.validateDocument(schema, document, Locale.ENGLISH) + + then: + validationErrors.any { it.validationErrorType == ValidationErrorType.VariableNotAllowed } + } + + def "full validation accepts constant in directive on variable definition"() { + given: + def query = ''' + query ($v: Int @dir(arg: 42)) { x } + ''' + def document = TestUtil.parseQuery(query) + def validator = new Validator() + + when: + def validationErrors = validator.validateDocument(schema, document, Locale.ENGLISH) + + then: + !validationErrors.any { it.validationErrorType == ValidationErrorType.VariableNotAllowed } + } +}