[Bug] Stop graphql-depth-limit crashes and related MRT/insights issues (#401)

+4 -3

client/src/webpages/dashboard/rules/dashboard/RulesDashboard.tsx

··· 42 42 GQLRuleStatus, 43 43 GQLSignalType, 44 44 GQLUserPermission, 45 + namedOperations, 45 46 useGQLAddFavoriteRuleMutation, 46 47 useGQLDeleteRuleMutation, 47 48 useGQLRemoveFavoriteRuleMutation, ··· 222 223 const onDeleteRule = (id: string) => { 223 224 deleteRule({ 224 225 variables: { id }, 225 - refetchQueries: [{ query: RULES_QUERY }], 226 + refetchQueries: [namedOperations.Query.Rules], 226 227 }); 227 228 }; 228 229 const onAddFavoriteRule = useCallback( ··· 232 233 variables: { 233 234 ruleId, 234 235 }, 235 - refetchQueries: [{ query: RULES_QUERY }], 236 + refetchQueries: [namedOperations.Query.Rules], 236 237 }); 237 238 }, 238 239 [addFavoriteRule], ··· 244 245 variables: { 245 246 ruleId, 246 247 }, 247 - refetchQueries: [{ query: RULES_QUERY }], 248 + refetchQueries: [namedOperations.Query.Rules], 248 249 }); 249 250 }, 250 251 [removeFavoriteRule],

+15 -7

client/src/webpages/dashboard/rules/info/insights/RuleInsightsSamplesTable.tsx

··· 843 843 */ 844 844 export function flattenRuleExecutionSampleForCSV( 845 845 topLevelKey: string | null, 846 - json: object, 846 + json: object | null | undefined, 847 847 ) { 848 848 let result: { [key: string]: string } = {}; 849 + if (json == null || Array.isArray(json)) { 850 + if (topLevelKey != null) { 851 + result[topLevelKey] = json == null ? '' : JSON.stringify(json); 852 + } 853 + return result; 854 + } 849 855 Object.entries(json).forEach(([key, value]) => { 850 - if (typeof value === 'object') { 856 + const fullKey = topLevelKey ? `${topLevelKey}:${key}` : key; 857 + if (value != null && typeof value === 'object' && !Array.isArray(value)) { 851 858 result = { 852 859 ...omit(result, key), 853 - ...flattenRuleExecutionSampleForCSV( 854 - topLevelKey ? `${topLevelKey}:${key}` : key, 855 - value, 856 - ), 860 + ...flattenRuleExecutionSampleForCSV(fullKey, value), 857 861 }; 862 + } else if (Array.isArray(value)) { 863 + result[fullKey] = JSON.stringify(value); 864 + } else if (value == null) { 865 + result[fullKey] = ''; 858 866 } else { 859 - result[topLevelKey ? `${topLevelKey}:${key}` : key] = value; 867 + result[fullKey] = value; 860 868 } 861 869 }); 862 870 return result;

+20

client/src/webpages/dashboard/rules/info/insights/tests/RuleInsightsSamplesTable.test.ts

··· 71 71 'a:b:c:d:e': 1, 72 72 }); 73 73 }); 74 + it('Null nested value renders as an empty cell', () => { 75 + const content = { a: 1, b: null, c: { c1: null, c2: 'x' } }; 76 + expect(flattenRuleExecutionSampleForCSV(null, content)).toMatchObject({ 77 + a: 1, 78 + b: '', 79 + 'c:c1': '', 80 + 'c:c2': 'x', 81 + }); 82 + }); 83 + it('Null top-level input does not throw', () => { 84 + expect(() => flattenRuleExecutionSampleForCSV(null, null)).not.toThrow(); 85 + expect(flattenRuleExecutionSampleForCSV(null, null)).toMatchObject({}); 86 + }); 87 + it('Array values are stringified rather than recursed into', () => { 88 + const content = { tags: ['a', 'b'], n: 1 }; 89 + expect(flattenRuleExecutionSampleForCSV(null, content)).toMatchObject({ 90 + tags: '["a","b"]', 91 + n: 1, 92 + }); 93 + }); 74 94 }); 75 95 });

+6 -6

client/src/webpages/dashboard/rules/rule_form/RuleForm.tsx

··· 27 27 GQLConditionConjunction, 28 28 GQLRuleStatus, 29 29 GQLUserPermission, 30 + namedOperations, 30 31 useGQLContentRuleFormConfigQuery, 31 32 useGQLCreateContentRuleMutation, 32 33 useGQLCreateUserRuleMutation, ··· 41 42 import useRouteQueryParams from '../../../../routing/useRouteQueryParams'; 42 43 import { DAY, HOUR, MONTH, WEEK } from '../../../../utils/time'; 43 44 import { ModalInfo } from '../../types/ModalInfo'; 44 - import { RULES_QUERY } from '../dashboard/RulesDashboard'; 45 45 import TextTokenInput from '../TextTokenInput'; 46 46 import { 47 47 ConditionInput, ··· 644 644 const onDeleteRule = (id: string) => { 645 645 deleteRule({ 646 646 variables: { id }, 647 - refetchQueries: [{ query: RULES_QUERY }], 647 + refetchQueries: [namedOperations.Query.Rules], 648 648 }); 649 649 }; 650 650 ··· 918 918 expirationTime: state.expirationTime ? format(state.expirationTime, 'yyyy-MM-dd HH:mm') : undefined, 919 919 }, 920 920 }, 921 - refetchQueries: [{ query: RULES_QUERY }], 921 + refetchQueries: [namedOperations.Query.Rules], 922 922 }); 923 923 }; 924 924 ··· 943 943 }, 944 944 }, 945 945 refetchQueries: [ 946 - { query: RULES_QUERY }, 946 + namedOperations.Query.Rules, 947 947 { query: RULE_QUERY, variables: { id } }, 948 948 ], 949 949 }); ··· 965 965 expirationTime: state.expirationTime ? format(state.expirationTime, 'yyyy-MM-dd HH:mm') : undefined, 966 966 }, 967 967 }, 968 - refetchQueries: [{ query: RULES_QUERY }], 968 + refetchQueries: [namedOperations.Query.Rules], 969 969 }); 970 970 }; 971 971 ··· 989 989 }, 990 990 }, 991 991 refetchQueries: [ 992 - { query: RULES_QUERY }, 992 + namedOperations.Query.Rules, 993 993 { query: RULE_QUERY, variables: { id } }, 994 994 ], 995 995 });

+2 -2

server/api.ts

··· 19 19 import cors from 'cors'; 20 20 import express, { type ErrorRequestHandler } from 'express'; 21 21 import session from 'express-session'; 22 - import depthLimit from 'graphql-depth-limit'; 23 22 import { buildContext, GraphQLLocalStrategy } from 'graphql-passport'; 24 23 import helmet from 'helmet'; 25 24 import passport from 'passport'; ··· 37 36 import { passwordMatchesHash } from './services/userManagementService/index.js'; 38 37 import typeDefs from './graphql/schema.js'; 39 38 import { authSchemaWrapper } from './graphql/utils/authorization.js'; 39 + import { safeDepthLimit } from './graphql/utils/safeDepthLimit.js'; 40 40 import { type Dependencies } from './iocContainer/index.js'; 41 41 import { isEnvTrue, safeGetEnvInt } from './iocContainer/utils.js'; 42 42 import controllers from './routes/index.js'; ··· 388 388 ? [ApolloServerPluginLandingPageDisabled()] 389 389 : []), 390 390 ], 391 - validationRules: [depthLimit(safeGetEnvInt('GRAPHQL_MAX_DEPTH', 10))], 391 + validationRules: [safeDepthLimit(safeGetEnvInt('GRAPHQL_MAX_DEPTH', 10))], 392 392 introspection: process.env.NODE_ENV !== 'production', 393 393 formatError(formattedError, error) { 394 394 // unwrapResolverError removes the GraphQLError wrapper added by graphql-js

+13 -8

server/graphql/modules/manualReviewTool.ts

··· 1630 1630 async jobs(queue, { ids: jobIds, limit }, context) { 1631 1631 const { orgId, id: queueId } = queue; 1632 1632 1633 - if (!jobIds) { 1633 + if (jobIds == null) { 1634 1634 return context.services.ManualReviewToolService.getAllJobsForQueue({ 1635 1635 orgId, 1636 1636 queueId, 1637 1637 limit: limit ?? undefined, 1638 1638 }); 1639 - } else { 1640 - return context.services.ManualReviewToolService.getJobsForQueue({ 1641 - orgId, 1642 - queueId, 1643 - jobIds, 1644 - isAppealsQueue: queue.isAppealsQueue, 1645 - }); 1639 + } 1640 + // Empty array means "filter to no IDs" -> result is always []. Short-circuit 1641 + // so we don't open a Bull/Redis queue handle per reviewable queue on every 1642 + // MRT page load before a job has been dequeued. 1643 + if (jobIds.length === 0) { 1644 + return []; 1646 1645 } 1646 + return context.services.ManualReviewToolService.getJobsForQueue({ 1647 + orgId, 1648 + queueId, 1649 + jobIds, 1650 + isAppealsQueue: queue.isAppealsQueue, 1651 + }); 1647 1652 }, 1648 1653 async pendingJobCount(queue, _, context) { 1649 1654 const { orgId, id: queueId } = queue;

+88

server/graphql/utils/safeDepthLimit.test.ts

··· 1 + import { 2 + buildSchema, 3 + parse, 4 + validate, 5 + type DocumentNode, 6 + type GraphQLSchema, 7 + } from 'graphql'; 8 + 9 + import { safeDepthLimit } from './safeDepthLimit.js'; 10 + 11 + const schema: GraphQLSchema = buildSchema(` 12 + type Query { 13 + me: User 14 + } 15 + type User { 16 + id: ID! 17 + name: String 18 + friends: [User!]! 19 + } 20 + `); 21 + 22 + function parseQuery(source: string): DocumentNode { 23 + return parse(source); 24 + } 25 + 26 + describe('safeDepthLimit', () => { 27 + test('rejects queries deeper than the configured maximum', () => { 28 + const doc = parseQuery(` 29 + query { 30 + me { 31 + friends { 32 + friends { 33 + friends { 34 + id 35 + } 36 + } 37 + } 38 + } 39 + } 40 + `); 41 + const errors = validate(schema, doc, [safeDepthLimit(2)]); 42 + expect(errors.length).toBeGreaterThan(0); 43 + expect(errors[0].message).toMatch(/exceeds maximum operation depth/); 44 + }); 45 + 46 + test('allows queries within the configured maximum', () => { 47 + const doc = parseQuery(` 48 + query { 49 + me { 50 + id 51 + } 52 + } 53 + `); 54 + const errors = validate(schema, doc, [safeDepthLimit(5)]); 55 + expect(errors).toEqual([]); 56 + }); 57 + 58 + test('does not throw when a query references an undefined fragment', () => { 59 + const doc = parseQuery(` 60 + query { 61 + me { 62 + ...MissingFields 63 + } 64 + } 65 + `); 66 + expect(() => validate(schema, doc, [safeDepthLimit(10)])).not.toThrow(); 67 + }); 68 + 69 + test('logs the offending operation name when it falls back', () => { 70 + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); 71 + const error = jest.spyOn(console, 'error').mockImplementation(() => {}); 72 + try { 73 + const doc = parseQuery(` 74 + query MyOp { 75 + me { ...MissingFields } 76 + } 77 + `); 78 + validate(schema, doc, [safeDepthLimit(10)]); 79 + expect(warn).toHaveBeenCalledTimes(1); 80 + const message = warn.mock.calls[0][0] as string; 81 + expect(message).toContain('safeDepthLimit'); 82 + expect(message).toContain('MyOp'); 83 + } finally { 84 + warn.mockRestore(); 85 + error.mockRestore(); 86 + } 87 + }); 88 + });

+37

server/graphql/utils/safeDepthLimit.ts

··· 1 + import depthLimit from 'graphql-depth-limit'; 2 + import { Kind, type ValidationContext } from 'graphql'; 3 + 4 + // `graphql-depth-limit@1.1.0` throws when a query references an undefined 5 + // fragment (it dereferences `undefined.kind`). Swallow so other validation 6 + // rules can report the real error instead of crashing the request with a 500. 7 + export function safeDepthLimit( 8 + maxDepth: number, 9 + options?: Parameters<typeof depthLimit>[1], 10 + callback?: Parameters<typeof depthLimit>[2], 11 + ) { 12 + const inner = depthLimit(maxDepth, options, callback); 13 + return (context: ValidationContext) => { 14 + try { 15 + return inner(context); 16 + } catch (err) { 17 + const message = err instanceof Error ? err.message : String(err); 18 + // eslint-disable-next-line no-console 19 + console.warn( 20 + '[safeDepthLimit] depth-limit validation crashed; falling back. ' + 21 + `operations=[${getOperationNames(context).join(',')}] ` + 22 + `error=${message}`, 23 + ); 24 + return {}; 25 + } 26 + }; 27 + } 28 + 29 + function getOperationNames(context: ValidationContext): string[] { 30 + const names: string[] = []; 31 + for (const def of context.getDocument().definitions) { 32 + if (def.kind === Kind.OPERATION_DEFINITION) { 33 + names.push(def.name?.value ?? '<anonymous>'); 34 + } 35 + } 36 + return names; 37 + }

Configure Feed

Configure Feed