Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redact any common secrets from fetch debug logging #553

Merged
merged 2 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/lib/fetch-wrapper.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import { inspect } from "node:util";

import { container } from "../config/container.mjs";
import { redactedStringify } from "./formatting/redact.mjs";

// this wrapper exists for only one reason: logging
// in the future, it could also be extended for error-handling,
Expand All @@ -24,7 +25,7 @@ export default async function fetchWrapper(url, options) {
let body;
if (isJSON) {
body = await response.json();
logMessage += ` with body:\n${JSON.stringify(body, null, 2)}`;
logMessage += ` with body:\n${redactedStringify(body, null, 2)}`;
}

logger.debug(logMessage, "fetch");
Expand Down
24 changes: 17 additions & 7 deletions src/lib/formatting/redact.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const REDACT_FILL = 8;

/**
* Redacts a string by replacing everything except the first and last four characters with asterisks.
* If the string is too short to display both the first and last four characters, the first four
Expand All @@ -7,26 +9,32 @@
* @returns {string} The redacted string.
*/
export function redact(text) {
if (!text) return text;
if (text === null || text === undefined) return text;

// If the text is not a string, we can't redact it, but instead of throwing an error,
// just return a string of asterisks. We fail open because this is effectively a logging
// function and we don't want to break the application.
if (typeof text !== "string") {
return "*".repeat(REDACT_FILL);
}

// If the string is less than 12 characters long, it is completely replaced with asterisks.
// This is so we can guarantee that the redacted string is at least 8 characters long.
// This is so we can guarantee that the redacted string is at least REDACT_FILL characters long.
// This aligns with minimum password lengths.
if (text.length < 12) {
return "*".repeat(text.length);
return "*".repeat(REDACT_FILL);
}

// If the string is less than 16, we can't redact both, so display the last four only.
if (text.length < 16) {
const lastFour = text.slice(-4);
return `${"*".repeat(text.length - 4)}${lastFour}`;
return `${"*".repeat(REDACT_FILL)}${lastFour}`;
}

// Otherwise, redact the middle of the string and keep the first and last four characters.
const firstFour = text.slice(0, 4);
const lastFour = text.slice(-4);
const middleLength = text.length - 8;
return `${firstFour}${"*".repeat(middleLength)}${lastFour}`;
return `${firstFour}${"*".repeat(REDACT_FILL)}${lastFour}`;
}

/**
Expand All @@ -51,7 +59,9 @@ export function redactedStringify(obj, replacer, space) {
.replace(/-/g, "");
if (
normalizedKey.includes("secret") ||
normalizedKey.includes("accountkey")
normalizedKey.includes("accountkey") ||
normalizedKey.includes("refreshtoken") ||
normalizedKey.includes("accesstoken")
) {
return redact(resolvedReplaced(key, value));
}
Expand Down
66 changes: 52 additions & 14 deletions test/lib/formatting/redact.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,27 @@ describe("redact", () => {
expect(redact(undefined)).to.be.undefined;
});

it("returns a string of asterisks for non-string values", () => {
expect(redact({})).to.equal("********");
expect(redact([])).to.equal("********");
expect(redact(123)).to.equal("********");
expect(redact(true)).to.equal("********");
expect(redact(false)).to.equal("********");
});

it("completely redacts strings shorter than 12 characters", () => {
expect(redact("short")).to.equal("*****");
expect(redact("mediumtext")).to.equal("**********");
expect(redact("short")).to.equal("********");
expect(redact("mediumtext")).to.equal("********");
});

it("keeps last 4 characters for strings between 12 and 15 characters", () => {
expect(redact("123456789012")).to.equal("********9012");
expect(redact("1234567890123")).to.equal("*********0123");
expect(redact("1234567890123")).to.equal("********0123");
});

it("keeps first and last 4 characters for strings 16 or more characters", () => {
expect(redact("1234567890123456")).to.equal("1234********3456");
expect(redact("12345678901234567")).to.equal("1234*********4567");
expect(redact("12345678901234567")).to.equal("1234********4567");
});
});

Expand All @@ -40,10 +48,10 @@ describe("redactedStringify", () => {
const result = JSON.parse(redactedStringify(obj));

expect(result.normal).to.equal("visible");
expect(result.secret).to.equal("*******");
expect(result.mySecret).to.equal("*********-too");
expect(result["account-key"]).to.equal("***********");
expect(result.bigSecret).to.equal("this*************cret");
expect(result.secret).to.equal("********");
expect(result.mySecret).to.equal("********-too");
expect(result["account-key"]).to.equal("********");
expect(result.bigSecret).to.equal("this********cret");
});

it("redacts keys containing 'accountkey'", () => {
Expand All @@ -55,10 +63,40 @@ describe("redactedStringify", () => {
};
const result = JSON.parse(redactedStringify(obj));

expect(result.accountkey).to.equal("******");
expect(result.account_key).to.equal("*********0123");
expect(result.accountkey).to.equal("********");
expect(result.account_key).to.equal("********0123");
expect(result.myaccountkey).to.equal("1234********3456");
expect(result.longaccountkey).to.equal("test**********ey-1");
expect(result.longaccountkey).to.equal("test********ey-1");
});

it("redacts keys containing 'accesstoken'", () => {
const obj = {
accesstoken: "secret",
access_token: "1234567890123",
myaccesstoken: "1234567890123456",
longaccesstoken: "test-access-token-1",
};
const result = JSON.parse(redactedStringify(obj));

expect(result.accesstoken).to.equal("********");
expect(result.access_token).to.equal("********0123");
expect(result.myaccesstoken).to.equal("1234********3456");
expect(result.longaccesstoken).to.equal("test********en-1");
});

it("redacts keys containing 'refreshtoken'", () => {
const obj = {
refreshtoken: "secret",
refresh_token: "1234567890123",
myrefreshtoken: "1234567890123456",
longrefreshtoken: "test-refresh-token-1",
};
const result = JSON.parse(redactedStringify(obj));

expect(result.refreshtoken).to.equal("********");
expect(result.refresh_token).to.equal("********0123");
expect(result.myrefreshtoken).to.equal("1234********3456");
expect(result.longrefreshtoken).to.equal("test********en-1");
});

it("respects custom replacer function", () => {
Expand All @@ -72,9 +110,9 @@ describe("redactedStringify", () => {

const result = JSON.parse(redactedStringify(obj, replacer));

expect(result.secret).to.equal("*******");
expect(result.secret).to.equal("********");
expect(result.normal).to.equal("SHOW-ME");
expect(result.longSecret).to.equal("1234************************9012");
expect(result.longSecret).to.equal("1234********9012");
});

it("respects space parameter for formatting", () => {
Expand All @@ -89,7 +127,7 @@ describe("redactedStringify", () => {
expect(formatted).to.include(" ");
expect(JSON.parse(formatted)).to.deep.equal({
normal: "visible",
secret: "*******",
secret: "********",
longSecret: "1234********3456",
});
});
Expand Down
Loading