From 253110ac6f696fcd03ccd725db928b73bbdbd986 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 3 Jul 2024 15:25:36 +0100 Subject: [PATCH 1/6] Detect secrets in error messages. --- packages/backend-core/src/environment.ts | 15 ++++++++ .../src/middleware/errorHandling.ts | 11 +++++- packages/backend-core/src/security/secrets.ts | 20 +++++++++++ .../src/security/tests/secrets.spec.ts | 35 +++++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 packages/backend-core/src/security/secrets.ts create mode 100644 packages/backend-core/src/security/tests/secrets.spec.ts diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 685f2988ad..0296e1fd48 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -205,6 +205,21 @@ const environment = { OPENAI_API_KEY: process.env.OPENAI_API_KEY, } +type EnvironmentKey = keyof typeof environment +export const SECRETS: EnvironmentKey[] = [ + "API_ENCRYPTION_KEY", + "COUCH_DB_PASSWORD", + "COUCH_DB_SQL_URL", + "COUCH_DB_URL", + "GOOGLE_CLIENT_SECRET", + "INTERNAL_API_KEY_FALLBACK", + "INTERNAL_API_KEY", + "JWT_SECRET", + "MINIO_ACCESS_KEY", + "MINIO_SECRET_KEY", + "REDIS_PASSWORD", +] + // clean up any environment variable edge cases for (let [key, value] of Object.entries(environment)) { // handle the edge case of "0" to disable an environment variable diff --git a/packages/backend-core/src/middleware/errorHandling.ts b/packages/backend-core/src/middleware/errorHandling.ts index 08f9f3214d..3f6d069337 100644 --- a/packages/backend-core/src/middleware/errorHandling.ts +++ b/packages/backend-core/src/middleware/errorHandling.ts @@ -1,6 +1,7 @@ import { APIError } from "@budibase/types" import * as errors from "../errors" import environment from "../environment" +import { stringContainsSecret } from "../security/secrets" export async function errorHandling(ctx: any, next: any) { try { @@ -17,7 +18,7 @@ export async function errorHandling(ctx: any, next: any) { let error: APIError = { message: err.message, - status: status, + status, validationErrors: err.validation, error: errors.getPublicError(err), } @@ -27,6 +28,14 @@ export async function errorHandling(ctx: any, next: any) { error.stack = err.stack } + if (stringContainsSecret(JSON.stringify(error))) { + error = { + message: "Unexpected error", + status, + error: "Unexpected error", + } + } + ctx.body = error } } diff --git a/packages/backend-core/src/security/secrets.ts b/packages/backend-core/src/security/secrets.ts new file mode 100644 index 0000000000..24fb556bdb --- /dev/null +++ b/packages/backend-core/src/security/secrets.ts @@ -0,0 +1,20 @@ +import environment, { SECRETS } from "../environment" + +export function stringContainsSecret(str: string) { + if (str.includes("-----BEGIN PRIVATE KEY-----")) { + return true + } + + for (const key of SECRETS) { + const value = environment[key] + if (typeof value !== "string") { + continue + } + + if (str.includes(value)) { + return true + } + } + + return false +} diff --git a/packages/backend-core/src/security/tests/secrets.spec.ts b/packages/backend-core/src/security/tests/secrets.spec.ts new file mode 100644 index 0000000000..19bf174973 --- /dev/null +++ b/packages/backend-core/src/security/tests/secrets.spec.ts @@ -0,0 +1,35 @@ +import { randomUUID } from "crypto" +import environment, { SECRETS } from "../../environment" +import { stringContainsSecret } from "../secrets" + +describe("secrets", () => { + describe("stringContainsSecret", () => { + it.each(SECRETS)("detects that a string contains a secret in: %s", key => { + const needle = randomUUID() + const haystack = `this is a secret: ${needle}` + const old = environment[key] + environment._set(key, needle) + + try { + expect(stringContainsSecret(haystack)).toBe(true) + } finally { + environment._set(key, old) + } + }) + + it.each(SECRETS)( + "detects that a string does not contain a secret in: %s", + key => { + const needle = randomUUID() + const haystack = `this does not contain a secret` + const old = environment[key] + environment._set(key, needle) + try { + expect(stringContainsSecret(haystack)).toBe(false) + } finally { + environment._set(key, old) + } + } + ) + }) +}) From d9b94c1dcfc8a8b64b85fece38b7761ffa91a4f9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 3 Jul 2024 15:35:46 +0100 Subject: [PATCH 2/6] Don't detect empty strings. --- packages/backend-core/src/security/secrets.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/backend-core/src/security/secrets.ts b/packages/backend-core/src/security/secrets.ts index 24fb556bdb..65bc33a1dc 100644 --- a/packages/backend-core/src/security/secrets.ts +++ b/packages/backend-core/src/security/secrets.ts @@ -7,7 +7,7 @@ export function stringContainsSecret(str: string) { for (const key of SECRETS) { const value = environment[key] - if (typeof value !== "string") { + if (typeof value !== "string" || value === "") { continue } From bab3c077274d714ce59e23de0faf89ef2be9419f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 3 Jul 2024 16:33:32 +0100 Subject: [PATCH 3/6] Add a couple more secrets. --- packages/backend-core/src/environment.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 0296e1fd48..e06d51f918 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -208,6 +208,7 @@ const environment = { type EnvironmentKey = keyof typeof environment export const SECRETS: EnvironmentKey[] = [ "API_ENCRYPTION_KEY", + "BB_ADMIN_USER_PASSWORD", "COUCH_DB_PASSWORD", "COUCH_DB_SQL_URL", "COUCH_DB_URL", @@ -217,6 +218,7 @@ export const SECRETS: EnvironmentKey[] = [ "JWT_SECRET", "MINIO_ACCESS_KEY", "MINIO_SECRET_KEY", + "OPENAI_API_KEY", "REDIS_PASSWORD", ] From 16e293a9ff5919d7b077ef4f726d8bd0259b8f57 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 4 Jul 2024 09:55:36 +0100 Subject: [PATCH 4/6] Fix tests. --- packages/backend-core/src/middleware/errorHandling.ts | 10 +++++----- packages/backend-core/src/security/secrets.ts | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/backend-core/src/middleware/errorHandling.ts b/packages/backend-core/src/middleware/errorHandling.ts index 3f6d069337..6ceda9cd3a 100644 --- a/packages/backend-core/src/middleware/errorHandling.ts +++ b/packages/backend-core/src/middleware/errorHandling.ts @@ -23,11 +23,6 @@ export async function errorHandling(ctx: any, next: any) { error: errors.getPublicError(err), } - if (environment.isTest() && ctx.headers["x-budibase-include-stacktrace"]) { - // @ts-ignore - error.stack = err.stack - } - if (stringContainsSecret(JSON.stringify(error))) { error = { message: "Unexpected error", @@ -36,6 +31,11 @@ export async function errorHandling(ctx: any, next: any) { } } + if (environment.isTest() && ctx.headers["x-budibase-include-stacktrace"]) { + // @ts-ignore + error.stack = err.stack + } + ctx.body = error } } diff --git a/packages/backend-core/src/security/secrets.ts b/packages/backend-core/src/security/secrets.ts index 65bc33a1dc..ab7ea36728 100644 --- a/packages/backend-core/src/security/secrets.ts +++ b/packages/backend-core/src/security/secrets.ts @@ -12,6 +12,7 @@ export function stringContainsSecret(str: string) { } if (str.includes(value)) { + throw new Error(`String contains secret: ${key}=${value}`) return true } } From 257ee8fb70247838f09fa6d44ce9e554595bfae4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 4 Jul 2024 10:46:09 +0100 Subject: [PATCH 5/6] Fix tests actually. --- packages/backend-core/src/security/secrets.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/backend-core/src/security/secrets.ts b/packages/backend-core/src/security/secrets.ts index ab7ea36728..65bc33a1dc 100644 --- a/packages/backend-core/src/security/secrets.ts +++ b/packages/backend-core/src/security/secrets.ts @@ -12,7 +12,6 @@ export function stringContainsSecret(str: string) { } if (str.includes(value)) { - throw new Error(`String contains secret: ${key}=${value}`) return true } } From 8595a3ab40762d78627f0ca5c7d6aeb9ec7ccdbd Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 4 Jul 2024 13:55:30 +0100 Subject: [PATCH 6/6] Update account-portal submodule to latest master. --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index ff16525b73..b03e584e46 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit ff16525b73c5751d344f5c161a682609c0a993f2 +Subproject commit b03e584e465f620b49a1b688ff4afc973e6c0758