Wire inbox api#29
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR implements end-to-end encrypted message retrieval and client-side decryption. The server adds a database query to return user-received messages; the client obtains and stores a private key during login, then decrypts each message before rendering in the inbox view with graceful fallback to plaintext placeholders. ChangesEncrypted message flow: server retrieval, client key management, and inbox decryption
Sequence Diagram(s)sequenceDiagram
participant LoginForm
participant login as api.login()
participant crypto as crypto.subtle
participant storage as api._sessionPrivateKey
LoginForm->>login: username, password
login->>login: parse wrapped_private_key from response
login->>crypto: decrypt using password
alt decryption succeeds
crypto-->>login: decrypted key material
login->>crypto: importKey(X25519)
crypto-->>login: CryptoKey
login->>storage: setPrivateKey(CryptoKey)
else decryption fails
login->>login: log error, continue
end
login-->>LoginForm: login success
sequenceDiagram
participant renderInbox
participant privateKey as api.getPrivateKey()
participant crypto as crypto.subtle
participant decryptMsg as decryptMessage()
participant messageList as rendered messages
renderInbox->>renderInbox: map messages to decrypt tasks
loop for each message
renderInbox->>privateKey: obtain stored X25519 key
alt required encryption fields present
renderInbox->>crypto: importKey(message.ephemeral_pk)
crypto-->>renderInbox: sender public key
renderInbox->>decryptMsg: decrypt ciphertext/nonce/pubkey
alt decryption succeeds
decryptMsg-->>renderInbox: plaintext content
else decryption fails
renderInbox->>renderInbox: use '(encrypted)' placeholder
end
else fields missing
renderInbox->>renderInbox: use '(encrypted)' placeholder
end
end
renderInbox->>messageList: render message cards from decrypted objects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client-web/js/api.js (1)
63-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear stale in-memory private key on every login attempt.
login()currently keeps the previous_sessionPrivateKeywhen the new response has no key (or unwrap fails), which can leak key state across account switches in the same tab.Suggested fix
export async function login(username, password) { + clearPrivateKey(); const data = await request('POST', '/auth/login', { body: { username, password } }); @@ if (data.wrapped_private_key) { try { @@ setPrivateKey(privKey); } catch { + clearPrivateKey(); // Placeholder key material (test users) or corrupted key — login still succeeds, // inbox will see getPrivateKey() === null and skip decryption. } }🤖 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 `@client-web/js/api.js` around lines 63 - 84, The login flow currently can retain a previous in-memory private key when the new response lacks wrapped_private_key or unwrap fails; to fix, explicitly clear the session private key at the start of login (or at least before attempting unwrap) by calling setPrivateKey(null), and also ensure you call setPrivateKey(null) in the branch where data.wrapped_private_key is missing and inside the catch block around decryptPrivateKey/EncryptedPrivateKey.fromJSON so any previous _sessionPrivateKey is removed on failed or keyless logins.
🧹 Nitpick comments (1)
whatsas.service (1)
1-19: Remember to reload systemd and restart the service.After modifying the unit file, run the following commands to apply the changes:
sudo systemctl daemon-reload sudo systemctl restart whatsas.service sudo systemctl status whatsas.service🤖 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 `@whatsas.service` around lines 1 - 19, You updated the systemd unit for the WhatSaS Flask API (whatsas.service) but didn’t apply the change; reload systemd and restart the service so the new ExecStart, EnvironmentFile, and other settings take effect by running the systemd daemon-reload and then restart whatsas.service, and verify it’s running with a status check (i.e., run the equivalent of sudo systemctl daemon-reload, sudo systemctl restart whatsas.service, and sudo systemctl status whatsas.service).
🤖 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 `@app.py`:
- Around line 1-2: The file app.py conflicts with the existing server/app
package and can shadow imports; rename this module (e.g., to dev_app.py) and
update any imports/usages accordingly: move the Flask app definition (the
variable named app and the Flask(__name__) instantiation) into the new module
name and change import sites (for example any "from app import ..." such as the
"from app import create_app" usage in server/run.py) to import from dev_app (or
the chosen new filename) so module resolution is deterministic and does not
collide with the server/app package.
In `@client-web/js/views.js`:
- Around line 32-37: hexToBytes silently accepts non-hex characters causing NaN
-> 0 coercion and misleading failures; validate the input before conversion by
checking the hex string matches /^[0-9a-fA-F]+$/ and has even length (or
validate each pair with /^[0-9a-fA-F]{2}$/) and throw a descriptive Error if
validation fails, and also guard the parseInt result inside hexToBytes (e.g., if
parseInt(...) is NaN) to throw if any byte cannot be parsed; update the function
hexToBytes to perform these checks before creating/assigning into the
Uint8Array.
In `@server/app/messages/routes.py`:
- Around line 31-34: The response for GET /messages is returning the DB column
ephemeral_pk but the inbox decrypt flow expects ephemeral_public_key; update the
messages route that builds the response (the SQL SELECT or response mapping in
routes.py) so that ephemeral_pk is emitted as ephemeral_public_key (e.g., alias
ephemeral_pk AS ephemeral_public_key or rename the dict key produced by the
function that handles the SELECT results) ensuring the field name matches what
the inbox decrypt flow (ephemeral_public_key) consumes.
---
Outside diff comments:
In `@client-web/js/api.js`:
- Around line 63-84: The login flow currently can retain a previous in-memory
private key when the new response lacks wrapped_private_key or unwrap fails; to
fix, explicitly clear the session private key at the start of login (or at least
before attempting unwrap) by calling setPrivateKey(null), and also ensure you
call setPrivateKey(null) in the branch where data.wrapped_private_key is missing
and inside the catch block around decryptPrivateKey/EncryptedPrivateKey.fromJSON
so any previous _sessionPrivateKey is removed on failed or keyless logins.
---
Nitpick comments:
In `@whatsas.service`:
- Around line 1-19: You updated the systemd unit for the WhatSaS Flask API
(whatsas.service) but didn’t apply the change; reload systemd and restart the
service so the new ExecStart, EnvironmentFile, and other settings take effect by
running the systemd daemon-reload and then restart whatsas.service, and verify
it’s running with a status check (i.e., run the equivalent of sudo systemctl
daemon-reload, sudo systemctl restart whatsas.service, and sudo systemctl status
whatsas.service).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 566af6bf-ec0f-4dc5-91d6-bededf7d3d15
📒 Files selected for processing (6)
app.pyclient-web/js/api.jsclient-web/js/views.jsdocs/transcript_sarah.mdserver/app/messages/routes.pywhatsas.service
Aoibheannmangan
approved these changes
May 25, 2026
This was referenced May 25, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Chores