fix: dispatch request-logger hides negative numbers, dates and date times (#1506)
* fix: dispatch request-logger hides negative numbers, dates and date times * test: add unit test for undici log agent * fix: deepsource issue
This commit is contained in:
99
apps/tasks/src/test/undici-log-agent-override.spec.ts
Normal file
99
apps/tasks/src/test/undici-log-agent-override.spec.ts
Normal file
@@ -0,0 +1,99 @@
|
|||||||
|
import type { Dispatcher } from "undici";
|
||||||
|
import { describe, expect, test, vi } from "vitest";
|
||||||
|
|
||||||
|
import { logger } from "@homarr/log";
|
||||||
|
|
||||||
|
import { LoggingAgent } from "~/undici-log-agent-override";
|
||||||
|
|
||||||
|
vi.mock("undici", () => {
|
||||||
|
return {
|
||||||
|
Agent: class Agent {
|
||||||
|
dispatch(_options: Dispatcher.DispatchOptions, _handler: Dispatcher.DispatchHandlers): boolean {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
},
|
||||||
|
setGlobalDispatcher: () => undefined,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
const REDACTED = "REDACTED";
|
||||||
|
|
||||||
|
describe("LoggingAgent should log all requests", () => {
|
||||||
|
test("should log all requests", () => {
|
||||||
|
// Arrange
|
||||||
|
const infoLogSpy = vi.spyOn(logger, "info");
|
||||||
|
const agent = new LoggingAgent();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
agent.dispatch({ origin: "https://homarr.dev", path: "/", method: "GET" }, {});
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(infoLogSpy).toHaveBeenCalledWith("Dispatching request https://homarr.dev/ (0 headers)");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should show amount of headers", () => {
|
||||||
|
// Arrange
|
||||||
|
const infoLogSpy = vi.spyOn(logger, "info");
|
||||||
|
const agent = new LoggingAgent();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
agent.dispatch(
|
||||||
|
{
|
||||||
|
origin: "https://homarr.dev",
|
||||||
|
path: "/",
|
||||||
|
method: "GET",
|
||||||
|
headers: {
|
||||||
|
"Content-Type": "text/html",
|
||||||
|
"User-Agent": "Mozilla/5.0",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{},
|
||||||
|
);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(infoLogSpy).toHaveBeenCalledWith(expect.stringContaining("(2 headers)"));
|
||||||
|
});
|
||||||
|
|
||||||
|
test.each([
|
||||||
|
["/?hex=a3815e8ada2ef9a31", `/?hex=${REDACTED}`],
|
||||||
|
["/?uuid=f7c3f65e-c511-4f90-ba9a-3fd31418bd49", `/?uuid=${REDACTED}`],
|
||||||
|
["/?password=complexPassword123", `/?password=${REDACTED}`],
|
||||||
|
[
|
||||||
|
// JWT for John Doe
|
||||||
|
"/?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c",
|
||||||
|
`/?jwt=${REDACTED}`,
|
||||||
|
],
|
||||||
|
["/?one=a1&two=b2&three=c3", `/?one=${REDACTED}&two=${REDACTED}&three=${REDACTED}`],
|
||||||
|
["/?numberWith13Chars=1234567890123", `/?numberWith13Chars=${REDACTED}`],
|
||||||
|
[`/?stringWith13Chars=${"a".repeat(13)}`, `/?stringWith13Chars=${REDACTED}`],
|
||||||
|
])("should redact sensitive data in url https://homarr.dev%s", (path, expected) => {
|
||||||
|
// Arrange
|
||||||
|
const infoLogSpy = vi.spyOn(logger, "info");
|
||||||
|
const agent = new LoggingAgent();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
agent.dispatch({ origin: "https://homarr.dev", path, method: "GET" }, {});
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(infoLogSpy).toHaveBeenCalledWith(expect.stringContaining(` https://homarr.dev${expected} `));
|
||||||
|
});
|
||||||
|
test.each([
|
||||||
|
["empty", "/?empty"],
|
||||||
|
["numbers with max 12 chars", "/?number=123456789012"],
|
||||||
|
["true", "/?true=true"],
|
||||||
|
["false", "/?false=false"],
|
||||||
|
["strings with max 12 chars", `/?short=${"a".repeat(12)}`],
|
||||||
|
["dates", "/?date=2022-01-01"],
|
||||||
|
["date times", "/?datetime=2022-01-01T00:00:00.000Z"],
|
||||||
|
])("should not redact values that are %s", (_reason, path) => {
|
||||||
|
// Arrange
|
||||||
|
const infoLogSpy = vi.spyOn(logger, "info");
|
||||||
|
const agent = new LoggingAgent();
|
||||||
|
|
||||||
|
// Act
|
||||||
|
agent.dispatch({ origin: "https://homarr.dev", path, method: "GET" }, {});
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(infoLogSpy).toHaveBeenCalledWith(expect.stringContaining(` https://homarr.dev${path} `));
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3,7 +3,7 @@ import { Agent, setGlobalDispatcher } from "undici";
|
|||||||
|
|
||||||
import { logger } from "@homarr/log";
|
import { logger } from "@homarr/log";
|
||||||
|
|
||||||
class LoggingAgent extends Agent {
|
export class LoggingAgent extends Agent {
|
||||||
constructor(...props: ConstructorParameters<typeof Agent>) {
|
constructor(...props: ConstructorParameters<typeof Agent>) {
|
||||||
super(...props);
|
super(...props);
|
||||||
}
|
}
|
||||||
@@ -15,15 +15,17 @@ class LoggingAgent extends Agent {
|
|||||||
// some integrations use query parameters for auth
|
// some integrations use query parameters for auth
|
||||||
url.searchParams.forEach((value, key) => {
|
url.searchParams.forEach((value, key) => {
|
||||||
if (value === "") return; // Skip empty values
|
if (value === "") return; // Skip empty values
|
||||||
if (/^\d{1,12}$/.test(value)) return; // Skip small numbers
|
if (/^-?\d{1,12}$/.test(value)) return; // Skip small numbers
|
||||||
if (value === "true" || value === "false") return; // Skip boolean values
|
if (value === "true" || value === "false") return; // Skip boolean values
|
||||||
if (/^[a-zA-Z]{1,12}$/.test(value)) return; // Skip short strings
|
if (/^[a-zA-Z]{1,12}$/.test(value)) return; // Skip short strings
|
||||||
|
if (/^\d{4}-\d{2}-\d{2}$/.test(value)) return; // Skip dates
|
||||||
|
if (/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/.test(value)) return; // Skip date times
|
||||||
|
|
||||||
url.searchParams.set(key, "REDACTED");
|
url.searchParams.set(key, "REDACTED");
|
||||||
});
|
});
|
||||||
|
|
||||||
logger.info(
|
logger.info(
|
||||||
`Dispatching request ${url.toString().replaceAll("=&", "&")} (${Object.keys(options.headers as object).length} headers)`,
|
`Dispatching request ${url.toString().replaceAll("=&", "&")} (${Object.keys(options.headers ?? {}).length} headers)`,
|
||||||
);
|
);
|
||||||
return super.dispatch(options, handler);
|
return super.dispatch(options, handler);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user