Skip to content

Commit

Permalink
Merge pull request #506 from appsignal/add-request-headers-allowlist
Browse files Browse the repository at this point in the history
Add request headers allowlist
  • Loading branch information
luismiramirez authored Nov 23, 2021
2 parents 6c00cd1 + 4c11f36 commit c938cbd
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
---

Collect request headers from express requests by default. See also our new `requestHeaders` config
option, added in the @appsignal/nodejs package.
23 changes: 21 additions & 2 deletions packages/express/src/middleware/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { Client } from "@appsignal/nodejs"

import { HashMap } from "@appsignal/types"
import {
Request,
Response,
NextFunction,
ErrorRequestHandler,
RequestHandler
} from "express"
import { IncomingHttpHeaders } from "http"

/**
* Returns an Express middleware that can augment the current span
Expand All @@ -33,7 +34,8 @@ export function expressMiddleware(appsignal: Client): RequestHandler {
res.end = function (this: Response) {
res.end = originalEnd

const { method = "GET", params = {}, query = {} } = req
const { method = "GET", params = {}, query = {}, headers } = req
const filteredHeaders = filterHeaders(headers, appsignal)

// if there is no error passed to `next()`, the span name will
// be updated to match the current path
Expand All @@ -48,6 +50,7 @@ export function expressMiddleware(appsignal: Client): RequestHandler {
// @TODO: keep an eye on this
// @ts-ignore
span.setSampleData("params", { ...params, ...query })
span.setSampleData("environment", filteredHeaders)

return res.end.apply(this, arguments as any)
}
Expand Down Expand Up @@ -80,3 +83,19 @@ export function expressErrorHandler(appsignal: Client): ErrorRequestHandler {
return next(err)
}
}

function filterHeaders(
headers: IncomingHttpHeaders,
appsignal: Client
): HashMap<any> {
let filtered: HashMap<any> = {}
const headersAllowList = appsignal.config.data.requestHeaders || []

headersAllowList.forEach(key => {
if (headers[key] != undefined) {
filtered[key] = headers[key]
}
})

return filtered
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
bump: "patch"
---

The `requestHeaders` config option is now available. An allow list that gives the ability to define
which request headers you want to be shown in sample detail views. The default is a list of common
headers that do not include [personal identifiable information](https://docs.appsignal.com/appsignal/gdpr.html#allowed-request-headers-only).
Read more about [request headers](https://docs.appsignal.com/application/header-filtering.html) on our documentation website.
4 changes: 3 additions & 1 deletion packages/nodejs/global.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
export {}

import { Client } from "./src/interfaces"

declare global {
var __APPSIGNAL__: any
var __APPSIGNAL__: Client
}
10 changes: 10 additions & 0 deletions packages/nodejs/src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ describe("Configuration", () => {
ignoreNamespaces: [],
log: "file",
logPath: "/tmp",
requestHeaders: [
"accept",
"accept-charset",
"accept-encoding",
"accept-language",
"cache-control",
"connection",
"content-length",
"range"
],
transactionDebugMode: false
}

Expand Down
6 changes: 5 additions & 1 deletion packages/nodejs/src/cli/diagnose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,5 +416,9 @@ export class Diagnose {
}

function format_value(value: any) {
return util.inspect(value)
if (typeof value == "object") {
return JSON.stringify(value)
} else {
return util.inspect(value)
}
}
10 changes: 10 additions & 0 deletions packages/nodejs/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ export class Configuration {
ignoreNamespaces: [],
log: "file",
logPath: this._tmpdir(),
requestHeaders: [
"accept",
"accept-charset",
"accept-encoding",
"accept-language",
"cache-control",
"connection",
"content-length",
"range"
],
transactionDebugMode: false
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/nodejs/src/config/configmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export const JS_TO_RUBY_MAPPING: { [key: string]: string } = {
log: "log",
logPath: "log_path",
name: "name",
requestHeaders: "request_headers",
revision: "revision",
runningInContainer: "running_in_container",
transactionDebugMode: "transaction_debug_mode",
Expand Down
20 changes: 18 additions & 2 deletions packages/nodejs/src/instrumentation/http/lifecycle/incoming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
*/

import { Tracer } from "../../../interfaces"
import { HashMap } from "@appsignal/types"
import { parse } from "url"
import { IncomingMessage, ServerResponse } from "http"
import { IncomingMessage, ServerResponse, IncomingHttpHeaders } from "http"

// explicitly ignore some urls that we can't guarantee groupings on, or
// routes that cause known issues.
Expand All @@ -32,8 +33,9 @@ function incomingRequest(
}

const [req, res] = args as [IncomingMessage, ServerResponse]
const { method = "GET", url = "/" } = req
const { method = "GET", url = "/", headers } = req
const { pathname, query } = parse(url)
const allowedHeaders = filterHeaders(headers)

// don't start a span for ignored urls
if (url && DEFAULT_IGNORED_URLS.some(el => el.test(url))) {
Expand All @@ -58,6 +60,7 @@ function incomingRequest(
.setName(`${method} ${pathname === "/" ? pathname : "[unknown route]"}`)
.setCategory("process_request.http")
.set("method", method)
.setSampleData("environment", allowedHeaders)
.setSampleData("params", query ? { query } : {})

return tracer.withSpan(rootSpan, span => {
Expand All @@ -83,6 +86,19 @@ function incomingRequest(
}
}

function filterHeaders(headers: IncomingHttpHeaders): HashMap<any> {
let filtered: HashMap<any> = {}
const headersAllowList = global.__APPSIGNAL__.config.data.requestHeaders || []

headersAllowList.forEach(key => {
if (headers[key] != undefined) {
filtered[key] = headers[key]
}
})

return filtered
}

export function getPatchIncomingRequestFunction(tracer: Tracer) {
return function (original: (event: string, ...args: unknown[]) => boolean) {
return incomingRequest(original, tracer)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/diagnose
Submodule diagnose updated 1 files
+21 −0 spec/diagnose_spec.rb

0 comments on commit c938cbd

Please sign in to comment.