Conversation
|
|
||
| balance, err := client.AccountAPTBalance(addr) | ||
| if err != nil { | ||
| s.logger.Warnw("failed to get balance for account, skipping", "account", account, "address", addr.String(), "error", err) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Copilot Autofix
AI about 3 hours ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
|
|
||
| acc := utils.Ed25519PublicKeyToAddress(ed25519PublicKey) | ||
| fromAddress := acc.String() | ||
| a.baseLogger.Infow("TestingAptosWriteCap: EnqueueCRE - resolved from address", "fromAddress", fromAddress) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
To fix the issue, avoid logging the full fromAddress value derived from authKey, or log only a non-sensitive, obfuscated version. This preserves functionality while preventing clear-text storage of potentially sensitive account identifiers.
The best low-impact fix is to change the log statement at line 281 in relayer/txm/txm.go to either (a) omit the address value entirely, or (b) log only a truncated/masked version (for example, first/last few characters) that is sufficient for debugging but useless for an attacker. No other logic relies on this log output, so we do not need to change any control flow or data structures.
Concretely, in relayer/txm/txm.go:
- Replace the existing
Infowcall that logs"fromAddress", fromAddresswith a version that either:- does not include the
"fromAddress"key at all, or - logs an obfuscated string constructed from
fromAddress, such as a prefix and suffix plus a length.
- does not include the
- This can be done inline without new imports. For example:
maskedFromAddress := fromAddress[:6] + "..." + fromAddress[len(fromAddress)-4:]- Then log
"fromAddress", maskedFromAddress.
- Ensure we only modify the logging, leaving subsequent parsing of
fromAddressand transaction creation unchanged.
No new methods or external libraries are required; this is a simple string manipulation in the existing file.
| @@ -278,7 +278,11 @@ | ||
|
|
||
| acc := utils.Ed25519PublicKeyToAddress(ed25519PublicKey) | ||
| fromAddress := acc.String() | ||
| a.baseLogger.Infow("TestingAptosWriteCap: EnqueueCRE - resolved from address", "fromAddress", fromAddress) | ||
| maskedFromAddress := fromAddress | ||
| if len(fromAddress) > 10 { | ||
| maskedFromAddress = fromAddress[:6] + "..." + fromAddress[len(fromAddress)-4:] | ||
| } | ||
| a.baseLogger.Infow("TestingAptosWriteCap: EnqueueCRE - resolved from address", "fromAddress", maskedFromAddress) | ||
|
|
||
| fromAccountAddress := &aptos.AccountAddress{} | ||
| err = fromAccountAddress.ParseStringRelaxed(fromAddress) |
| fromAccountAddress := &aptos.AccountAddress{} | ||
| err = fromAccountAddress.ParseStringRelaxed(fromAddress) | ||
| if err != nil { | ||
| a.baseLogger.Errorw("TestingAptosWriteCap: EnqueueCRE - failed to parse from address", "fromAddress", fromAddress, "error", err) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
To fix the issue, we should avoid logging the full fromAddress value directly. Instead, we can either (a) omit it entirely from logs, or (b) log only a redacted form (for example, the first and last few characters), which is enough for debugging while not exposing the complete derived auth key. This addresses the CodeQL concern about clear-text logging of sensitive data without changing any functional behavior of the transaction processing.
The most targeted, non-invasive fix is:
- Keep computing
fromAddressas before and using it for parsing and transaction construction. - Change the two log statements that include
"fromAddress", fromAddressto instead include a redacted value, e.g.RedactAddress(fromAddress). - Implement a small helper
RedactAddressinrelayer/utils/address.gothat takes an address string and returns a redacted representation (e.g.0x1234...abcd), falling back sensibly for short strings. This keeps logic centralized and reusable. - Use only standard library functionality in the helper to avoid extra dependencies.
Concretely:
- In
relayer/utils/address.go, add a new helper functionRedactAddress(addr string) stringafter the existing functions. - In
relayer/txm/txm.go, update:- The info log at line 281 to log
"fromAddress", utils.RedactAddress(fromAddress)instead of the full address. - The error log at line 286 similarly to log the redacted address instead of the full one.
- The info log at line 281 to log
No changes are required to imports beyond using the already imported github.com/smartcontractkit/chainlink-aptos/relayer/utils in txm.go. The new helper in address.go uses only len and string slicing, so no new imports are necessary there.
| @@ -278,12 +278,12 @@ | ||
|
|
||
| acc := utils.Ed25519PublicKeyToAddress(ed25519PublicKey) | ||
| fromAddress := acc.String() | ||
| a.baseLogger.Infow("TestingAptosWriteCap: EnqueueCRE - resolved from address", "fromAddress", fromAddress) | ||
| a.baseLogger.Infow("TestingAptosWriteCap: EnqueueCRE - resolved from address", "fromAddress", utils.RedactAddress(fromAddress)) | ||
|
|
||
| fromAccountAddress := &aptos.AccountAddress{} | ||
| err = fromAccountAddress.ParseStringRelaxed(fromAddress) | ||
| if err != nil { | ||
| a.baseLogger.Errorw("TestingAptosWriteCap: EnqueueCRE - failed to parse from address", "fromAddress", fromAddress, "error", err) | ||
| a.baseLogger.Errorw("TestingAptosWriteCap: EnqueueCRE - failed to parse from address", "fromAddress", utils.RedactAddress(fromAddress), "error", err) | ||
| return fmt.Errorf("failed to parse from address: %+w", err) | ||
| } | ||
|
|
| @@ -67,3 +67,14 @@ | ||
|
|
||
| return Ed25519PublicKeyToAddress(ed25519.PublicKey(publicKey)), nil | ||
| } | ||
|
|
||
| // RedactAddress returns a partially obfuscated representation of an address string | ||
| // suitable for logging without exposing the full underlying value. | ||
| func RedactAddress(addr string) string { | ||
| const keep = 6 | ||
| n := len(addr) | ||
| if n <= 2*keep { | ||
| return addr | ||
| } | ||
| return addr[:keep] + "..." + addr[n-keep:] | ||
| } |
|
|
||
| select { | ||
| case a.broadcastChan <- transactionID: | ||
| ctxLogger.Infow("TestingAptosWriteCap: EnqueueCRE - tx enqueued to broadcast channel", "fromAddr", fromAddress, "transactionID", transactionID) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 4 hours ago
In general, to fix clear-text logging of sensitive information, you either (1) remove the sensitive data from logs entirely, or (2) obfuscate/redact it so logs retain usefulness without exposing full values. Here, the sensitive value is fromAddress, an account address string derived from an auth key. The safest change that preserves existing functionality is to stop logging the full fromAddress while leaving all transaction processing logic intact.
The single best fix is to modify the log line on line 325 in relayer/txm/txm.go so it no longer includes "fromAddr", fromAddress. Since the rest of the system may depend on this log message existing (e.g., for debugging or metrics parsing), we keep the log but only report non-sensitive fields like transactionID. There is already another log earlier (resolved from address) that also logs fromAddress; ideally that should also be adjusted, but the CodeQL alert specifically highlights line 325, so we will minimally fix that sink. No new imports or helpers are required; we simply adjust the arguments to Infow in the same file.
Concretely:
- In
relayer/txm/txm.go, locate theselectblock where the transaction ID is sent ona.broadcastChan. - Replace the
Infowcall on line 325 to remove the"fromAddr", fromAddresskey/value pair, leaving just the message and"transactionID", transactionID. - No additional methods, types, or configuration are needed.
| @@ -322,7 +322,7 @@ | ||
|
|
||
| select { | ||
| case a.broadcastChan <- transactionID: | ||
| ctxLogger.Infow("TestingAptosWriteCap: EnqueueCRE - tx enqueued to broadcast channel", "fromAddr", fromAddress, "transactionID", transactionID) | ||
| ctxLogger.Infow("TestingAptosWriteCap: EnqueueCRE - tx enqueued to broadcast channel", "transactionID", transactionID) | ||
| default: | ||
| // if the channel is full, we drop the transaction. | ||
| // we do this instead of setting the tx in `a.transactions` post-broadcast to avoid a race |
TODO:
smartcontractkit/chainlink#21431