diff --git a/packages/server/logging/expressLogging.ts b/packages/server/logging/expressLogging.ts index 04851ba23..0f3237bcc 100644 --- a/packages/server/logging/expressLogging.ts +++ b/packages/server/logging/expressLogging.ts @@ -7,7 +7,7 @@ import type { SerializedResponse } from 'pino' import type { GenReqId } from 'pino-http' import type { IncomingMessage, ServerResponse } from 'http' import { ensureError, type Optional } from '@speckle/shared' -import { getRequestPath } from '@/modules/core/helpers/server' +import { getRequestParameters, getRequestPath } from '@/modules/core/helpers/server' import { get } from 'lodash' const REQUEST_ID_HEADER = 'x-request-id' @@ -39,6 +39,17 @@ export const sanitizeHeaders = (headers: Record) => ) ) +export const sanitizeQueryParams = ( + query: Record +) => { + Object.keys(query).forEach(function (key) { + if (['code', 'state'].includes(key.toLocaleLowerCase())) { + query[key] = '******' + } + }) + return query +} + export const LoggingExpressMiddleware = HttpLogger({ logger, autoLogging: true, @@ -122,7 +133,9 @@ export const LoggingExpressMiddleware = HttpLogger({ id: req.raw.id, method: req.raw.method, path: getRequestPath(req.raw), - // Allowlist useful headers + // Denylist potentially sensitive query parameters + pathParameters: sanitizeQueryParams(getRequestParameters(req.raw)), + // Denylist potentially sensitive headers headers: sanitizeHeaders(req.raw.headers) } }), diff --git a/packages/server/modules/core/helpers/server.ts b/packages/server/modules/core/helpers/server.ts index 7413ef4d3..26a6ce228 100644 --- a/packages/server/modules/core/helpers/server.ts +++ b/packages/server/modules/core/helpers/server.ts @@ -1,10 +1,21 @@ +import { getServerOrigin } from '@/modules/shared/helpers/envHelper' import type { Request } from 'express' import type { IncomingMessage } from 'http' import { get } from 'lodash' +import { parse } from 'url' export const getRequestPath = (req: IncomingMessage | Request) => { - const path = ((get(req, 'originalUrl') || get(req, 'url') || '') as string).split( - '?' - )[0] - return path?.length ? path : null + const maybeUrl = get(req, 'originalUrl') || get(req, 'url') || ('' as string) + const url = new URL(maybeUrl, getServerOrigin()) + const path = url.pathname + if (!path || !path.length) return null + if (path === '/') return null + + return path +} + +export const getRequestParameters = (req: IncomingMessage | Request) => { + const maybeUrl = get(req, 'originalUrl') || get(req, 'url') || '' + const url = parse(maybeUrl, true) + return url.query || {} }