Skip to content

Commit 6985a38

Browse files
authored
fix: GraphQL error messages disclose required input field names when public introspection is disabled ([GHSA-2fgh-8j2g-w354](GHSA-2fgh-8j2g-w354)) (#10567)
1 parent dbd4281 commit 6985a38

2 files changed

Lines changed: 149 additions & 2 deletions

File tree

spec/ParseGraphQLServer.spec.js

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -930,6 +930,132 @@ describe('ParseGraphQLServer', () => {
930930
expect(error.message).toContain('secretAdminTask');
931931
}
932932
});
933+
934+
it('should strip required-field names from base coercion errors without master or maintenance key', async () => {
935+
const schemaController = await parseServer.config.databaseController.loadSchema();
936+
await schemaController.addClassIfNotExists('TestReqClass', {
937+
secretRequiredField: { type: 'String', required: true },
938+
});
939+
await resetGraphQLCache();
940+
941+
try {
942+
await apolloClient.mutate({
943+
mutation: gql`
944+
mutation Create($input: CreateTestReqClassInput!) {
945+
createTestReqClass(input: $input) {
946+
testReqClass {
947+
id
948+
}
949+
}
950+
}
951+
`,
952+
variables: { input: { fields: {} } },
953+
});
954+
fail('should have thrown a coercion error');
955+
} catch (e) {
956+
const error = getReturnedError(e);
957+
// The base graphql-js "... was not provided." coercion message carries no
958+
// "Did you mean" clause, so it discloses the required custom field name to a
959+
// caller who only has the public application id. It must be redacted.
960+
expect(error.message).not.toContain('secretRequiredField');
961+
// The message is duplicated into extensions.stacktrace in non-production;
962+
// ensure the identifier does not leak through any returned field.
963+
expect(JSON.stringify(error)).not.toContain('secretRequiredField');
964+
}
965+
});
966+
967+
it('should keep required-field names in base coercion errors with master key', async () => {
968+
const schemaController = await parseServer.config.databaseController.loadSchema();
969+
await schemaController.addClassIfNotExists('TestReqClass', {
970+
secretRequiredField: { type: 'String', required: true },
971+
});
972+
await resetGraphQLCache();
973+
974+
try {
975+
await apolloClient.mutate({
976+
mutation: gql`
977+
mutation Create($input: CreateTestReqClassInput!) {
978+
createTestReqClass(input: $input) {
979+
testReqClass {
980+
id
981+
}
982+
}
983+
}
984+
`,
985+
variables: { input: { fields: {} } },
986+
context: {
987+
headers: {
988+
'X-Parse-Master-Key': 'test',
989+
},
990+
},
991+
});
992+
fail('should have thrown a coercion error');
993+
} catch (e) {
994+
const error = getReturnedError(e);
995+
expect(error.message).toContain('secretRequiredField');
996+
}
997+
});
998+
999+
it('should keep required-field names in base coercion errors when public introspection is enabled', async () => {
1000+
const parseServer = await reconfigureServer();
1001+
await createGQLFromParseServer(parseServer, { graphQLPublicIntrospection: true });
1002+
const schemaController = await parseServer.config.databaseController.loadSchema();
1003+
await schemaController.addClassIfNotExists('TestReqClass', {
1004+
secretRequiredField: { type: 'String', required: true },
1005+
});
1006+
await resetGraphQLCache();
1007+
1008+
try {
1009+
await apolloClient.mutate({
1010+
mutation: gql`
1011+
mutation Create($input: CreateTestReqClassInput!) {
1012+
createTestReqClass(input: $input) {
1013+
testReqClass {
1014+
id
1015+
}
1016+
}
1017+
}
1018+
`,
1019+
variables: { input: { fields: {} } },
1020+
});
1021+
fail('should have thrown a coercion error');
1022+
} catch (e) {
1023+
const error = getReturnedError(e);
1024+
expect(error.message).toContain('secretRequiredField');
1025+
}
1026+
});
1027+
1028+
it('should strip required-field names from inline-literal coercion errors without master or maintenance key', async () => {
1029+
const schemaController = await parseServer.config.databaseController.loadSchema();
1030+
await schemaController.addClassIfNotExists('TestReqClass', {
1031+
secretRequiredField: { type: 'String', required: true },
1032+
});
1033+
await resetGraphQLCache();
1034+
1035+
try {
1036+
// Input written inline in the operation (not via a variable) is validated by
1037+
// ValuesOfCorrectTypeRule, which emits a type-qualified message
1038+
// ('Field "<Type>.<field>" of required type ...'), disclosing both the generated
1039+
// input type name (which embeds the class name) and the required field name.
1040+
await apolloClient.mutate({
1041+
mutation: gql`
1042+
mutation Create {
1043+
createTestReqClass(input: { fields: {} }) {
1044+
testReqClass {
1045+
id
1046+
}
1047+
}
1048+
}
1049+
`,
1050+
});
1051+
fail('should have thrown a validation error');
1052+
} catch (e) {
1053+
const error = getReturnedError(e);
1054+
expect(error.message).not.toContain('secretRequiredField');
1055+
expect(error.message).not.toContain('CreateTestReqClass');
1056+
expect(JSON.stringify(error)).not.toContain('secretRequiredField');
1057+
}
1058+
});
9331059
});
9341060

9351061

src/GraphQL/ParseGraphQLServer.js

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,27 @@ const IntrospectionControlPlugin = (publicIntrospection) => ({
6464
const stripSchemaSuggestion = message =>
6565
typeof message === 'string' ? message.replace(/ ?Did you mean(.+?)\?$/, '') : message;
6666

67+
// graphql-js also emits a base input-coercion message that names a schema
68+
// identifier WITHOUT a "Did you mean" clause, so the suggestion strip above
69+
// cannot reach it: when a required custom input field is omitted, coerceInputValue
70+
// returns 'Field "<name>" of required type "<type>" was not provided.', disclosing
71+
// a field name the caller never supplied. Redact the quoted identifiers from this
72+
// template while preserving the error shape, for callers that are not allowed to
73+
// introspect. The sibling coercion messages ('... is not defined by type "<type>".',
74+
// 'Expected type "<type>" to be an object.') are intentionally left intact: they
75+
// only echo an input type name the caller already referenced in the operation, so
76+
// they disclose nothing the caller did not already provide.
77+
const stripSchemaCoercionIdentifiers = message =>
78+
typeof message === 'string'
79+
? message.replace(
80+
/Field "[^"]*" of required type "[^"]*" was not provided\./g,
81+
'Field of required type was not provided.'
82+
)
83+
: message;
84+
85+
const stripSchemaIdentifiers = message =>
86+
stripSchemaCoercionIdentifiers(stripSchemaSuggestion(message));
87+
6788
const SchemaSuggestionsControlPlugin = (publicIntrospection) => ({
6889
requestDidStart: async (requestContext) => ({
6990
willSendResponse: async () => {
@@ -84,9 +105,9 @@ const SchemaSuggestionsControlPlugin = (publicIntrospection) => ({
84105
? body.initialResult.errors
85106
: undefined;
86107
errors?.forEach(error => {
87-
error.message = stripSchemaSuggestion(error.message);
108+
error.message = stripSchemaIdentifiers(error.message);
88109
if (Array.isArray(error.extensions?.stacktrace)) {
89-
error.extensions.stacktrace = error.extensions.stacktrace.map(stripSchemaSuggestion);
110+
error.extensions.stacktrace = error.extensions.stacktrace.map(stripSchemaIdentifiers);
90111
}
91112
});
92113
},

0 commit comments

Comments
 (0)