Skip to content

Commit

Permalink
Redact any common secrets from fetch debug logging (#553)
Browse files Browse the repository at this point in the history
  • Loading branch information
ecooper authored Jan 15, 2025
1 parent bfb125b commit ca540aa
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 22 deletions.
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

0 comments on commit ca540aa

Please sign in to comment.