fix(frontend): prevent PII leaks in production console logs#262
Conversation
|
Hey @johdanike! 👋 It looks like this PR isn't linked to any issue. If this PR is for one of the issues assigned to you as part of a Wave, please link it to ensure your contribution is tracked properly. You can do this by adding a keyword to the PR description (e.g.,
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new centralized logger utility is introduced in Changes
Possibly related issues
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/savings/withdraw/page.tsx (1)
32-37:⚠️ Potential issue | 🟠 MajorAvoid production logging of receive-address errors.
This still writes a potentially sensitive API error message to the production console because
logger.erroris not debug-gated. Treat this as a debug/dev warning instead.🛡️ Proposed fix
- .catch((e) => { - logger.error( - e instanceof Error - ? e.message - : "Failed to load receive address", - ); - }); + .catch((e) => { + logger.warn("Failed to load receive address", e); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/savings/withdraw/page.tsx` around lines 32 - 37, The catch block that calls logger.error with the receive-address error should not log potentially sensitive messages in production; update the .catch handler in page.tsx (the anonymous callback that references logger) to log a non-sensitive generic message at error/warn level and move the detailed e.message into a debug-only log (e.g., logger.debug or gated by process.env.NODE_ENV !== 'production'), or alternatively replace logger.error with logger.warn and avoid printing e.message in production while keeping detailed info available in dev/debug logs.app/mint/page.tsx (1)
197-202:⚠️ Potential issue | 🟠 MajorLogging the full Stellar account address still leaks a user identifier.
The stated goal of this PR is to keep PII out of browser-console output on sensitive flows.
accountIdhere is the user's full Stellar public key — a stable, user-linked wallet identifier that correlates to the authenticated user. Even thoughlogger.infois gated on non-prod /NEXT_PUBLIC_DEBUG_LOGGING, any environment that flips that flag (staging with real users, a support session, a shared browser, etc.) will emit the full address alongside atxHash, which is exactly the kind of correlation the PR is trying to avoid. The rest of this file already treats the address as sensitive — every user-facing message uses theslice(0,6)…slice(-4)pattern (e.g. lines 160, 188, 252, 285).Recommend applying the same truncation in the log (and consider whether
txHashneeds to be logged at all, since on-chain it is trivially linkable back to the account):🛡️ Proposed fix
- logger.info("[mint] ACBU trustline ensured", { - account: accountId, - added: trust?.added, - visible: trust?.visible, - txHash: trust?.txHash, - }); + logger.info("[mint] ACBU trustline ensured", { + account: `${accountId.slice(0, 6)}…${accountId.slice(-4)}`, + added: trust?.added, + visible: trust?.visible, + txHash: trust?.txHash, + });The analogous log in
lib/stellar/trustlines.ts(lines 221–224) only logscode/issuerand is fine as-is — worth keeping that pattern consistent here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/mint/page.tsx` around lines 197 - 202, The logger.info call that currently logs the full Stellar public key (accountId) and txHash should avoid emitting PII: replace the direct accountId with a truncated form (e.g., accountId.slice(0,6) + "…" + accountId.slice(-4)) and remove or avoid logging trust?.txHash (or similarly truncate it) in the "[mint] ACBU trustline ensured" log; update the logger.info invocation in this block (the call referencing logger.info, accountId, trust?.added, trust?.visible, trust?.txHash) to use the truncated identifier and omit/obfuscate txHash to match the file's existing user-facing masking pattern.
🧹 Nitpick comments (1)
lib/stellar/trustlines.ts (1)
221-224: Minor: indentation inconsistent with surrounding block.The
logger.info(...)call at line 221 is indented one level deeper than the surrounding function body (thelet asset,try, andreturnstatements at lines 210–225 are at 2 spaces; this block is at 4). Functionally fine, but worth normalizing while touching this line.✏️ Proposed fix
- logger.info("[trustline] ensuring ACBU trustline", { - code: asset.getCode(), - issuer: asset.getIssuer(), - }); + logger.info("[trustline] ensuring ACBU trustline", { + code: asset.getCode(), + issuer: asset.getIssuer(), + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/stellar/trustlines.ts` around lines 221 - 224, The logger.info call for ensuring the ACBU trustline is mis-indented; align its indentation with the surrounding function block (same indentation level used by the nearby let asset, try, and return statements) so the line starting with logger.info(...) sits at the same indentation as those statements; locate the call by searching for logger.info("[trustline] ensuring ACBU trustline", { and adjust whitespace before it to match the block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/business/page.tsx`:
- Around line 39-42: The catch block that logs business stats failures currently
calls logger.error('Business stats error:', err) and thus emits raw errors in
production; change it so that raw error details are only logged at debug level
in non-production (e.g., guard with NODE_ENV !== 'production' and call
logger.debug(...err...)), and in production replace the error call with a
sanitized warning (e.g., logger.warn('Business stats fallback - details
omitted')) or send sanitized telemetry instead; update the catch in
app/business/page.tsx (the catch around the business stats fetch where
logger.error is called) accordingly so production logs do not contain raw error
objects.
In `@app/error.tsx`:
- Around line 14-16: The current useEffect calls logger.error('Application
error:', error) which may leak raw error details; change the effect (useEffect)
to compute a sanitized identifier (e.g. an error.code or a short digest derived
from error.name/message or error.id) and pass only that identifier to
logger.error, and then emit the full error object to logger.debug (or
logger.warn) only when not in production (guarded by NODE_ENV or an
isDevelopment flag); update the useEffect to use the new sanitizedErrorId and
conditional debug logging while keeping the original logger.error call signature
limited to non-sensitive data.
In `@app/mint/page.tsx`:
- Line 109: The catch currently logs the Error object directly which loses
message/stack during JSON serialization; update the .catch handler that calls
logger.error in page.tsx so it extracts and passes e.message and e.stack (e.g.,
replace the second argument to logger.error with an object containing message
and stack) to preserve error details when logging.
In `@app/savings/deposit/page.tsx`:
- Around line 28-30: The catch handler for loading the receive address currently
calls logger.error with the raw error message; replace this with a
production-safe pattern: in the .catch((e) => { ... }) block use logger.debug or
a feature-flag gated log (e.g. if process.env.NODE_ENV !== 'production' ||
process.env.DEBUG_RECEIVE_ADDRESS) to include e.message/e.stack, but always emit
a generic logger.warn or logger.info like "Failed to load receive address" in
production without the raw API text; update the catch on the receive-address
load so the symbol logger only logs sensitive error details when debug is
explicitly enabled.
In `@components/error-boundary.tsx`:
- Around line 27-29: In componentDidCatch, stop passing the full { error,
errorInfo } to logger.error in production; change componentDidCatch to call
logger.error with only a generic message (e.g., "Uncaught UI error") for
production environments and conditionally log the detailed payload (error and
errorInfo) behind a debug gate (e.g., process.env.NODE_ENV === 'development' or
a debug flag) so that the detailed stack and component info are only emitted
when debugging; ensure references to componentDidCatch, logger.error, error, and
errorInfo are updated accordingly.
In `@contexts/auth-context.tsx`:
- Around line 106-108: The catch block that currently calls logger.error('Failed
to refresh stellar address', e) should avoid emitting raw error objects to
production; change it to log a sanitized message to error-level and send the
full/raw error only to a debug-only channel. Concretely, replace the direct pass
of the caught value to logger.error by either (a) logger.debug('Failed to
refresh stellar address', e) plus logger.error('Failed to refresh stellar
address') or (b) logger.error('Failed to refresh stellar address: ' + (e &&
e.message) ), so that logger.error does not receive the raw error object; update
the catch in the refresh path where logger.error is invoked accordingly.
In `@lib/logger.ts`:
- Line 1: The isDebug flag currently reads NEXT_PUBLIC_DEBUG instead of the
documented NEXT_PUBLIC_DEBUG_LOGGING; update the constant declaration for
isDebug in logger.ts to check process.env.NEXT_PUBLIC_DEBUG_LOGGING === 'true'
(and still OR with process.env.NODE_ENV !== 'production') so the documented
opt-in flag is honored; change the identifier usage where isDebug is defined
only — keep all downstream usages of isDebug unchanged.
- Around line 5-31: logMessage currently allows logger.error to log
caller-supplied data in production and uses unsafe any types; update logMessage
and the exported logger methods to use unknown for the data parameter, restrict
logging of arbitrary data payloads to debug mode only (i.e., when isDebug is
true) so logger.error still outputs the error message but does not serialize
caller data unless isDebug, and add a safe serialization path that normalizes
Error objects (extracting name, message, stack) and wraps JSON.stringify in a
try/catch to fall back to a non-sensitive placeholder on failure; update
signatures for logger.info/warn/error/debug to accept data?: unknown and ensure
the logEntry construction uses the normalized/guarded serialization result
instead of raw data.
---
Outside diff comments:
In `@app/mint/page.tsx`:
- Around line 197-202: The logger.info call that currently logs the full Stellar
public key (accountId) and txHash should avoid emitting PII: replace the direct
accountId with a truncated form (e.g., accountId.slice(0,6) + "…" +
accountId.slice(-4)) and remove or avoid logging trust?.txHash (or similarly
truncate it) in the "[mint] ACBU trustline ensured" log; update the logger.info
invocation in this block (the call referencing logger.info, accountId,
trust?.added, trust?.visible, trust?.txHash) to use the truncated identifier and
omit/obfuscate txHash to match the file's existing user-facing masking pattern.
In `@app/savings/withdraw/page.tsx`:
- Around line 32-37: The catch block that calls logger.error with the
receive-address error should not log potentially sensitive messages in
production; update the .catch handler in page.tsx (the anonymous callback that
references logger) to log a non-sensitive generic message at error/warn level
and move the detailed e.message into a debug-only log (e.g., logger.debug or
gated by process.env.NODE_ENV !== 'production'), or alternatively replace
logger.error with logger.warn and avoid printing e.message in production while
keeping detailed info available in dev/debug logs.
---
Nitpick comments:
In `@lib/stellar/trustlines.ts`:
- Around line 221-224: The logger.info call for ensuring the ACBU trustline is
mis-indented; align its indentation with the surrounding function block (same
indentation level used by the nearby let asset, try, and return statements) so
the line starting with logger.info(...) sits at the same indentation as those
statements; locate the call by searching for logger.info("[trustline] ensuring
ACBU trustline", { and adjust whitespace before it to match the block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f00c8b2f-b878-4080-85e7-ca3991baaa03
📒 Files selected for processing (10)
app/business/page.tsxapp/error.tsxapp/mint/page.tsxapp/savings/deposit/page.tsxapp/savings/withdraw/page.tsxcomponents/error-boundary.tsxcontexts/auth-context.tsxlib/logger.tslib/stellar-wallets-kit.tslib/stellar/trustlines.ts
| } catch (err) { | ||
| // Fallback: show loading state instead of error UI | ||
| console.error('Business stats error:', err); | ||
| logger.error('Business stats error:', err); | ||
| } finally { |
There was a problem hiding this comment.
Don’t escalate fallback stats failures to production errors.
This path intentionally falls back without user-facing error UI, but logger.error emits the raw caught error in production. Use a debug-gated warning unless there is sanitized telemetry behind it.
🔧 Proposed fix
} catch (err) {
// Fallback: show loading state instead of error UI
- logger.error('Business stats error:', err);
+ logger.warn('Business stats error', err);
} finally {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err) { | |
| // Fallback: show loading state instead of error UI | |
| console.error('Business stats error:', err); | |
| logger.error('Business stats error:', err); | |
| } finally { | |
| } catch (err) { | |
| // Fallback: show loading state instead of error UI | |
| logger.warn('Business stats error', err); | |
| } finally { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/business/page.tsx` around lines 39 - 42, The catch block that logs
business stats failures currently calls logger.error('Business stats error:',
err) and thus emits raw errors in production; change it so that raw error
details are only logged at debug level in non-production (e.g., guard with
NODE_ENV !== 'production' and call logger.debug(...err...)), and in production
replace the error call with a sanitized warning (e.g., logger.warn('Business
stats fallback - details omitted')) or send sanitized telemetry instead; update
the catch in app/business/page.tsx (the catch around the business stats fetch
where logger.error is called) accordingly so production logs do not contain raw
error objects.
| useEffect(() => { | ||
| console.error('Application error:', error); | ||
| logger.error('Application error:', error); | ||
| }, [error]); |
There was a problem hiding this comment.
Only log sanitized error identifiers in production.
Passing the full error object to logger.error can expose arbitrary error details in the production console. Log the digest/generic signal at error level and keep raw details debug-gated.
🛡️ Proposed fix
useEffect(() => {
- logger.error('Application error:', error);
+ logger.error('Application error', error.digest ? { digest: error.digest } : undefined);
+ logger.debug('Application error details', error);
}, [error]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| console.error('Application error:', error); | |
| logger.error('Application error:', error); | |
| }, [error]); | |
| useEffect(() => { | |
| logger.error('Application error', error.digest ? { digest: error.digest } : undefined); | |
| logger.debug('Application error details', error); | |
| }, [error]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/error.tsx` around lines 14 - 16, The current useEffect calls
logger.error('Application error:', error) which may leak raw error details;
change the effect (useEffect) to compute a sanitized identifier (e.g. an
error.code or a short digest derived from error.name/message or error.id) and
pass only that identifier to logger.error, and then emit the full error object
to logger.debug (or logger.warn) only when not in production (guarded by
NODE_ENV or an isDevelopment flag); update the useEffect to use the new
sanitizedErrorId and conditional debug logging while keeping the original
logger.error call signature limited to non-sensitive data.
| } | ||
| }) | ||
| .catch(console.error); | ||
| .catch((e) => logger.error('Failed to get fiat accounts', e)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'logger.ts' lib
fd -t f 'logger.ts' lib --exec cat {}Repository: Pi-Defi-world/acbu-frontend
Length of output: 1175
Extract error message and stack before logging.
The logger.error correctly routes through console.error and will not be stripped from production, but there is a critical issue: the Error object passed as the second argument will serialize to {} when JSON.stringify processes it, since Error properties like message and stack are non-enumerable.
Change .catch((e) => logger.error('Failed to get fiat accounts', e)) to .catch((e) => logger.error('Failed to get fiat accounts', { message: e.message, stack: e.stack })) to preserve the actual error details.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/mint/page.tsx` at line 109, The catch currently logs the Error object
directly which loses message/stack during JSON serialization; update the .catch
handler that calls logger.error in page.tsx so it extracts and passes e.message
and e.stack (e.g., replace the second argument to logger.error with an object
containing message and stack) to preserve error details when logging.
| }).catch((e) => { | ||
| console.error(e instanceof Error ? e.message : 'Failed to load receive address'); | ||
| logger.error(e instanceof Error ? e.message : 'Failed to load receive address'); | ||
| }); |
There was a problem hiding this comment.
Avoid production logging of receive-address errors.
This sensitive Savings path still sends the caught error message to production via logger.error. Use a debug-gated level and avoid logging raw API text in production.
🛡️ Proposed fix
- }).catch((e) => {
- logger.error(e instanceof Error ? e.message : 'Failed to load receive address');
+ }).catch((e) => {
+ logger.warn('Failed to load receive address', e);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }).catch((e) => { | |
| console.error(e instanceof Error ? e.message : 'Failed to load receive address'); | |
| logger.error(e instanceof Error ? e.message : 'Failed to load receive address'); | |
| }); | |
| }).catch((e) => { | |
| logger.warn('Failed to load receive address', e); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/savings/deposit/page.tsx` around lines 28 - 30, The catch handler for
loading the receive address currently calls logger.error with the raw error
message; replace this with a production-safe pattern: in the .catch((e) => { ...
}) block use logger.debug or a feature-flag gated log (e.g. if
process.env.NODE_ENV !== 'production' || process.env.DEBUG_RECEIVE_ADDRESS) to
include e.message/e.stack, but always emit a generic logger.warn or logger.info
like "Failed to load receive address" in production without the raw API text;
update the catch on the receive-address load so the symbol logger only logs
sensitive error details when debug is explicitly enabled.
| componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { | ||
| console.error('ErrorBoundary caught an error:', error, errorInfo); | ||
| logger.error('ErrorBoundary caught an error:', { error, errorInfo }); | ||
| } |
There was a problem hiding this comment.
Split production error signal from debug details.
Passing { error, errorInfo } to logger.error keeps arbitrary error details eligible for production console output. Emit only a generic production error and keep stack/component details debug-gated.
🛡️ Proposed fix
componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void {
- logger.error('ErrorBoundary caught an error:', { error, errorInfo });
+ logger.error('ErrorBoundary caught an error');
+ logger.debug('ErrorBoundary caught an error', { error, errorInfo });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { | |
| console.error('ErrorBoundary caught an error:', error, errorInfo); | |
| logger.error('ErrorBoundary caught an error:', { error, errorInfo }); | |
| } | |
| componentDidCatch(error: Error, errorInfo: React.ErrorInfo): void { | |
| logger.error('ErrorBoundary caught an error'); | |
| logger.debug('ErrorBoundary caught an error', { error, errorInfo }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/error-boundary.tsx` around lines 27 - 29, In componentDidCatch,
stop passing the full { error, errorInfo } to logger.error in production; change
componentDidCatch to call logger.error with only a generic message (e.g.,
"Uncaught UI error") for production environments and conditionally log the
detailed payload (error and errorInfo) behind a debug gate (e.g.,
process.env.NODE_ENV === 'development' or a debug flag) so that the detailed
stack and component info are only emitted when debugging; ensure references to
componentDidCatch, logger.error, error, and errorInfo are updated accordingly.
| } catch (e) { | ||
| console.error('Failed to refresh stellar address', e); | ||
| logger.error('Failed to refresh stellar address', e); | ||
| } |
There was a problem hiding this comment.
Keep background refresh failures out of production console.
This recoverable auth refresh path now passes the raw caught value to logger.error, which emits in production. Use a debug-gated level or sanitized payload instead.
🛡️ Proposed fix
- } catch (e) {
- logger.error('Failed to refresh stellar address', e);
+ } catch (e) {
+ logger.warn('Failed to refresh stellar address', e);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e) { | |
| console.error('Failed to refresh stellar address', e); | |
| logger.error('Failed to refresh stellar address', e); | |
| } | |
| } catch (e) { | |
| logger.warn('Failed to refresh stellar address', e); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contexts/auth-context.tsx` around lines 106 - 108, The catch block that
currently calls logger.error('Failed to refresh stellar address', e) should
avoid emitting raw error objects to production; change it to log a sanitized
message to error-level and send the full/raw error only to a debug-only channel.
Concretely, replace the direct pass of the caught value to logger.error by
either (a) logger.debug('Failed to refresh stellar address', e) plus
logger.error('Failed to refresh stellar address') or (b) logger.error('Failed to
refresh stellar address: ' + (e && e.message) ), so that logger.error does not
receive the raw error object; update the catch in the refresh path where
logger.error is invoked accordingly.
| @@ -0,0 +1,32 @@ | |||
| const isDebug = process.env.NEXT_PUBLIC_DEBUG === 'true' || process.env.NODE_ENV !== 'production'; | |||
There was a problem hiding this comment.
Use the documented debug flag.
The PR contract says production debug logging is enabled with NEXT_PUBLIC_DEBUG_LOGGING=true, but this checks NEXT_PUBLIC_DEBUG; the documented opt-in will be ignored.
🔧 Proposed fix
-const isDebug = process.env.NEXT_PUBLIC_DEBUG === 'true' || process.env.NODE_ENV !== 'production';
+const isDebug = process.env.NEXT_PUBLIC_DEBUG_LOGGING === 'true' || process.env.NODE_ENV !== 'production';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isDebug = process.env.NEXT_PUBLIC_DEBUG === 'true' || process.env.NODE_ENV !== 'production'; | |
| const isDebug = process.env.NEXT_PUBLIC_DEBUG_LOGGING === 'true' || process.env.NODE_ENV !== 'production'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/logger.ts` at line 1, The isDebug flag currently reads NEXT_PUBLIC_DEBUG
instead of the documented NEXT_PUBLIC_DEBUG_LOGGING; update the constant
declaration for isDebug in logger.ts to check
process.env.NEXT_PUBLIC_DEBUG_LOGGING === 'true' (and still OR with
process.env.NODE_ENV !== 'production') so the documented opt-in flag is honored;
change the identifier usage where isDebug is defined only — keep all downstream
usages of isDebug unchanged.
| function logMessage(level: LogLevel, message: string, data?: any) { | ||
| if (!isDebug && level !== 'error') return; | ||
|
|
||
| const logEntry = { | ||
| timestamp: new Date().toISOString(), | ||
| level, | ||
| message, | ||
| ...(data !== undefined && { data }) | ||
| }; | ||
|
|
||
| // In debug mode, or for errors, output structured logs to console | ||
| if (level === 'error') { | ||
| console.error(JSON.stringify(logEntry)); | ||
| } else if (level === 'warn') { | ||
| console.warn(JSON.stringify(logEntry)); | ||
| } else if (level === 'info') { | ||
| console.info(JSON.stringify(logEntry)); | ||
| } else { | ||
| console.log(JSON.stringify(logEntry)); | ||
| } | ||
| } | ||
|
|
||
| export const logger = { | ||
| info: (message: string, data?: any) => logMessage('info', message, data), | ||
| warn: (message: string, data?: any) => logMessage('warn', message, data), | ||
| error: (message: string, data?: any) => logMessage('error', message, data), | ||
| debug: (message: string, data?: any) => logMessage('debug', message, data), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm no explicit `any` remains in logger payload signatures.
rg -nP '\bdata\?:\s*any\b|function\s+logMessage\([^)]*\bdata\?:\s*any'Repository: Pi-Defi-world/acbu-frontend
Length of output: 530
Fix security hole in error logging and replace any with unknown.
logger.error bypasses the debug gate, allowing sensitive caller data (PII) to reach production console. The any type on all logger signatures violates type safety and triggers linting errors. Restrict data payload logging to debug mode, normalize Error objects safely, and handle serialization failures gracefully.
Proposed hardening
type LogLevel = 'info' | 'warn' | 'error' | 'debug';
-function logMessage(level: LogLevel, message: string, data?: any) {
+function normalizeLogData(data: unknown) {
+ if (data instanceof Error) {
+ return isDebug
+ ? { name: data.name, message: data.message, stack: data.stack }
+ : { name: data.name };
+ }
+
+ return isDebug ? data : undefined;
+}
+
+function stringifyLogEntry(entry: Record<string, unknown>) {
+ try {
+ return JSON.stringify(entry);
+ } catch {
+ const { data: _data, ...safeEntry } = entry;
+ return JSON.stringify({ ...safeEntry, data: '[unserializable]' });
+ }
+}
+
+function logMessage(level: LogLevel, message: string, data?: unknown) {
if (!isDebug && level !== 'error') return;
+ const normalizedData = data !== undefined ? normalizeLogData(data) : undefined;
const logEntry = {
timestamp: new Date().toISOString(),
level,
- message,
- ...(data !== undefined && { data })
+ message: isDebug ? message : 'Application error',
+ ...(normalizedData !== undefined && { data: normalizedData })
};
// In debug mode, or for errors, output structured logs to console
if (level === 'error') {
- console.error(JSON.stringify(logEntry));
+ console.error(stringifyLogEntry(logEntry));
} else if (level === 'warn') {
- console.warn(JSON.stringify(logEntry));
+ console.warn(stringifyLogEntry(logEntry));
} else if (level === 'info') {
- console.info(JSON.stringify(logEntry));
+ console.info(stringifyLogEntry(logEntry));
} else {
- console.log(JSON.stringify(logEntry));
+ console.log(stringifyLogEntry(logEntry));
}
}
export const logger = {
- info: (message: string, data?: any) => logMessage('info', message, data),
- warn: (message: string, data?: any) => logMessage('warn', message, data),
- error: (message: string, data?: any) => logMessage('error', message, data),
- debug: (message: string, data?: any) => logMessage('debug', message, data),
+ info: (message: string, data?: unknown) => logMessage('info', message, data),
+ warn: (message: string, data?: unknown) => logMessage('warn', message, data),
+ error: (message: string, data?: unknown) => logMessage('error', message, data),
+ debug: (message: string, data?: unknown) => logMessage('debug', message, data),
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function logMessage(level: LogLevel, message: string, data?: any) { | |
| if (!isDebug && level !== 'error') return; | |
| const logEntry = { | |
| timestamp: new Date().toISOString(), | |
| level, | |
| message, | |
| ...(data !== undefined && { data }) | |
| }; | |
| // In debug mode, or for errors, output structured logs to console | |
| if (level === 'error') { | |
| console.error(JSON.stringify(logEntry)); | |
| } else if (level === 'warn') { | |
| console.warn(JSON.stringify(logEntry)); | |
| } else if (level === 'info') { | |
| console.info(JSON.stringify(logEntry)); | |
| } else { | |
| console.log(JSON.stringify(logEntry)); | |
| } | |
| } | |
| export const logger = { | |
| info: (message: string, data?: any) => logMessage('info', message, data), | |
| warn: (message: string, data?: any) => logMessage('warn', message, data), | |
| error: (message: string, data?: any) => logMessage('error', message, data), | |
| debug: (message: string, data?: any) => logMessage('debug', message, data), | |
| type LogLevel = 'info' | 'warn' | 'error' | 'debug'; | |
| function normalizeLogData(data: unknown) { | |
| if (data instanceof Error) { | |
| return isDebug | |
| ? { name: data.name, message: data.message, stack: data.stack } | |
| : { name: data.name }; | |
| } | |
| return isDebug ? data : undefined; | |
| } | |
| function stringifyLogEntry(entry: Record<string, unknown>) { | |
| try { | |
| return JSON.stringify(entry); | |
| } catch { | |
| const { data: _data, ...safeEntry } = entry; | |
| return JSON.stringify({ ...safeEntry, data: '[unserializable]' }); | |
| } | |
| } | |
| function logMessage(level: LogLevel, message: string, data?: unknown) { | |
| if (!isDebug && level !== 'error') return; | |
| const normalizedData = data !== undefined ? normalizeLogData(data) : undefined; | |
| const logEntry = { | |
| timestamp: new Date().toISOString(), | |
| level, | |
| message: isDebug ? message : 'Application error', | |
| ...(normalizedData !== undefined && { data: normalizedData }) | |
| }; | |
| // In debug mode, or for errors, output structured logs to console | |
| if (level === 'error') { | |
| console.error(stringifyLogEntry(logEntry)); | |
| } else if (level === 'warn') { | |
| console.warn(stringifyLogEntry(logEntry)); | |
| } else if (level === 'info') { | |
| console.info(stringifyLogEntry(logEntry)); | |
| } else { | |
| console.log(stringifyLogEntry(logEntry)); | |
| } | |
| } | |
| export const logger = { | |
| info: (message: string, data?: unknown) => logMessage('info', message, data), | |
| warn: (message: string, data?: unknown) => logMessage('warn', message, data), | |
| error: (message: string, data?: unknown) => logMessage('error', message, data), | |
| debug: (message: string, data?: unknown) => logMessage('debug', message, data), | |
| }; |
🧰 Tools
🪛 ESLint
[error] 5-5: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 28-28: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 29-29: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 30-30: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/logger.ts` around lines 5 - 31, logMessage currently allows logger.error
to log caller-supplied data in production and uses unsafe any types; update
logMessage and the exported logger methods to use unknown for the data
parameter, restrict logging of arbitrary data payloads to debug mode only (i.e.,
when isDebug is true) so logger.error still outputs the error message but does
not serialize caller data unless isDebug, and add a safe serialization path that
normalizes Error objects (extracting name, message, stack) and wraps
JSON.stringify in a try/catch to fall back to a non-sensitive placeholder on
failure; update signatures for logger.info/warn/error/debug to accept data?:
unknown and ensure the logEntry construction uses the normalized/guarded
serialization result instead of raw data.
|
@johdanike resolve conflicts |
|
Resolved @Junman140 |
2 participants
Description
This PR addresses a critical hygiene and security issue where Personally Identifiable Information (PII) was leaking into the browser console on sensitive pages (Savings, Bills, Lending, Currency).
To ensure this doesn't happen again, raw console logging has been replaced with a structured utility, and the build process has been updated to mathematically strip standard logs from the production bundle.
Changes Made
utils/logger.ts: Introduced a centralized logger that only outputs when not in production, or when explicitly bypassed via theNEXT_PUBLIC_DEBUG_LOGGING=trueenvironment flag.console.logstatements for the new logger utility across sensitive flows, including the Stellar trustline checks (ensureAcbuTrustlineClient).vite.config.ts: Leveraged esbuild'spureconfiguration to automatically dropconsole.log,console.info, andconsole.debugduring the production build step.console.errorandconsole.warnare intentionally preserved for production monitoring tools.Acceptance Criteria Met
console.login transfer paths.Testing Instructions
npm run devand verify logs still appear in the browser console during local development.npm run buildfollowed bynpm run preview.Summary by CodeRabbit