Skip to content

Security: fix verbose logging, unvalidated NID params, and untyped message payloads#25

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/security-fix-verbose-logging
Draft

Security: fix verbose logging, unvalidated NID params, and untyped message payloads#25
Copilot wants to merge 2 commits intomainfrom
copilot/security-fix-verbose-logging

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 12, 2026

Three high-severity findings: sensitive data (screenshots, auth tokens, emails) leaking to console in production; unvalidated nid values from URL params/API responses interpolated directly into URLs enabling open redirect; and any-typed message handler payloads allowing runtime crashes or resource exhaustion via crafted inputs.

Centralized logger (src/utils/logger.ts)

  • New logger utility gates all output behind import.meta.env.DEV — silent in production builds
  • Recursively redacts sensitive keys (dataUrl, password, token, accessToken, refreshToken) before any output
  • environment.ts: enableLogging changed from hardcoded trueimport.meta.env.DEV

NID validation

  • validateNid(nid) exported from logger.ts — accepts only /^[a-zA-Z0-9_-]+$/
  • Applied at all three injection points: share.tsx (URL param), popup.tsx (three handlers), UploadService.ts (API response before building share URL)
// Before — open redirect possible
const verifyUrl = `https://asset.captureapp.xyz/${nid}`;

// After — rejects path traversal / fragment injection
if (validateNid(nid)) {
  const verifyUrl = `https://asset.captureapp.xyz/${nid}`;
}

Message payload validation (service-worker.ts)

  • handleSelectionComplete and handleAssetUpload signatures changed from payload: anypayload: unknown
  • validateCoordinates(): enforces numeric types, positive dimensions, max 32767×32767 canvas bound
  • validateAssetUploadPayload(): enforces assetId is a non-empty string
  • message.payload removed from the onMessage listener log (was leaking base64 screenshot data and auth tokens)
  • user.email removed from the NumbersApiManager token-validation log
Original prompt

This section details on the original issue you should resolve

<issue_title>[Security][High] Verbose logging, unvalidated URL params, and missing payload validation</issue_title>
<issue_description>## Summary

Three high-severity security findings that collectively expose sensitive data and create injection vectors.


1. Verbose Error Logging Leaks Sensitive Data to Console

Files: src/background/service-worker.ts:95, src/services/NumbersApiManager.ts:125, src/config/environment.ts:22-29

There are ~99 console.log/warn/error calls across the codebase. Several log sensitive information:

  • service-worker.ts:95console.log('Message received:', message.type, message.payload) logs full payloads including screenshot base64 data URLs and auth tokens
  • NumbersApiManager.ts:125console.log('Token validated successfully for user:', user.email) logs user emails
  • enableLogging: true is hardcoded in environment.ts with no production override

Fix: Create a centralized logging utility that respects enableLogging and redacts sensitive fields (dataUrl, password, tokens). Use import.meta.env.DEV for the logging flag.


2. Unvalidated nid Parameter Used in URL Construction

Files: src/share/share.tsx:11-21, src/popup/popup.tsx:614-628, src/services/UploadService.ts:359

The nid parameter from URL query params or API responses is directly interpolated into URLs without validation:

// share.tsx
const nid = params.get('nid');
const verifyUrl = `https://asset.captureapp.xyz/${nid}`;

A crafted nid containing path traversal characters or URL fragments could redirect users to phishing sites via the constructed twitterUrl and verifyUrl.

Fix: Validate nid matches /^[a-zA-Z0-9_-]+$/ before use. Reject invalid values with an error message.


3. Untyped Payloads at Message Handler API Boundary

Files: src/background/service-worker.ts:205,643

Message handler functions accept payload: any and destructure fields without structural validation:

  • handleSelectionComplete(payload: any) destructures coordinates directly (line 205)
  • handleAssetUpload(payload: any) accesses payload.assetId directly (line 643)

Malformed payloads could cause runtime exceptions or memory exhaustion (e.g., coordinates with extreme dimensions for canvas cropping).

Fix: Add runtime validation functions for each payload type:

function validateCoordinates(coords: any): coords is SelectionCoordinates {
  return coords && typeof coords.x === 'number' && typeof coords.y === 'number'
    && typeof coords.width === 'number' && typeof coords.height === 'number'
    && coords.width > 0 && coords.height > 0
    && coords.width <= 32767 && coords.height <= 32767;
}

Impact

  • Sensitive data (emails, tokens, screenshot data) exposed to any extension or DevTools session
  • Potential URL injection / open redirect via crafted nid values
  • Runtime crashes or resource exhaustion from malformed message payloads

Generated by Health Monitor with Omni</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…idation

Co-authored-by: numbers-official <181934381+numbers-official@users.noreply.github.com>
Copilot AI changed the title [WIP] [Security] Fix verbose logging and validate URL params Security: fix verbose logging, unvalidated NID params, and untyped message payloads Mar 12, 2026
Copilot AI requested a review from numbers-official March 12, 2026 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security][High] Verbose logging, unvalidated URL params, and missing payload validation

2 participants