feat: add passwordless desktop authentication - #166
Merged
Conversation
Soulter
marked this pull request as ready for review
August 7, 2026 09:14
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src-tauri/src/backend/http.rs" line_range="279-284" />
<code_context>
Some(token)
}
+fn sanitize_http_header_value(value: &str) -> Option<&str> {
+ if value.is_empty() || value.contains('\r') || value.contains('\n') {
+ return None;
+ }
+ Some(value)
+}
+
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Tighten header value sanitization to reject broader control characters and invalid bytes
This currently only rejects empties and CR/LF. Since this value is written directly into an HTTP header, it would be safer to also reject other control characters (0x00–0x1F, 0x7F) and possibly non-ASCII bytes to avoid header parsing or injection issues. You could either enforce the same constraints as `DesktopSessionSecret` (e.g., `[0-9a-f]`) or require `value.chars().all(|c| c.is_ascii_graphic())`, which also makes this helper safer for future reuse.
```suggestion
fn sanitize_http_header_value(value: &str) -> Option<&str> {
if value.is_empty() {
return None;
}
// Only allow printable ASCII (no control characters, no spaces, no non-ASCII)
if !value.chars().all(|c| c.is_ascii_graphic()) {
return None;
}
Some(value)
}
```
</issue_to_address>
### Comment 2
<location path="src-tauri/src/bridge_bootstrap.js" line_range="186-189" />
<code_context>
locale: value,
});
+ let desktopAuthRefreshPromise = null;
+ const refreshDesktopAuthSession = () => {
+ if (desktopAuthRefreshPromise) {
+ return desktopAuthRefreshPromise;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle rejected bridge invocations to avoid unhandled promise rejections in refreshDesktopAuthSession
The async IIFE inside `refreshDesktopAuthSession` awaits `invokeBridge(BRIDGE_COMMANDS.GET_AUTH_TOKEN)` without a try/catch. If `invokeBridge` rejects, `desktopAuthRefreshPromise` will reject and `void refreshDesktopAuthSession()` callers will cause unhandled promise rejections, which is noisy given this runs at startup and on a timer.
Please wrap the `invokeBridge` call in a try/catch and return a normalized `{ ok: false, reason: '...' }` on failure (logging via `devWarn`), so the refresh loop and lifecycle hooks stay resilient even when the bridge layer fails.
</issue_to_address>
### Comment 3
<location path="src-tauri/src/bridge/commands.rs" line_range="216-225" />
<code_context>
state.bridge_state(&app_handle)
}
+#[tauri::command]
+pub(crate) async fn desktop_bridge_get_auth_token(
+ app_handle: AppHandle,
+) -> DesktopAuthBridgeResult {
+ let task_result = tauri::async_runtime::spawn_blocking(move || {
+ let state = app_handle.state::<BackendState>();
+ state.request_desktop_auth_session()
+ })
+ .await;
+
+ match task_result {
+ Ok(Some(session)) => DesktopAuthBridgeResult {
+ ok: true,
+ token: Some(session.token),
+ username: Some(session.username),
+ reason: None,
+ },
+ Ok(None) => DesktopAuthBridgeResult {
+ ok: false,
+ token: None,
+ username: None,
+ reason: Some("Desktop passwordless authentication is unavailable.".to_string()),
+ },
+ Err(_) => DesktopAuthBridgeResult {
+ ok: false,
+ token: None,
</code_context>
<issue_to_address>
**suggestion:** Differentiate and log spawn_blocking failures for the desktop auth task
The `Err(_)` arm currently hides all `spawn_blocking` failures behind a generic response and no logging, which impairs production debugging. Please log the error (using the contents of the `Err`) and, ideally, distinguish between panic vs join/runtime errors while keeping the bridge response stable for the frontend.
Suggested implementation:
```rust
Ok(None) => DesktopAuthBridgeResult {
ok: false,
token: None,
username: None,
reason: Some("Desktop passwordless authentication is unavailable.".to_string()),
},
Err(join_err) => {
// Differentiate and log spawn_blocking failures while keeping a stable bridge response.
if join_err.is_panic() {
log::error!(
"desktop_bridge_get_auth_token: desktop auth task panicked: {:?}",
join_err
);
} else {
log::error!(
"desktop_bridge_get_auth_token: desktop auth task failed to complete: {:?}",
join_err
);
}
DesktopAuthBridgeResult {
ok: false,
token: None,
username: None,
reason: Some("Desktop authentication task failed.".to_string()),
}
},
}
```
1. Ensure that a suitable logging macro is in scope:
- If this file (or a common prelude) does *not* already import it, add `use log::error;` at the top and replace `log::error!` with `error!`, or
- If the project uses `tracing`, switch the calls to `tracing::error!` (or `error!` if that macro is already in scope).
2. If you prefer using `append_desktop_log` for user-facing desktop logs, you can supplement the `log::error!` calls with something like:
```rust
append_desktop_log(
&app_handle,
"desktop_bridge_get_auth_token: desktop auth task panicked; see logs for details",
);
```
Adjust the exact signature and usage to match how `append_desktop_log` is used elsewhere in this file.
</issue_to_address>
### Comment 4
<location path="src-tauri/src/backend/http.rs" line_range="15" />
<code_context>
+ BackendState, DESKTOP_AUTH_REQUEST_TIMEOUT_MS, GRACEFUL_RESTART_START_TIME_TIMEOUT_MS,
+};
+
+#[derive(Default)]
+struct BackendRequestOptions<'a> {
+ auth_token: Option<&'a str>,
</code_context>
<issue_to_address>
**issue (complexity):** Consider inlining the new desktop-auth options into the existing `request_backend_response_bytes` function signature instead of using an internal variant and `BackendRequestOptions` struct to reduce indirection and lifetimes.
You can keep the new desktop-auth behavior while simplifying the indirection and lifetime plumbing by folding `BackendRequestOptions` back into the main function and using explicit parameters.
### 1. Remove `BackendRequestOptions` and `_internal` variant
Replace the internal function + options struct with an extended signature on the original function:
```rust
pub(crate) fn request_backend_response_bytes(
&self,
method: &str,
api_path: &str,
timeout_ms: u64,
body: Option<&str>,
auth_token: Option<&str>,
desktop_session_secret: Option<&str>,
require_loopback: bool,
) -> Option<Vec<u8>> {
let base = Url::parse(&self.backend_url).ok()?;
let request_url = base.join(api_path).ok()?;
if request_url.scheme() != "http" {
return None;
}
let host = request_url.host_str()?;
let port = request_url.port_or_known_default().unwrap_or(80);
let timeout = Duration::from_millis(timeout_ms.max(50));
let addrs = (host, port).to_socket_addrs().ok()?;
let mut stream = addrs
.into_iter()
.find_map(|address| {
if require_loopback && !is_loopback_socket_address(&address) {
return None;
}
TcpStream::connect_timeout(&address, timeout).ok()
})?;
let _ = stream.set_read_timeout(Some(timeout));
let _ = stream.set_write_timeout(Some(timeout));
let mut request_target = request_url.path().to_string();
if let Some(query) = request_url.query() {
request_target.push('?');
request_target.push_str(query);
}
if request_target.is_empty() {
request_target = "/".to_string();
}
let payload = body.unwrap_or("");
let authorization_header = auth_token
.and_then(sanitize_authorization_token)
.map(|token| format!("Authorization: Bearer {token}\r\n"))
.unwrap_or_default();
let desktop_session_header = desktop_session_secret
.and_then(sanitize_http_header_value)
.map(|secret| format!("{DESKTOP_SESSION_HEADER}: {secret}\r\n"))
.unwrap_or_default();
let request = format!(
"{method} {request_target} HTTP/1.1\r\n\
Host: {host}\r\n\
Accept: application/json\r\n\
Accept-Encoding: identity\r\n\
Connection: close\r\n\
{authorization_header}\
{desktop_session_header}\
Content-Type: application/json\r\n\
Content-Length: {}\r\n\
\r\n\
{}",
payload.len(),
payload
);
if stream.write_all(request.as_bytes()).is_err() {
return None;
}
read_http_response_bytes(&mut stream)
}
```
This removes:
- the lifetime-bearing `BackendRequestOptions<'a>`
- the public + internal function split
- the `..BackendRequestOptions::default()` ceremony at callsites
while preserving the loopback filter and header logic.
### 2. Keep existing callers simple
Existing usages can remain almost unchanged by passing the new parameters with obvious defaults:
```rust
// old behavior: no desktop session, no loopback restriction
let payload = self.request_backend_json(
"GET",
"/api/stat/start-time",
GRACEFUL_RESTART_START_TIME_TIMEOUT_MS,
None,
None,
)?;
```
Assuming `request_backend_json` delegates to `request_backend_response_bytes`, update that delegation only:
```rust
fn request_backend_json(
&self,
method: &str,
api_path: &str,
timeout_ms: u64,
body: Option<&str>,
auth_token: Option<&str>,
) -> Option<serde_json::Value> {
let response = self.request_backend_response_bytes(
method,
api_path,
timeout_ms,
body,
auth_token,
None, // desktop_session_secret
false, // require_loopback
)?;
http_response::parse_http_json_response(&response)
}
```
Callers that don’t care about the new behavior still pass only `auth_token` at their level; the extra parameters are centralized in this helper.
### 3. Use the extended function in `request_desktop_auth_session`
Your desktop session callsite becomes straightforward, without an options object:
```rust
pub(crate) fn request_desktop_auth_session(&self) -> Option<DesktopAuthSession> {
let response = self.request_backend_response_bytes(
"POST",
DESKTOP_SESSION_ENDPOINT,
DESKTOP_AUTH_REQUEST_TIMEOUT_MS,
Some("{}"),
None, // no Authorization header here
Some(self.desktop_session_secret.as_str()),
true, // require_loopback
)?;
let payload = http_response::parse_http_json_response(&response)?;
parse_desktop_auth_session(&payload)
}
```
This keeps:
- loopback-only connections for desktop sessions
- the new `DESKTOP_SESSION_HEADER`
- all the new tests and helper functions (`is_loopback_socket_address`, `sanitize_http_header_value`, `parse_desktop_auth_session`)
but removes the extra abstraction and lifetimes, making the API surface and callsites simpler while retaining all functionality.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
Why
Desktop users can otherwise be blocked by the random first-run dashboard password before logs are accessible, or by an expired JWT after the password has been forgotten. The desktop process already controls the local backend and can establish a narrowly scoped local session.
Backend dependency
Requires AstrBotDevs/AstrBot#9585.
Security boundaries
Validation
cargo fmt --checkcargo test -- --test-threads=1(162 passed)cargo clippy --all-targets -- -D warningsuv run --project /Users/moonshot/AstrBot-1 pnpm run test:prepare-resources(128 passed)Summary by Sourcery
Introduce a passwordless desktop authentication flow that uses a per-process secret and loopback-only backend session endpoint, wiring it through the native bridge, backend launcher, and WebUI bootstrap while preserving existing password login as a fallback.
New Features:
Enhancements:
Build:
Documentation:
Tests: