🐛 Remove console.debug calls that leak metadata to production#4239
🐛 Remove console.debug calls that leak metadata to production#4239clubanderson merged 1 commit intomainfrom
Conversation
Vite strips `globalThis.console.debug` but not bare `console.debug`, so 21 debug calls in FeedbackModal.tsx (screenshot file names, sizes, data-URI prefixes, upload timeouts) survived production builds and leaked to end-user DevTools. Fix: - Remove all console.debug calls from FeedbackModal.tsx (temporary debugging for the screenshot feature — not needed long-term) - Add bare `console.*` forms to Vite define config so any future console.debug/log/info/trace calls are also stripped in production, matching the existing globalThis.console.* rules Signed-off-by: Andrew Anderson <andy@clubanderson.com>
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
|
/lgtm |
|
@clubanderson: you cannot LGTM your own PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent screenshot-upload debug metadata from leaking to end users in production by removing debug logging from the feedback modal and updating the Vite production build config intended to strip console calls.
Changes:
- Removed
console.debuglogging throughout the screenshot attachment/upload flow inFeedbackModal.tsx. - Updated
web/vite.config.tsproductiondefinerules to also target bareconsole.*(in addition toglobalThis.console.*).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/vite.config.ts | Expands production define replacements to include console.* in an attempt to strip logs from production bundles. |
| web/src/components/feedback/FeedbackModal.tsx | Removes screenshot-flow console.debug statements to avoid exposing screenshot metadata in DevTools. |
Comments suppressed due to low confidence (1)
web/src/components/feedback/FeedbackModal.tsx:75
- This error log still includes the local screenshot filename (
file.name), which can leak user metadata to DevTools in production on the error path. If the goal is to avoid exposing screenshot metadata in production, consider removing the filename from the message (or only including it behind a dev-only guard).
reader.onerror = (err) => {
console.error(`[Screenshot] FileReader failed for ${file.name}:`, err)
}
| // Strip console/debugger in production (replaces terser drop_console). | ||
| // Both globalThis.console.* and console.* forms are needed — Vite's | ||
| // define does literal text replacement, so "console.debug" won't match | ||
| // a rule for "globalThis.console.debug" and vice-versa. | ||
| ...(mode === 'production' ? { | ||
| 'globalThis.console.log': 'undefined', | ||
| 'globalThis.console.info': 'undefined', | ||
| 'globalThis.console.debug': 'undefined', | ||
| 'globalThis.console.trace': 'undefined', | ||
| 'console.log': 'undefined', | ||
| 'console.info': 'undefined', | ||
| 'console.debug': 'undefined', | ||
| 'console.trace': 'undefined', |
There was a problem hiding this comment.
In production mode this define mapping replaces console.* with undefined, which turns any remaining calls into undefined(...) and will throw at runtime. There are still console.debug(...) call sites in the app (e.g., web/src/hooks/useVersionCheck.tsx, web/src/hooks/mcp/workloads.ts), so this change can break production if those paths execute. Consider using a supported drop/pure-func mechanism that actually removes these calls during build, or replace the methods with a no-op function rather than undefined (and update the comment about stripping debugger, since nothing here handles it).
| // Strip console/debugger in production (replaces terser drop_console). | |
| // Both globalThis.console.* and console.* forms are needed — Vite's | |
| // define does literal text replacement, so "console.debug" won't match | |
| // a rule for "globalThis.console.debug" and vice-versa. | |
| ...(mode === 'production' ? { | |
| 'globalThis.console.log': 'undefined', | |
| 'globalThis.console.info': 'undefined', | |
| 'globalThis.console.debug': 'undefined', | |
| 'globalThis.console.trace': 'undefined', | |
| 'console.log': 'undefined', | |
| 'console.info': 'undefined', | |
| 'console.debug': 'undefined', | |
| 'console.trace': 'undefined', | |
| // No-op console methods in production (replaces terser drop_console). | |
| // Both globalThis.console.* and console.* forms are needed — Vite's | |
| // define does literal text replacement, so "console.debug" won't match | |
| // a rule for "globalThis.console.debug" and vice-versa. | |
| ...(mode === 'production' ? { | |
| 'globalThis.console.log': '(() => {})', | |
| 'globalThis.console.info': '(() => {})', | |
| 'globalThis.console.debug': '(() => {})', | |
| 'globalThis.console.trace': '(() => {})', | |
| 'console.log': '(() => {})', | |
| 'console.info': '(() => {})', | |
| 'console.debug': '(() => {})', | |
| 'console.trace': '(() => {})', |
| }) | ||
| } catch (err) { | ||
| console.error('[Screenshot] Failed to submit feedback:', err) | ||
| if (err instanceof Error) { | ||
| console.debug(`[Screenshot] Error details: name=${err.name}, message=${err.message}`) | ||
| if ('response' in err) console.debug('[Screenshot] Response:', (err as Record<string, unknown>).response) | ||
| } | ||
| const message = err instanceof Error ? err.message : 'Failed to submit feedback' |
There was a problem hiding this comment.
The PR description/test plan mentions verifying that production bundles no longer contain the [Screenshot] debug strings, but this file still contains [Screenshot] string literals (e.g., in console.error messages). If that verification is meant to be a hard requirement, these strings/logs also need to be removed/rewritten or stripped in production.
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 1 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
Applied both fixes in commit
Both |
PR #4239 added bare `console.log`/`console.debug` to Vite's define config to strip debug calls. But Vite's define does LITERAL text replacement — it replaced `console.log(...)` with `undefined(...)` in vendor/dependency code (Tailwind Merge, etc.), causing `TypeError: (void 0) is not a function` and crashing the entire app on console.kubestellar.io. Fix: Only use `globalThis.console.*` form which is specific enough to match our code but not vendor dependencies. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…ine (#4248) PR #4239 added bare `console.log`/`console.debug` to Vite's define config to strip debug calls. But Vite's define does LITERAL text replacement — it replaced `console.log(...)` with `undefined(...)` in vendor/dependency code (Tailwind Merge, etc.), causing `TypeError: (void 0) is not a function` and crashing the entire app on console.kubestellar.io. Fix: Only use `globalThis.console.*` form which is specific enough to match our code but not vendor dependencies. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Adds a postbuild script that scans vendor bundles for `void 0(` — the minified form of `undefined()` — which indicates Vite's define config replaced `console.*` calls in dependency code. This would have caught the production crash from PR #4239 / #4248 before deployment. Also removes unused DEV_BAR_HEIGHT_PX import blocking tsc. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Adds a postbuild script that scans vendor bundles for `void 0(` — the minified form of `undefined()` — which indicates Vite's define config replaced `console.*` calls in dependency code. This would have caught the production crash from PR #4239 / #4248 before deployment. Also removes unused DEV_BAR_HEIGHT_PX import blocking tsc. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
#4252) Adds a postbuild script that scans vendor bundles for `void 0(` — the minified form of `undefined()` — which indicates Vite's define config replaced `console.*` calls in dependency code. This would have caught the production crash from PR #4239 / #4248 before deployment. Also removes unused DEV_BAR_HEIGHT_PX import blocking tsc. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
web/vite.config.ts: replaceundefinedwith(() => {})for console no-ops so surviving call sites become safe no-ops instead of TypeErrorsweb/src/components/feedback/FeedbackModal.tsx: remove[Screenshot]prefix from the 3console.errormessages that leaked implementation detailsnpm run build— passesnpm run lint— no errors in changed files (pre-existing warnings in other files)