Skip to content

Commit

Permalink
Don't log API response by default (#4478)
Browse files Browse the repository at this point in the history
  • Loading branch information
penalosa authored Nov 21, 2023
1 parent 199e1a1 commit 7b54350
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/silly-bats-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Don't log sensitive data to the Wrangler debug log file by default. This includes API request headers and responses.
12 changes: 5 additions & 7 deletions packages/wrangler/src/cfetch/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,9 @@ export async function performApiFetch(
);
const logHeaders = cloneHeaders(headers);
delete logHeaders["Authorization"];
logger.debug("HEADERS:", JSON.stringify(logHeaders, null, 2));
logger.debug(
"INIT:",
JSON.stringify({ ...init, headers: logHeaders }, null, 2)
);
logger.debugWithSanitization("HEADERS:", JSON.stringify(logHeaders, null, 2));

logger.debugWithSanitization("INIT:", JSON.stringify({ ...init }, null, 2));
logger.debug("-- END CF API REQUEST");
return await fetch(`${getCloudflareApiBaseUrl()}${resource}${queryString}`, {
method,
Expand Down Expand Up @@ -82,8 +80,8 @@ export async function fetchInternal<ResponseType>(
);
const logHeaders = cloneHeaders(response.headers);
delete logHeaders["Authorization"];
logger.debug("HEADERS:", JSON.stringify(logHeaders, null, 2));
logger.debug("RESPONSE:", jsonText);
logger.debugWithSanitization("HEADERS:", JSON.stringify(logHeaders, null, 2));
logger.debugWithSanitization("RESPONSE:", jsonText);
logger.debug("-- END CF API RESPONSE");

// HTTP 204 and HTTP 205 responses do not return a body. We need to special-case this
Expand Down
9 changes: 7 additions & 2 deletions packages/wrangler/src/dev/create-worker-preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ export async function createPreviewSession(

const switchedExchangeUrl = switchHost(exchange_url, ctx.host).toString();

logger.debug(`-- START EXCHANGE API REQUEST: GET ${switchedExchangeUrl}`);
logger.debugWithSanitization(
"-- START EXCHANGE API REQUEST:",
` GET ${switchedExchangeUrl}`
);

logger.debug("-- END EXCHANGE API REQUEST");
const exchangeResponse = await fetch(switchedExchangeUrl, {
signal: abortSignal,
Expand All @@ -172,7 +176,8 @@ export async function createPreviewSession(
exchangeResponse.status
);
logger.debug("HEADERS:", JSON.stringify(exchangeResponse.headers, null, 2));
logger.debug("RESPONSE:", bodyText);
logger.debugWithSanitization("RESPONSE:", bodyText);

logger.debug("-- END EXCHANGE API RESPONSE");

const { inspector_websocket, prewarm, token } = parseJSON<{
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/dev/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,8 +460,8 @@ function configureProxyServer({
method,
url
);
logger.debug("HEADERS", JSON.stringify(headers, null, 2));
logger.debug("PREVIEW TOKEN", previewToken);
logger.debugWithSanitization("HEADERS", JSON.stringify(headers, null, 2));
logger.debugWithSanitization("PREVIEW TOKEN", previewToken);

request.on("response", (responseHeaders) => {
const status = responseHeaders[":status"] ?? 500;
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/environment-variables/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type VariableNames =
| "WRANGLER_CLIENT_ID"
| "WRANGLER_LOG"
| "WRANGLER_LOG_PATH"
| "WRANGLER_LOG_SANITIZE"
| "WRANGLER_REVOKE_URL"
| "WRANGLER_SEND_METRICS"
| "WRANGLER_TOKEN_URL";
Expand Down
8 changes: 8 additions & 0 deletions packages/wrangler/src/environment-variables/misc-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,11 @@ export const getCloudflareApiBaseUrl = getEnvironmentVariableFactory({
? "https://api.staging.cloudflare.com/client/v4"
: "https://api.cloudflare.com/client/v4",
});

// Should we sanitize debug logs? By default we do, since debug logs could be added to GitHub issues and shouldn't include sensitive information
export const getSanitizeLogs = getEnvironmentVariableFactory({
variableName: "WRANGLER_LOG_SANITIZE",
defaultValue() {
return "true";
},
});
11 changes: 11 additions & 0 deletions packages/wrangler/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import chalk from "chalk";
import CLITable from "cli-table3";
import { formatMessagesSync } from "esbuild";
import { getEnvironmentVariableFactory } from "./environment-variables/factory";
import { getSanitizeLogs } from "./environment-variables/misc-variables";
import { appendToDebugLogFile } from "./utils/debug-log-file";
import type { Message } from "esbuild";
export const LOGGER_LEVELS = {
Expand Down Expand Up @@ -54,6 +55,16 @@ export class Logger {
columns = process.stdout.columns;

debug = (...args: unknown[]) => this.doLog("debug", args);
debugWithSanitization = (label: string, ...args: unknown[]) => {
if (getSanitizeLogs() === "false") {
this.doLog("debug", [label, ...args]);
} else {
this.doLog("debug", [
label,
"omitted; set WRANGLER_LOG_SANITIZE=false to include sanitized data",
]);
}
};
info = (...args: unknown[]) => this.doLog("info", args);
log = (...args: unknown[]) => this.doLog("log", args);
warn = (...args: unknown[]) => this.doLog("warn", args);
Expand Down

0 comments on commit 7b54350

Please sign in to comment.