feat(webview_apis): WebSocket bridge for live-webview APIs, Gmail first#869
Merged
senamakel merged 7 commits intotinyhumansai:mainfrom Apr 24, 2026
Merged
feat(webview_apis): WebSocket bridge for live-webview APIs, Gmail first#869senamakel merged 7 commits intotinyhumansai:mainfrom
senamakel merged 7 commits intotinyhumansai:mainfrom
Conversation
…build steps - Removed separate frontend build commands from the Windows and release workflows, as the vite build is now handled during the Tauri app build step. - Increased Node.js memory limit for the Tauri build process to prevent out-of-memory errors on macOS ARM64 runners. This change streamlines the build process and reduces memory pressure during builds.
- Added a WebSocket server and client for the webview_apis bridge, enabling communication between the Tauri shell and core sidecar. - Implemented JSON-RPC methods for Gmail operations, allowing for real-time interaction with the live webview. - Introduced new modules for handling requests and responses, including a router for method dispatching. - Updated existing Gmail command wrappers to utilize the new bridge for enhanced functionality. This change enhances the integration capabilities of the application, allowing for more dynamic interactions with Gmail through the webview.
The initial gmail/session.rs matcher required both `https://mail.google.com/` URL prefix AND the `#openhuman-account-<id>` fragment. In practice Gmail rewrites the URL to `#inbox` / `#search/...` as soon as the user routes, so the fragment check dropped the tab and `list_labels` failed with `no matching page target` every time the inbox was actually loaded. Relax to fragment OR placeholder-title marker OR any mail.google.com URL — mirrors the per-account cdp::session opener. Re-export placeholder_marker from cdp::mod so gmail::session can reach it. Verified end-to-end: curl → core JSON-RPC → WS bridge → Tauri CDP attach → DOMSnapshot → list_labels returns `ok` against a live Gmail session.
Contributor
📝 WalkthroughWalkthroughAdds a loopback WebSocket JSON‑RPC bridge between core and the Tauri webview plus a CDP-driven Gmail connector (reads/writes/parsers/sessions). Introduces server/client/router, webview API schemas, new Gmail parsing/fetch modules, Tauri commands, tests, and a mock bridge script. Changes
Sequence DiagramsequenceDiagram
participant Webview as Tauri Webview
participant Server as WebviewAPIs Server
participant Router as Router
participant Core as Core Client
participant Gmail as Gmail Module
participant CDP as CDP Session
Webview->>Server: WebSocket send {kind, id, method, params}
Server->>Router: parse envelope -> dispatch(method, params)
Router->>Gmail: invoke cdp_* helper (e.g., list_labels)
Gmail->>CDP: attach(account_id)
Gmail->>CDP: fetch DOM / Network.loadNetworkResource -> IO.read* (base64 chunks)
CDP-->>Gmail: response body
Gmail->>CDP: detach(session)
Gmail-->>Router: return result
Router->>Server: wrap as response {kind,response,id,ok,result/error}
Server-->>Core: deliver response
Core-->>Webview: (if proxying) send response envelope back
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src-tauri/src/lib.rs (1)
628-655:⚠️ Potential issue | 🟠 MajorAbort startup when the bridge fails to bind.
If
OPENHUMAN_WEBVIEW_APIS_PORTis already set andwebview_apis::start()fails, this just logs and continues. That leaves the stale port in the process environment, so the core child can inherit it and connect to whichever loopback process already owns that port. Please clear the env var and fail setup before creatingcore_handle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/lib.rs` around lines 628 - 655, If webview_apis::start() returns Err, remove the PORT_ENV from the process environment and abort startup instead of proceeding to create the core handle; specifically, in the Err(err) arm for webview_apis::start().await call, call std::env::remove_var(webview_apis::server::PORT_ENV) (to clear any stale OPENHUMAN_WEBVIEW_APIS_PORT) and then stop setup by returning an Err or otherwise exiting early so core_process::CoreProcessHandle::new / core_process::default_core_port() / core_process::default_core_run_mode(daemon_mode) are never executed.
🧹 Nitpick comments (7)
app/src-tauri/Cargo.toml (1)
52-54: Update comment to reflect dual WebSocket usage.The comment describes
tokio-tungsteniteas only a "WebSocket client for the Chrome DevTools Protocol," but this PR also introduces a WebSocket server for the webview_apis bridge. Consider updating the comment to reflect both uses.📝 Suggested comment update
-# WebSocket client for the Chrome DevTools Protocol (used to talk to the -# embedded CEF instance over `--remote-debugging-port=9222` and read -# IndexedDB / drive `Runtime.evaluate` for the WhatsApp recipe). +# WebSocket support for: +# - Client: Chrome DevTools Protocol connections to the embedded CEF instance +# over `--remote-debugging-port=9222` for IndexedDB reads and `Runtime.evaluate` +# - Server: webview_apis bridge accepting connections from the core sidecar tokio-tungstenite = { version = "0.24", default-features = false, features = ["connect", "handshake"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/Cargo.toml` around lines 52 - 54, Update the existing comment that mentions tokio-tungstenite as only a "WebSocket client for the Chrome DevTools Protocol" to state that tokio-tungstenite is used both as a WebSocket client (for the Chrome DevTools Protocol / remote-debugging port usage and IndexedDB/Runtime.evaluate access) and as a WebSocket server (for the webview_apis bridge), referencing tokio-tungstenite, Chrome DevTools Protocol, and the webview_apis bridge so readers understand the dual role.src/core/all.rs (1)
77-79: Consider adding awebview_apisnamespace description for CLI help completeness.You registered the domain in both registries; adding
namespace_description("webview_apis")keeps help/discovery output consistent.Also applies to: 174-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/all.rs` around lines 77 - 79, Add a namespace_description("webview_apis") entry wherever you register the webview_apis domain so CLI help/discovery shows a description; specifically, next to the call controllers.extend(crate::openhuman::webview_apis::all_webview_apis_registered_controllers()) and the second registration occurrence (around the other registry at line ~174), invoke namespace_description("webview_apis") on the same registry/builder object so both registries include the description.app/src-tauri/src/webview_apis/router.rs (1)
16-17: Add branch/error-path logging around dispatch outcomes.You log entry, but missing logs for success/failure outcomes makes bridge debugging harder.
As per coding guidelines, "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error paths."
Also applies to: 74-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/webview_apis/router.rs` around lines 16 - 17, The dispatch entry logs but lacks outcome logging—update the match in router.rs inside the dispatch handling (the match on method and the surrounding dispatch logic) to log success and error paths: add debug/info logs on successful branches (include method and any result identifier), and error/error-level logs on failure branches and panics (include method, error details, and contextual data); also add exit/summary logging after dispatch completes and mirror this additional logging around the related logic referenced near the 74-80 range so callers can trace branch decisions and failures.src/openhuman/webview_apis/schemas.rs (1)
221-298: Keepschemas.rsregistry-only and move handler logic intorpc.rs.This file now owns param validation and bridge calls for every operation, so the schema module is carrying domain behavior instead of just registration metadata. Please move the
handle_gmail_*implementations into a siblingrpc.rsand haveschemas.rsdelegate there.As per coding guidelines,
src/openhuman/*/schemas.rs: "Rust core domains must include aschemas.rsfile that definesschemas,all_controller_schemas,all_registered_controllers, andhandle_*functions delegating to domainrpc.rs."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/webview_apis/schemas.rs` around lines 221 - 298, Move the actual implementations of the handle_gmail_* functions out of schemas.rs into a new/nearby rpc.rs: cut the async Box::pin blocks from handle_gmail_list_labels, handle_gmail_list_messages, handle_gmail_search, handle_gmail_get_message, handle_gmail_send, handle_gmail_trash, and handle_gmail_add_label and paste them into rpc.rs as pub async functions (or pub fn returning ControllerFuture) keeping the same names and logic (including param validation and client::request calls); in schemas.rs replace each implementation with a thin delegating stub that calls the corresponding function in rpc (e.g. return rpc::handle_gmail_list_labels(params)), ensure rpc.rs is imported (use crate::openhuman::...::rpc) and keep schemas.rs as registry-only alongside maintaining symbols like schemas, all_controller_schemas, and all_registered_controllers unchanged.app/src-tauri/src/gmail/reads.rs (2)
168-188: System label detection is English-only.The
is_system_labelcatalog only matches English names. Gmail localizes aria-labels, so users with non-English locales will see all labels classified as"user"instead of"system".If locale-agnostic detection is needed, consider pattern-matching on DOM structure (e.g., system labels appear in a specific nav section) or noting this as a known limitation in the module docs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/reads.rs` around lines 168 - 188, The is_system_label function currently matches only English label strings (function: is_system_label), which breaks for localized Gmail aria-labels; fix by replacing string-only checks with a locale-agnostic approach—e.g., detect system labels from DOM structure or CSS context where they are rendered (move detection out of content-based matching in is_system_label and instead accept a structural flag or selector-based check), or alternatively add a documented limitation and load a locale mapping table to translate localized names before calling is_system_label; update callers to use the new structural/normalized detection path and adjust function signature/name if needed (e.g., from is_system_label(name: &str) to is_system_label(name: &str, is_in_system_section: bool) or similar).
114-119: Label ID uses display name — document this limitation.The
idfield is set toname.clone(), which differs from Gmail's internal label IDs (e.g.,INBOX,Label_123). This is reasonable given DOM scraping constraints, but downstream callers (likeadd_label) expecting stable IDs for API calls may encounter mismatches.Consider documenting this in the
GmailLabeltype or adding a TODO for retrieving actual IDs via Network interception in follow-up work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/reads.rs` around lines 114 - 119, The GmailLabel struct currently uses the display name for its id (see the out.push(GmailLabel { id: name.clone(), ... }) in reads.rs), which doesn't match Gmail's internal stable IDs; update documentation for the GmailLabel type (or add a clear TODO on the GmailLabel definition) to state that id is the display name and not the Gmail API/internal ID (e.g., INBOX, Label_123), and note that callers like add_label must not assume stable/internal IDs and that retrieving true IDs via network interception is a planned follow-up.app/src-tauri/src/gmail/mod.rs (1)
161-209: Add entry logging to Tauri commands.The Tauri commands are entry points from the frontend but lack logging. While the underlying
cdp_*helpers log viareads.rs, adding entry-point logging here helps distinguish frontend invocations from bridge invocations in traces.📋 Example for gmail_list_labels
#[tauri::command] pub async fn gmail_list_labels(account_id: String) -> Result<Vec<GmailLabel>, String> { + log::debug!("[gmail][tauri] gmail_list_labels account_id={}", account_id); cdp_list_labels(&account_id).await }As per coding guidelines: "Add substantial development-oriented logging at entry/exit points."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/mod.rs` around lines 161 - 209, Add entry (and optional exit) logging to each Tauri command to differentiate frontend invocations from internal bridge calls: inside gmail_list_labels, gmail_list_messages, gmail_search, gmail_get_message, gmail_send, gmail_trash, and gmail_add_label log a concise entry message (e.g., "tauri::gmail_list_labels called") including non-sensitive contextual fields such as account_id, message_id, label, query, limit or a safe summary of request; call the existing logger/tracing facility used elsewhere in the module before delegating to the cdp_* helper and optionally log the result or error on return to aid tracing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/gmail/session.rs`:
- Around line 46-50: The fallback predicate in connect_and_attach_matching is
account-agnostic and may attach to the wrong Gmail tab; update the closure used
for connect_and_attach_matching so the third branch no longer matches all
"https://mail.google.com/" pages blindly — require the URL to also contain the
same account identifier or fragment (e.g., the "u=" query param or the fragment
string) or the marker before accepting it. Concretely, in the predicate closure
that references fragment and marker, parse or derive the account id from
fragment (or check for "u=" in URLs) and change the starts_with branch to
something like url.starts_with("https://mail.google.com/") &&
(url.contains(&fragment) || url.contains(&account_id) || title==marker) so only
the same-account Gmail tab is matched by connect_and_attach_matching.
- Around line 30-35: The debug log in the log::debug! call is emitting raw
account identifiers and markers (account_id and marker); replace those with a
redacted or hashed representation before logging (e.g., compute a short hash or
mask of account_id and marker) and log the sanitized values alongside fragment;
update the log::debug! invocation that currently references account_id,
fragment, marker so it uses the sanitized variables instead of the raw ones.
In `@app/src-tauri/src/gmail/writes.rs`:
- Line 13: The debug logs in gmail/writes.rs currently print the raw account_id
(log::debug!("[gmail][{account_id}] ...")), which must be redacted; update all
occurrences of log::debug! that include account_id (the three lines noted) to
pass a redacted or hashed representation instead (e.g., call a helper like
redact_account_id(account_id) or hash_account_id(account_id) that returns a
non-reversible token or a truncated form), implement that helper if it doesn't
exist, and use it in place of account_id in the debug messages to avoid logging
raw PII.
In `@app/src-tauri/src/webview_apis/router.rs`:
- Around line 19-67: The route handlers currently accept empty or
whitespace-only strings because read_string(¶ms, "account_id") (and other
required string reads like "message_id" and "label") do not validate content;
update the code so required string params are rejected if empty/blank by either:
1) changing read_string to trim and return an Err for empty strings (preferred),
or 2) adding/using a new helper read_nonblank_string that trims and errors on
empty input, and replace calls in the Gmail handlers (all occurrences calling
read_string for required ids in the match arms, e.g. the "gmail.list_labels",
"gmail.list_messages", "gmail.search", "gmail.get_message", "gmail.send",
"gmail.trash", "gmail.add_label" branches) so they use the nonblank validator;
keep read_optional_string behavior unchanged for optional params.
- Around line 92-97: The generic function read_optional_string currently returns
a hardcoded error message referencing 'label'; update its error handling to
include the actual key parameter so callers see the correct field name (e.g.,
change the Err branch to format!("invalid '{}': expected string", key) or
similar). Locate read_optional_string and replace the literal 'label' in the
Err(format!(...)) with the key variable to produce a contextual error message.
In `@app/src-tauri/src/webview_apis/server.rs`:
- Around line 42-182: The server currently accepts any loopback WebSocket
client; require a per-launch unguessable token to authenticate requests:
generate/provide the token from the Tauri side into the core on startup (passed
via env or API) and store it when start() runs, then validate it for each
incoming request in handle_frame; specifically, add a stored LAUNCH_TOKEN (or
reuse PORT_ENV-style var) at start() and set it alongside RESOLVED_PORT, then in
handle_frame inspect the envelope.params (or expect a top-level "auth_token"
param) and reject with encode_response(Response::error(...)) if the token is
missing or does not match the stored token; apply changes to start(),
handle_frame, and any Request parsing to ensure unauthenticated frames are
logged and denied.
In `@src/openhuman/webview_apis/client.rs`:
- Around line 119-207: The cached sender in self.sink must be cleared when
either the writer or reader task exits; modify ensure_connected so you clone
Arc<Mutex<Option<mpsc::Sender<String>>>> (self.sink) into both spawned tasks
and, on each task's exit path (after sending Close or after draining pending),
acquire that mutex and set the Option to None; keep existing behavior of
draining/failed pending in the reader task (pending remains
Arc::clone(&self.pending)) but add the sink-clearing step in both the writer
closure (after its loop/Close send) and the reader closure (before/after
draining pending) so future calls to ensure_connected will reconnect instead of
reusing a stale sender.
In `@src/openhuman/webview_apis/schemas.rs`:
- Around line 315-320: The current require_number function accepts non-integer
numbers which lets invalid values like -1 or 1.5 reach the Tauri router; update
require_number to, when the key is "limit", inspect the Value::Number using
n.as_u64() and return an Err if it is not an unsigned integer or if the value
exceeds u32::MAX (so only valid u32 values are accepted), otherwise keep the
existing numeric check for other keys; use the function name require_number and
the params: &Map<String, Value>, key: &str to locate and modify the logic and
return the same Result<(), String> error messages on failure.
---
Outside diff comments:
In `@app/src-tauri/src/lib.rs`:
- Around line 628-655: If webview_apis::start() returns Err, remove the PORT_ENV
from the process environment and abort startup instead of proceeding to create
the core handle; specifically, in the Err(err) arm for
webview_apis::start().await call, call
std::env::remove_var(webview_apis::server::PORT_ENV) (to clear any stale
OPENHUMAN_WEBVIEW_APIS_PORT) and then stop setup by returning an Err or
otherwise exiting early so core_process::CoreProcessHandle::new /
core_process::default_core_port() /
core_process::default_core_run_mode(daemon_mode) are never executed.
---
Nitpick comments:
In `@app/src-tauri/Cargo.toml`:
- Around line 52-54: Update the existing comment that mentions tokio-tungstenite
as only a "WebSocket client for the Chrome DevTools Protocol" to state that
tokio-tungstenite is used both as a WebSocket client (for the Chrome DevTools
Protocol / remote-debugging port usage and IndexedDB/Runtime.evaluate access)
and as a WebSocket server (for the webview_apis bridge), referencing
tokio-tungstenite, Chrome DevTools Protocol, and the webview_apis bridge so
readers understand the dual role.
In `@app/src-tauri/src/gmail/mod.rs`:
- Around line 161-209: Add entry (and optional exit) logging to each Tauri
command to differentiate frontend invocations from internal bridge calls: inside
gmail_list_labels, gmail_list_messages, gmail_search, gmail_get_message,
gmail_send, gmail_trash, and gmail_add_label log a concise entry message (e.g.,
"tauri::gmail_list_labels called") including non-sensitive contextual fields
such as account_id, message_id, label, query, limit or a safe summary of
request; call the existing logger/tracing facility used elsewhere in the module
before delegating to the cdp_* helper and optionally log the result or error on
return to aid tracing.
In `@app/src-tauri/src/gmail/reads.rs`:
- Around line 168-188: The is_system_label function currently matches only
English label strings (function: is_system_label), which breaks for localized
Gmail aria-labels; fix by replacing string-only checks with a locale-agnostic
approach—e.g., detect system labels from DOM structure or CSS context where they
are rendered (move detection out of content-based matching in is_system_label
and instead accept a structural flag or selector-based check), or alternatively
add a documented limitation and load a locale mapping table to translate
localized names before calling is_system_label; update callers to use the new
structural/normalized detection path and adjust function signature/name if
needed (e.g., from is_system_label(name: &str) to is_system_label(name: &str,
is_in_system_section: bool) or similar).
- Around line 114-119: The GmailLabel struct currently uses the display name for
its id (see the out.push(GmailLabel { id: name.clone(), ... }) in reads.rs),
which doesn't match Gmail's internal stable IDs; update documentation for the
GmailLabel type (or add a clear TODO on the GmailLabel definition) to state that
id is the display name and not the Gmail API/internal ID (e.g., INBOX,
Label_123), and note that callers like add_label must not assume stable/internal
IDs and that retrieving true IDs via network interception is a planned
follow-up.
In `@app/src-tauri/src/webview_apis/router.rs`:
- Around line 16-17: The dispatch entry logs but lacks outcome logging—update
the match in router.rs inside the dispatch handling (the match on method and the
surrounding dispatch logic) to log success and error paths: add debug/info logs
on successful branches (include method and any result identifier), and
error/error-level logs on failure branches and panics (include method, error
details, and contextual data); also add exit/summary logging after dispatch
completes and mirror this additional logging around the related logic referenced
near the 74-80 range so callers can trace branch decisions and failures.
In `@src/core/all.rs`:
- Around line 77-79: Add a namespace_description("webview_apis") entry wherever
you register the webview_apis domain so CLI help/discovery shows a description;
specifically, next to the call
controllers.extend(crate::openhuman::webview_apis::all_webview_apis_registered_controllers())
and the second registration occurrence (around the other registry at line ~174),
invoke namespace_description("webview_apis") on the same registry/builder object
so both registries include the description.
In `@src/openhuman/webview_apis/schemas.rs`:
- Around line 221-298: Move the actual implementations of the handle_gmail_*
functions out of schemas.rs into a new/nearby rpc.rs: cut the async Box::pin
blocks from handle_gmail_list_labels, handle_gmail_list_messages,
handle_gmail_search, handle_gmail_get_message, handle_gmail_send,
handle_gmail_trash, and handle_gmail_add_label and paste them into rpc.rs as pub
async functions (or pub fn returning ControllerFuture) keeping the same names
and logic (including param validation and client::request calls); in schemas.rs
replace each implementation with a thin delegating stub that calls the
corresponding function in rpc (e.g. return
rpc::handle_gmail_list_labels(params)), ensure rpc.rs is imported (use
crate::openhuman::...::rpc) and keep schemas.rs as registry-only alongside
maintaining symbols like schemas, all_controller_schemas, and
all_registered_controllers unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7c2c279-7938-4980-aea6-f25ac101b5ab
📒 Files selected for processing (19)
app/src-tauri/Cargo.tomlapp/src-tauri/src/cdp/mod.rsapp/src-tauri/src/gmail/mod.rsapp/src-tauri/src/gmail/reads.rsapp/src-tauri/src/gmail/session.rsapp/src-tauri/src/gmail/types.rsapp/src-tauri/src/gmail/writes.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/webview_apis/mod.rsapp/src-tauri/src/webview_apis/router.rsapp/src-tauri/src/webview_apis/server.rsscripts/mock-webview-bridge.mjssrc/core/all.rssrc/openhuman/mod.rssrc/openhuman/webview_apis/client.rssrc/openhuman/webview_apis/mod.rssrc/openhuman/webview_apis/schemas.rssrc/openhuman/webview_apis/types.rstests/webview_apis_bridge.rs
Comment on lines
+30
to
+35
| log::debug!( | ||
| "[gmail][{}] attaching CDP session fragment={} marker={}", | ||
| account_id, | ||
| fragment, | ||
| marker | ||
| ); |
Contributor
There was a problem hiding this comment.
Avoid logging raw account identifiers and markers.
These debug logs can expose user identifiers in plain text. Redact/hash account-scoped fields before logging.
Suggested patch
+fn redact_account_id(account_id: &str) -> String {
+ if account_id.is_empty() {
+ return "anon".to_string();
+ }
+ let keep = account_id.chars().take(3).collect::<String>();
+ format!("{keep}***")
+}
+
pub async fn attach(account_id: &str) -> Result<(CdpConn, String), String> {
let fragment = target_url_fragment(account_id);
let marker = placeholder_marker(account_id);
+ let account_tag = redact_account_id(account_id);
log::debug!(
- "[gmail][{}] attaching CDP session fragment={} marker={}",
- account_id,
- fragment,
- marker
+ "[gmail][{}] attaching CDP session",
+ account_tag
);
@@
- log::debug!("[gmail][{}] attached session={}", account_id, session);
+ log::debug!("[gmail][{}] attached CDP session", account_tag);
Ok((cdp, session))
}As per coding guidelines, "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields."
Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/session.rs` around lines 30 - 35, The debug log in
the log::debug! call is emitting raw account identifiers and markers (account_id
and marker); replace those with a redacted or hashed representation before
logging (e.g., compute a short hash or mask of account_id and marker) and log
the sanitized values alongside fragment; update the log::debug! invocation that
currently references account_id, fragment, marker so it uses the sanitized
variables instead of the raw ones.
| use super::types::{Ack, GmailSendRequest, SendAck}; | ||
|
|
||
| pub async fn send(account_id: &str, _req: GmailSendRequest) -> Result<SendAck, String> { | ||
| log::debug!("[gmail][{account_id}] send (not implemented)"); |
Contributor
There was a problem hiding this comment.
Redact account identifiers in write-op debug logs.
These logs currently emit raw account_id; please redact/hash before writing debug output.
Suggested patch
+fn redact_account_id(account_id: &str) -> String {
+ if account_id.is_empty() {
+ "anon".to_string()
+ } else {
+ let keep = account_id.chars().take(3).collect::<String>();
+ format!("{keep}***")
+ }
+}
+
pub async fn send(account_id: &str, _req: GmailSendRequest) -> Result<SendAck, String> {
- log::debug!("[gmail][{account_id}] send (not implemented)");
+ log::debug!(
+ "[gmail][{}] send (not implemented)",
+ redact_account_id(account_id)
+ );
@@
pub async fn trash(account_id: &str, _message_id: String) -> Result<Ack, String> {
- log::debug!("[gmail][{account_id}] trash (not implemented)");
+ log::debug!(
+ "[gmail][{}] trash (not implemented)",
+ redact_account_id(account_id)
+ );
@@
) -> Result<Ack, String> {
- log::debug!("[gmail][{account_id}] add_label (not implemented)");
+ log::debug!(
+ "[gmail][{}] add_label (not implemented)",
+ redact_account_id(account_id)
+ );As per coding guidelines, "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields."
Also applies to: 21-21, 32-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/writes.rs` at line 13, The debug logs in
gmail/writes.rs currently print the raw account_id
(log::debug!("[gmail][{account_id}] ...")), which must be redacted; update all
occurrences of log::debug! that include account_id (the three lines noted) to
pass a redacted or hashed representation instead (e.g., call a helper like
redact_account_id(account_id) or hash_account_id(account_id) that returns a
non-reversible token or a truncated form), implement that helper if it doesn't
exist, and use it in place of account_id in the debug messages to avoid logging
raw PII.
Comment on lines
+42
to
+182
| pub async fn start() -> Result<u16, String> { | ||
| if STARTED.get().is_some() { | ||
| return Ok(resolved_port()); | ||
| } | ||
|
|
||
| let requested = std::env::var(PORT_ENV) | ||
| .ok() | ||
| .and_then(|s| s.parse::<u16>().ok()) | ||
| .unwrap_or(0); | ||
|
|
||
| let addr: SocketAddr = format!("127.0.0.1:{requested}") | ||
| .parse() | ||
| .map_err(|e| format!("[webview_apis] bad addr: {e}"))?; | ||
| let listener = TcpListener::bind(addr) | ||
| .await | ||
| .map_err(|e| format!("[webview_apis] bind {addr} failed: {e}"))?; | ||
| let bound = listener | ||
| .local_addr() | ||
| .map_err(|e| format!("[webview_apis] local_addr: {e}"))?; | ||
| let port = bound.port(); | ||
| RESOLVED_PORT.store(port, Ordering::SeqCst); | ||
| let _ = STARTED.set(()); | ||
|
|
||
| log::info!("[webview_apis] server listening on {bound}"); | ||
|
|
||
| tokio::spawn(async move { | ||
| loop { | ||
| match listener.accept().await { | ||
| Ok((stream, peer)) => { | ||
| log::info!("[webview_apis] accepted connection from {peer}"); | ||
| tokio::spawn(async move { | ||
| if let Err(e) = handle_connection(stream).await { | ||
| log::warn!("[webview_apis] connection {peer} ended: {e}"); | ||
| } else { | ||
| log::info!("[webview_apis] connection {peer} closed cleanly"); | ||
| } | ||
| }); | ||
| } | ||
| Err(e) => { | ||
| log::warn!("[webview_apis] accept failed: {e}"); | ||
| tokio::time::sleep(Duration::from_millis(200)).await; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| Ok(port) | ||
| } | ||
|
|
||
| async fn handle_connection(stream: tokio::net::TcpStream) -> Result<(), String> { | ||
| let ws = tokio_tungstenite::accept_async(stream) | ||
| .await | ||
| .map_err(|e| format!("ws handshake: {e}"))?; | ||
| let (mut sink, mut stream) = ws.split(); | ||
|
|
||
| // Responses from per-request tasks fan in here and are written back | ||
| // in order. 32 is plenty — the core sidecar issues one request at a | ||
| // time per op in the common path. | ||
| let (tx, mut rx) = mpsc::channel::<String>(32); | ||
|
|
||
| let writer = tokio::spawn(async move { | ||
| while let Some(msg) = rx.recv().await { | ||
| if let Err(e) = sink.send(Message::Text(msg)).await { | ||
| log::warn!("[webview_apis] ws send failed: {e}"); | ||
| break; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| while let Some(msg) = stream.next().await { | ||
| match msg { | ||
| Ok(Message::Text(text)) => { | ||
| let tx = tx.clone(); | ||
| tokio::spawn(async move { | ||
| let reply = handle_frame(&text).await; | ||
| if let Err(_e) = tx.send(reply).await { | ||
| log::warn!("[webview_apis] response channel closed before send"); | ||
| } | ||
| }); | ||
| } | ||
| Ok(Message::Binary(_)) => { | ||
| log::debug!("[webview_apis] ignoring binary frame"); | ||
| } | ||
| Ok(Message::Ping(p)) => { | ||
| // tungstenite auto-responds to Ping at the protocol layer; | ||
| // log for visibility. | ||
| log::trace!("[webview_apis] ping {} bytes", p.len()); | ||
| } | ||
| Ok(Message::Close(_)) => { | ||
| log::debug!("[webview_apis] peer requested close"); | ||
| break; | ||
| } | ||
| Ok(_) => {} | ||
| Err(e) => { | ||
| return Err(format!("ws recv: {e}")); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| drop(tx); | ||
| let _ = writer.await; | ||
| Ok(()) | ||
| } | ||
|
|
||
| async fn handle_frame(text: &str) -> String { | ||
| let envelope: Request = match serde_json::from_str(text) { | ||
| Ok(v) => v, | ||
| Err(e) => { | ||
| log::warn!("[webview_apis] bad request frame: {e}"); | ||
| return encode_response(Response::error("<unknown>", format!("bad frame: {e}"))); | ||
| } | ||
| }; | ||
| if envelope.kind != "request" { | ||
| return encode_response(Response::error( | ||
| &envelope.id, | ||
| format!("unsupported envelope kind '{}'", envelope.kind), | ||
| )); | ||
| } | ||
| let params = envelope.params.unwrap_or_default(); | ||
| let started = std::time::Instant::now(); | ||
| let result = router::dispatch(&envelope.method, params).await; | ||
| let ms = started.elapsed().as_millis(); | ||
| match result { | ||
| Ok(value) => { | ||
| log::debug!( | ||
| "[webview_apis] {} id={} ok in {ms}ms", | ||
| envelope.method, | ||
| envelope.id | ||
| ); | ||
| encode_response(Response::ok(&envelope.id, value)) | ||
| } | ||
| Err(e) => { | ||
| log::warn!( | ||
| "[webview_apis] {} id={} err in {ms}ms: {e}", | ||
| envelope.method, | ||
| envelope.id | ||
| ); | ||
| encode_response(Response::error(&envelope.id, e)) | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Require per-launch authentication on the localhost bridge.
This listener accepts arbitrary loopback WebSocket clients and exposes Gmail read/write operations backed by the user's logged-in session. Loopback is not a trust boundary; any local process that discovers the port can list, send, trash, or relabel mail. Please add an unguessable per-launch token passed from Tauri to core and reject unauthenticated requests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/webview_apis/server.rs` around lines 42 - 182, The server
currently accepts any loopback WebSocket client; require a per-launch
unguessable token to authenticate requests: generate/provide the token from the
Tauri side into the core on startup (passed via env or API) and store it when
start() runs, then validate it for each incoming request in handle_frame;
specifically, add a stored LAUNCH_TOKEN (or reuse PORT_ENV-style var) at start()
and set it alongside RESOLVED_PORT, then in handle_frame inspect the
envelope.params (or expect a top-level "auth_token" param) and reject with
encode_response(Response::error(...)) if the token is missing or does not match
the stored token; apply changes to start(), handle_frame, and any Request
parsing to ensure unauthenticated frames are logged and denied.
Comment on lines
+315
to
+320
| fn require_number(params: &Map<String, Value>, key: &str) -> Result<(), String> { | ||
| match params.get(key) { | ||
| Some(Value::Number(_)) => Ok(()), | ||
| Some(_) => Err(format!("invalid '{key}': expected number")), | ||
| None => Err(format!("missing required param '{key}'")), | ||
| } |
Contributor
There was a problem hiding this comment.
Validate limit as an integer before forwarding it.
require_number accepts values like -1 and 1.5, but the Tauri router reads limit as u32. Those inputs currently cross the process boundary and only fail on the shell side. Validate as_u64()/u32 here so the JSON-RPC error is immediate and consistent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/webview_apis/schemas.rs` around lines 315 - 320, The current
require_number function accepts non-integer numbers which lets invalid values
like -1 or 1.5 reach the Tauri router; update require_number to, when the key is
"limit", inspect the Value::Number using n.as_u64() and return an Err if it is
not an unsigned integer or if the value exceeds u32::MAX (so only valid u32
values are accepted), otherwise keep the existing numeric check for other keys;
use the function name require_number and the params: &Map<String, Value>, key:
&str to locate and modify the logic and return the same Result<(), String> error
messages on failure.
Both ops now return real data from the live Gmail session via CDP. list_messages — fetches the stable Gmail Atom feed at `mail.google.com/mail/u/0/feed/atom[/<label>]` using Network.loadNetworkResource + IO.read (no JS eval), then parses the XML into the 20 most recent unread messages. Honors `limit` and optional `label`. New `atom.rs` has a minimal hand-rolled XML parser with named and numeric HTML entity decoding; no new XML crate dependency. get_message — fetches Gmail's print-view endpoint (permthid=thread-f:…) using the same CDP fetch primitive and parses the flat plaintext layout Gmail emits today (subject via <title>, account-owner line, sender line, human-readable date, To:, body). Picks the LAST email-like line before `To:` as the sender, which skips the account-owner header Gmail prepends. Handles both RFC 2822 and the "Thu, Apr 23, 2026 at 9:01 PM" human shape for dates. cdp_fetch.rs — thin wrapper around Network.loadNetworkResource plus IO.read + IO.close, with frame-id discovery via Page.getFrameTree, so reads can issue authenticated GETs through the attached CDP session without touching JS. Also fixes the entity decoder that was treating UTF-8 bytes as Latin-1 codepoints — now char-aware and supports `&#NN;` / `&#xHH;`. Verified curl end-to-end against a live logged-in Gmail session: list_messages returned five real unread inbox entries with correct sender, subject, and dates; get_message returned subject, sender, to, date_ms, and body for the first message. search remains stubbed for a follow-up (needs Page.navigate to `#search/<q>` plus response observation).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/gmail/atom.rs`:
- Around line 64-75: The parser currently hardcodes labels = ["INBOX"] for every
GmailMessage (in the GmailMessage construction), which corrupts label metadata
when list_messages fetches per-label feeds; update the parse(...) function (and
its callers such as reads::list_messages) to accept a source_label
(Option<String> or Vec<String>) and use that value to populate the
GmailMessage.labels field instead of the literal ["INBOX"], or alternatively let
reads::list_messages set/overwrite message.labels after parsing; adjust all
calls to parse(...) so the real feed label is threaded through and used when
constructing the GmailMessage.
In `@app/src-tauri/src/gmail/cdp_fetch.rs`:
- Line 37: The debug line log::debug!("[gmail-cdp-fetch] session={session}
frame={frame_id} url={url}"); leaks PII/IDs; in get_message replace it with a
non-sensitive log: either log only the operation/result (e.g., "gmail-cdp-fetch
get_message start/complete") or emit redacted/hashes of identifiers (e.g.,
hash(session), hash(frame_id), hash(url) or mask URL path/query) before logging;
update the log invocation in the get_message flow (the log::debug call) to avoid
printing raw session, frame_id, or the full url.
- Around line 39-45: The current call to cdp.call("Network.enable", json!({}),
Some(session)).await maps any error into an Err and returns early; change this
to a best-effort call that logs the failure and continues instead of propagating
the error. Specifically, replace the map_err + ? behavior around
cdp.call("Network.enable", ...) so that on Err(e) you emit a warning (e.g., via
the crate's logging/tracing) noting "Network.enable failed" and the error
details, but do not return—allow the function to proceed and let the subsequent
cdp.call("Network.loadNetworkResource", ...) be the real success check. Ensure
the call remains awaited so side-effects occur and keep it idempotent (no retry
logic needed).
In `@app/src-tauri/src/gmail/mod.rs`:
- Around line 30-38: The capability table in the module doc comment is out of
date: update the table entries for `list_messages` and `get_message` from "stub"
to "working" (or the appropriate status used for implemented features) in the
top-of-file doc block in gmail/mod.rs so the docs accurately reflect the new
implementations of the `list_messages` and `get_message` paths.
In `@app/src-tauri/src/gmail/print_view.rs`:
- Around line 281-287: The current looks_like_print_view() heuristic returns
true if either the Gmail title or a "From:" header is present, causing false
positives; change it to require both markers (i.e., ensure
html.contains("<title>Gmail -") AND the lowercased html contains ">from:</") or
replace/add a stronger print-view-specific anchor check inside
looks_like_print_view() so that get_message only proceeds when both indicators
(the tab title and the From: header) are present; update the function body
accordingly.
- Around line 274-278: The code currently treats timezone-less "human" Gmail
dates as UTC by computing secs = days * 86400 + hh*3600 + mm*60 and returning
Some(secs.saturating_mul(1000)); instead, detect that the parsed date has no
timezone/offset and return None for that shape (do not compute or emit an
absolute timestamp); modify the block around days_from_civil / secs to
early-return None when the mailbox timezone is unavailable, leaving other
timezone-aware parsing paths unchanged so callers receive None for timezone-less
human dates.
In `@app/src-tauri/src/gmail/reads.rs`:
- Around line 205-268: The current parse_aria_label and is_system_label hardcode
English strings; change parsing to be locale-agnostic by (1) updating
parse_aria_label to extract a trailing integer token using a numeric-only
check/regex (find last token matching ^\d+$) and strip it as the unread count
without looking for English words like "unread"/"messages", and (2) stop using
localized display names in is_system_label and instead use a canonical label
identifier provided by the DOM (e.g., a data-label-id / aria or dataset value)
when available—modify the code that calls parse_aria_label to prefer that
canonical id and update is_system_label to match that set of canonical IDs
(INBOX, SPAM, TRASH, SENT, DRAFT, STARRED, SNOOZED, IMPORTANT, SCHEDULED, CHATS,
CATEGORIES/Promotions/Social/Forums canonical keys) as the primary check,
falling back to a lowercased display-name check only if no canonical id is
present.
- Around line 24-25: The logs currently interpolate raw identifiers (account_id,
label names, message IDs) in functions like list_labels (and the other Gmail
functions emitting logs around labels/messages); replace those interpolations
with redacted or hashed representations: create/Use a small helper (e.g.,
redact_id or hash_and_truncate) that takes account_id, label.name, message_id
and returns a non-reversible short token (hash prefix or constant "<redacted>"),
then update the log::debug calls in list_labels and the other Gmail logging
sites to log the operation and the redacted token instead of the raw identifier.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d235ecb-2163-46d1-befa-30624868debd
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/src-tauri/Cargo.tomlapp/src-tauri/src/gmail/atom.rsapp/src-tauri/src/gmail/cdp_fetch.rsapp/src-tauri/src/gmail/mod.rsapp/src-tauri/src/gmail/print_view.rsapp/src-tauri/src/gmail/reads.rs
Comment on lines
+64
to
+75
| out.push(GmailMessage { | ||
| id, | ||
| thread_id: None, | ||
| from, | ||
| to: Vec::new(), | ||
| cc: Vec::new(), | ||
| subject: title, | ||
| snippet: summary, | ||
| body: None, | ||
| date_ms, | ||
| labels: vec!["INBOX".into()], | ||
| unread: true, |
Contributor
There was a problem hiding this comment.
Don't stamp every Atom entry as INBOX.
list_messages() also fetches per-label feeds, but this parser hardcodes labels = ["INBOX"] for every message. Calls like list_messages(..., Some("Spam")) will therefore return incorrect label metadata. Please thread the source label into parse(...) or let reads::list_messages overwrite labels before returning.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/atom.rs` around lines 64 - 75, The parser currently
hardcodes labels = ["INBOX"] for every GmailMessage (in the GmailMessage
construction), which corrupts label metadata when list_messages fetches
per-label feeds; update the parse(...) function (and its callers such as
reads::list_messages) to accept a source_label (Option<String> or Vec<String>)
and use that value to populate the GmailMessage.labels field instead of the
literal ["INBOX"], or alternatively let reads::list_messages set/overwrite
message.labels after parsing; adjust all calls to parse(...) so the real feed
label is threaded through and used when constructing the GmailMessage.
| // `Network.loadNetworkResource` needs a frameId to charge the | ||
| // request against — we use the main frame of the target. | ||
| let frame_id = main_frame_id(cdp, session).await?; | ||
| log::debug!("[gmail-cdp-fetch] session={session} frame={frame_id} url={url}"); |
Contributor
There was a problem hiding this comment.
Redact the fetch URL and session identifiers from logs.
This debug line records the full Gmail URL plus CDP session/frame identifiers. For get_message, the URL embeds stable Gmail message/thread IDs, so this will leak user data into routine logs. Please log the operation/result only, or hash/redact identifiers before emitting them.
As per coding guidelines, "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/cdp_fetch.rs` at line 37, The debug line
log::debug!("[gmail-cdp-fetch] session={session} frame={frame_id} url={url}");
leaks PII/IDs; in get_message replace it with a non-sensitive log: either log
only the operation/result (e.g., "gmail-cdp-fetch get_message start/complete")
or emit redacted/hashes of identifiers (e.g., hash(session), hash(frame_id),
hash(url) or mask URL path/query) before logging; update the log invocation in
the get_message flow (the log::debug call) to avoid printing raw session,
frame_id, or the full url.
Comment on lines
+39
to
+45
| // Network must be enabled for loadNetworkResource to work on some | ||
| // CDP builds. Enabling is idempotent — safe to call every fetch. | ||
| let _ = cdp | ||
| .call("Network.enable", json!({}), Some(session)) | ||
| .await | ||
| .map_err(|e| format!("Network.enable: {e}"))?; | ||
|
|
Contributor
There was a problem hiding this comment.
Make Network.enable best-effort here.
The rest of the CDP integration already treats domain-enable calls as compatibility shims, because some builds reject them even when the real operation still works. Failing the whole Gmail fetch on Network.enable turns that quirk into a hard outage. Please log and continue, then let Network.loadNetworkResource be the real success check.
Suggested change
- let _ = cdp
- .call("Network.enable", json!({}), Some(session))
- .await
- .map_err(|e| format!("Network.enable: {e}"))?;
+ if let Err(e) = cdp.call("Network.enable", json!({}), Some(session)).await {
+ log::debug!("[gmail-cdp-fetch] Network.enable failed: {}", e);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Network must be enabled for loadNetworkResource to work on some | |
| // CDP builds. Enabling is idempotent — safe to call every fetch. | |
| let _ = cdp | |
| .call("Network.enable", json!({}), Some(session)) | |
| .await | |
| .map_err(|e| format!("Network.enable: {e}"))?; | |
| // Network must be enabled for loadNetworkResource to work on some | |
| // CDP builds. Enabling is idempotent — safe to call every fetch. | |
| if let Err(e) = cdp.call("Network.enable", json!({}), Some(session)).await { | |
| log::debug!("[gmail-cdp-fetch] Network.enable failed: {}", e); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/cdp_fetch.rs` around lines 39 - 45, The current call
to cdp.call("Network.enable", json!({}), Some(session)).await maps any error
into an Err and returns early; change this to a best-effort call that logs the
failure and continues instead of propagating the error. Specifically, replace
the map_err + ? behavior around cdp.call("Network.enable", ...) so that on
Err(e) you emit a warning (e.g., via the crate's logging/tracing) noting
"Network.enable failed" and the error details, but do not return—allow the
function to proceed and let the subsequent
cdp.call("Network.loadNetworkResource", ...) be the real success check. Ensure
the call remains awaited so side-effects occur and keep it idempotent (no retry
logic needed).
Comment on lines
+30
to
+38
| //! | Op | Status | | ||
| //! | ------------ | ------------- | | ||
| //! | `list_labels` | **working** | | ||
| //! | `list_messages` | stub | | ||
| //! | `search` | stub | | ||
| //! | `get_message` | stub | | ||
| //! | `send` | stub | | ||
| //! | `trash` | stub | | ||
| //! | `add_label` | stub | |
Contributor
There was a problem hiding this comment.
Update the capability table.
The docs here still mark list_messages and get_message as stubs, but this PR now implements both paths. Leaving the table stale will mislead the next person touching the connector.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/mod.rs` around lines 30 - 38, The capability table in
the module doc comment is out of date: update the table entries for
`list_messages` and `get_message` from "stub" to "working" (or the appropriate
status used for implemented features) in the top-of-file doc block in
gmail/mod.rs so the docs accurately reflect the new implementations of the
`list_messages` and `get_message` paths.
Comment on lines
+274
to
+278
| let days = days_from_civil(year, mon, day); | ||
| // Human date has no TZ — treat as local-naive (we don't know the | ||
| // mailbox's TZ), so use UTC. Caller sees the absolute millis. | ||
| let secs = days * 86_400 + hh as i64 * 3600 + mm as i64 * 60; | ||
| Some(secs.saturating_mul(1000)) |
Contributor
There was a problem hiding this comment.
Don't invent UTC timestamps from timezone-less Gmail dates.
The Thu, Apr 23, 2026 at 6:12 PM form has no offset. Converting it straight to unix millis as if it were UTC makes date_ms wrong for every non-UTC mailbox. If the timezone is unavailable here, it is safer to return None for this shape than emit a shifted absolute timestamp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/print_view.rs` around lines 274 - 278, The code
currently treats timezone-less "human" Gmail dates as UTC by computing secs =
days * 86400 + hh*3600 + mm*60 and returning Some(secs.saturating_mul(1000));
instead, detect that the parsed date has no timezone/offset and return None for
that shape (do not compute or emit an absolute timestamp); modify the block
around days_from_civil / secs to early-return None when the mailbox timezone is
unavailable, leaving other timezone-aware parsing paths unchanged so callers
receive None for timezone-less human dates.
Comment on lines
+281
to
+287
| fn looks_like_print_view(html: &str) -> bool { | ||
| // Gmail print-view pages always contain these two markers: | ||
| // - `<title>Gmail -` — the browser tab title | ||
| // - a `From:`-labelled header row | ||
| // Login / error pages contain neither. | ||
| html.contains("<title>Gmail -") || html.to_ascii_lowercase().contains(">from:</") | ||
| } |
Contributor
There was a problem hiding this comment.
Tighten the print-view detection heuristic.
looks_like_print_view() currently accepts pages that contain either the Gmail - ... title or a From: marker. That makes false positives likely on redirects/error pages that still render a Gmail title, and get_message can then return a bogus parsed message instead of a parse failure. Please require both markers or another stronger print-view-specific anchor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/print_view.rs` around lines 281 - 287, The current
looks_like_print_view() heuristic returns true if either the Gmail title or a
"From:" header is present, causing false positives; change it to require both
markers (i.e., ensure html.contains("<title>Gmail -") AND the lowercased html
contains ">from:</") or replace/add a stronger print-view-specific anchor check
inside looks_like_print_view() so that get_message only proceeds when both
indicators (the tab title and the From: header) are present; update the function
body accordingly.
Comment on lines
+24
to
+25
| pub async fn list_labels(account_id: &str) -> Result<Vec<GmailLabel>, String> { | ||
| log::debug!("[gmail][{account_id}] list_labels"); |
Contributor
There was a problem hiding this comment.
Redact account, label, and message identifiers from Gmail logs.
These debug statements emit raw account_id, label names, and message IDs. Those are user identifiers/data and will end up in normal app logs. Please keep the operation-level logging, but hash or omit the identifiers themselves.
As per coding guidelines, "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields".
Also applies to: 48-51, 117-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/reads.rs` around lines 24 - 25, The logs currently
interpolate raw identifiers (account_id, label names, message IDs) in functions
like list_labels (and the other Gmail functions emitting logs around
labels/messages); replace those interpolations with redacted or hashed
representations: create/Use a small helper (e.g., redact_id or
hash_and_truncate) that takes account_id, label.name, message_id and returns a
non-reversible short token (hash prefix or constant "<redacted>"), then update
the log::debug calls in list_labels and the other Gmail logging sites to log the
operation and the redacted token instead of the raw identifier.
Comment on lines
+205
to
+268
| /// Gmail's aria-labels look like: | ||
| /// `"Inbox 23 unread"`, `"Inbox, 23 unread messages"`, | ||
| /// `"Starred"`, `"Drafts 4"`, `"Spam, 1"`. | ||
| /// Peel any trailing `N unread(messages)?` / `N` count off and return | ||
| /// the plain label name plus the parsed unread count if present. | ||
| fn parse_aria_label(aria: &str) -> (String, Option<u64>) { | ||
| let mut name = aria.trim().to_string(); | ||
|
|
||
| // 1. Strip English descriptors in order from most-specific to least. | ||
| // Keep going until no more of these match, which covers labels | ||
| // like "Spam, 1 unread messages" that chain two suffixes. | ||
| loop { | ||
| let lower = name.to_ascii_lowercase(); | ||
| let stripped_len = ["unread messages", "unread", "messages"] | ||
| .iter() | ||
| .find(|suf| lower.ends_with(*suf)) | ||
| .map(|suf| name.len() - suf.len()); | ||
| match stripped_len { | ||
| Some(n) => { | ||
| name.truncate(n); | ||
| name = name.trim_end_matches([' ', ',']).to_string(); | ||
| } | ||
| None => break, | ||
| } | ||
| } | ||
|
|
||
| // 2. Now name is e.g. "Inbox 23" or "Spam, 1" or "Starred". Peel off | ||
| // a trailing integer (with any comma/space separator) as the | ||
| // unread count. | ||
| let mut unread: Option<u64> = None; | ||
| if let Some(last) = name.split(|c: char| c == ' ' || c == ',').next_back() { | ||
| if !last.is_empty() { | ||
| if let Ok(n) = last.parse::<u64>() { | ||
| unread = Some(n); | ||
| let cut = name.len() - last.len(); | ||
| name.truncate(cut); | ||
| name = name.trim_end_matches([' ', ',']).to_string(); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| (name.trim().to_string(), unread) | ||
| } | ||
|
|
||
| fn is_system_label(name: &str) -> bool { | ||
| matches!( | ||
| name, | ||
| "Inbox" | ||
| | "Starred" | ||
| | "Snoozed" | ||
| | "Important" | ||
| | "Sent" | ||
| | "Drafts" | ||
| | "Scheduled" | ||
| | "All Mail" | ||
| | "Spam" | ||
| | "Trash" | ||
| | "Chats" | ||
| | "Categories" | ||
| | "Updates" | ||
| | "Promotions" | ||
| | "Social" | ||
| | "Forums" | ||
| ) |
Contributor
There was a problem hiding this comment.
The label parser is English-only.
Both unread-count extraction and system-label classification depend on English aria-label text ("unread", "Spam", "Trash", etc.). Gmail localizes those strings, so non-English accounts will lose unread counts and misclassify core folders as "user". This needs a locale-stable anchor, or the API should avoid baking English-only label semantics into the returned data.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/reads.rs` around lines 205 - 268, The current
parse_aria_label and is_system_label hardcode English strings; change parsing to
be locale-agnostic by (1) updating parse_aria_label to extract a trailing
integer token using a numeric-only check/regex (find last token matching ^\d+$)
and strip it as the unread count without looking for English words like
"unread"/"messages", and (2) stop using localized display names in
is_system_label and instead use a canonical label identifier provided by the DOM
(e.g., a data-label-id / aria or dataset value) when available—modify the code
that calls parse_aria_label to prefer that canonical id and update
is_system_label to match that set of canonical IDs (INBOX, SPAM, TRASH, SENT,
DRAFT, STARRED, SNOOZED, IMPORTANT, SCHEDULED, CHATS,
CATEGORIES/Promotions/Social/Forums canonical keys) as the primary check,
falling back to a lowercased display-name check only if no canonical id is
present.
Lift-and-shift of the actionable items from tinyhumansai#869 (review) with full test coverage for the new logic. **Correctness** * lib.rs — bridge-start failure now clears OPENHUMAN_WEBVIEW_APIS_PORT and returns Err from setup so the core child can't inherit a stale port and connect to whichever loopback process owns it. * gmail/session.rs — replace the single permissive predicate with a two-pass attach: first the account-anchored fragment/title, then a broad mail.google.com/* fallback. The fallback now warns loudly and only runs when the strong match produced no target. * webview_apis/client.rs — writer and reader tasks both clear self.sink on exit so ensure_connected reconnects instead of handing out a dead mpsc::Sender cached in the OnceLock. * webview_apis/router.rs — read_string now trims and rejects blank required params at the WS entry point; read_optional_string error now echoes the actual param name (was hard-coded 'label'); dispatch wraps outcomes with success/warn traces so bridge debugging has a single chokepoint. * webview_apis/rpc.rs (new) — require_u32 replaces the old require_number so 'limit' rejects negatives, fractions, and u32 overflow at the core JSON-RPC edge rather than surfacing as downstream errors. **Convention** * Split handler bodies out of schemas.rs into a sibling rpc.rs per the project rule "core domains must include a schemas.rs that … [has] handle_* functions delegating to domain rpc.rs". * Add namespace_description("webview_apis") so CLI discovery shows a description alongside the schemas. **Observability + docs** * gmail/mod.rs — entry-point log::debug in each #[tauri::command] so frontend invocations are distinguishable from webview_apis bridge calls in traces. * gmail/types.rs / reads.rs — doc the GmailLabel.id scrape limitation (display name, not Gmail's stable internal id) and the English-only is_system_label table, with follow-up pointers in the plan. * Cargo.toml — tokio-tungstenite comment now describes both CDP client and webview_apis-server uses. **Deliberately not taken** * Launch-token auth on the loopback WS server. The port is 127.0.0.1, set via env-var from Tauri to the core child on the same user / same machine; matches the trust model of core_rpc_relay and CDP's remote-debugging port. Added crypto handshake here would be noise. * Redacting account_id / marker from debug logs. account_id is a user-assigned webview label (e.g. "gmail-dev"), not PII, and every other scanner in this shell (discord_scanner, slack_scanner, whatsapp_scanner, …) logs it unredacted. Tests: 59 passing in OpenHuman app lib, 5 in core webview_apis (2 new rpc helper tests), 2 bridge integration tests, 12 gmail parser tests.
Member
Author
|
Addressed the CodeRabbit review in Fixed (9 of 11):
Deliberately not taken (2 of 11):
Tests: 59 passing in the app lib (+2 new router tests), 5 in core |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
app/src-tauri/src/gmail/reads.rs (2)
249-274:⚠️ Potential issue | 🟠 MajorEnglish-only label classification is documented but limits non-English users.
The
is_system_labelfunction relies on an English catalog. Users on localized Gmail will see system labels (Inbox, Spam, Trash, etc.) misclassified as"user". The limitation is acknowledged in the doc comment (lines 249-253), which is good.For a more robust solution, consider parsing the canonical label ID from DOM attributes (e.g.,
data-label-id) if Gmail exposes them, or maintain a mapping of locale-specific names.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/reads.rs` around lines 249 - 274, is_system_label currently matches only English label names so localized Gmail labels get misclassified; update the detector to prefer canonical/localization-agnostic identifiers: modify the is_system_label logic to first look up a stable DOM attribute (e.g., data-label-id or other canonical id exposed by Gmail) from the label element and treat known canonical ids as system labels, and if that attribute is absent fall back to the existing English-name matches (function name: is_system_label). If DOM attributes aren’t available, implement a locale-aware mapping table (e.g., a HashSet per locale) and consult it before falling back to English names, and add comments/tests around both code paths.
24-25:⚠️ Potential issue | 🟠 MajorRedact identifiers in Gmail operation logs.
These debug logs emit raw
account_id, label names, and message IDs. As per coding guidelines: "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields."Consider logging only operation names and counts without raw identifiers, or use a consistent redaction helper across the Gmail module.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/reads.rs` around lines 24 - 25, The debug log in list_labels (and other Gmail ops) currently emits raw account_id and other identifiers; replace these with non-sensitive messages and a redaction helper (e.g., add a redact_identifier or redact_gmail_id function in the gmail module) and use it wherever account_id, label names, message IDs, or similar fields are logged (for example, in list_labels and any functions returning Vec<GmailLabel>) so logs only include operation names and safe metadata like counts, not raw PII or IDs.app/src-tauri/src/gmail/session.rs (2)
69-72:⚠️ Potential issue | 🟠 MajorFallback predicate can attach to the wrong Gmail account.
The fallback at line 70 matches any
https://mail.google.com/URL without account disambiguation. If multiple Gmail tabs are open, this can bind to another user's session and return data for the wrong account.Consider removing the fallback entirely or requiring an account-specific signal (e.g., URL contains
u=<index>or fragment) even in the fallback path:Suggested patch
- let (cdp, session) = - connect_and_attach_matching(|t| t.url.starts_with("https://mail.google.com/")) + // Require at least a mail.google.com URL that contains the account fragment + // or a /u/<n>/ path segment to reduce cross-account mismatch risk. + let (cdp, session) = connect_and_attach_matching(|t| { + t.url.starts_with("https://mail.google.com/") + && (t.url.contains(&fragment) || t.url.contains("/u/")) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/session.rs` around lines 69 - 72, The predicate passed to connect_and_attach_matching (the closure using t.url.starts_with("https://mail.google.com/")) is too broad and can attach to the wrong Gmail tab when multiple accounts are open; update that predicate to require an account-specific signal (for example check the URL contains the account index or fragment like "u=<index>" or another unique identifier tied to account_id) or remove this fallback entirely so it never matches a generic "https://mail.google.com/" URL; ensure the error message still references account_id when mapping the error from connect_and_attach_matching.
30-35:⚠️ Potential issue | 🟠 MajorRedact account identifiers in debug logs.
The debug logs emit raw
account_id,fragment, andmarkervalues. These can expose user identifiers in plain text. As per coding guidelines: "Never log secrets, raw JWTs, API keys, credentials, or full PII; redact or omit sensitive fields."Consider redacting or hashing account-scoped fields before logging:
Suggested approach
+fn redact_id(id: &str) -> String { + if id.len() <= 3 { return "***".to_string(); } + format!("{}***", &id[..3]) +} + pub async fn attach(account_id: &str) -> Result<(CdpConn, String), String> { let fragment = target_url_fragment(account_id); let marker = placeholder_marker(account_id); + let tag = redact_id(account_id); log::debug!( - "[gmail][{}] attaching CDP session fragment={} marker={}", - account_id, - fragment, - marker + "[gmail][{}] attaching CDP session", + tag );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/session.rs` around lines 30 - 35, The debug log currently prints raw account-scoped values (account_id, fragment, marker) which may expose PII; update the logging in the gmail session (the log::debug call that references account_id, fragment, marker) to redact or pseudonymize those fields before printing (e.g., replace with a fixed "[REDACTED]" or a stable hash of account_id/fragment/marker), and ensure you centralize the transformation (helper like redact_id or hash_identifier) so the log call uses the transformed values rather than the raw variables.app/src-tauri/src/gmail/mod.rs (1)
30-38:⚠️ Potential issue | 🟡 MinorUpdate the capability table to reflect implemented operations.
The table marks
list_messagesandget_messageas "stub", butreads.rsnow implements both:
list_messages(reads.rs:43-71) fetches the Atom feed and parses messagesget_message(reads.rs:116-130) fetches the print-view and parses itSuggested fix
//! | Op | Status | //! | ------------ | ------------- | //! | `list_labels` | **working** | -//! | `list_messages` | stub | +//! | `list_messages` | **working** | //! | `search` | stub | -//! | `get_message` | stub | +//! | `get_message` | **working** | //! | `send` | stub |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/gmail/mod.rs` around lines 30 - 38, The capability table in gmail/mod.rs is out of date: update the table rows for `list_messages` and `get_message` from "stub" to "working" since reads.rs now implements `list_messages` (parses Atom feed) and `get_message` (parses print-view); locate the table comment block in gmail/mod.rs and change the status cells for `list_messages` and `get_message` to reflect "working" (matching the implemented functions in reads.rs).
🧹 Nitpick comments (4)
src/openhuman/webview_apis/schemas.rs (1)
197-209: Consider panicking for unknown function names in development.The fallback returns an "unknown" schema which will pass registry validation but produce confusing behavior at runtime. Since
schemas()is called with string literals, a panic would catch typos at compile/test time:Optional hardening
- _other => ControllerSchema { - namespace: "webview_apis", - function: "unknown", - ... - }, + other => panic!("unknown webview_apis function: {other}"),This matches patterns in other
schemas.rsfiles that don't silently degrade.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/webview_apis/schemas.rs` around lines 197 - 209, Replace the silent fallback in the match that returns a ControllerSchema with namespace "webview_apis" and function "unknown" by panicking when an unrecognized function name is encountered: locate the match arm handling _other in the schemas() mapping (the branch that constructs ControllerSchema with "unknown"), and change it to panic with a clear message that includes the unexpected function name so typos are caught early (e.g., panic!("Unknown webview_apis controller function: {}", func_name)); keep ControllerSchema/FieldSchema types referenced for context.src/openhuman/webview_apis/client.rs (1)
36-40: Global singleton prevents concurrent client instances but limits testability.The
OnceLock<Client>pattern ensures a single connection but makes unit testing harder (tests share state). For this internal bridge client, the tradeoff is reasonable.If testing becomes problematic, consider injecting the client via a trait or resettable global.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/webview_apis/client.rs` around lines 36 - 40, The global OnceLock<Client> (CLIENT) and client() factory prevents replacing the Client in tests; change to a test-friendly injectable abstraction: define a BridgeClient trait implemented by Client, replace the static OnceLock<Client> with a concurrency-safe holder (e.g., a static RwLock<Option<Box<dyn BridgeClient + Send + Sync>>> or OnceLock<Box<dyn BridgeClient + Send + Sync>>) and update client() to return the trait object/reference; add a set_test_client(boxed_client) helper to install a mock in tests and ensure production initialization still uses Client::new when none is set. Ensure all uses of CLIENT and client() are updated to the trait type so tests can inject mocks while production uses the real Client implementation.app/src-tauri/src/webview_apis/router.rs (1)
121-127: Consider adding explicit error messages forread_u32edge cases.The current implementation combines "missing" and "invalid" into one error. While functional, separating these cases would improve debuggability:
Optional refinement
fn read_u32(params: &Map<String, Value>, key: &str) -> Result<u32, String> { - params - .get(key) - .and_then(|v| v.as_u64()) - .and_then(|n| u32::try_from(n).ok()) - .ok_or_else(|| format!("missing or invalid u32 param '{key}'")) + match params.get(key) { + Some(Value::Number(n)) => { + let u = n.as_u64() + .ok_or_else(|| format!("invalid '{key}': expected non-negative integer"))?; + u32::try_from(u) + .map_err(|_| format!("invalid '{key}': exceeds u32 max")) + } + Some(_) => Err(format!("invalid '{key}': expected number")), + None => Err(format!("missing required param '{key}'")), + } }This would align with the validation semantics in
src/openhuman/webview_apis/rpc.rs::require_u32(see context snippet 2).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/webview_apis/router.rs` around lines 121 - 127, The read_u32 function currently folds "missing" and "invalid" into a single error; change it to return distinct errors for the three edge cases by first checking params.get(key) and returning "missing param '{key}'" if absent, then checking value.as_u64() and returning "invalid type for '{key}': expected unsigned integer" if the value isn't numeric, and finally attempting u32::try_from(n) and returning "out of range for '{key}': value {n} does not fit in u32" on conversion failure; keep the function name read_u32 and mirror the validation wording used by require_u32 for consistency.src/openhuman/webview_apis/rpc.rs (1)
119-133: Caplimitto an operational max, not justu32::MAX.Type validation is solid, but allowing values up to 4B can become a load/rate risk once list/search are fully implemented. Prefer a domain cap (e.g., 100–1000) and fail fast.
Suggested guard
+const MAX_GMAIL_LIMIT: u32 = 500; + fn require_u32(params: &Map<String, Value>, key: &str) -> Result<(), String> { match params.get(key) { Some(Value::Number(n)) => { let u = n .as_u64() .ok_or_else(|| format!("invalid '{key}': expected non-negative integer"))?; if u > u32::MAX as u64 { return Err(format!("invalid '{key}': exceeds u32 max")); } + if u > MAX_GMAIL_LIMIT as u64 { + return Err(format!("invalid '{key}': exceeds max {}", MAX_GMAIL_LIMIT)); + } Ok(()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/webview_apis/rpc.rs` around lines 119 - 133, The current require_u32 allows values up to u32::MAX which is dangerous for params like "limit"; change the validation to enforce a practical operational cap: introduce a constant (e.g., OPERATIONAL_MAX_LIMIT = 1000) and in require_u32 (or a new helper require_limited_u32) check after parsing the u64 that the value <= OPERATIONAL_MAX_LIMIT and return an Err with a clear message (e.g., "invalid 'limit': exceeds allowed maximum of {OPERATIONAL_MAX_LIMIT}") when it exceeds; ensure callers that pass the "limit" key use this capped validation or switch to the new helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/gmail/mod.rs`:
- Around line 172-240: Logs in the Tauri command handlers leak raw identifiers;
update each handler (gmail_list_labels, gmail_list_messages, gmail_search,
gmail_get_message, gmail_send, gmail_trash, gmail_add_label) to log redacted
versions of account_id, message_id, label, etc., using the same redaction helper
used in the session/reads modules (call it the project's redact helper, e.g.,
redact_identifier/redact_string) instead of interpolating raw values, and for
request bodies/loggable collections keep the existing length logging (e.g.,
body.len(), to.len()) while redacting their contents.
In `@src/openhuman/webview_apis/rpc.rs`:
- Around line 21-98: Handlers like handle_gmail_list_labels /
handle_gmail_list_messages / handle_gmail_search / handle_gmail_get_message /
handle_gmail_send / handle_gmail_trash / handle_gmail_add_label lack structured
entry/exit and error logging, so add stable, grep-friendly logs at the start and
just before returning success (e.g. "[webview_apis][gmail][entry]" and
"[webview_apis][gmail][exit]") and explicit error-path logs when validation
(require_string/require_u32/read_required) or client::request(...) fails;
include correlation fields such as method/op and a request_id/correlation_id
(but do not log raw user data like account_id/message_id values), ensure logs
use the project logging macro/utility used elsewhere (structured key/value
fields) and include the operation name (e.g., gmail_list_messages) and error
details in error logs to aid debugging.
---
Duplicate comments:
In `@app/src-tauri/src/gmail/mod.rs`:
- Around line 30-38: The capability table in gmail/mod.rs is out of date: update
the table rows for `list_messages` and `get_message` from "stub" to "working"
since reads.rs now implements `list_messages` (parses Atom feed) and
`get_message` (parses print-view); locate the table comment block in
gmail/mod.rs and change the status cells for `list_messages` and `get_message`
to reflect "working" (matching the implemented functions in reads.rs).
In `@app/src-tauri/src/gmail/reads.rs`:
- Around line 249-274: is_system_label currently matches only English label
names so localized Gmail labels get misclassified; update the detector to prefer
canonical/localization-agnostic identifiers: modify the is_system_label logic to
first look up a stable DOM attribute (e.g., data-label-id or other canonical id
exposed by Gmail) from the label element and treat known canonical ids as system
labels, and if that attribute is absent fall back to the existing English-name
matches (function name: is_system_label). If DOM attributes aren’t available,
implement a locale-aware mapping table (e.g., a HashSet per locale) and consult
it before falling back to English names, and add comments/tests around both code
paths.
- Around line 24-25: The debug log in list_labels (and other Gmail ops)
currently emits raw account_id and other identifiers; replace these with
non-sensitive messages and a redaction helper (e.g., add a redact_identifier or
redact_gmail_id function in the gmail module) and use it wherever account_id,
label names, message IDs, or similar fields are logged (for example, in
list_labels and any functions returning Vec<GmailLabel>) so logs only include
operation names and safe metadata like counts, not raw PII or IDs.
In `@app/src-tauri/src/gmail/session.rs`:
- Around line 69-72: The predicate passed to connect_and_attach_matching (the
closure using t.url.starts_with("https://mail.google.com/")) is too broad and
can attach to the wrong Gmail tab when multiple accounts are open; update that
predicate to require an account-specific signal (for example check the URL
contains the account index or fragment like "u=<index>" or another unique
identifier tied to account_id) or remove this fallback entirely so it never
matches a generic "https://mail.google.com/" URL; ensure the error message still
references account_id when mapping the error from connect_and_attach_matching.
- Around line 30-35: The debug log currently prints raw account-scoped values
(account_id, fragment, marker) which may expose PII; update the logging in the
gmail session (the log::debug call that references account_id, fragment, marker)
to redact or pseudonymize those fields before printing (e.g., replace with a
fixed "[REDACTED]" or a stable hash of account_id/fragment/marker), and ensure
you centralize the transformation (helper like redact_id or hash_identifier) so
the log call uses the transformed values rather than the raw variables.
---
Nitpick comments:
In `@app/src-tauri/src/webview_apis/router.rs`:
- Around line 121-127: The read_u32 function currently folds "missing" and
"invalid" into a single error; change it to return distinct errors for the three
edge cases by first checking params.get(key) and returning "missing param
'{key}'" if absent, then checking value.as_u64() and returning "invalid type for
'{key}': expected unsigned integer" if the value isn't numeric, and finally
attempting u32::try_from(n) and returning "out of range for '{key}': value {n}
does not fit in u32" on conversion failure; keep the function name read_u32 and
mirror the validation wording used by require_u32 for consistency.
In `@src/openhuman/webview_apis/client.rs`:
- Around line 36-40: The global OnceLock<Client> (CLIENT) and client() factory
prevents replacing the Client in tests; change to a test-friendly injectable
abstraction: define a BridgeClient trait implemented by Client, replace the
static OnceLock<Client> with a concurrency-safe holder (e.g., a static
RwLock<Option<Box<dyn BridgeClient + Send + Sync>>> or OnceLock<Box<dyn
BridgeClient + Send + Sync>>) and update client() to return the trait
object/reference; add a set_test_client(boxed_client) helper to install a mock
in tests and ensure production initialization still uses Client::new when none
is set. Ensure all uses of CLIENT and client() are updated to the trait type so
tests can inject mocks while production uses the real Client implementation.
In `@src/openhuman/webview_apis/rpc.rs`:
- Around line 119-133: The current require_u32 allows values up to u32::MAX
which is dangerous for params like "limit"; change the validation to enforce a
practical operational cap: introduce a constant (e.g., OPERATIONAL_MAX_LIMIT =
1000) and in require_u32 (or a new helper require_limited_u32) check after
parsing the u64 that the value <= OPERATIONAL_MAX_LIMIT and return an Err with a
clear message (e.g., "invalid 'limit': exceeds allowed maximum of
{OPERATIONAL_MAX_LIMIT}") when it exceeds; ensure callers that pass the "limit"
key use this capped validation or switch to the new helper.
In `@src/openhuman/webview_apis/schemas.rs`:
- Around line 197-209: Replace the silent fallback in the match that returns a
ControllerSchema with namespace "webview_apis" and function "unknown" by
panicking when an unrecognized function name is encountered: locate the match
arm handling _other in the schemas() mapping (the branch that constructs
ControllerSchema with "unknown"), and change it to panic with a clear message
that includes the unexpected function name so typos are caught early (e.g.,
panic!("Unknown webview_apis controller function: {}", func_name)); keep
ControllerSchema/FieldSchema types referenced for context.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1725119d-6f97-43c7-b02c-8634fee5f820
📒 Files selected for processing (12)
app/src-tauri/Cargo.tomlapp/src-tauri/src/gmail/mod.rsapp/src-tauri/src/gmail/reads.rsapp/src-tauri/src/gmail/session.rsapp/src-tauri/src/gmail/types.rsapp/src-tauri/src/lib.rsapp/src-tauri/src/webview_apis/router.rssrc/core/all.rssrc/openhuman/webview_apis/client.rssrc/openhuman/webview_apis/mod.rssrc/openhuman/webview_apis/rpc.rssrc/openhuman/webview_apis/schemas.rs
✅ Files skipped from review due to trivial changes (1)
- app/src-tauri/src/gmail/types.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/openhuman/webview_apis/mod.rs
- app/src-tauri/Cargo.toml
Comment on lines
+172
to
+240
| #[tauri::command] | ||
| pub async fn gmail_list_labels(account_id: String) -> Result<Vec<GmailLabel>, String> { | ||
| log::debug!("[gmail][tauri] gmail_list_labels account_id={account_id}"); | ||
| cdp_list_labels(&account_id).await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn gmail_list_messages( | ||
| account_id: String, | ||
| limit: u32, | ||
| label: Option<String>, | ||
| ) -> Result<Vec<GmailMessage>, String> { | ||
| log::debug!( | ||
| "[gmail][tauri] gmail_list_messages account_id={account_id} limit={limit} label={label:?}" | ||
| ); | ||
| cdp_list_messages(&account_id, limit, label).await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn gmail_search( | ||
| account_id: String, | ||
| query: String, | ||
| limit: u32, | ||
| ) -> Result<Vec<GmailMessage>, String> { | ||
| log::debug!( | ||
| "[gmail][tauri] gmail_search account_id={account_id} query_len={} limit={limit}", | ||
| query.len() | ||
| ); | ||
| cdp_search(&account_id, query, limit).await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn gmail_get_message( | ||
| account_id: String, | ||
| message_id: String, | ||
| ) -> Result<GmailMessage, String> { | ||
| log::debug!("[gmail][tauri] gmail_get_message account_id={account_id} message_id={message_id}"); | ||
| cdp_get_message(&account_id, message_id).await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn gmail_send(account_id: String, request: GmailSendRequest) -> Result<SendAck, String> { | ||
| log::debug!( | ||
| "[gmail][tauri] gmail_send account_id={account_id} to={} cc={} bcc={} body_len={}", | ||
| request.to.len(), | ||
| request.cc.len(), | ||
| request.bcc.len(), | ||
| request.body.len() | ||
| ); | ||
| cdp_send(&account_id, request).await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn gmail_trash(account_id: String, message_id: String) -> Result<Ack, String> { | ||
| log::debug!("[gmail][tauri] gmail_trash account_id={account_id} message_id={message_id}"); | ||
| cdp_trash(&account_id, message_id).await | ||
| } | ||
|
|
||
| #[tauri::command] | ||
| pub async fn gmail_add_label( | ||
| account_id: String, | ||
| message_id: String, | ||
| label: String, | ||
| ) -> Result<Ack, String> { | ||
| log::debug!( | ||
| "[gmail][tauri] gmail_add_label account_id={account_id} message_id={message_id} label={label}" | ||
| ); | ||
| cdp_add_label(&account_id, message_id, label).await | ||
| } |
Contributor
There was a problem hiding this comment.
Tauri command logs expose raw identifiers.
Similar to the reads.rs logging issue, the #[tauri::command] handlers log raw account_id, message_id, and label values. Consider using the same redaction approach recommended for the session/reads modules.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src-tauri/src/gmail/mod.rs` around lines 172 - 240, Logs in the Tauri
command handlers leak raw identifiers; update each handler (gmail_list_labels,
gmail_list_messages, gmail_search, gmail_get_message, gmail_send, gmail_trash,
gmail_add_label) to log redacted versions of account_id, message_id, label,
etc., using the same redaction helper used in the session/reads modules (call it
the project's redact helper, e.g., redact_identifier/redact_string) instead of
interpolating raw values, and for request bodies/loggable collections keep the
existing length logging (e.g., body.len(), to.len()) while redacting their
contents.
Comment on lines
+21
to
+98
| pub fn handle_gmail_list_labels(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| let labels: Vec<GmailLabel> = client::request("gmail.list_labels", params).await?; | ||
| finish(RpcOutcome::single_log( | ||
| labels, | ||
| "[webview_apis] gmail_list_labels ok", | ||
| )) | ||
| }) | ||
| } | ||
|
|
||
| pub fn handle_gmail_list_messages(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| require_u32(¶ms, "limit")?; | ||
| let messages: Vec<GmailMessage> = client::request("gmail.list_messages", params).await?; | ||
| finish(RpcOutcome::single_log( | ||
| messages, | ||
| "[webview_apis] gmail_list_messages ok", | ||
| )) | ||
| }) | ||
| } | ||
|
|
||
| pub fn handle_gmail_search(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| require_string(¶ms, "query")?; | ||
| require_u32(¶ms, "limit")?; | ||
| let messages: Vec<GmailMessage> = client::request("gmail.search", params).await?; | ||
| finish(RpcOutcome::single_log( | ||
| messages, | ||
| "[webview_apis] gmail_search ok", | ||
| )) | ||
| }) | ||
| } | ||
|
|
||
| pub fn handle_gmail_get_message(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| require_string(¶ms, "message_id")?; | ||
| let msg: GmailMessage = client::request("gmail.get_message", params).await?; | ||
| finish(RpcOutcome::single_log( | ||
| msg, | ||
| "[webview_apis] gmail_get_message ok", | ||
| )) | ||
| }) | ||
| } | ||
|
|
||
| pub fn handle_gmail_send(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| let _: GmailSendRequest = read_required(¶ms, "request")?; | ||
| let ack: SendAck = client::request("gmail.send", params).await?; | ||
| finish(RpcOutcome::single_log(ack, "[webview_apis] gmail_send ok")) | ||
| }) | ||
| } | ||
|
|
||
| pub fn handle_gmail_trash(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| require_string(¶ms, "message_id")?; | ||
| let ack: Ack = client::request("gmail.trash", params).await?; | ||
| finish(RpcOutcome::single_log(ack, "[webview_apis] gmail_trash ok")) | ||
| }) | ||
| } | ||
|
|
||
| pub fn handle_gmail_add_label(params: Map<String, Value>) -> ControllerFuture { | ||
| Box::pin(async move { | ||
| require_string(¶ms, "account_id")?; | ||
| require_string(¶ms, "message_id")?; | ||
| require_string(¶ms, "label")?; | ||
| let ack: Ack = client::request("gmail.add_label", params).await?; | ||
| finish(RpcOutcome::single_log( | ||
| ack, | ||
| "[webview_apis] gmail_add_label ok", | ||
| )) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Add structured entry/error logging around each bridge operation.
These handlers currently emit success logs only; validation failures and client::request(...) failures return without local diagnostic context. Add explicit entry/exit and error-path logs (with stable prefixes + correlation fields like method/op, not raw user data) at this boundary.
Proposed pattern
+const LOG_PREFIX: &str = "[webview_apis][rpc]";
+
pub fn handle_gmail_list_labels(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
+ let op = "gmail_list_labels";
+ tracing::debug!("{LOG_PREFIX} entry op={op}");
require_string(¶ms, "account_id")?;
- let labels: Vec<GmailLabel> = client::request("gmail.list_labels", params).await?;
+ let labels: Vec<GmailLabel> = client::request("gmail.list_labels", params)
+ .await
+ .map_err(|e| {
+ tracing::warn!("{LOG_PREFIX} bridge_error op={op} err={e}");
+ e
+ })?;
+ tracing::debug!("{LOG_PREFIX} exit op={op} status=ok");
finish(RpcOutcome::single_log(
labels,
"[webview_apis] gmail_list_labels ok",
))
})
}As per coding guidelines: **/*.{rs,ts,tsx} must include substantial development-oriented logging at entry/exit, branches, external calls, and error paths; and src/**/*.rs logs must use stable grep-friendly prefixes with correlation fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/webview_apis/rpc.rs` around lines 21 - 98, Handlers like
handle_gmail_list_labels / handle_gmail_list_messages / handle_gmail_search /
handle_gmail_get_message / handle_gmail_send / handle_gmail_trash /
handle_gmail_add_label lack structured entry/exit and error logging, so add
stable, grep-friendly logs at the start and just before returning success (e.g.
"[webview_apis][gmail][entry]" and "[webview_apis][gmail][exit]") and explicit
error-path logs when validation (require_string/require_u32/read_required) or
client::request(...) fails; include correlation fields such as method/op and a
request_id/correlation_id (but do not log raw user data like
account_id/message_id values), ensure logs use the project logging macro/utility
used elsewhere (structured key/value fields) and include the operation name
(e.g., gmail_list_messages) and error details in error logs to aid debugging.
5 tasks
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
1 participant
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
webview_apisWebSocket bridge between the core sidecar (client) and the Tauri shell (server) on127.0.0.1:$OPENHUMAN_WEBVIEW_APIS_PORT, soopenhuman.webview_apis_*JSON-RPC methods from the core reach live-webview CDP operations in the Tauri shell.list_labelsworking end-to-end (CDPDOMSnapshot.captureSnapshotvia the user's logged-in session);list_messages/search/get_message/send/trash/add_labelscaffolded with structured "not implemented" errors so the API surface is visible now.cdp_*helper inapp/src-tauri/src/gmail/, shared between a#[tauri::command](frontend path) and the bridge router (core path). Future connectors (Notion, Slack, …) drop in the same way.OPENHUMAN_DEV_AUTO_GMAIL=<account-id>opens the Gmail webview on boot socurlagainst the core RPC port hits a live CDP target without clicking through the UI.tests/webview_apis_bridge.rsspins up a mock WS server in-process and round-tripsGmailLabel+ error responses through the real core client.Problem
We want a debuggable, typed API layer on top of logged-in webviews (data-connect-style), callable both from curl/JSON-RPC today and from the agent tomorrow. Gmail is the first target. Constraints:
Network.*MITM,Input.*).core_rpc_relay/ HTTP), but core → Tauri did not exist.Solution
New
webview_apismodule on both sides sharing a JSON-over-WS envelope:Port coordination: Tauri binds first (
OPENHUMAN_WEBVIEW_APIS_PORTenv override, else OS-assigned), then sets the env var before spawning core. Core reads it lazily on first request.session::attachuses a belt-and-braces match: fragment OR placeholder title ORmail.google.com/*URL prefix. Gmail rewrites URL fragments to#inbox/#search/…as soon as it routes, so the third fallback is load-bearing.Submission Checklist
cargo testcoversgmail::reads::parse_aria_label, router param validation, controller registry wiring, client envelope types.tests/webview_apis_bridge.rsboots a mock WS server in-process and round-trips through the realclient::request.OPENHUMAN_DEV_AUTO_GMAIL=gmail-devagainst a live logged-in Gmail session.//!////covering purpose and consumer path.Impact
feature = "cef"and return a clear error on wry builds (wry has no remote-debugging port). No change to the wry code path.OPENHUMAN_WEBVIEW_APIS_PORT(OS-picked by default). No external surface.127.0.0.1only, and the core connects using the inherited env var. Both processes already trust each other via the existingcore_rpc_relaychannel, so this matches the current trust boundary.tokio-tungsteniteserver feature (handshake) added toapp/src-tauri. Core'stokio-tungstenitealready had full features.list_messages/search/get_messageviaNetwork.responseReceived+getResponseBody; write ops viaInput.*; genericConnectortrait after a second connector lands.Related
Summary by CodeRabbit
New Features
Tests