fix: 🐛 panic on unknown keys#169
Conversation
Added extra logs
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/!(ltsm)/**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds panic-safe wrappers around LTSM/WASM E2EE operations and standardizes structured zerolog context for group decryption error logs; replaces ad-hoc logs with a single helper while preserving existing control flow. ChangesE2EE Group Decryption Safety and Observability
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/connector/handle_message.go (2)
46-75: ⚡ Quick winConsider adding a godoc comment.
While the function is well-structured and follows zerolog best practices, a brief godoc comment would improve maintainability by documenting its purpose and expected usage pattern.
📝 Suggested godoc addition
+// groupDecryptLogContext enriches a zerolog event with structured fields for E2EE group decryption logging. +// It includes message metadata, chunk information, and decoded sender/group key IDs when available. +// Decoding errors are captured as *_error fields rather than omitted. func groupDecryptLogContext(evt *zerolog.Event, msg *line.Message, chatMID string, opType int) *zerolog.Event {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/connector/handle_message.go` around lines 46 - 75, Add a short godoc comment above the groupDecryptLogContext function that explains its purpose (building a zerolog event with metadata for group E2EE message decryption logging), the expected inputs (evt *zerolog.Event, msg *line.Message, chatMID string, opType int) and key behaviors (which fields are added, when e2ee_version is included, and that sender/group key IDs are decoded from the last two chunks if present). Keep it one or two sentences and use the standard Go godoc style starting with the function name.
134-135: 💤 Low valueConsider consolidating redundant key ID fields.
Each logging call adds
Int("key_id", ...)explicitly, and thengroupDecryptLogContextalso decodes and addsInt("group_key_id", ...)from the message chunks. In normal operation these values should be identical, creating field redundancy in the logs.While not harmful (and potentially useful for detecting mismatches), you could simplify by either:
- Removing the explicit
Int("key_id", ...)and relying on the helper'sgroup_key_idfield, or- Passing the key ID to the helper and having it use a consistent field name
♻️ Example: Remove explicit key_id field
-groupDecryptLogContext(lc.UserLogin.Bridge.Log.Debug().Err(errFetch).Int("key_id", gkID), msg, portalIDStr, opType). +groupDecryptLogContext(lc.UserLogin.Bridge.Log.Debug().Err(errFetch), msg, portalIDStr, opType). Msg("Prefetch group key before decrypt failed")Apply similarly to lines 144, 148, and 153. The
group_key_idfield from the helper provides the same information.Also applies to: 144-145, 148-149, 153-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/connector/handle_message.go` around lines 134 - 135, The logs currently duplicate the key id by calling Int("key_id", gkID) before invoking groupDecryptLogContext which itself adds Int("group_key_id", ...) from the message chunks; remove the explicit Int("key_id", gkID) occurrences (the Debug().Err(errFetch).Int("key_id", gkID) calls) and rely on groupDecryptLogContext's group_key_id field for consistency (apply the same change to the other similar sites noted), or alternatively refactor groupDecryptLogContext to accept gkID and emit a single canonical field name (e.g., group_key_id) so only one key-id field appears in logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/runner.go`:
- Around line 148-156: The LoginUnwrapKeyChain flow still calls
r.rt.E2EEKeyExportKey(keyPtr) directly and can panic; update LoginUnwrapKeyChain
to call the existing exportE2EEKeyPanicSafe(keyPtr) helper (the same one used by
ChannelUnwrapGroupSharedKey) instead of calling r.rt.E2EEKeyExportKey directly,
propagate its (exported, err) return values and handle the error as
LoginUnwrapKeyChain currently expects so any panic from E2EEKeyExportKey is
converted to an error rather than crashing.
---
Nitpick comments:
In `@pkg/connector/handle_message.go`:
- Around line 46-75: Add a short godoc comment above the groupDecryptLogContext
function that explains its purpose (building a zerolog event with metadata for
group E2EE message decryption logging), the expected inputs (evt *zerolog.Event,
msg *line.Message, chatMID string, opType int) and key behaviors (which fields
are added, when e2ee_version is included, and that sender/group key IDs are
decoded from the last two chunks if present). Keep it one or two sentences and
use the standard Go godoc style starting with the function name.
- Around line 134-135: The logs currently duplicate the key id by calling
Int("key_id", gkID) before invoking groupDecryptLogContext which itself adds
Int("group_key_id", ...) from the message chunks; remove the explicit
Int("key_id", gkID) occurrences (the Debug().Err(errFetch).Int("key_id", gkID)
calls) and rely on groupDecryptLogContext's group_key_id field for consistency
(apply the same change to the other similar sites noted), or alternatively
refactor groupDecryptLogContext to accept gkID and emit a single canonical field
name (e.g., group_key_id) so only one key-id field appears in logs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 490424f1-eed9-46e9-a05b-9a67a5ca18d6
📒 Files selected for processing (2)
pkg/connector/handle_message.gopkg/runner.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-docker
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Usego fmtfor code formatting across all Go files
Usegoimportswith-local "github.com/highesttt/matrix-line-messenger"flag to group project-local imports correctly
Usezerologfor logging throughout the codebase
Do not useMsgfin logging; useMsgwith structured fields instead
UseStringerinterface where applicable in Go code
Files:
pkg/runner.gopkg/connector/handle_message.go
**/!(ltsm)/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/!(ltsm)/**/*.go: Runstaticcheckon all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Rungo veton all Go files excludingpkg/ltsmpackage (transpiled WASM code)
Files:
pkg/runner.gopkg/connector/handle_message.go
🧠 Learnings (1)
📚 Learning: 2026-05-29T10:13:37.093Z
Learnt from: CR
Repo: beeper/matrix-line-messenger PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-29T10:13:37.093Z
Learning: Applies to pkg/ltsm/wbc_generated.go : Do not manually edit `pkg/ltsm/wbc_generated.go` as it is transpiled WASM code
Applied to files:
pkg/runner.go
🔇 Additional comments (3)
pkg/connector/handle_message.go (2)
38-44: LGTM!
153-154: Excellent addition: logging decrypt failure after key refresh.This new log statement provides valuable visibility when decryption fails even after successfully fetching and unwrapping a fresh group key, which helps diagnose persistent E2EE issues.
pkg/runner.go (1)
141-146: LGTM!Also applies to: 158-186, 498-507, 596-599, 630-634
…failed-to-decrypt-the-data-data-authentication-failure
highesttt
deleted the
168-bug-failed-to-decrypt-the-data-data-authentication-failure
branch
1 participant
This pr adds extra logs when handling messages and fixes a panic on receiving unknown keys.
Confirmed to work by 2 users