Merge pull request #56 from arup-group/jm/dev/sync-upstream

Jm/dev/sync upstream

packages/fileimport-service/package.json

@@ -24,7 +24,6 @@
   },
   "dependencies": {
     "@speckle/shared": "workspace:^",
-    "aws-sdk": "^2.1075.0",
     "bcrypt": "^5.0.1",
     "crypto-random-string": "^3.3.1",
     "knex": "^2.4.1",

packages/fileimport-service/requirements.txt

@@ -1,3 +1,5 @@
 numpy-stl==3.0.0
 specklepy==2.9.1
 structlog==22.3.0
+numpy==1.24.2 # not directly required, pinned to avoid a vulnerability in <1.22.2
+python-util==1.2.1 # not directly required, peer dependency of numpy-stl

packages/frontend/Dockerfile

@@ -28,7 +28,9 @@ COPY packages/frontend ./packages/frontend/
 COPY packages/shared ./packages/shared/
 
 # This way the foreach only builds the frontend and its deps
-RUN yarn workspaces foreach run build
+RUN --mount=type=secret,id=posthog_api_key \
+    export VITE_POSTHOG_API_KEY=$(cat /run/secrets/posthog_api_key) && \
+    yarn workspaces foreach run build
 
 RUN DEBIAN_FRONTEND=noninteractive \
     apt-get -q update && \

packages/preview-service/Dockerfile

@@ -1,7 +1,7 @@
 # NOTE: Docker context should be set to git root directory, to include the viewer
 ARG NODE_ENV=production
 
-FROM node:18.14.1-bullseye-slim as build-stage
+FROM node:18.15.0-bullseye-slim as build-stage
 
 ARG NODE_ENV
 ENV NODE_ENV=${NODE_ENV}
@@ -35,7 +35,7 @@ COPY packages/preview-service ./packages/preview-service/
 # This way the foreach only builds the frontend and its deps
 RUN yarn workspaces foreach run build
 
-FROM node:18.14.1-bullseye-slim as node
+FROM node:18.15.0-bullseye-slim as node
 
 RUN apt-get update && \
   DEBIAN_FRONTEND=noninteractive apt-get install -y \

packages/server/.mocharc.js

@@ -2,7 +2,7 @@
 
 /** @type {import("mocha").MochaOptions} */
 const config = {
-  spec: ['modules/**/*.spec.js', 'modules/**/*.spec.ts'],
+  spec: ['modules/**/*.spec.js', 'modules/**/*.spec.ts', 'logging/**/*.spec.js'],
   require: ['ts-node/register', 'test/hooks.js'],
   slow: 0,
   timeout: '150000',

packages/server/Dockerfile

@@ -1,7 +1,7 @@
 ARG NODE_ENV=production
 ARG SPECKLE_SERVER_VERSION=custom
 
-FROM node:18.14.1-bullseye-slim as build-stage
+FROM node:18.15.0-bullseye-slim as build-stage
 ARG NODE_ENV
 ARG SPECKLE_SERVER_VERSION
 WORKDIR /speckle-server
@@ -38,7 +38,7 @@ RUN yarn workspaces foreach run build
 
 # install only production dependencies
 # we need a clean environment, free of build dependencies
-FROM node:18.14.1-bullseye-slim as dependency-stage
+FROM node:18.15.0-bullseye-slim as dependency-stage
 ARG NODE_ENV
 ARG SPECKLE_SERVER_VERSION
 
@@ -54,7 +54,9 @@ COPY packages/objectloader/package.json ./packages/objectloader/
 WORKDIR /speckle-server/packages/server
 RUN yarn workspaces focus --production
 
-FROM node:18.14.1-bullseye-slim as production-stage
+FROM node:18.15.0-bullseye-slim as production-stage
+ARG NODE_ENV
+ARG SPECKLE_SERVER_VERSION
 ARG FILE_SIZE_LIMIT_MB=100
 ENV FILE_SIZE_LIMIT_MB=${FILE_SIZE_LIMIT_MB} \
     NODE_ENV=${NODE_ENV} \

packages/server/app.ts

@@ -44,6 +44,7 @@ import { get, has, isString, toNumber } from 'lodash'
 import {
   authContextMiddleware,
   buildContext,
+  determineClientIpAddressMiddleware,
   mixpanelTrackerHelperMiddleware
 } from '@/modules/shared/middleware'
 
@@ -191,6 +192,7 @@ export async function init() {
   await knex.migrate.latest()
 
   app.use(DetermineRequestIdMiddleware)
+  app.use(determineClientIpAddressMiddleware)
   app.use(LoggingExpressMiddleware)
 
   if (process.env.COMPRESSION) {

packages/server/logging/apolloPlugin.js

@@ -1,8 +1,11 @@
+/* eslint-disable camelcase */
 /* istanbul ignore file */
-// const { logger } = require('@/logging/logging')
 const Sentry = require('@sentry/node')
 const { ApolloError } = require('apollo-server-express')
 const prometheusClient = require('prom-client')
+const { graphqlLogger } = require('@/logging/logging')
+const { redactSensitiveVariables } = require('@/logging/loggingHelper')
+const { GraphQLError } = require('graphql')
 
 const metricCallCount = new prometheusClient.Counter({
   name: 'speckle_server_apollo_calls',
@@ -20,13 +23,30 @@ module.exports = {
           return
         }
 
+        let logger = ctx.context.log || graphqlLogger
+
+        const op = `GQL ${ctx.operation.operation} ${ctx.operation.selectionSet.selections[0].name.value}`
+        const name = `GQL ${ctx.operation.selectionSet.selections[0].name.value}`
+        const kind = ctx.operation.operation
+        const query = ctx.request.query
+        const variables = ctx.request.variables
+
+        logger = logger.child({
+          graphql_operation_kind: kind,
+          graphql_query: query,
+          graphql_variables: redactSensitiveVariables(variables),
+          graphql_operation_value: op,
+          grqphql_operation_name: name
+        })
+
         const transaction = Sentry.startTransaction({
-          op: `GQL ${ctx.operation.operation} ${ctx.operation.selectionSet.selections[0].name.value}`,
-          name: `GQL ${ctx.operation.selectionSet.selections[0].name.value}`
+          op,
+          name
         })
 
         try {
           const actionName = `${ctx.operation.operation} ${ctx.operation.selectionSet.selections[0].name.value}`
+          logger = logger.child({ actionName })
           metricCallCount.labels(actionName).inc()
           // logger.debug(actionName)
         } catch (e) {
@@ -35,18 +55,35 @@ module.exports = {
 
         Sentry.configureScope((scope) => scope.setSpan(transaction))
         ctx.request.transaction = transaction
+        ctx.context.log = logger
       },
       didEncounterErrors(ctx) {
         if (!ctx.operation) return
 
+        let logger = ctx.context.log || graphqlLogger
+
         for (const err of ctx.errors) {
           if (err instanceof ApolloError) {
             continue
           }
+
+          const kind = ctx.operation.operation
+          const query = ctx.request.query
+          const variables = ctx.request.variables
+
+          if (err.path) {
+            logger = logger.child({ 'query-path': err.path.join(' > ') })
+          }
+          if (err instanceof GraphQLError && err.extensions?.code === 'FORBIDDEN') {
+            logger.info(err, 'graphql error')
+          } else {
+            logger.error(err, 'graphql error')
+          }
+
           Sentry.withScope((scope) => {
-            scope.setTag('kind', ctx.operation.operation)
-            scope.setExtra('query', ctx.request.query)
-            scope.setExtra('variables', ctx.request.variables)
+            scope.setTag('kind', kind)
+            scope.setExtra('query', query)
+            scope.setExtra('variables', variables)
             if (err.path) {
               // We can also add the path as breadcrumb
               scope.addBreadcrumb({
@@ -60,6 +97,9 @@ module.exports = {
         }
       },
       willSendResponse(ctx) {
+        const logger = ctx.context.log || graphqlLogger
+        logger.info('graphql response')
+
         if (ctx.request.transaction) {
           ctx.request.transaction.finish()
         }

packages/server/logging/expressLogging.ts

@@ -44,12 +44,19 @@ export const LoggingExpressMiddleware = HttpLogger({
         method: req.raw.method,
         path: req.raw.url?.split('?')[0], // Remove query params which might be sensitive
         // Allowlist useful headers
-        headers: {
-          host: req.raw.headers.host,
-          'user-agent': req.raw.headers['user-agent'],
-          'x-request-id': req.raw.headers[REQUEST_ID_HEADER],
-          referer: req.raw.headers.referer
-        }
+        headers: Object.fromEntries(
+          Object.entries(req.raw.headers).filter(
+            ([key]) =>
+              ![
+                'cookie',
+                'authorization',
+                'cf-connecting-ip',
+                'true-client-ip',
+                'x-real-ip',
+                'x-forwarded-for'
+              ].includes(key.toLocaleLowerCase())
+          )
+        )
       }
     }),
     res: pino.stdSerializers.wrapResponseSerializer((res) => {
@@ -61,15 +68,7 @@ export const LoggingExpressMiddleware = HttpLogger({
       return {
         statusCode: res.raw.statusCode,
         // Allowlist useful headers
-        headers: {
-          'content-length': resRaw.raw.headers['content-length'],
-          'content-type': resRaw.raw.headers['content-type'],
-          'retry-after': resRaw.raw.headers['retry-after'],
-          'x-ratelimit-remaining': resRaw.raw.headers['x-ratelimit-remaining'],
-          'x-ratelimit-reset': resRaw.raw.headers['x-ratelimit-reset'],
-          'x-request-id': resRaw.raw.headers['x-request-id'],
-          'x-speckle-meditation': resRaw.raw.headers['x-speckle-meditation']
-        }
+        headers: resRaw.raw.headers
       }
     })
   }
@@ -81,6 +80,7 @@ export const DetermineRequestIdMiddleware = (
   next: NextFunction
 ) => {
   const id = DetermineRequestId(req)
+  req.headers[REQUEST_ID_HEADER] = id
   res.setHeader(REQUEST_ID_HEADER, id)
   next()
 }

packages/server/logging/logging.ts

@@ -25,3 +25,4 @@ export const servicesLogger = extendLoggerComponent(logger, 'services')
 export const rateLimiterLogger = extendLoggerComponent(logger, 'rate-limiter')
 export const redisLogger = extendLoggerComponent(logger, 'redis')
 export const mixpanelLogger = extendLoggerComponent(logger, 'mixpanel')
+export const graphqlLogger = extendLoggerComponent(logger, 'graphql')

packages/server/logging/loggingHelper.js

@@ -0,0 +1,36 @@
+const redactSensitiveVariables = (variables) => {
+  if (!variables) {
+    return variables
+  }
+
+  if (Array.isArray(variables)) {
+    return variables.map((v) => redactSensitiveVariables(v))
+  }
+
+  if (typeof variables !== 'object') {
+    return variables
+  }
+
+  return Object.entries(variables).reduce((acc, [key, val]) => {
+    if (typeof val === 'object') {
+      acc[key] = redactSensitiveVariables(val)
+      return acc
+    }
+
+    if (
+      ['email', 'emailaddress', 'email_address', 'emails'].includes(
+        key.toLocaleLowerCase()
+      )
+    ) {
+      acc[key] = '[REDACTED]'
+      return acc
+    }
+
+    acc[key] = val
+    return acc
+  }, {})
+}
+
+module.exports = {
+  redactSensitiveVariables
+}

packages/server/logging/loggingHelper.spec.js

@@ -0,0 +1,78 @@
+const expect = require('chai').expect
+const { redactSensitiveVariables } = require('@/logging/loggingHelper')
+
+describe('loggingHelper', () => {
+  describe('filterSensitiveVariables', () => {
+    it('should filter sensitive variables at root', () => {
+      const variables = {
+        email: 'exampleValue',
+        emailaddress: 'exampleValue',
+        // eslint-disable-next-line camelcase
+        email_address: 'exampleValue',
+        emails: 'exampleValue',
+        notsensitive: 'exampleValue'
+      }
+      const result = redactSensitiveVariables(variables)
+      expect(result).to.deep.equal({
+        email: '[REDACTED]',
+        emailaddress: '[REDACTED]',
+        // eslint-disable-next-line camelcase
+        email_address: '[REDACTED]',
+        emails: '[REDACTED]',
+        notsensitive: 'exampleValue'
+      })
+    })
+    it('should filter nested sensitive variables', () => {
+      const variables = {
+        nest1: {
+          email: 'exampleValue',
+          emailaddress: 'exampleValue',
+          // eslint-disable-next-line camelcase
+          email_address: 'exampleValue',
+          emails: 'exampleValue'
+        }
+      }
+      const result = redactSensitiveVariables(variables)
+      expect(result).to.deep.equal({
+        nest1: {
+          email: '[REDACTED]',
+          emailaddress: '[REDACTED]',
+          // eslint-disable-next-line camelcase
+          email_address: '[REDACTED]',
+          emails: '[REDACTED]'
+        }
+      })
+    })
+    it('should filter sensitive variables in tree structure', () => {
+      const variables = {
+        nest1: {
+          nest2: {
+            // eslint-disable-next-line camelcase
+            email_address: 'exampleValue',
+            emails: 'exampleValue',
+            notsensitive: 'exampleValue'
+          },
+          nest3: {
+            email: 'exampleValue',
+            emailaddress: 'exampleValue'
+          }
+        }
+      }
+      const result = redactSensitiveVariables(variables)
+      expect(result).to.deep.equal({
+        nest1: {
+          nest2: {
+            // eslint-disable-next-line camelcase
+            email_address: '[REDACTED]',
+            emails: '[REDACTED]',
+            notsensitive: 'exampleValue'
+          },
+          nest3: {
+            email: '[REDACTED]',
+            emailaddress: '[REDACTED]'
+          }
+        }
+      })
+    })
+  })
+})