Add verbose output with tiered logging levels (-v and -vv)#3
Conversation
| logger.debug(`→ ${options.method} ${options.url}`); | ||
| } | ||
| }; | ||
|
|
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix this, we should avoid logging potentially sensitive URLs in clear text, especially for OAuth token exchanges, while preserving the usefulness of HTTP logs. The minimal, backward-compatible change is to sanitize or truncate URLs before including them in log messages. A common pattern is to log only the path and host (or even just a label) and to strip query parameters entirely, since they frequently contain secrets.
Concretely, in src/services/logger.ts, we can introduce a small helper function (within the snippet shown) such as sanitizeUrlForLogging(url: string): string that attempts to parse the URL and returns a version with no query string or fragment, and falls back to the original string if parsing fails. Then we adjust logHttpRequest and logHttpResponse to use this sanitized URL string in their human-readable messages (→ ... and ← ...) instead of the raw options.url. The structured log fields (url: options.url) can reasonably continue to store the full URL if the logging backend is configured with appropriate redaction or access control, but if you want stricter protection you can swap those to the sanitized URL as well; here, to minimize functionality changes while addressing CodeQL’s complaint (which targets the string interpolation sink), we will sanitize only the interpolated strings.
Implementation details:
- Add a
sanitizeUrlForLoggingfunction somewhere abovelogHttpRequestinsrc/services/logger.ts. It will:- Try
new URL(url)(which is globally available in Node 18+ and modern JS runtimes; no new imports needed). - Return
${parsed.origin}${parsed.pathname}(no search, no hash). - On failure, return the original
urlto avoid breaking logging on unusual inputs.
- Try
- Update:
- In
logHttpRequest, for both verbosity branches, replaceoptions.urlin template strings withsanitizeUrlForLogging(options.url). - In
logHttpResponse, do the same in both template strings.
- In
- No external dependencies are required; we use the built-in
URLclass.
| @@ -54,6 +54,15 @@ | ||
|
|
||
| export const getLogger = () => logger; | ||
|
|
||
| const sanitizeUrlForLogging = (url: string): string => { | ||
| try { | ||
| const parsed = new URL(url); | ||
| return `${parsed.origin}${parsed.pathname}`; | ||
| } catch { | ||
| return url; | ||
| } | ||
| }; | ||
|
|
||
| // HTTP request logging utilities | ||
| export const logHttpRequest = (options: { | ||
| method: string; | ||
| @@ -61,6 +70,7 @@ | ||
| headers?: Record<string, string>; | ||
| body?: unknown; | ||
| }) => { | ||
| const safeUrl = sanitizeUrlForLogging(options.url); | ||
| if (verbosityLevel >= 2) { | ||
| logger.debug( | ||
| { | ||
| @@ -70,10 +80,10 @@ | ||
| headers: options.headers, | ||
| body: options.body, | ||
| }, | ||
| `→ ${options.method} ${options.url}`, | ||
| `→ ${options.method} ${safeUrl}`, | ||
| ); | ||
| } else if (verbosityLevel === 1) { | ||
| logger.debug(`→ ${options.method} ${options.url}`); | ||
| logger.debug(`→ ${options.method} ${safeUrl}`); | ||
| } | ||
| }; | ||
|
|
||
| @@ -86,6 +94,7 @@ | ||
| body?: unknown; | ||
| duration?: number; | ||
| }) => { | ||
| const safeUrl = sanitizeUrlForLogging(options.url); | ||
| if (verbosityLevel >= 2) { | ||
| logger.debug( | ||
| { | ||
| @@ -98,13 +107,13 @@ | ||
| body: options.body, | ||
| duration: options.duration, | ||
| }, | ||
| `← ${options.status} ${options.method} ${options.url} ${ | ||
| `← ${options.status} ${options.method} ${safeUrl} ${ | ||
| options.duration ? `(${options.duration}ms)` : "" | ||
| }`, | ||
| ); | ||
| } else if (verbosityLevel === 1) { | ||
| logger.debug( | ||
| `← ${options.status} ${options.method} ${options.url} ${ | ||
| `← ${options.status} ${options.method} ${safeUrl} ${ | ||
| options.duration ? `(${options.duration}ms)` : "" | ||
| }`, | ||
| ); |
|
|
||
| if (Array.isArray(obj)) { | ||
| return obj.map(redactSensitiveFields); |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
General fix approach: avoid logging sensitive data directly; for HTTP logs, that usually means (a) redacting or omitting tokens and secrets in headers, bodies, and URLs, and (b) logging only non-sensitive parts (e.g., method, status, host, and path) or a sanitized URL. In this case, the specific problem is that the string interpolation in logHttpResponse includes options.url unchanged.
Best fix in this codebase without changing behavior more than necessary:
- Keep the structured log object unchanged (it already includes
urland the redactedbody), becausepino’s redaction andredactSensitiveFieldshandle JSON data; this may still containurlbut structured logs are already somewhat controlled. - Change the human-readable message strings used in
logger.debugfor responses to omit or sanitize the URL, so no clear-text token-bearing URLs appear in logs. - Reuse that same sanitized format for both verbosity branches in
logHttpResponseto keep behavior consistent. - Optionally (and very low impact) also adjust the request-side message (
logHttpRequest) similarly for symmetry, but the specific CodeQL alert is onlogHttpResponse, so the minimum required harmless change is there.
Concretely:
- In
src/services/logger.ts, inlogHttpResponse, replace:- The message template in the
verbosityLevel >= 2branch from
`← ${options.status} ${options.method} ${options.url} ${...}`
to something like
`← ${options.status} ${options.method} ${options.duration ? ... : ""}`
(no URL). - The message template in the
verbosityLevel === 1branch similarly, omittingoptions.url.
- The message template in the
- No new imports or helpers are needed; we only modify the interpolated string.
This preserves the core functionality (status, method, and latency logging) while removing the sensitive url from the log message.
| @@ -98,13 +98,13 @@ | ||
| body: options.body, | ||
| duration: options.duration, | ||
| }, | ||
| `← ${options.status} ${options.method} ${options.url} ${ | ||
| `← ${options.status} ${options.method} ${ | ||
| options.duration ? `(${options.duration}ms)` : "" | ||
| }`, | ||
| ); | ||
| } else if (verbosityLevel === 1) { | ||
| logger.debug( | ||
| `← ${options.status} ${options.method} ${options.url} ${ | ||
| `← ${options.status} ${options.method} ${ | ||
| options.duration ? `(${options.duration}ms)` : "" | ||
| }`, | ||
| ); |
Added standard -v and --verbose flags for enabling verbose output, while keeping --debug as an alias. This follows common CLI conventions where -v is the standard flag for verbose logging. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit adds comprehensive verbose output support with two levels of verbosity, along with fixes for ES module compatibility issues. - **Level 1 (-v, --info)**: Basic output showing HTTP method, URL, status, and duration - **Level 2 (-vv, --debug)**: Full output including headers, request/response bodies with sensitive field redaction - Introduced `loggedFetch()` wrapper that automatically logs HTTP requests - Extracts logging details from actual requests/responses, eliminating hardcoded duplication - Automatic timing and status code capture - Smart JSON response parsing for level 2 verbosity - Automatic redaction of `access_token` and `accessToken` fields in logs - Recursive redaction for nested objects - Follows existing redaction patterns for Authorization headers - Added `__dirname` and `__filename` shims in require-shim.js for dependencies expecting CommonJS globals - Marked pino-pretty as external dependency to avoid bundling worker thread code that breaks at runtime - Changed from pino transport API to direct stream usage to eliminate worker thread dependency - Added visual indicator when verbose output is enabled - Clear messaging distinguishing between basic and full verbosity levels - `-v, --verbose`: Incrementing flag (use multiple times for higher levels) - `--info`: Alias for level 1 verbosity - `--debug`: Alias for level 2 verbosity - DRY principle: Single `loggedFetch()` wrapper handles all HTTP logging - No hardcoded method/status values in logging calls - Automatic body parsing based on content-type - Level-aware logging prevents unnecessary work at lower verbosity Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
91bbd50 to
9d5fb5e
Compare
Summary
This PR adds comprehensive verbose output support with two levels of verbosity, along with fixes for ES module compatibility issues that were preventing verbose mode from working.
Features Added
Tiered Verbosity Levels
-v,--info): Basic output showing HTTP method, URL, status code, and duration-vv,--debug): Full output including headers, request/response bodies with automatic sensitive field redactionAutomatic HTTP Logging
loggedFetch()wrapper that automatically logs HTTP requestsSecurity Enhancements
access_tokenandaccessTokenfields in response logsBug Fixes
ES Module Compatibility Issues
__dirname is not definederror: Added__dirnameand__filenameshims inrequire-shim.jsfor dependencies expecting CommonJS globalspino-prettyas external dependency to avoid bundling worker thread code that breaks at runtimeThese fixes ensure that verbose mode actually works when the CLI is installed globally.
CLI Options
-v, --verbose: Incrementing flag (can be used multiple times:-vv)--info: Alias for level 1 verbosity (-v)--debug: Alias for level 2 verbosity (-vv)Examples
Implementation Highlights
loggedFetch()wrapper handles all HTTP logging(verbose output enabled)or(verbose output enabled: full details)when enabledTesting
Tested with:
godaddy -v auth login- Basic logging worksgodaddy -vv auth login- Full logging works with redacted tokensgodaddy --info env get- Alias worksgodaddy --debug env get- Alias works🤖 Generated with Claude Code