Skip to content

fix(ng-dev/caretaker): redact raw error objects from caretaker logs#3759

Open
josephperrott wants to merge 1 commit into
angular:mainfrom
josephperrott:fix/sec-caretaker-token-leakage-ce4ab3d8
Open

fix(ng-dev/caretaker): redact raw error objects from caretaker logs#3759
josephperrott wants to merge 1 commit into
angular:mainfrom
josephperrott:fix/sec-caretaker-token-leakage-ce4ab3d8

Conversation

@josephperrott
Copy link
Copy Markdown
Member

Addresses ce4ab3d8. Redacts raw error objects in caretaker logs to prevent sensitive token leakage.

@josephperrott josephperrott added the target: patch This PR is targeted for the next patch release label Jun 6, 2026
@josephperrott josephperrott requested a review from alan-agius4 June 6, 2026 02:20
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves error logging across several caretaker modules by safely handling caught errors, ensuring stack is defined before logging, and converting non-Error objects to strings. A review comment suggests simplifying the logging logic in update-github-team.ts to avoid duplicate log entries, as Error.prototype.stack already includes the error message.

Comment on lines +99 to +106
if (e instanceof Error) {
Log.debug(e.message);
if (e.stack) {
Log.debug(e.stack);
}
} else {
Log.debug(String(e));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Node.js, Error.prototype.stack already includes the error message as its first line. Logging both e.message and e.stack at the same Log.debug level results in duplicate log entries. We can simplify this by logging e.stack if it exists, and falling back to e.message otherwise.

    if (e instanceof Error) {
      Log.debug(e.stack ?? e.message);
    } else {
      Log.debug(String(e));
    }

@josephperrott josephperrott force-pushed the fix/sec-caretaker-token-leakage-ce4ab3d8 branch from 47c3424 to 520fb26 Compare June 6, 2026 02:24
@josephperrott josephperrott added action: merge The PR is ready for merge by the caretaker and removed target: patch This PR is targeted for the next patch release labels Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant