fix: redact RPC tokens from logged deal errors#453
Merged
Conversation
rjan90
changed the title
rjan90
requested review from
SgtPooki and
silent-cipher
and removed request for
silent-cipher
Contributor
There was a problem hiding this comment.
Pull request overview
Redacts sensitive RPC credentials (e.g., query-string tokens) from error strings that end up in logs and persisted deal failure messages.
Changes:
- Added URL-based redaction utilities and applied them to
toStructuredError(message/stack/string throws). - Persist a redacted
deal.errorMessagewhen deal creation fails. - Added regression tests for URL redaction and deal failure persistence.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/backend/src/deal/deal.service.ts | Redacts persisted deal failure message before saving. |
| apps/backend/src/deal/deal.service.spec.ts | Adds test ensuring persisted failure message is redacted. |
| apps/backend/src/common/logging.ts | Introduces URL redaction and applies it to structured error serialization. |
| apps/backend/src/common/logging.spec.ts | Adds tests covering redaction across message/stack/cause and string throws. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rvagg
approved these changes
Apr 16, 2026
Contributor
rvagg
left a comment
rvagg
left a comment
There was a problem hiding this comment.
sgtm but the copilot suggestion is important to fix before merging
rvagg
reviewed
Apr 16, 2026
Co-authored-by: Rod Vagg <rod@vagg.org>
Co-authored-by: Rod Vagg <rod@vagg.org>
SgtPooki
approved these changes
Apr 16, 2026
github-project-automation
Bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
4 tasks
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.
What changed
Why
A
429from the configured Filecoin RPC can bubble up throughviemwith the full request URL in the error message. Dealbot was forwarding that raw message into logs and intodeals.error_message, which could expose RPC query-string tokens.Impact
This keeps the existing error flow intact while replacing values for sensitive URL credentials and query parameters like
token,api_key,apikey,access_token, andauthorizationwithREDACTEDin the affected dealbot surfaces.Root cause
viemincludes the full request URL inHttpRequestError/ContractFunctionExecutionErrormessages, and dealbot was serializing and persistingerror.messagewithout any redaction.Validation
pnpm test -- logging.spec.ts deal.service.spec.ts(backend suite passed)pnpm typecheck