Skip to content

Commit

Permalink
[SECURITY] Implement a feature to disable the suggestion when a Graph…
Browse files Browse the repository at this point in the history
…QL query fails (#3411)

* implement a feature to disable suggestion for security

* implement test for the feature to disable suggestion
  • Loading branch information
tomoikey authored Dec 8, 2024
1 parent b2ed608 commit 3114065
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
16 changes: 15 additions & 1 deletion graphql/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/vektah/gqlparser/v2/gqlerror"
"github.com/vektah/gqlparser/v2/parser"
"github.com/vektah/gqlparser/v2/validator"
"github.com/vektah/gqlparser/v2/validator/rules"

"github.com/99designs/gqlgen/graphql"
"github.com/99designs/gqlgen/graphql/errcode"
Expand All @@ -24,7 +25,8 @@ type Executor struct {
recoverFunc graphql.RecoverFunc
queryCache graphql.Cache[*ast.QueryDocument]

parserTokenLimit int
parserTokenLimit int
disableSuggestion bool
}

var _ graphql.GraphExecutor = &Executor{}
Expand Down Expand Up @@ -177,6 +179,10 @@ func (e *Executor) SetParserTokenLimit(limit int) {
e.parserTokenLimit = limit
}

func (e *Executor) SetDisableSuggestion(value bool) {
e.disableSuggestion = value
}

// parseQuery decodes the incoming query and validates it, pulling from cache if present.
//
// NOTE: This should NOT look at variables, they will change per request. It should only parse and
Expand Down Expand Up @@ -216,6 +222,14 @@ func (e *Executor) parseQuery(
return nil, gqlerror.List{gqlErr}
}

// swap out the FieldsOnCorrectType rule with one that doesn't provide suggestions
if e.disableSuggestion {
validator.RemoveRule("FieldsOnCorrectType")

rule := rules.FieldsOnCorrectTypeRuleWithoutSuggestions
validator.AddRule(rule.Name, rule.RuleFunc)
}

listErr := validator.Validate(e.es.Schema(), doc)
if len(listErr) != 0 {
for _, e := range listErr {
Expand Down
16 changes: 16 additions & 0 deletions graphql/executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,22 @@ func TestExecutor(t *testing.T) {
})
}

func TestExecutorDisableSuggestion(t *testing.T) {
exec := testexecutor.New()
t.Run("by default, the error message will include suggestions", func(t *testing.T) {
resp := query(exec, "", "{nam}")
assert.Equal(t, "", string(resp.Data))
assert.Equal(t, "input:1: Cannot query field \"nam\" on type \"Query\". Did you mean \"name\"?\n", resp.Errors.Error())
})

t.Run("disable suggestion, the error message will not include suggestions", func(t *testing.T) {
exec.SetDisableSuggestion(true)
resp := query(exec, "", "{nam}")
assert.Equal(t, "", string(resp.Data))
assert.Equal(t, "input:1: Cannot query field \"nam\" on type \"Query\".\n", resp.Errors.Error())
})
}

type testParamMutator struct {
Mutate func(context.Context, *graphql.RawParams) *gqlerror.Error
}
Expand Down
4 changes: 4 additions & 0 deletions graphql/handler/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ func (s *Server) SetParserTokenLimit(limit int) {
s.exec.SetParserTokenLimit(limit)
}

func (s *Server) SetDisableSuggestion(value bool) {
s.exec.SetDisableSuggestion(value)
}

func (s *Server) Use(extension graphql.HandlerExtension) {
s.exec.Use(extension)
}
Expand Down

0 comments on commit 3114065

Please sign in to comment.