Skip to content

Commit d96c945

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)) (#10566)
1 parent 293f60e commit d96c945

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
@@ -1231,6 +1231,132 @@ describe('ParseGraphQLServer', () => {
12311231
expect(error.message).toContain('secretAdminTask');
12321232
}
12331233
});
1234+
1235+
it('should strip required-field names from base coercion errors without master or maintenance key', async () => {
1236+
const schemaController = await parseServer.config.databaseController.loadSchema();
1237+
await schemaController.addClassIfNotExists('TestReqClass', {
1238+
secretRequiredField: { type: 'String', required: true },
1239+
});
1240+
await resetGraphQLCache();
1241+
1242+
try {
1243+
await apolloClient.mutate({
1244+
mutation: gql`
1245+
mutation Create($input: CreateTestReqClassInput!) {
1246+
createTestReqClass(input: $input) {
1247+
testReqClass {
1248+
id
1249+
}
1250+
}
1251+
}
1252+
`,
1253+
variables: { input: { fields: {} } },
1254+
});
1255+
fail('should have thrown a coercion error');
1256+
} catch (e) {
1257+
const error = getReturnedError(e);
1258+
// The base graphql-js "... was not provided." coercion message carries no
1259+
// "Did you mean" clause, so it discloses the required custom field name to a
1260+
// caller who only has the public application id. It must be redacted.
1261+
expect(error.message).not.toContain('secretRequiredField');
1262+
// The message is duplicated into extensions.stacktrace in non-production;
1263+
// ensure the identifier does not leak through any returned field.
1264+
expect(JSON.stringify(error)).not.toContain('secretRequiredField');
1265+
}
1266+
});
1267+
1268+
it('should keep required-field names in base coercion errors with master key', async () => {
1269+
const schemaController = await parseServer.config.databaseController.loadSchema();
1270+
await schemaController.addClassIfNotExists('TestReqClass', {
1271+
secretRequiredField: { type: 'String', required: true },
1272+
});
1273+
await resetGraphQLCache();
1274+
1275+
try {
1276+
await apolloClient.mutate({
1277+
mutation: gql`
1278+
mutation Create($input: CreateTestReqClassInput!) {
1279+
createTestReqClass(input: $input) {
1280+
testReqClass {
1281+
id
1282+
}
1283+
}
1284+
}
1285+
`,
1286+
variables: { input: { fields: {} } },
1287+
context: {
1288+
headers: {
1289+
'X-Parse-Master-Key': 'test',
1290+
},
1291+
},
1292+
});
1293+
fail('should have thrown a coercion error');
1294+
} catch (e) {
1295+
const error = getReturnedError(e);
1296+
expect(error.message).toContain('secretRequiredField');
1297+
}
1298+
});
1299+
1300+
it('should keep required-field names in base coercion errors when public introspection is enabled', async () => {
1301+
const parseServer = await reconfigureServer();
1302+
await createGQLFromParseServer(parseServer, { graphQLPublicIntrospection: true });
1303+
const schemaController = await parseServer.config.databaseController.loadSchema();
1304+
await schemaController.addClassIfNotExists('TestReqClass', {
1305+
secretRequiredField: { type: 'String', required: true },
1306+
});
1307+
await resetGraphQLCache();
1308+
1309+
try {
1310+
await apolloClient.mutate({
1311+
mutation: gql`
1312+
mutation Create($input: CreateTestReqClassInput!) {
1313+
createTestReqClass(input: $input) {
1314+
testReqClass {
1315+
id
1316+
}
1317+
}
1318+
}
1319+
`,
1320+
variables: { input: { fields: {} } },
1321+
});
1322+
fail('should have thrown a coercion error');
1323+
} catch (e) {
1324+
const error = getReturnedError(e);
1325+
expect(error.message).toContain('secretRequiredField');
1326+
}
1327+
});
1328+
1329+
it('should strip required-field names from inline-literal coercion errors without master or maintenance key', async () => {
1330+
const schemaController = await parseServer.config.databaseController.loadSchema();
1331+
await schemaController.addClassIfNotExists('TestReqClass', {
1332+
secretRequiredField: { type: 'String', required: true },
1333+
});
1334+
await resetGraphQLCache();
1335+
1336+
try {
1337+
// Input written inline in the operation (not via a variable) is validated by
1338+
// ValuesOfCorrectTypeRule, which emits a type-qualified message
1339+
// ('Field "<Type>.<field>" of required type ...'), disclosing both the generated
1340+
// input type name (which embeds the class name) and the required field name.
1341+
await apolloClient.mutate({
1342+
mutation: gql`
1343+
mutation Create {
1344+
createTestReqClass(input: { fields: {} }) {
1345+
testReqClass {
1346+
id
1347+
}
1348+
}
1349+
}
1350+
`,
1351+
});
1352+
fail('should have thrown a validation error');
1353+
} catch (e) {
1354+
const error = getReturnedError(e);
1355+
expect(error.message).not.toContain('secretRequiredField');
1356+
expect(error.message).not.toContain('CreateTestReqClass');
1357+
expect(JSON.stringify(error)).not.toContain('secretRequiredField');
1358+
}
1359+
});
12341360
});
12351361

12361362

src/GraphQL/ParseGraphQLServer.js

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

107+
// graphql-js also emits a base input-coercion message that names a schema
108+
// identifier WITHOUT a "Did you mean" clause, so the suggestion strip above
109+
// cannot reach it: when a required custom input field is omitted, coerceInputValue
110+
// returns 'Field "<name>" of required type "<type>" was not provided.', disclosing
111+
// a field name the caller never supplied. Redact the quoted identifiers from this
112+
// template while preserving the error shape, for callers that are not allowed to
113+
// introspect. The sibling coercion messages ('... is not defined by type "<type>".',
114+
// 'Expected type "<type>" to be an object.') are intentionally left intact: they
115+
// only echo an input type name the caller already referenced in the operation, so
116+
// they disclose nothing the caller did not already provide.
117+
const stripSchemaCoercionIdentifiers = message =>
118+
typeof message === 'string'
119+
? message.replace(
120+
/Field "[^"]*" of required type "[^"]*" was not provided\./g,
121+
'Field of required type was not provided.'
122+
)
123+
: message;
124+
125+
const stripSchemaIdentifiers = message =>
126+
stripSchemaCoercionIdentifiers(stripSchemaSuggestion(message));
127+
107128
const SchemaSuggestionsControlPlugin = (publicIntrospection) => ({
108129
requestDidStart: async (requestContext) => ({
109130
willSendResponse: async () => {
@@ -124,9 +145,9 @@ const SchemaSuggestionsControlPlugin = (publicIntrospection) => ({
124145
? body.initialResult.errors
125146
: undefined;
126147
errors?.forEach(error => {
127-
error.message = stripSchemaSuggestion(error.message);
148+
error.message = stripSchemaIdentifiers(error.message);
128149
if (Array.isArray(error.extensions?.stacktrace)) {
129-
error.extensions.stacktrace = error.extensions.stacktrace.map(stripSchemaSuggestion);
150+
error.extensions.stacktrace = error.extensions.stacktrace.map(stripSchemaIdentifiers);
130151
}
131152
});
132153
},

0 commit comments

Comments
 (0)