diff --git a/CHANGELOG.md b/CHANGELOG.md index afa7641f50a..637f1b2e880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ The version headers in this history reflect the versions of Apollo Server itself - `apollo-engine-reporting`: **BEHAVIOR CHANGE**: If the error returned from the `engine.rewriteError` hook has an `extensions` property, that property will be used instead of the original error's extensions. Document that changes to most other `GraphQLError` fields by `engine.rewriteError` are ignored. [PR #2932](https://github.com/apollographql/apollo-server/pull/2932) - `apollo-engine-reporting`: **BEHAVIOR CHANGE**: The `engine.maskErrorDetails` option, deprecated by `engine.rewriteError` in v2.5.0, now behaves a bit more like the new option: while all error messages will be redacted, they will still show up on the appropriate nodes in a trace. [PR #2932](https://github.com/apollographql/apollo-server/pull/2932) +- `apollo-engine-reporting`: **BEHAVIOR CHANGE**: By default, send no GraphQL variable values to Apollo's servers instead of sending all variable values. Adding the new EngineReportingOption `sendVariableValues` to send some or all variable values, possibly after transforming them. This replaces the `privateVariables` option, which is now deprecated. [PR #2931](https://github.com/apollographql/apollo-server/pull/2931) + + > Note: In order to keep shipping all GraphQL variable values to Apollo Engine, pass in the option: + > + > `new ApolloServer({engine: {sendVariableValues: {all: true}}})`. + ### v2.6.7 diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md index 890eebcdf48..a831458e7b6 100644 --- a/docs/source/api/apollo-server.md +++ b/docs/source/api/apollo-server.md @@ -119,7 +119,7 @@ new ApolloServer({ * `engine`: <`EngineReportingOptions`> | boolean - Provided the `ENGINE_API_KEY` environment variable is set, the engine reporting agent will be started automatically. The API key can also be provided as the `apiKey` field in an object passed as the `engine` field. See the [EngineReportingOptions](#enginereportingoptions) section for a full description of how to configure the reporting agent, including how to blacklist variables. When using the Engine proxy, this option should be set to false. + Provided the `ENGINE_API_KEY` environment variable is set, the engine reporting agent will be started automatically. The API key can also be provided as the `apiKey` field in an object passed as the `engine` field. See the [EngineReportingOptions](#enginereportingoptions) section for a full description of how to configure the reporting agent, including how to include variable values and HTTP headers. When using the Engine proxy, this option should be set to false. * `persistedQueries`: <`Object`> | false @@ -342,20 +342,59 @@ addMockFunctionsToSchema({ By default, errors sending reports to Engine servers will be logged to standard error. Specify this function to process errors in a different way. - -* `privateVariables`: Array | boolean - - A case-sensitive list of names of variables whose values should not be sent - to Apollo servers, or 'true' to leave out all variables. In the former - case, the report will indicate that each private variable was redacted in - the latter case, no variables are sent at all. - -* `privateHeaders`: Array | boolean - - A case-insensitive list of names of HTTP headers whose values should not be - sent to Apollo servers, or 'true' to leave out all HTTP headers. Unlike - with privateVariables, names of dropped headers are not reported. - + +* `sendVariableValues`: { transform: (options: { variables: Record, operationString?: string } ) => Record } + | { exceptNames: Array<String> } + | { onlyNames: Array<String> } + | { none: true } + | { all: true } + + By default, Apollo Server does not send the values of any GraphQL variables to Apollo's servers, because variable values often contain the private data of your app's users. If you'd like variable values to be included in traces, set this option. This option can take several forms: + + - `{ none: true }`: Don't send any variable values. **(DEFAULT)** + - `{ all: true }`: Send all variable values. + - `{ transform: ({ variables, operationString}) => { ... } }`: A custom function for modifying variable values. Keys added by the custom function will be removed, and keys removed will be added back with an empty value. + - `{ exceptNames: [...] }`: A case-sensitive list of names of variables whose values should not be sent to Apollo servers. + - `{ onlyNames: [...] }`: A case-sensitive list of names of variables whose values will be sent to Apollo servers. + + Defaults to not sending any variable values if both this parameter and the deprecated `privateVariables` are not set. + The report will indicate each private variable key whose value was redacted by `{ none: true }` or `{ exceptNames: [...]` }. + +* `privateVariables`: Array<String> | boolean + + > Will be deprecated in 3.0. Use the option `sendVariableValues` instead. + Passing an array into `privateVariables` is equivalent to + passing in `{ exceptNames: array } ` to `sendVariableValues`, and passing in `true` or `false` is equivalent + to passing ` { none: true } ` or ` { all: true }`, respectively. + + > Note: An error will be thrown if both this deprecated option and its replacement, `sendVariableValues` are defined. + In order to preserve the old default of `privateVariables`, which sends all variables and their values, pass in the `sendVariableValues` option: + `new ApolloServer({engine: {sendVariableValues: {all: true}}})`. + +* `sendHeaders`: { exceptNames: Array<String> } | { onlyNames: Array<String> } | { all: boolean } | { none: boolean } + By default, Apollo Server does not send the list of HTTP request headers and values to + Apollo's servers, to protect private data of your app's users. If you'd like this information included in traces, + set this option. This option can take several forms: + + - `{ none: true }`: Drop all HTTP request headers. **(DEFAULT)** + - `{ all: true }`: Send the values of all HTTP request headers. + - `{ exceptNames: [...] }`: A case-insensitive list of names of HTTP headers whose values should not be sent to Apollo servers. + - `{ onlyNames: [...] }`: A case-insensitive list of names of HTTP headers whose values will be sent to Apollo servers. + + Defaults to not sending any request header names and values if both this parameter and the deprecated `privateHeaders` are not set. + Unlike with `sendVariableValues`, names of dropped headers are not reported. + The headers 'authorization', 'cookie', and 'set-cookie' are never reported. + +* `privateHeaders`: Array<String> | boolean + + > Will be deprecated in 3.0. Use the `sendHeaders` option instead. + Passing an array into `privateHeaders` is equivalent to passing ` { exceptNames: array } ` into `sendHeaders`, and + passing `true` or `false` is equivalent to passing in ` { none: true } ` and ` { all: true }`, respectively. + + > Note: An error will be thrown if both this deprecated option and its replacement, `sendHeaders`, are defined. + In order to preserve the old default of `privateHeaders`, which sends all request headers and their values, pass in the `sendHeaders` option: + `new ApolloServer({engine: {sendHeaders: {all: true}}})`. + * `handleSignals`: boolean By default, EngineReportingAgent listens for the 'SIGINT' and 'SIGTERM' diff --git a/packages/apollo-engine-reporting/src/__tests__/agent.test.ts b/packages/apollo-engine-reporting/src/__tests__/agent.test.ts index 3a6c76ced79..ff6ac43ab6a 100644 --- a/packages/apollo-engine-reporting/src/__tests__/agent.test.ts +++ b/packages/apollo-engine-reporting/src/__tests__/agent.test.ts @@ -1,4 +1,8 @@ -import { signatureCacheKey } from '../agent'; +import { + signatureCacheKey, + handleLegacyOptions, + EngineReportingOptions, +} from '../agent'; describe('signature cache key', () => { it('generates without the operationName', () => { @@ -11,3 +15,92 @@ describe('signature cache key', () => { ); }); }); + +describe("test handleLegacyOptions(), which converts the deprecated privateVariable and privateHeaders options to the new options' formats", () => { + it('Case 1: privateVariables/privateHeaders == False; same as all', () => { + const optionsPrivateFalse: EngineReportingOptions = { + privateVariables: false, + privateHeaders: false, + }; + handleLegacyOptions(optionsPrivateFalse); + expect(optionsPrivateFalse.privateVariables).toBe(undefined); + expect(optionsPrivateFalse.sendVariableValues).toEqual({ all: true }); + expect(optionsPrivateFalse.privateHeaders).toBe(undefined); + expect(optionsPrivateFalse.sendHeaders).toEqual({ all: true }); + }); + + it('Case 2: privateVariables/privateHeaders == True; same as none', () => { + const optionsPrivateTrue: EngineReportingOptions = { + privateVariables: true, + privateHeaders: true, + }; + handleLegacyOptions(optionsPrivateTrue); + expect(optionsPrivateTrue.privateVariables).toBe(undefined); + expect(optionsPrivateTrue.sendVariableValues).toEqual({ none: true }); + expect(optionsPrivateTrue.privateHeaders).toBe(undefined); + expect(optionsPrivateTrue.sendHeaders).toEqual({ none: true }); + }); + + it('Case 3: privateVariables/privateHeaders set to an array', () => { + const privateArray: Array = ['t1', 't2']; + const optionsPrivateArray: EngineReportingOptions = { + privateVariables: privateArray, + privateHeaders: privateArray, + }; + handleLegacyOptions(optionsPrivateArray); + expect(optionsPrivateArray.privateVariables).toBe(undefined); + expect(optionsPrivateArray.sendVariableValues).toEqual({ + exceptNames: privateArray, + }); + expect(optionsPrivateArray.privateHeaders).toBe(undefined); + expect(optionsPrivateArray.sendHeaders).toEqual({ + exceptNames: privateArray, + }); + }); + + it('Case 4: privateVariables/privateHeaders are null or undefined; no change', () => { + const optionsPrivateFalse: EngineReportingOptions = { + privateVariables: undefined, + privateHeaders: null, // null is not a valid TS input, but check the output anyways + }; + handleLegacyOptions(optionsPrivateFalse); + expect(optionsPrivateFalse.privateVariables).toBe(undefined); + expect(optionsPrivateFalse.sendVariableValues).toBe(undefined); + expect(optionsPrivateFalse.privateHeaders).toBe(undefined); + expect(optionsPrivateFalse.sendHeaders).toBe(undefined); + }); + + it('Case 5: throws error when both the new and old options are set', () => { + const optionsBothVariables: EngineReportingOptions = { + privateVariables: true, + sendVariableValues: { none: true }, + }; + expect(() => { + handleLegacyOptions(optionsBothVariables); + }).toThrow(); + const optionsBothHeaders: EngineReportingOptions = { + privateHeaders: true, + sendHeaders: { none: true }, + }; + expect(() => { + handleLegacyOptions(optionsBothHeaders); + }).toThrow(); + }); + + it('Case 6: the passed in options are not modified if deprecated fields were not set', () => { + const optionsNotDeprecated: EngineReportingOptions = { + sendVariableValues: { exceptNames: ['test'] }, + sendHeaders: { all: true }, + }; + const output: EngineReportingOptions = { + sendVariableValues: { exceptNames: ['test'] }, + sendHeaders: { all: true }, + }; + handleLegacyOptions(optionsNotDeprecated); + expect(optionsNotDeprecated).toEqual(output); + + const emptyInput: EngineReportingOptions = {}; + handleLegacyOptions(emptyInput); + expect(emptyInput).toEqual({}); + }); +}); diff --git a/packages/apollo-engine-reporting/src/__tests__/extension.test.ts b/packages/apollo-engine-reporting/src/__tests__/extension.test.ts index c61eccbfd75..7d6dc27daca 100644 --- a/packages/apollo-engine-reporting/src/__tests__/extension.test.ts +++ b/packages/apollo-engine-reporting/src/__tests__/extension.test.ts @@ -6,7 +6,14 @@ import { import { Trace } from 'apollo-engine-reporting-protobuf'; import { graphql } from 'graphql'; import { Request } from 'node-fetch'; -import { EngineReportingExtension } from '../extension'; +import { + EngineReportingExtension, + makeTraceDetails, + makeTraceDetailsLegacy, + makeHTTPRequestHeaders, + makeHTTPRequestHeadersLegacy, +} from '../extension'; +import { Headers } from 'apollo-server-env'; import { InMemoryLRUCache } from 'apollo-server-caching'; test('trace construction', async () => { @@ -83,3 +90,271 @@ test('trace construction', async () => { requestDidEnd(); // XXX actually write some tests }); + +/** + * TESTS FOR sendVariableValues REPORTING OPTION + */ +const variables: Record = { + testing: 'testing', + t2: 2, +}; + +describe('check variableJson output for sendVariableValues null or undefined (default)', () => { + it('Case 1: No keys/values in variables to be filtered/not filtered', () => { + const emptyOutput = new Trace.Details(); + expect(makeTraceDetails({}, null)).toEqual(emptyOutput); + expect(makeTraceDetails({}, undefined)).toEqual(emptyOutput); + expect(makeTraceDetails({})).toEqual(emptyOutput); + }); + it('Case 2: Filter all variables', () => { + const filteredOutput = new Trace.Details(); + Object.keys(variables).forEach(name => { + filteredOutput.variablesJson[name] = ''; + }); + expect(makeTraceDetails(variables)).toEqual(filteredOutput); + expect(makeTraceDetails(variables, null)).toEqual(filteredOutput); + expect(makeTraceDetails(variables, undefined)).toEqual(filteredOutput); + }); +}); + +describe('check variableJson output for sendVariableValues all/none type', () => { + it('Case 1: No keys/values in variables to be filtered/not filtered', () => { + const emptyOutput = new Trace.Details(); + expect(makeTraceDetails({}, { all: true })).toEqual(emptyOutput); + expect(makeTraceDetails({}, { none: true })).toEqual(emptyOutput); + }); + + const filteredOutput = new Trace.Details(); + Object.keys(variables).forEach(name => { + filteredOutput.variablesJson[name] = ''; + }); + + const nonFilteredOutput = new Trace.Details(); + Object.keys(variables).forEach(name => { + nonFilteredOutput.variablesJson[name] = JSON.stringify(variables[name]); + }); + + it('Case 2: Filter all variables', () => { + expect(makeTraceDetails(variables, { none: true })).toEqual(filteredOutput); + }); + + it('Case 3: Do not filter variables', () => { + expect(makeTraceDetails(variables, { all: true })).toEqual( + nonFilteredOutput, + ); + }); + + it('Case 4: Check behavior for invalid inputs', () => { + expect(makeTraceDetails(variables, { none: false })).toEqual( + nonFilteredOutput, + ); + + expect(makeTraceDetails(variables, { all: false })).toEqual(filteredOutput); + }); +}); + +describe('variableJson output for sendVariableValues exceptNames: Array type', () => { + it('array contains some values not in keys', () => { + const privateVariablesArray: string[] = ['testing', 'notInVariables']; + const expectedVariablesJson = { + testing: '', + t2: JSON.stringify(2), + }; + expect( + makeTraceDetails(variables, { exceptNames: privateVariablesArray }) + .variablesJson, + ).toEqual(expectedVariablesJson); + }); + + it('none=true equivalent to exceptNames=[all variables]', () => { + const privateVariablesArray: string[] = ['testing', 't2']; + expect(makeTraceDetails(variables, { none: true }).variablesJson).toEqual( + makeTraceDetails(variables, { exceptNames: privateVariablesArray }) + .variablesJson, + ); + }); +}); + +describe('variableJson output for sendVariableValues onlyNames: Array type', () => { + it('array contains some values not in keys', () => { + const privateVariablesArray: string[] = ['t2', 'notInVariables']; + const expectedVariablesJson = { + testing: '', + t2: JSON.stringify(2), + }; + expect( + makeTraceDetails(variables, { onlyNames: privateVariablesArray }) + .variablesJson, + ).toEqual(expectedVariablesJson); + }); + + it('all=true equivalent to onlyNames=[all variables]', () => { + const privateVariablesArray: string[] = ['testing', 't2']; + expect(makeTraceDetails(variables, { all: true }).variablesJson).toEqual( + makeTraceDetails(variables, { onlyNames: privateVariablesArray }) + .variablesJson, + ); + }); + + it('none=true equivalent to onlyNames=[]', () => { + const privateVariablesArray: string[] = []; + expect(makeTraceDetails(variables, { none: true }).variablesJson).toEqual( + makeTraceDetails(variables, { onlyNames: privateVariablesArray }) + .variablesJson, + ); + }); +}); + +describe('variableJson output for sendVariableValues transform: custom function type', () => { + it('test custom function that redacts every variable to some value', () => { + const modifiedValue = 100; + const customModifier = (input: { + variables: Record; + }): Record => { + const out: Record = Object.create(null); + Object.keys(input.variables).map((name: string) => { + out[name] = modifiedValue; + }); + return out; + }; + + // Expected output + const output = new Trace.Details(); + Object.keys(variables).forEach(name => { + output.variablesJson[name] = JSON.stringify(modifiedValue); + }); + + expect(makeTraceDetails(variables, { transform: customModifier })).toEqual( + output, + ); + }); + + const origKeys = Object.keys(variables); + const firstKey = origKeys[0]; + const secondKey = origKeys[1]; + + const modifier = (input: { + variables: Record; + }): Record => { + const out: Record = Object.create(null); + Object.keys(input.variables).map((name: string) => { + out[name] = null; + }); + // remove the first key, and then add a new key + delete out[firstKey]; + out['newkey'] = 'blah'; + return out; + }; + + it('original keys in variables should match the modified keys', () => { + expect( + Object.keys( + makeTraceDetails(variables, { transform: modifier }).variablesJson, + ).sort(), + ).toEqual(origKeys.sort()); + }); + + it('expect empty string for keys removed by the custom modifier', () => { + expect( + makeTraceDetails(variables, { transform: modifier }).variablesJson[ + firstKey + ], + ).toEqual(''); + }); + + it('expect stringify(null) for values set to null by custom modifier', () => { + expect( + makeTraceDetails(variables, { transform: modifier }).variablesJson[ + secondKey + ], + ).toEqual(JSON.stringify(null)); + }); +}); + +describe('Catch circular reference error during JSON.stringify', () => { + const circularReference = {}; + circularReference['this'] = circularReference; + + const circularVariables = { + bad: circularReference, + }; + + expect( + makeTraceDetails(circularVariables, { all: true }).variablesJson['bad'], + ).toEqual(JSON.stringify('[Unable to convert value to JSON]')); +}); + +function makeTestHTTP(): Trace.HTTP { + return new Trace.HTTP({ + method: Trace.HTTP.Method.UNKNOWN, + host: null, + path: null, + }); +} + +/** + * TESTS FOR THE sendHeaders REPORTING OPTION + */ +const headers = new Headers(); +headers.append('name', 'value'); +headers.append('authorization', 'blahblah'); // THIS SHOULD NEVER BE SENT + +const headersOutput = { name: new Trace.HTTP.Values({ value: ['value'] }) }; + +describe('tests for the sendHeaders reporting option', () => { + it('sendHeaders defaults to hiding all', () => { + const http = makeTestHTTP(); + // sendHeaders: null is not a valid TS input, but check the output anyways + makeHTTPRequestHeaders(http, headers, null); + expect(http.requestHeaders).toEqual({}); + makeHTTPRequestHeaders(http, headers, undefined); + expect(http.requestHeaders).toEqual({}); + makeHTTPRequestHeaders(http, headers); + expect(http.requestHeaders).toEqual({}); + }); + + it('sendHeaders.all and sendHeaders.none', () => { + const httpSafelist = makeTestHTTP(); + makeHTTPRequestHeaders(httpSafelist, headers, { all: true }); + expect(httpSafelist.requestHeaders).toEqual(headersOutput); + + const httpBlocklist = makeTestHTTP(); + makeHTTPRequestHeaders(httpBlocklist, headers, { none: true }); + expect(httpBlocklist.requestHeaders).toEqual({}); + }); + + it('invalid inputs for sendHeaders.all and sendHeaders.none', () => { + const httpSafelist = makeTestHTTP(); + makeHTTPRequestHeaders(httpSafelist, headers, { none: false }); + expect(httpSafelist.requestHeaders).toEqual(headersOutput); + + const httpBlocklist = makeTestHTTP(); + makeHTTPRequestHeaders(httpBlocklist, headers, { all: false }); + expect(httpBlocklist.requestHeaders).toEqual({}); + }); + + it('test sendHeaders.exceptNames', () => { + const except: String[] = ['name', 'notinheaders']; + const http = makeTestHTTP(); + makeHTTPRequestHeaders(http, headers, { exceptNames: except }); + expect(http.requestHeaders).toEqual({}); + }); + + it('test sendHeaders.onlyNames', () => { + // headers that should never be sent (such as "authorization") should still be removed if in includeHeaders + const include: String[] = ['name', 'authorization', 'notinheaders']; + const http = makeTestHTTP(); + makeHTTPRequestHeaders(http, headers, { onlyNames: include }); + expect(http.requestHeaders).toEqual(headersOutput); + }); + + it('authorization, cookie, and set-cookie headers should never be sent', () => { + headers.append('cookie', 'blahblah'); + headers.append('set-cookie', 'blahblah'); + const http = makeTestHTTP(); + makeHTTPRequestHeaders(http, headers, { all: true }); + expect(http.requestHeaders['authorization']).toBe(undefined); + expect(http.requestHeaders['cookie']).toBe(undefined); + expect(http.requestHeaders['set-cookie']).toBe(undefined); + }); +}); diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts index 2535a42a4a8..b05ed2e8861 100644 --- a/packages/apollo-engine-reporting/src/agent.ts +++ b/packages/apollo-engine-reporting/src/agent.ts @@ -25,6 +25,25 @@ export interface ClientInfo { clientReferenceId?: string; } +export type SendValuesBaseOptions = + | { onlyNames: Array } + | { exceptNames: Array } + | { all: true } + | { none: true }; + +type VariableValueTransformOptions = { + variables: Record; + operationString?: string; +}; + +export type VariableValueOptions = + | { + transform: ( + options: VariableValueTransformOptions, + ) => Record; + } + | SendValuesBaseOptions; + export type GenerateClientInfo = ( requestContext: GraphQLRequestContext, ) => ClientInfo; @@ -82,16 +101,57 @@ export interface EngineReportingOptions { */ reportErrorFunction?: (err: Error) => void; /** - * A case-sensitive list of names of variables whose values should not be sent - * to Apollo servers, or 'true' to leave out all variables. In the former - * case, the report will indicate that each private variable was redacted; in - * the latter case, no variables are sent at all. + * By default, Apollo Server does not send the values of any GraphQL variables to Apollo's servers, because variable + * values often contain the private data of your app's users. If you'd like variable values to be included in traces, set this option. + * This option can take several forms: + * - { none: true }: don't send any variable values (DEFAULT) + * - { all: true}: send all variable values + * - { transform: ... }: a custom function for modifying variable values. Keys added by the custom function will + * be removed, and keys removed will be added back with an empty value. + * - { exceptNames: ... }: a case-sensitive list of names of variables whose values should not be sent to Apollo servers + * - { onlyNames: ... }: A case-sensitive list of names of variables whose values will be sent to Apollo servers + * + * Defaults to not sending any variable values if both this parameter and + * the deprecated `privateVariables` are not set. The report will + * indicate each private variable key whose value was redacted by { none: true } or { exceptNames: [...] }. + * + * TODO(helen): Add new flag to the trace details (and modify the protobuf message structure) to indicate the type of modification. Then, add the following description to the docs: + * "The report will indicate that variable values were modified by a custom function, or will list all private variables redacted." + * TODO(helen): LINK TO EXAMPLE FUNCTION? e.g. a function recursively search for keys to be blocklisted + */ + sendVariableValues?: VariableValueOptions; + /** + * [DEPRECATED] Use sendVariableValues + * Passing an array into privateVariables is equivalent to passing { exceptNames: array } into + * sendVariableValues, and passing in `true` or `false` is equivalent to passing { none: true } or + * { all: true }, respectively. + * + * An error will be thrown if both this deprecated option and its replacement, sendVariableValues are defined. */ privateVariables?: Array | boolean; /** - * A case-insensitive list of names of HTTP headers whose values should not be - * sent to Apollo servers, or 'true' to leave out all HTTP headers. Unlike - * with privateVariables, names of dropped headers are not reported. + * By default, Apollo Server does not send the list of HTTP headers and values to + * Apollo's servers, to protect private data of your app's users. If you'd like this information included in traces, + * set this option. This option can take several forms: + * + * - { none: true } to drop all HTTP request headers (DEFAULT) + * - { all: true } to send the values of all HTTP request headers + * - { exceptNames: Array } A case-insensitive list of names of HTTP headers whose values should not be + * sent to Apollo servers + * - { onlyNames: Array }: A case-insensitive list of names of HTTP headers whose values will be sent to Apollo servers + * + * Defaults to not sending any request header names and values if both this parameter and + * the deprecated `privateHeaders` are not set. + * Unlike with sendVariableValues, names of dropped headers are not reported. + * The headers 'authorization', 'cookie', and 'set-cookie' are never reported. + */ + sendHeaders?: SendValuesBaseOptions; + /** + * [DEPRECATED] Use sendHeaders + * Passing an array into privateHeaders is equivalent to passing { exceptNames: array } into sendHeaders, and + * passing `true` or `false` is equivalent to passing in { none: true } and { all: true }, respectively. + * + * An error will be thrown if both this deprecated option and its replacement, sendHeaders are defined. */ privateHeaders?: Array | boolean; /** @@ -203,6 +263,9 @@ export class EngineReportingAgent { }); }); } + + // Handle the legacy options: privateVariables and privateHeaders + handleLegacyOptions(this.options); } public newExtension(): EngineReportingExtension { @@ -478,3 +541,57 @@ function createSignatureCache(): InMemoryLRUCache { export function signatureCacheKey(queryHash: string, operationName: string) { return `${queryHash}${operationName && ':' + operationName}`; } + +// Helper function to modify the EngineReportingOptions if the deprecated fields 'privateVariables' and 'privateHeaders' +// were set. +// - Throws an error if both the deprecated option and its replacement (e.g. 'privateVariables' and 'sendVariableValues') were set. +// - Otherwise, wraps the deprecated option into objects that can be passed to the new replacement field (see the helper +// function makeSendValuesBaseOptionsFromLegacy), and deletes the deprecated field from the options +export function handleLegacyOptions( + options: EngineReportingOptions, +): void { + // Handle the legacy option: privateVariables + if ( + typeof options.privateVariables !== 'undefined' && + options.sendVariableValues + ) { + throw new Error( + "You have set both the 'sendVariableValues' and the deprecated 'privateVariables' options. Please only set 'sendVariableValues'.", + ); + } else if (typeof options.privateVariables !== 'undefined') { + if (options.privateVariables !== null) { + options.sendVariableValues = makeSendValuesBaseOptionsFromLegacy( + options.privateVariables, + ); + } + delete options.privateVariables; + } + + // Handle the legacy option: privateHeaders + if (typeof options.privateHeaders !== 'undefined' && options.sendHeaders) { + throw new Error( + "You have set both the 'sendHeaders' and the deprecated 'privateHeaders' options. Please only set 'sendHeaders'.", + ); + } else if (typeof options.privateHeaders !== 'undefined') { + if (options.privateHeaders !== null) { + options.sendHeaders = makeSendValuesBaseOptionsFromLegacy( + options.privateHeaders, + ); + } + delete options.privateHeaders; + } +} + +// This helper wraps non-null inputs from the deprecated options 'privateVariables' and 'privateHeaders' into +// objects that can be passed to the new replacement options, 'sendVariableValues' and 'sendHeaders' +function makeSendValuesBaseOptionsFromLegacy( + legacyPrivateOption: Array | boolean, +): SendValuesBaseOptions { + return Array.isArray(legacyPrivateOption) + ? { + exceptNames: legacyPrivateOption, + } + : legacyPrivateOption + ? { none: true } + : { all: true }; +} diff --git a/packages/apollo-engine-reporting/src/extension.ts b/packages/apollo-engine-reporting/src/extension.ts index 88b8e3dac0c..1a65393f8e0 100644 --- a/packages/apollo-engine-reporting/src/extension.ts +++ b/packages/apollo-engine-reporting/src/extension.ts @@ -1,4 +1,4 @@ -import { Request, WithRequired } from 'apollo-server-env'; +import { Request, Headers, WithRequired } from 'apollo-server-env'; import { GraphQLResolveInfo, @@ -15,6 +15,8 @@ import { EngineReportingOptions, GenerateClientInfo, AddTraceArgs, + VariableValueOptions, + SendValuesBaseOptions, } from './agent'; import { GraphQLRequestContext } from 'apollo-server-core/dist/requestPipelineAPI'; @@ -91,33 +93,13 @@ export class EngineReportingExtension host: null, path: null, }); - if (this.options.privateHeaders !== true) { - for (const [key, value] of o.request.headers) { - if ( - this.options.privateHeaders && - Array.isArray(this.options.privateHeaders) && - // We assume that most users only have a few private headers, or will - // just set privateHeaders to true; we can change this linear-time - // operation if it causes real performance issues. - this.options.privateHeaders.some(privateHeader => { - // Headers are case-insensitive, and should be compared as such. - return privateHeader.toLowerCase() === key.toLowerCase(); - }) - ) { - continue; - } - - switch (key) { - case 'authorization': - case 'cookie': - case 'set-cookie': - break; - default: - this.trace.http!.requestHeaders![key] = new Trace.HTTP.Values({ - value: [value], - }); - } - } + + if (this.options.sendHeaders) { + makeHTTPRequestHeaders( + this.trace.http, + o.request.headers, + this.options.sendHeaders, + ); if (o.requestContext.metrics.persistedQueryHit) { this.trace.persistedQueryHit = true; @@ -127,41 +109,12 @@ export class EngineReportingExtension } } - if (this.options.privateVariables !== true && o.variables) { - // Note: we explicitly do *not* include the details.rawQuery field. The - // Engine web app currently does nothing with this other than store it in - // the database and offer it up via its GraphQL API, and sending it means - // that using calculateSignature to hide sensitive data in the query - // string is ineffective. - this.trace.details = new Trace.Details(); - Object.keys(o.variables).forEach(name => { - if ( - this.options.privateVariables && - Array.isArray(this.options.privateVariables) && - // We assume that most users will have only a few private variables, - // or will just set privateVariables to true; we can change this - // linear-time operation if it causes real performance issues. - this.options.privateVariables.includes(name) - ) { - // Special case for private variables. Note that this is a different - // representation from a variable containing the empty string, as that - // will be sent as '""'. - this.trace.details!.variablesJson![name] = ''; - } else { - try { - this.trace.details!.variablesJson![name] = JSON.stringify( - o.variables![name], - ); - } catch (e) { - // This probably means that the value contains a circular reference, - // causing `JSON.stringify()` to throw a TypeError: - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Issue_with_JSON.stringify()_when_serializing_circular_references - this.trace.details!.variablesJson![name] = JSON.stringify( - '[Unable to convert value to JSON]', - ); - } - } - }); + if (o.variables) { + this.trace.details = makeTraceDetails( + o.variables, + this.options.sendVariableValues, + o.queryString, + ); } const clientInfo = this.generateClientInfo(o.requestContext); @@ -424,3 +377,125 @@ function defaultGenerateClientInfo({ request }: GraphQLRequestContext) { return {}; } } + +// Creates trace details from request variables, given a specification for modifying +// values of private or sensitive variables. +// The details will include all variable names and their (possibly hidden or modified) values. +// If sendVariableValues is {all: bool}, {none: bool} or {exceptNames: Array}, the option will act similarly to +// to the to-be-deprecated options.privateVariables, except that the redacted variable +// names will still be visible in the UI even if the values are hidden. +// If sendVariableValues is null or undefined, we default to the {none: true} case. +export function makeTraceDetails( + variables: Record, + sendVariableValues?: VariableValueOptions, + operationString?: string, +): Trace.Details { + const details = new Trace.Details(); + const variablesToRecord = (() => { + if (sendVariableValues && 'transform' in sendVariableValues) { + // Custom function to allow user to specify what variablesJson will look like + const originalKeys = Object.keys(variables); + const modifiedVariables = sendVariableValues.transform({ + variables: variables, + operationString: operationString, + }); + return cleanModifiedVariables(originalKeys, modifiedVariables); + } else { + return variables; + } + })(); + + // Note: we explicitly do *not* include the details.rawQuery field. The + // Engine web app currently does nothing with this other than store it in + // the database and offer it up via its GraphQL API, and sending it means + // that using calculateSignature to hide sensitive data in the query + // string is ineffective. + Object.keys(variablesToRecord).forEach(name => { + if ( + !sendVariableValues || + ('none' in sendVariableValues && sendVariableValues.none) || + ('all' in sendVariableValues && !sendVariableValues.all) || + ('exceptNames' in sendVariableValues && + // We assume that most users will have only a few variables values to hide, + // or will just set {none: true}; we can change this + // linear-time operation if it causes real performance issues. + sendVariableValues.exceptNames.includes(name)) || + ('onlyNames' in sendVariableValues && + !sendVariableValues.onlyNames.includes(name)) + ) { + // Special case for private variables. Note that this is a different + // representation from a variable containing the empty string, as that + // will be sent as '""'. + details.variablesJson![name] = ''; + } else { + try { + details.variablesJson![name] = + typeof variablesToRecord[name] === 'undefined' + ? '' + : JSON.stringify(variablesToRecord[name]); + } catch (e) { + details.variablesJson![name] = JSON.stringify( + '[Unable to convert value to JSON]', + ); + } + } + }); + return details; +} + +// Helper for makeTraceDetails() to enforce that the keys of a modified 'variables' +// matches that of the original 'variables' +function cleanModifiedVariables( + originalKeys: Array, + modifiedVariables: Record, +): Record { + const cleanedVariables: Record = Object.create(null); + originalKeys.forEach(name => { + cleanedVariables[name] = modifiedVariables[name]; + }); + return cleanedVariables; +} + +export function makeHTTPRequestHeaders( + http: Trace.IHTTP, + headers: Headers, + sendHeaders?: SendValuesBaseOptions, +): void { + if ( + !sendHeaders || + ('none' in sendHeaders && sendHeaders.none) || + ('all' in sendHeaders && !sendHeaders.all) + ) { + return; + } + for (const [key, value] of headers) { + const lowerCaseKey = key.toLowerCase(); + if ( + ('exceptNames' in sendHeaders && + // We assume that most users only have a few headers to hide, or will + // just set {none: true} ; we can change this linear-time + // operation if it causes real performance issues. + sendHeaders.exceptNames.some(exceptHeader => { + // Headers are case-insensitive, and should be compared as such. + return exceptHeader.toLowerCase() === lowerCaseKey; + })) || + ('onlyNames' in sendHeaders && + !sendHeaders.onlyNames.some(header => { + return header.toLowerCase() === lowerCaseKey; + })) + ) { + continue; + } + + switch (key) { + case 'authorization': + case 'cookie': + case 'set-cookie': + break; + default: + http!.requestHeaders![key] = new Trace.HTTP.Values({ + value: [value], + }); + } + } +}