diff --git a/.limetech/ai-review-markers/fix-ws-introspection-policy-ee1178220d13.json b/.limetech/ai-review-markers/fix-ws-introspection-policy-ee1178220d13.json new file mode 100644 index 0000000000..15f097b4a9 --- /dev/null +++ b/.limetech/ai-review-markers/fix-ws-introspection-policy-ee1178220d13.json @@ -0,0 +1,8 @@ +{ + "disposition": "PATCH", + "forecast_commit": "5322e5d57d50618c837b371412f766492b02633c", + "reviewed_sha": "8f2ead51e46f37ebe8f07835f5a6895005b3fdc3", + "schema": "limetech.ai-review-marker.v2", + "stage": "final", + "unresolved_proportionality_findings": [] +} diff --git a/api/src/unraid-api/graph/graph.module.ts b/api/src/unraid-api/graph/graph.module.ts index 902505a3a0..8a947b942f 100644 --- a/api/src/unraid-api/graph/graph.module.ts +++ b/api/src/unraid-api/graph/graph.module.ts @@ -19,7 +19,7 @@ import '@app/unraid-api/graph/auth/auth-action.enum.js'; import { createDynamicIntrospectionPlugin } from '@app/unraid-api/graph/introspection-plugin.js'; import { - graphqlWsOptions, + createGraphqlWsOptions, maxGraphqlTokens, requestProtection, } from '@app/unraid-api/graph/request-protection.js'; @@ -61,7 +61,7 @@ import { PluginModule } from '@app/unraid-api/plugin/plugin.module.js'; createDynamicIntrospectionPlugin(isSandboxEnabled), createSandboxPlugin(), ] as any[], - subscriptions: { 'graphql-ws': graphqlWsOptions }, + subscriptions: { 'graphql-ws': createGraphqlWsOptions(isSandboxEnabled) }, buildSchemaOptions: { dateScalarMode: 'isoDate', // Only add directive when not in test environment to avoid GraphQL version conflicts diff --git a/api/src/unraid-api/graph/introspection-plugin.ts b/api/src/unraid-api/graph/introspection-plugin.ts index d3c62bfb0a..1a9372cc26 100644 --- a/api/src/unraid-api/graph/introspection-plugin.ts +++ b/api/src/unraid-api/graph/introspection-plugin.ts @@ -64,7 +64,7 @@ const hasBlockedIntrospectionField = ( return blocked; }); -const isBlockedIntrospectionOperation = ( +export const isBlockedIntrospectionOperation = ( operation: OperationDefinitionNode, document: DocumentNode ): boolean => { diff --git a/api/src/unraid-api/graph/request-protection.spec.ts b/api/src/unraid-api/graph/request-protection.spec.ts index a034f6b4a0..d8520b60fa 100644 --- a/api/src/unraid-api/graph/request-protection.spec.ts +++ b/api/src/unraid-api/graph/request-protection.spec.ts @@ -1,7 +1,13 @@ +import { createServer } from 'node:http'; +import { createRequire } from 'node:module'; + import { buildSchema, parse, validate } from 'graphql'; +import { createClient } from 'graphql-ws'; +import { useServer } from 'graphql-ws/use/ws'; import { describe, expect, it } from 'vitest'; +import WebSocket, { WebSocketServer } from 'ws'; -import { requestProtection } from '@app/unraid-api/graph/request-protection.js'; +import { createGraphqlWsOptions, requestProtection } from '@app/unraid-api/graph/request-protection.js'; const schema = buildSchema('type Query { value: String, child: Query }'); @@ -60,3 +66,76 @@ describe('requestProtection', () => { expect(validateRequest('query First { value } query Second { value }')).toHaveLength(0); }); }); + +describe('WebSocket introspection policy', () => { + it('checks the selected operation before execution and reads sandbox changes on the same connection', async () => { + const graphql: typeof import('graphql') = createRequire(import.meta.url)('graphql'); + const socketSchema = graphql.buildSchema('type Query { value: String }'); + let resolverCalls = 0; + let sandboxEnabled = false; + socketSchema.getQueryType()!.getFields().value.resolve = () => { + resolverCalls++; + return 'ok'; + }; + const server = createServer(); + const sockets = new WebSocketServer({ server, path: '/graphql' }); + const disposable = useServer( + { + schema: socketSchema, + ...createGraphqlWsOptions(() => sandboxEnabled), + }, + sockets + ); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const address = server.address(); + if (!address || typeof address === 'string') throw new Error('Expected TCP listener'); + const client = createClient({ + url: `ws://127.0.0.1:${address.port}/graphql`, + webSocketImpl: WebSocket, + lazy: false, + retryAttempts: 0, + }); + const run = async (query: string, operationName?: string) => { + const iterator = client.iterate({ query, operationName }); + try { + return (await iterator.next()).value; + } finally { + await iterator.return?.(); + } + }; + try { + for (const query of [ + '{ __schema { queryType { name } } value }', + '{ metadata: __type(name: "Query") { name } value }', + 'query { ...Metadata value } fragment Metadata on Query { __schema { queryType { name } } }', + '{ ... on Query { __schema @include(if: true) { queryType { name } } } value }', + ]) { + await expect(run(query)).rejects.toEqual([ + expect.objectContaining({ extensions: { code: 'INTROSPECTION_DISABLED' } }), + ]); + } + expect(resolverCalls).toBe(0); + const operations = + 'query Normal { value __typename } query Metadata { __schema { queryType { name } } }'; + expect((await run(operations, 'Normal'))?.data).toEqual({ + value: 'ok', + __typename: 'Query', + }); + await expect(run(operations, 'Metadata')).rejects.toBeDefined(); + sandboxEnabled = true; + expect((await run(operations, 'Metadata'))?.data).toEqual({ + __schema: { queryType: { name: 'Query' } }, + }); + sandboxEnabled = false; + await expect(run(operations, 'Metadata')).rejects.toBeDefined(); + const aliases = Array.from({ length: 101 }, (_, index) => `a${index}: value`).join(' '); + await expect(run(`{ ${aliases} }`)).rejects.toBeDefined(); + } finally { + await client.dispose(); + await disposable.dispose(); + await new Promise((resolve, reject) => + server.close((error) => (error ? reject(error) : resolve())) + ); + } + }); +}); diff --git a/api/src/unraid-api/graph/request-protection.ts b/api/src/unraid-api/graph/request-protection.ts index e1194028fc..1f3239b403 100644 --- a/api/src/unraid-api/graph/request-protection.ts +++ b/api/src/unraid-api/graph/request-protection.ts @@ -4,6 +4,8 @@ import type { DocumentNode, GraphQLSchema } from 'graphql'; import type { SubscribePayload } from 'graphql-ws'; import { ApolloArmor } from '@escape.tech/graphql-armor'; +import { isBlockedIntrospectionOperation } from '@app/unraid-api/graph/introspection-plugin.js'; + export const maxGraphqlTokens = 5_000; const graphqlForWs: typeof import('graphql') = createRequire(import.meta.url)('graphql'); @@ -26,11 +28,24 @@ export const requestProtection = new ApolloArmor({ const websocketValidationRules = [...graphqlForWs.specifiedRules, ...requestProtection.validationRules]; -export const graphqlWsOptions = { +export const createGraphqlWsOptions = (isSandboxEnabled: () => boolean) => ({ path: '/graphql', onSubscribe: (_context: unknown, _id: string, payload: SubscribePayload) => { try { - graphqlForWs.parse(payload.query, { maxTokens: maxGraphqlTokens }); + const document = graphqlForWs.parse(payload.query, { maxTokens: maxGraphqlTokens }); + const operation = graphqlForWs.getOperationAST(document, payload.operationName); + if ( + !isSandboxEnabled() && + operation && + isBlockedIntrospectionOperation(operation, document) + ) { + return [ + new graphqlForWs.GraphQLError( + 'GraphQL introspection is not allowed, but the current request is for introspection.', + { extensions: { code: 'INTROSPECTION_DISABLED' } } + ), + ]; + } } catch (error) { if (error instanceof graphqlForWs.GraphQLError) return [error]; throw error; @@ -40,4 +55,4 @@ export const graphqlWsOptions = { // Nest passes this option to graphql-ws, although its subscription config omits the type. validate: (schema: GraphQLSchema, document: DocumentNode) => graphqlForWs.validate(schema, document, websocketValidationRules), -}; +});