Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions mobile/lib/blocs/divine_auth/divine_auth_cubit.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:nostr_key_manager/nostr_key_manager.dart';
import 'package:openvine/services/auth_service.dart';
import 'package:openvine/services/pending_verification_service.dart';
import 'package:openvine/utils/invite_error_utils.dart';
import 'package:openvine/utils/sensitive_uri_for_logs.dart';
import 'package:openvine/utils/validators.dart';
import 'package:unified_logger/unified_logger.dart';

Expand Down Expand Up @@ -272,7 +273,7 @@ class DivineAuthCubit extends Cubit<DivineAuthState> {

if (result.verificationRequired && result.deviceCode != null) {
Log.info(
'Email verification required for $email',
'Email verification required for ${redactEmailForLogs(email)}',
name: 'DivineAuthCubit',
category: LogCategory.auth,
);
Expand Down Expand Up @@ -385,7 +386,7 @@ class DivineAuthCubit extends Cubit<DivineAuthState> {
/// Send password reset email
Future<void> sendPasswordResetEmail(String email) async {
Log.info(
'Sending password reset email to $email',
'Sending password reset email to ${redactEmailForLogs(email)}',
name: 'DivineAuthCubit',
category: LogCategory.auth,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:invite_api_client/invite_api_client.dart';
import 'package:keycast_flutter/keycast_flutter.dart';
import 'package:openvine/services/auth_service.dart';
import 'package:openvine/utils/invite_error_utils.dart';
import 'package:openvine/utils/sensitive_uri_for_logs.dart';
import 'package:unified_logger/unified_logger.dart';

part 'email_verification_state.dart';
Expand Down Expand Up @@ -72,7 +73,7 @@ class EmailVerificationCubit extends Cubit<EmailVerificationState> {
String? inviteCode,
}) {
Log.info(
'startPolling called for $email '
'startPolling called for ${redactEmailForLogs(email)} '
'(cubit=$hashCode, authSvc=${_authService.hashCode}, '
'hasExistingTimer=${_pollTimer != null})',
name: 'EmailVerificationCubit',
Expand Down
33 changes: 25 additions & 8 deletions mobile/lib/observability/reportable_error.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// ABOUTME: Marker interface + wrapper + PII sanitizer used to gate Bloc errors
// ABOUTME: forwarded to Crashlytics. See .claude/rules/error_handling.md.

import 'package:openvine/utils/sensitive_uri_for_logs.dart';

/// Marker interface signalling that an error is worth reporting to Crashlytics.
///
/// Errors that flow through `addError(error, st)` only reach the crash reporter
Expand Down Expand Up @@ -56,17 +58,32 @@ final class Reportable<T extends Object> implements ReportableError {

final RegExp _npubPattern = RegExp('npub1[a-z0-9]+');
final RegExp _nsecPattern = RegExp('nsec1[a-z0-9]+');
// Conservative email matcher: local-part of common chars then `@`, host with
// at least one `.`. Intentionally narrower than RFC 5322 — false negatives on
// exotic addresses are preferable to false positives on non-email strings.
final RegExp _emailPattern = RegExp(
r'[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}',
);

/// Strips Nostr `npub1…` and `nsec1…` identifiers from [input] before it is
/// forwarded to a third-party crash reporter.
/// Strips Nostr `npub1…` / `nsec1…` identifiers and email addresses from
/// [input] before it is forwarded to a third-party crash reporter.
///
/// Scope is intentionally narrow: only `npub` (public-key bech32) and `nsec`
/// (private-key bech32). Other Nostr-format references (`note1`, `nevent1`,
/// `nprofile1`) encode event/profile pointers, not secrets, and removing them
/// removes triage value. Call sites that need to redact those should do so
/// explicitly before constructing the error message.
/// Scope is intentionally narrow:
/// - Nostr: only `npub` (public-key bech32) and `nsec` (private-key bech32).
/// Other Nostr-format references (`note1`, `nevent1`, `nprofile1`) encode
/// event/profile pointers, not secrets, and removing them removes triage
/// value. Call sites that need to redact those should do so explicitly
/// before constructing the error message.
/// - Email: any RFC-5321-ish local@host.tld pattern. Replacement delegates to
/// [redactEmailForLogs] so log redaction and crash-report redaction stay in
/// lockstep.
String sanitizeForCrashReport(String input) {
return input
.replaceAll(_npubPattern, 'npub1<redacted>')
.replaceAll(_nsecPattern, 'nsec1<redacted>');
.replaceAll(_nsecPattern, 'nsec1<redacted>')
.replaceAllMapped(
_emailPattern,
// Reuse the log helper here so both redaction surfaces stay aligned.
(match) => redactEmailForLogs(match.group(0)!),
Comment thread
realmeylisdev marked this conversation as resolved.
);
}
4 changes: 3 additions & 1 deletion mobile/lib/screens/auth/email_verification_screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import 'package:openvine/providers/route_feed_providers.dart';
import 'package:openvine/screens/auth/welcome_screen.dart';
import 'package:openvine/screens/explore_screen.dart';
import 'package:openvine/services/auth_service.dart';
import 'package:openvine/utils/sensitive_uri_for_logs.dart';
import 'package:unified_logger/unified_logger.dart';
import 'package:url_launcher/url_launcher.dart';

Expand Down Expand Up @@ -150,7 +151,8 @@ class _EmailVerificationScreenState

if (pending != null) {
Log.info(
'Found persisted verification data for ${pending.email}, '
'Found persisted verification data for '
'${redactEmailForLogs(pending.email)}, '
'attempting auto-login flow',
name: 'EmailVerificationScreen',
category: LogCategory.auth,
Expand Down
8 changes: 5 additions & 3 deletions mobile/lib/services/pending_verification_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// ABOUTME: Enables auto-login when app is cold-started via email verification deep link

import 'package:flutter_secure_storage/flutter_secure_storage.dart';
import 'package:openvine/utils/sensitive_uri_for_logs.dart';
import 'package:unified_logger/unified_logger.dart';

/// Data class representing pending email verification credentials
Expand Down Expand Up @@ -65,7 +66,7 @@ class PendingVerificationService {
_storage.write(key: _keyInviteCode, value: inviteCode),
]);
Log.info(
'Saved pending verification for $email',
'Saved pending verification for ${redactEmailForLogs(email)}',
name: 'PendingVerificationService',
category: LogCategory.auth,
);
Expand Down Expand Up @@ -121,7 +122,8 @@ class PendingVerificationService {
// Check expiration
if (pending.isExpired) {
Log.info(
'Pending verification for $email has expired, clearing',
'Pending verification for ${redactEmailForLogs(email)} has expired, '
'clearing',
name: 'PendingVerificationService',
category: LogCategory.auth,
);
Expand All @@ -130,7 +132,7 @@ class PendingVerificationService {
}

Log.info(
'Loaded pending verification for $email',
'Loaded pending verification for ${redactEmailForLogs(email)}',
name: 'PendingVerificationService',
category: LogCategory.auth,
);
Expand Down
31 changes: 29 additions & 2 deletions mobile/lib/utils/sensitive_uri_for_logs.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// ABOUTME: Redacts secrets from URIs before writing them to diagnostic logs.
// ABOUTME: Preserves schemes, hosts, paths, and Nostr path segments unchanged except /invite/code.
// ABOUTME: Redacts secrets and PII from URIs and free-form text before
// ABOUTME: writing them to diagnostic logs (URIs, email addresses, Nostr keys
// ABOUTME: are handled here; Nostr key stripping for crash reports lives in
// ABOUTME: lib/observability/reportable_error.dart).

/// Placeholder for secrets in free-form log text or HTTP headers.
const redactedSensitiveLogPlaceholder = '[REDACTED]';
Expand Down Expand Up @@ -58,3 +60,28 @@ String redactUriStringForLogs(String uriString) {
}
return out;
}

/// Returns an email address safe for diagnostic logs.
///
/// - `user@example.com` → `u***@example.com`
/// - `a@b.co` → `a***@b.co` (single-char local-part still gets
/// a fixed-width mask so the original
/// length is not leaked)
/// - empty input or input missing `@` or with no `.` in the domain →
/// [redactedSensitiveLogPlaceholder]
///
/// The domain is preserved verbatim so ops can correlate failure patterns
/// across the same provider (e.g. "all gmail.com users are timing out")
/// without identifying individual accounts. Local-part collapses to a
/// fixed 4-character mask (`x***`) regardless of length.
String redactEmailForLogs(String email) {
if (email.isEmpty) return redactedSensitiveLogPlaceholder;
final atIndex = email.indexOf('@');
// No `@`, or `@` at position 0 (empty local-part) → not a usable email.
if (atIndex <= 0) return redactedSensitiveLogPlaceholder;
final domain = email.substring(atIndex + 1);
// Domain must contain at least one `.` to be a routable host.
if (!domain.contains('.')) return redactedSensitiveLogPlaceholder;
final firstChar = email.substring(0, 1);
return '$firstChar***@$domain';
}
54 changes: 54 additions & 0 deletions mobile/test/observability/reportable_error_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import 'package:flutter_test/flutter_test.dart';
import 'package:openvine/observability/reportable_error.dart';
import 'package:openvine/utils/sensitive_uri_for_logs.dart';

void main() {
group(Reportable, () {
Expand Down Expand Up @@ -111,5 +112,58 @@ void main() {

expect(sanitizeForCrashReport(input), equals(input));
});

// Issue #4254 — emails are PII and must not flow to Crashlytics.
test('replaces an email with the first-char + domain mask', () {
const input = 'verification failed for user@example.com after 3 retries';

expect(
sanitizeForCrashReport(input),
'verification failed for u***@example.com after 3 retries',
);
});

test('replaces multiple emails in the same string', () {
const input = 'forwarded alice@a.com to bob@b.io';
final out = sanitizeForCrashReport(input);

expect(out, contains('a***@a.com'));
expect(out, contains('b***@b.io'));
expect(out, isNot(contains('alice')));
expect(out, isNot(contains('bob')));
});

test('strips email PII alongside npub / nsec in one pass', () {
const input =
'user@example.com leaked nsec1qwertyuiopasdfghjklzxcvbnm0123456789ab '
'tied to npub1abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmnop';
final out = sanitizeForCrashReport(input);

expect(out, contains('u***@example.com'));
expect(out, contains('nsec1<redacted>'));
expect(out, contains('npub1<redacted>'));
expect(out, isNot(contains('user@example.com')));
});

test('preserves the domain so ops can correlate failure patterns', () {
// The whole point of partial (not opaque) redaction: ops can spot
// "all gmail.com users are failing" without identifying anyone.
const input = 'auth failed for alice@gmail.com';

expect(
sanitizeForCrashReport(input),
contains('@gmail.com'),
);
});

test('uses the shared email redactor so masking stays aligned', () {
const email = 'first.last+tag@example.com';

expect(
// This locks the crash-report fallback to the same mask shape as logs.
sanitizeForCrashReport('auth failed for $email'),
Comment thread
realmeylisdev marked this conversation as resolved.
'auth failed for ${redactEmailForLogs(email)}',
);
});
});
}
49 changes: 49 additions & 0 deletions mobile/test/screens/auth/email_verification_screen_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import 'package:openvine/providers/route_feed_providers.dart';
import 'package:openvine/screens/auth/email_verification_screen.dart';
import 'package:openvine/services/auth_service.dart';
import 'package:openvine/services/pending_verification_service.dart';
import 'package:unified_logger/unified_logger.dart';

import '../../helpers/test_provider_overrides.dart';

Expand Down Expand Up @@ -390,6 +391,54 @@ void main() {
verify(() => mockCubit.stopPolling()).called(greaterThan(0));
});

testWidgets('redacts persisted pending email in auto-login logs', (
tester,
) async {
await LogCaptureService().clearAllLogs();
when(() => mockPendingVerification.load()).thenAnswer(
(_) async => PendingVerification(
deviceCode: 'persisted-device-code',
verifier: 'persisted-verifier',
email: 'user@example.com',
createdAt: DateTime(2026),
),
);
when(
() => mockOAuth.verifyEmail(token: any(named: 'token')),
).thenAnswer((_) async => VerifyEmailResult(success: true));
when(
() => mockCubit.startPolling(
deviceCode: any(named: 'deviceCode'),
verifier: any(named: 'verifier'),
email: any(named: 'email'),
inviteCode: any(named: 'inviteCode'),
),
).thenReturn(null);

await tester.pumpWidget(createTestWidget(token: 'persisted-token'));
await tester.pump();
await tester.pump(const Duration(milliseconds: 10));

final logMessage = LogCaptureService()
.getRecentLogs()
.map((entry) => entry.message)
.lastWhere(
(message) =>
message.startsWith('Found persisted verification data for '),
);

expect(logMessage, contains('u***@example.com'));
expect(logMessage, isNot(contains('user@example.com')));
verify(() => mockOAuth.verifyEmail(token: 'persisted-token')).called(1);
verify(
() => mockCubit.startPolling(
deviceCode: 'persisted-device-code',
verifier: 'persisted-verifier',
email: 'user@example.com',
),
).called(1);
});

testWidgets(
're-verifies when token changes while already in token mode',
(tester) async {
Expand Down
65 changes: 65 additions & 0 deletions mobile/test/unit/utils/sensitive_uri_for_logs_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,69 @@ void main() {
expect(out, contains('code=$redactedUriComponentForLogs'));
});
});

/// Issue #4254 — redact email PII from auth-flow logs.
group('redactEmailForLogs', () {
test('partial-redacts a standard email, preserves domain', () {
expect(
redactEmailForLogs('user@example.com'),
equals('u***@example.com'),
);
});

test('partial-redacts a single-character local-part', () {
// Even one-char local-parts get the fixed `x***` mask — the original
// length must not be leaked.
expect(redactEmailForLogs('a@b.co'), equals('a***@b.co'));
});

test('preserves subdomains in the domain part', () {
expect(
redactEmailForLogs('alice@mail.corp.example.com'),
equals('a***@mail.corp.example.com'),
);
});

test('hides the full local-part even when it is long', () {
final out = redactEmailForLogs('first.last+tag@example.com');
expect(out, equals('f***@example.com'));
expect(out, isNot(contains('first')));
expect(out, isNot(contains('last')));
expect(out, isNot(contains('tag')));
});

test('returns the opaque placeholder for empty input', () {
expect(redactEmailForLogs(''), equals(redactedSensitiveLogPlaceholder));
});

test('returns the opaque placeholder for input without `@`', () {
expect(
redactEmailForLogs('not-an-email'),
equals(redactedSensitiveLogPlaceholder),
);
});

test('returns the opaque placeholder for empty local-part', () {
expect(
redactEmailForLogs('@example.com'),
equals(redactedSensitiveLogPlaceholder),
);
});

test('returns the opaque placeholder for a domain without a dot', () {
// `a@b` (no TLD) is not a routable host — fail closed.
expect(
redactEmailForLogs('a@b'),
equals(redactedSensitiveLogPlaceholder),
);
});

test('returns the opaque placeholder for whitespace-only input', () {
// No `@` → treated like any other malformed input.
expect(
redactEmailForLogs(' '),
equals(redactedSensitiveLogPlaceholder),
);
});
});
}
Loading