Skip to content

Commit a0ddb85

Browse files
authored
fix: GraphQL "Did you mean" validation suggestions disclose schema to unauthenticated callers ([GHSA-8cph-rgr4-g5vj](GHSA-8cph-rgr4-g5vj)) (#10468)
1 parent baa153c commit a0ddb85

2 files changed

Lines changed: 137 additions & 1 deletion

File tree

spec/ParseGraphQLServer.spec.js

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,115 @@ describe('ParseGraphQLServer', () => {
715715
})
716716
expect(introspection.data).toBeDefined();
717717
});
718+
719+
it('should strip "Did you mean" field suggestions from validation errors without master or maintenance key', async () => {
720+
try {
721+
await apolloClient.query({
722+
query: gql`
723+
query Typo {
724+
healt
725+
}
726+
`,
727+
});
728+
fail('should have thrown a validation error');
729+
} catch (e) {
730+
const message = e.networkError.result.errors[0].message;
731+
expect(message).toContain('Cannot query field "healt"');
732+
expect(message).not.toMatch(/Did you mean/);
733+
expect(message).not.toContain('health');
734+
}
735+
});
736+
737+
it('should strip "Did you mean" argument suggestions from validation errors without master or maintenance key', async () => {
738+
try {
739+
await apolloClient.query({
740+
query: gql`
741+
query UnknownArg {
742+
users(wher: {}) {
743+
edges {
744+
node {
745+
id
746+
}
747+
}
748+
}
749+
}
750+
`,
751+
});
752+
fail('should have thrown a validation error');
753+
} catch (e) {
754+
const message = e.networkError.result.errors[0].message;
755+
expect(message).toContain('Unknown argument "wher"');
756+
expect(message).not.toMatch(/Did you mean/);
757+
expect(message).not.toContain('"where"');
758+
}
759+
});
760+
761+
it('should keep "Did you mean" suggestions with master key', async () => {
762+
try {
763+
await apolloClient.query({
764+
query: gql`
765+
query Typo {
766+
healt
767+
}
768+
`,
769+
context: {
770+
headers: {
771+
'X-Parse-Master-Key': 'test',
772+
},
773+
},
774+
});
775+
fail('should have thrown a validation error');
776+
} catch (e) {
777+
const message = e.networkError.result.errors[0].message;
778+
expect(message).toContain('Cannot query field "healt"');
779+
expect(message).toMatch(/Did you mean/);
780+
expect(message).toContain('health');
781+
}
782+
});
783+
784+
it('should keep "Did you mean" suggestions with maintenance key', async () => {
785+
try {
786+
await apolloClient.query({
787+
query: gql`
788+
query Typo {
789+
healt
790+
}
791+
`,
792+
context: {
793+
headers: {
794+
'X-Parse-Maintenance-Key': 'test2',
795+
},
796+
},
797+
});
798+
fail('should have thrown a validation error');
799+
} catch (e) {
800+
const message = e.networkError.result.errors[0].message;
801+
expect(message).toContain('Cannot query field "healt"');
802+
expect(message).toMatch(/Did you mean/);
803+
expect(message).toContain('health');
804+
}
805+
});
806+
807+
it('should keep "Did you mean" suggestions when public introspection is enabled', async () => {
808+
const parseServer = await reconfigureServer();
809+
await createGQLFromParseServer(parseServer, { graphQLPublicIntrospection: true });
810+
811+
try {
812+
await apolloClient.query({
813+
query: gql`
814+
query Typo {
815+
healt
816+
}
817+
`,
818+
});
819+
fail('should have thrown a validation error');
820+
} catch (e) {
821+
const message = e.networkError.result.errors[0].message;
822+
expect(message).toContain('Cannot query field "healt"');
823+
expect(message).toMatch(/Did you mean/);
824+
expect(message).toContain('health');
825+
}
826+
});
718827
});
719828

720829

src/GraphQL/ParseGraphQLServer.js

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,33 @@ const IntrospectionControlPlugin = (publicIntrospection) => ({
5050

5151
});
5252

53+
// graphql-js validation rules (FieldsOnCorrectTypeRule, KnownArgumentNamesRule,
54+
// KnownTypeNamesRule, ...) embed "Did you mean ...?" hints sourced from the live
55+
// schema in their error messages. Those messages are returned to the caller
56+
// before didResolveOperation runs, so they sidestep IntrospectionControlPlugin
57+
// and disclose schema identifiers the introspection guard is meant to hide.
58+
// Strip the hint suffix for callers that are not allowed to introspect.
59+
const SchemaSuggestionsControlPlugin = (publicIntrospection) => ({
60+
requestDidStart: async (requestContext) => ({
61+
validationDidStart: async () => {
62+
if (publicIntrospection) {
63+
return;
64+
}
65+
const isMasterOrMaintenance =
66+
requestContext.contextValue.auth?.isMaster ||
67+
requestContext.contextValue.auth?.isMaintenance;
68+
if (isMasterOrMaintenance) {
69+
return;
70+
}
71+
return async (validationErrors) => {
72+
validationErrors?.forEach(error => {
73+
error.message = error.message.replace(/ ?Did you mean(.+?)\?$/, '');
74+
});
75+
};
76+
},
77+
}),
78+
});
79+
5380
class ParseGraphQLServer {
5481
parseGraphQLController: ParseGraphQLController;
5582

@@ -111,7 +138,7 @@ class ParseGraphQLServer {
111138
requestHeaders: ['X-Parse-Application-Id'],
112139
},
113140
introspection: this.config.graphQLPublicIntrospection,
114-
plugins: [ApolloServerPluginCacheControlDisabled(), IntrospectionControlPlugin(this.config.graphQLPublicIntrospection), createComplexityValidationPlugin(() => this.parseServer.config.requestComplexity)],
141+
plugins: [ApolloServerPluginCacheControlDisabled(), IntrospectionControlPlugin(this.config.graphQLPublicIntrospection), SchemaSuggestionsControlPlugin(this.config.graphQLPublicIntrospection), createComplexityValidationPlugin(() => this.parseServer.config.requestComplexity)],
115142
schema,
116143
});
117144
await apollo.start();

0 commit comments

Comments
 (0)