feat: redact username in logging - #45
Open
yCodeTech wants to merge 6 commits into
Open
Conversation
…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.
- 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.
There was a problem hiding this comment.
Pull request overview
This PR adds a reusable redaction utility to prevent leaking the current OS username in log output, and applies it to a few high-signal logging/data-access paths in the extension.
Changes:
- Added
redactUsernameinsrc/utils.tsto recursively redact usernames in strings across nested data structures. - Updated
Configuration.logDebugInfo()logging to redact usernames from environment data and language config file paths. - Updated
ExtensionDatato redact usernames inextensionPathand extension discovery paths when used for logging.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/utils.ts | Introduces redactUsername utility used for recursive username redaction in log-bound data. |
| src/configuration.ts | Applies username redaction to debug logging of environment/config path-related data. |
| src/extensionData.ts | Redacts username from extension path and discovery paths exposed for logging. |
- 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.
- 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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (3)
src/logger.ts:233
- Username redaction is currently only applied to the formatted meta output (via
formatMeta()), not to the mainmessagestring appended inlogMessage(). This still allows usernames to leak when call sites interpolate paths directly into the message (e.g.,logger.info(Loaded DEV_USER_EXTENSIONS_PATH: "${devPath}")insrc/utils.ts). Consider redacting themessagebefore appending it to the output channel (and ideally also forimportant()logs) so redaction is consistently applied.
data = lines.join(",\n");
}
return this.redactUsername(data);
}
src/logger.ts:299
- The warning text says "debug logs may not be redacted", but username redaction is applied to any log meta that goes through
formatMeta()(including info/warn/error). The message should be accurate so users understand the scope of the redaction failure.
this.warn("Could not determine OS username; debug logs may not be redacted before sharing.");
src/logger.ts:261
- PR description says a recursive
redactUsernameutility was added tosrc/utils.tsand applied across configuration/extension data accessors, but the implementation shown here adds redaction insideLoggerinstead. Please update the PR description (or align the implementation) so it accurately reflects where redaction lives and how it’s applied.
/**
* Redact the OS username from a string, replacing it with `<redacted>`, to avoid leaking
* it into debug logs that could be shared.
*
* @param {string} text The text to redact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new utility function to redact the current OS username from log output and applies it where necessary to prevent leaking personally identifiable information (usernames) in logs. The main changes involve adding the
redactUsernamefunction and updating logging statements and data accessors to use it, especially when logging paths or configuration data that may contain the username.Sensitive Data Redaction:
redactUsernamefunction insrc/utils.tsto sanitize strings, arrays, objects, and Maps by replacing occurrences of the current OS username with"<redacted>".src/configuration.tsto redact the username from environment variables and language config file paths before outputting them.ExtensionDatainsrc/extensionData.tsto redact the username from theextensionPathproperty and all extension discovery paths when accessed for logging.src/extensionData.tsto include the newredactUsernameutility.Utility Improvements:
osmodule import insrc/utils.tsto support cross-platform username detection.