Skip to content

Commit 5ec5c07

Browse files
authored
feat: redact username in logging paths (#45)
* fix: `getAllExtensionDiscoveryPaths` to prevent external mutation of paths. The `getAllExtensionDiscoveryPaths` method signature has always been typed as returning a readonly Map, but that's just for compile-time. For run-time it was still returning the actual Map which could be mutated externally. - Fixed `getAllExtensionDiscoveryPaths` ExtensionData method to return a new Map of the `extensionDiscoveryPaths` Map, to prevent external mutation of the paths. * feat: redact username in logging output - Implemented `redactUsername` utility function to sanitise logs by removing the OS username. - Updated logging statements to use the new redaction utility in: - `logDebugInfo` method in `Configuration` class. - `getAllExtensionDiscoveryPaths` and `prepareForLogging` methods in `ExtensionData` class. * refactor: `redactUsername` to be directly implemented into the Logger. - Moved `redactUsername` function from utils into Logger, simplifying the new method, and adding it's call into the `formatMeta` method after it's transformed the data into a string. By implementing the `redactUsername` method directly into Logger, we can ensure that any calls to logger with additional meta data, will have the username automatically redacted. So even if more logs are added in future, we don't forget to redact the usernames before Logger gets it. This also fixes various copilot review comments on the previous implementation because its no longer recursing into objects or arrays, it's just replacing directly on the string immediately before logging to output. - Updated logging statements to remove the old redaction utility in: - `logDebugInfo` method in `Configuration` class. - `getAllExtensionDiscoveryPaths` and `prepareForLogging` methods in `ExtensionData` class. * fix: missing node `os` import. * feat: add `warn` log level and add `warnOnRedactionFailure` method. - Introduced new `warn` log level. - Added `warn` enum option to the `logLevel` user setting and adjusted all the options descriptions. - Implemented `warn` method in `Logger` class to handle warning messages. - Updated `logLevels` in the `utils` interface to include `warn`. - Added `warn` level in `shouldLog` method in `Logger` and adjusted all the weights. - Added new `hasWarnedAboutRedactionFailure` property to determine whether the user has already been warned about a redaction failure. - Added new `warnOnRedactionFailure` method in Logger to warn users when the username couldn't be determined and redaction failed. This method uses the new `hasWarnedAboutRedactionFailure` property to check if it's already been outputted, as this is a once per session warning. It also uses the new `warn` logger method. * refactor: move `warn` method to below the `error method in Logger. * fix: wording of the redaction failure warning * fix: log messages that could have usernames weren't being redacted. - Added the `redactUsername` method call in the `logMessage` method to redact usernames from the log messages, as they could have them too. - Moved the method call to redact the meta data from `formatMeta` method into the `logMessage` method, so that the log message and data are both redacted from the same centralised method. * feat: skip username redaction if previous attempt failed. - Added a `hasWarnedAboutRedactionFailure` guard conditional at the top of the `redactUsername` method to skip redaction if a previous redact attempt failed. * refactor: `hasWarnedAboutRedactionFailure` property into a new name. - Changed the `hasWarnedAboutRedactionFailure` property flag to `skipRedaction` to better describe it's actual job of skipping redaction attempts after failure. - Changed references to the old `hasWarnedAboutRedactionFailure` property to use the new `skipRedaction` property in the `redactUsername` and `warnOnRedactionFailure` methods. - Removed the old `hasWarnedAboutRedactionFailure` guard conditional from the `warnOnRedactionFailure` method. This is because the method will only run when the new `skipRedaction` flag is false thanks to the guard at the top of `redactUsername` method. So the extra guard is now redundant. - Revised docblocks and code comments for clarity on the redaction behaviour.
1 parent fe73350 commit 5ec5c07

5 files changed

Lines changed: 88 additions & 6 deletions

File tree

package.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,14 @@
8585
"enum": [
8686
"debug",
8787
"info",
88+
"warn",
8889
"error",
8990
"off"
9091
],
9192
"markdownEnumDescriptions": [
92-
"Log debug, info, and errors",
93-
"Log info and errors",
93+
"Log debug, info, warnings, and errors",
94+
"Log info, warnings, and errors",
95+
"Log warnings and errors",
9496
"Log errors only",
9597
"Disable logging"
9698
],

src/configuration.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,7 @@ export class Configuration {
10021002
),
10031003
},
10041004
};
1005+
10051006
logger.debug("Environment:", env);
10061007

10071008
// Log the extension's user configuration settings.

src/extensionData.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,12 @@ export class ExtensionData {
350350

351351
/**
352352
* Get all extension discovery paths.
353+
* Used for logging the paths to the output channel.
353354
*
354355
* @returns {ReadonlyMap<keyof ExtensionPaths, string>} A read-only Map containing all extension discovery paths.
355356
*/
356357
public getAllExtensionDiscoveryPaths(): ReadonlyMap<keyof ExtensionPaths, string> {
357-
return this.extensionDiscoveryPaths;
358+
// Return a new Map to prevent external mutation of the internal state.
359+
return new Map(this.extensionDiscoveryPaths);
358360
}
359361
}

src/interfaces/utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export type LanguageId = string;
4444
export const logLevels = {
4545
debug: "debug",
4646
info: "info",
47+
warn: "warn",
4748
error: "error",
4849
off: "off",
4950
} as const;

src/logger.ts

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as os from "node:os";
12
import {OutputChannel, window} from "vscode";
23
import {LogLevel, logLevels} from "./interfaces/utils";
34

@@ -26,6 +27,15 @@ class Logger {
2627
*/
2728
private logLevel: LogLevel = "debug";
2829

30+
/**
31+
* Whether username redaction should be skipped, because it previously failed.
32+
* Once set, further attempts are not made since the username is unlikely to become
33+
* resolvable later in the same session.
34+
*
35+
* @type {boolean}
36+
*/
37+
private skipRedaction: boolean = false;
38+
2939
/***********
3040
* Methods *
3141
***********/
@@ -121,6 +131,17 @@ class Logger {
121131
}
122132
}
123133

134+
/**
135+
* Sends a warning log to the output channel.
136+
*
137+
* @param {string} message The message to be logged.
138+
*/
139+
public warn(message: string): void {
140+
if (this.shouldLog("warn")) {
141+
this.logMessage("WARN", message);
142+
}
143+
}
144+
124145
/**
125146
* Send an important message to the output channel.
126147
* This is a special log level that is always emitted regardless of the log level,
@@ -150,8 +171,9 @@ class Logger {
150171
private shouldLog(requiredLevel: LogLevel): boolean {
151172
// Numeric weights used for level comparison.
152173
const levelWeight: Record<LogLevel, number> = {
153-
debug: 3, // Emits debug, info, and error logs - the most verbose level.
154-
info: 2, // Emits info and error logs.
174+
debug: 4, // Emits debug, info, warn, and error logs - the most verbose level.
175+
info: 3, // Emits info, warn, and error logs.
176+
warn: 2, // Emits warn and error logs.
155177
error: 1, // Emits error logs only.
156178
off: 0, // Disables all logs, except for the special "important" logs that are always emitted.
157179
};
@@ -171,13 +193,18 @@ class Logger {
171193
if (!this.outputChannel) {
172194
this.setupOutputChannel();
173195
}
196+
197+
message = this.redactUsername(message);
198+
174199
const time = new Date().toLocaleTimeString();
175200

176201
// Output the log message to the output channel.
177202
this.outputChannel.append(`["${level}" - ${time}] ${message}`);
178203

179204
if (meta) {
180-
const data: string = this.formatMeta(message, meta);
205+
let data: string = this.formatMeta(message, meta);
206+
207+
data = this.redactUsername(data);
181208

182209
// Output the meta data to the output channel with a leading space.
183210
this.outputChannel.appendLine(` ${data}`);
@@ -232,6 +259,55 @@ class Logger {
232259

233260
return value;
234261
}
262+
263+
/**
264+
* Redact the OS username from a string, replacing it with `<redacted>`,
265+
* to avoid leaking it into the logs that could be shared.
266+
*
267+
* @param {string} text The text to redact.
268+
* @returns {string} If redaction was possible, returns the redacted text,
269+
* otherwise the original text.
270+
*/
271+
private redactUsername(text: string): string {
272+
// If redaction previously failed, skip attempting it again and return the original text.
273+
if (this.skipRedaction) {
274+
return text;
275+
}
276+
277+
let username: string;
278+
279+
// Get the current OS username using Node's userInfo() method which is
280+
// cross-platform compatible. It can throw an error in sandboxed/remote environments where
281+
// the username can't be determined. So catch any errors and return the original text
282+
// if we can't get the username.
283+
try {
284+
username = os.userInfo().username;
285+
} catch {
286+
this.warnOnRedactionFailure();
287+
return text;
288+
}
289+
290+
// If the username is empty, return the original text.
291+
if (!username) {
292+
this.warnOnRedactionFailure();
293+
return text;
294+
}
295+
296+
// Escape special characters in the username.
297+
const escapedUsername = username.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
298+
// Replace any occurrence of the username in the text with "<redacted>", case-insensitively, and return it.
299+
return text.replace(new RegExp(escapedUsername, "gi"), "<redacted>");
300+
}
301+
302+
/**
303+
* Warn once per session that debug logs may not have the username redacted,
304+
* so users don't unknowingly share it in a bug report.
305+
*/
306+
private warnOnRedactionFailure(): void {
307+
// Set the flag to skip further redaction attempts before warning to avoid recursion.
308+
this.skipRedaction = true;
309+
this.warn("Could not determine OS username; logs won't be redacted. Manually redact any sensitive information before sharing logs.");
310+
}
235311
}
236312

237313
export const logger = new Logger();

0 commit comments

Comments
 (0)