cosmo icon indicating copy to clipboard operation
cosmo copied to clipboard

Variable validation errors leak entire variable in statusMessage telemetry.

Open Sam-tesouro opened this issue 1 year ago • 4 comments

Component(s)

router, studio, otelcollector

Is your feature request related to a problem? Please describe.

When a request to a federated Supergraph doesn't match spec and an $variable is used for input the entire $variable is leaked on the statusMessage via telemetry, potentially leaking sensitive information. https://github.com/wundergraph/graphql-go-tools/blob/master/v2/pkg/variablesvalidation/variablesvalidation.go

The response will also contain sensitive information if the $variable has sensitive information included.

Describe the solution you'd like

It would be amazing to have a config item where I could globally enable/disable said $variable being included in statusMessage on telemetry and/or the Error message on the response.

Even better would be some way to define types containing PII that would then be automatically masked, either via router config or schema annotation, whilst still providing the additional benefit of returning the input $variable context.

Describe alternatives you've considered

It's possible to resolve the tracing leaks with an OTEL collector filter processor, but that unfortunately doesn't resolve the response also potentially including sensitive data.

Additional context

variableContent on the message leaks.

func (v *variablesVisitor) renderVariableInvalidObjectTypeError(typeName []byte, variablesNode astjson.Node) {
	out := &bytes.Buffer{}
	err := v.variables.PrintNode(variablesNode, out)
	if err != nil {
		v.err = err
		return
	}
	variableContent := out.String()
	v.err = &InvalidVariableError{
		Message: fmt.Sprintf(`Variable "$%s" got invalid value %s; Expected type "%s" to be an object.`, string(v.currentVariableName), variableContent, string(typeName)),
	}
}

func (v *variablesVisitor) renderVariableRequiredNotProvidedError(fieldName []byte, typeRef int) {
	out := &bytes.Buffer{}
	err := v.variables.PrintNode(v.variables.Nodes[v.currentVariableJsonNodeRef], out)
	if err != nil {
		v.err = err
		return
	}
	variableContent := out.String()
	out.Reset()
	err = v.definition.PrintType(typeRef, out)
	if err != nil {
		v.err = err
		return
	}
	v.err = &InvalidVariableError{
		Message: fmt.Sprintf(`Variable "$%s" got invalid value %s; Field "%s" of required type "%s" was not provided.`, string(v.currentVariableName), variableContent, string(fieldName), out.String()),
	}
}

Really appreciate the work y'all are doing!

Sam-tesouro avatar Sep 17 '24 17:09 Sam-tesouro

WunderGraph commits fully to Open Source and we want to make sure that we can help you as fast as possible. The roadmap is driven by our customers and we have to prioritize issues that are important to them. You can influence the priority by becoming a customer. Please contact us here.

github-actions[bot] avatar Sep 17 '24 18:09 github-actions[bot]

Hey @Sam-tesouro we've discussed this internally. What you could do is propose a PR on go tools that removes the verbose response, with a flag (debug) to re-enable it. Once this is merged in, you can add it in a second PR to cosmo and we discuss how the debug flag can be passed on.

In general, our idea is that we can run the Router in "default" mode with a less verbose error message for compliance reasons, but still have an option to run the router in "dev mode" which is more verbose.

If you check the configuration (https://cosmo-docs.wundergraph.com/router/configuration), we've already got DEV_MODE, so we could apply it in this case.

Let me know what you think?

jensneuse avatar Oct 02 '24 07:10 jensneuse

Good day @jensneuse, that sounds ideal to me. I will go ahead and work on an implementation for your review and touch base as necessary.

Thank you!

Sam-tesouro avatar Oct 02 '24 14:10 Sam-tesouro

Good day @jensneuse, that sounds ideal to me. I will go ahead and work on an implementation for your review and touch base as necessary.

Thank you!

sounds fantastic, looking forward to it!

jensneuse avatar Oct 02 '24 15:10 jensneuse

Much appreciated y'all!

Sam-tesouro avatar Apr 03 '25 19:04 Sam-tesouro