Debugging AEAD errors

[xmtptools]

This PR updates the iOS bindings to libxmtp version 4.7.0-dev.5d3f820.

Changes:

• Updated XMTP.podspec version to 4.7.0-dev.5d3f820
• Updated static binary (LibXMTPSwiftFFI) URL and checksum in Package.swift
• Updated dynamic binary (LibXMTPSwiftFFIDynamic) URL and checksum in Package.swift
• Updated Swift source files (xmtpv3.swift) for both static and dynamic bindings

Base branch: cv/xmtp-ios-app-data-tag-2025-11-14

[claude[bot]]

Claude finished @xmtptools's task —— View job

---

Code Review Complete ✓

I've completed a thorough review of this PR. Here's my feedback:

---

Summary

This PR updates the iOS bindings to libxmtp version 4.7.0-dev.5d3f820 and adds a processType parameter to help distinguish between main app and notification extension processes when debugging AEAD errors. The changes are generally well-implemented but there are several important issues to address.

---

Critical Issues 🔴

1. Unsafe UInt32 conversion without bounds checking (Client.swift:1144)

Location: Sources/XMTPiOS/Client.swift:1144

The conversion UInt32(maxFiles) can trap if maxFiles is negative or exceeds UInt32.max. This is a runtime crash risk.

Recommendation:

guard maxFiles >= 0 && maxFiles <= Int(UInt32.max) else {
    os_log("Invalid maxFiles (%{public}d); expected 0...%{public}d", 
           log: OSLog.default, type: .error, maxFiles, Int(UInt32.max))
    return
}
let maxFiles32 = UInt32(maxFiles)
try enterDebugWriter(
    directory: logDirectory.path,
    logLevel: logLevel.ffiLogLevel,
    rotation: rotationSchedule,
    maxFiles: maxFiles32,  // Use validated value
    processType: processType
)

2. Integer overflow risk in handle counter (Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift:364)

Location: This appears in the dynamic bindings file

The currentHandle += 1 operation can overflow and trap. Macroscope correctly identified this issue.

Recommendation: Use wrapping arithmetic operator &+= or implement overflow detection:

currentHandle &+= 1  // Wrapping increment

---

Code Quality Issues ⚠️

3. Inconsistent error handling in logging setup

Location: Sources/XMTPiOS/Client.swift:1090-1137

The activatePersistentLibXMTPLogWriter method silently returns on various error conditions without notifying the caller. While logging is non-critical, developers should be aware if logging activation fails.

Recommendation: Consider returning a Bool success indicator or throwing an error that callers can catch:

@discardableResult
static func activatePersistentLibXMTPLogWriter(
    logLevel: LogLevel,
    rotationSchedule: FfiLogRotation,
    maxFiles: Int,
    customLogDirectory: URL? = nil,
    processType: FfiProcessType = .main
) -> Bool {
    // ... existing code ...
    // Return false on failures, true on success
}

4. Signal handler installation is unsafe

Location: Sources/XMTPiOS/Client.swift:1122-1137

Installing a signal handler for SIGABRT within the logging setup function is problematic:

• Signal handlers should be installed once at app startup, not each time logging is activated
• The current implementation could overwrite app-level signal handlers
• Signal handler code must be async-signal-safe; calling exitDebugWriter() and os_log is not safe

Recommendation: Remove the signal handler entirely or document that this is a temporary debugging measure. If kept, move it to app initialization and make it optional:

// Consider removing this entirely or moving to a dedicated setup function
// that's called once at app startup

5. Missing version mismatch between Package.swift and XMTP.podspec

Location:

• Package.swift:28 references swift-bindings-1.7.0-dev.ad8a1dc
• XMTP.podspec:3 references version 4.7.0-dev.5d3f820

The PR title mentions updating to 1.7.0-dev.5d3f820 but Package.swift shows a different commit hash (ad8a1dc). This inconsistency could indicate:

• The Package.swift wasn't updated to the intended version
• There's a version mismatch between static/dynamic bindings

Recommendation: Verify that all version references are correct and consistent.

---

Test Coverage 📊

6. Limited test coverage for new processType parameter

Location: Tests/XMTPTests/ClientTests.swift:825-831

The test at line 830 uses processType: FfiProcessType.main, but there's no test for:

• Using .notificationExtension process type
• Verifying that different process types produce distinguishable logs
• Testing the interaction between multiple processes using different types

Recommendation: Add test coverage:

func testPersistentLoggingWithNotificationExtension() async throws {
    // Test that notification extension process type works
    Client.activatePersistentLibXMTPLogWriter(
        logLevel: .debug,
        rotationSchedule: .hourly,
        maxFiles: 3,
        customLogDirectory: logDirectory,
        processType: .notificationExtension
    )
    // Verify logs contain process type identifier
}

---

Performance Considerations ⚡

7. Large binary file addition

The addition of Sources/XMTPiOSDynamic/Libxmtp/xmtpv3.swift (+14,990 lines) significantly increases the repository size. While this is auto-generated code from uniffi, verify:

• Is dynamic linking truly needed, or can this be binary-only?
• Does this duplicate functionality with the static version?

8. Test file system operations without cleanup

Location: Tests/XMTPTests/ClientTests.swift:809-811

The test creates a temporary directory but uses defer to deactivate logging. If the test fails before the defer block, temp files may not be cleaned up properly.

Recommendation:

defer {
    Client.deactivatePersistentLibXMTPLogWriter()
    try? fileManager.removeItem(at: logDirectory)  // Clean up temp directory
}

---

Security Considerations 🔒

9. Log directory permissions not verified

Location: Sources/XMTPiOS/Client.swift:1098-1108

The code verifies write permissions by creating a test file, which is good. However, it doesn't verify that the directory isn't world-readable, which could leak sensitive debug information.

Recommendation: Add permission checks on iOS where applicable, or document that developers should use appropriate sandboxed directories.

10. Potential information disclosure in logs

The new debug logging with process type differentiation will capture more detailed information. Ensure:

• No sensitive data (keys, messages, PII) is logged
• Documentation warns developers about log file handling
• Consider log encryption for sensitive debugging scenarios

---

Best Practices & Recommendations 💡

11. Default parameter value is sensible

The default processType: FfiProcessType = .main in activatePersistentLibXMTPLogWriter (line 1074) is well-chosen, as most callers will be in the main app process.

12. Good use of Swift features

• Proper use of async/await throughout
• Good error handling with do-catch blocks
• Appropriate use of os_log for system logging

13. Code organization

The separation of static vs dynamic bindings is clean, though the duplication is concerning from a maintenance perspective.

---

Summary of Recommendations

Must Fix:

1. ✅ Add bounds checking for maxFiles UInt32 conversion (Crash risk)
2. ✅ Fix integer overflow in handle counter (Crash risk)
3. ✅ Verify version consistency between Package.swift and podspec

Should Fix: 4. Remove or relocate unsafe signal handler 5. Improve error communication from logging setup 6. Add test coverage for notification extension process type

Consider: 7. Add cleanup to test defer blocks 8. Document security considerations for log files 9. Review necessity of 15K line dynamic bindings file

---

Positive Aspects ✨

• Clean API design for the processType parameter
• Well-documented enum cases for FfiProcessType
• Comprehensive test coverage overall (800+ lines of tests)
• Good defensive programming in log directory setup (permission checks)
• Proper use of Swift actors for thread-safe API caching

---

[macroscopeapp[bot]]

Add process type to Client.configureLogging and update UniFFI bindings and package to debug AEAD errors

Introduce FfiProcessType and thread it into enterDebugWriter and enterDebugWriterWithLevel; update generated UniFFI bindings and checksums; bump the binary target to swift-bindings-1.7.0-dev.ad8a1dc and set CocoaPods version to 4.7.0-dev.5d3f820.

📍Where to Start

Start with Client.configureLogging in Client.swift, then review the FFI changes in xmtpv3.swift.

---

Macroscope summarized ef372cf.