-
Notifications
You must be signed in to change notification settings - Fork 904
Button and api route for uploading logs #1221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
use crate::{ArcLock, web_api::ManagerExt}; | ||
use std::{fs, path::PathBuf}; | ||
use tauri::{AppHandle, Manager}; | ||
|
||
async fn get_latest_log_file(app: &AppHandle) -> Option<PathBuf> { | ||
let logs_dir = app | ||
.state::<ArcLock<crate::App>>() | ||
.read() | ||
.await | ||
.logs_dir | ||
.clone(); | ||
|
||
let entries = fs::read_dir(&logs_dir).ok()?; | ||
let mut log_files: Vec<_> = entries | ||
.filter_map(|entry| { | ||
let entry = entry.ok()?; | ||
let path = entry.path(); | ||
if path.is_file() && path.file_name()?.to_str()?.contains("cap-desktop.log") { | ||
let metadata = fs::metadata(&path).ok()?; | ||
let modified = metadata.modified().ok()?; | ||
Some((path, modified)) | ||
} else { | ||
None | ||
} | ||
}) | ||
.collect(); | ||
|
||
log_files.sort_by(|a, b| b.1.cmp(&a.1)); | ||
log_files.first().map(|(path, _)| path.clone()) | ||
} | ||
|
||
pub async fn upload_log_file(app: &AppHandle) -> Result<(), String> { | ||
let log_file = get_latest_log_file(app).await.ok_or("No log file found")?; | ||
|
||
let metadata = | ||
fs::metadata(&log_file).map_err(|e| format!("Failed to read log file metadata: {}", e))?; | ||
let file_size = metadata.len(); | ||
|
||
const MAX_SIZE: u64 = 1 * 1024 * 1024; | ||
|
||
let log_content = if file_size > MAX_SIZE { | ||
let content = | ||
fs::read_to_string(&log_file).map_err(|e| format!("Failed to read log file: {}", e))?; | ||
|
||
let header = format!( | ||
"⚠️ Log file truncated (original size: {} bytes, showing last ~1MB)\n\n", | ||
file_size | ||
); | ||
let max_content_size = (MAX_SIZE as usize) - header.len(); | ||
|
||
if content.len() > max_content_size { | ||
let start_pos = content.len() - max_content_size; | ||
let truncated = &content[start_pos..]; | ||
if let Some(newline_pos) = truncated.find('\n') { | ||
format!("{}{}", header, &truncated[newline_pos + 1..]) | ||
} else { | ||
format!("{}{}", header, truncated) | ||
} | ||
} else { | ||
content | ||
} | ||
} else { | ||
fs::read_to_string(&log_file).map_err(|e| format!("Failed to read log file: {}", e))? | ||
}; | ||
|
||
let form = reqwest::multipart::Form::new() | ||
.text("log", log_content) | ||
.text("os", std::env::consts::OS) | ||
.text("version", env!("CARGO_PKG_VERSION")); | ||
|
||
let response = app | ||
.api_request("/api/desktop/logs", |client, url| { | ||
client.post(url).multipart(form) | ||
}) | ||
.await | ||
.map_err(|e| format!("Failed to upload logs: {}", e))?; | ||
|
||
if !response.status().is_success() { | ||
return Err(format!("Upload failed with status: {}", response.status())); | ||
} | ||
|
||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,31 +39,35 @@ impl From<String> for AuthedApiError { | |
} | ||
} | ||
|
||
fn apply_env_headers(req: reqwest::RequestBuilder) -> reqwest::RequestBuilder { | ||
let mut req = req.header("X-Cap-Desktop-Version", env!("CARGO_PKG_VERSION")); | ||
|
||
if let Ok(s) = std::env::var("VITE_VERCEL_AUTOMATION_BYPASS_SECRET") { | ||
req = req.header("x-vercel-protection-bypass", s); | ||
} | ||
|
||
req | ||
} | ||
Comment on lines
+42
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainScope the Vercel bypass header to trusted hosts only. Currently, x-vercel-protection-bypass is added to every outbound request if the env var is set. This risks leaking the secret to arbitrary hosts if server_url is misconfigured or user-controlled. Gate the header by hostname (e.g., only *.vercel.app) or an allowlist, and pass the URL to the helper so it can decide. Proposed minimal change: -fn apply_env_headers(req: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
+fn apply_env_headers(req: reqwest::RequestBuilder, base_url: &str) -> reqwest::RequestBuilder {
let mut req = req.header("X-Cap-Desktop-Version", env!("CARGO_PKG_VERSION"));
- if let Ok(s) = std::env::var("VITE_VERCEL_AUTOMATION_BYPASS_SECRET") {
- req = req.header("x-vercel-protection-bypass", s);
- }
+ if let Ok(s) = std::env::var("VITE_VERCEL_AUTOMATION_BYPASS_SECRET") {
+ if let Ok(u) = reqwest::Url::parse(base_url) {
+ if matches!(u.host_str(), Some(h) if h.ends_with(".vercel.app")) {
+ req = req.header("x-vercel-protection-bypass", s);
+ }
+ }
+ }
req
}
@@
- apply_env_headers(req).send().await
+ apply_env_headers(req, &url).send().await
@@
- apply_env_headers(build(client, url)).send().await
+ apply_env_headers(build(client.clone(), url.clone()), &url).send().await Please confirm the exact trusted hostnames/allowlist you want (prod, staging). Based on learnings. Also applies to: 70-71, 112-121 I'll verify the security concern and the proposed fix by examining the actual codebase.
Length of output: 2923 🏁 Script executed: #!/bin/bash
# Find all occurrences of apply_env_headers call sites
rg -n "apply_env_headers" apps/desktop/src-tauri/src/web_api.rs -A 2 -B 2 Length of output: 384 🏁 Script executed: #!/bin/bash
# Understand function signatures and URL availability
rg -n "fn apply_env_headers|\.send\(\)" apps/desktop/src-tauri/src/web_api.rs -B 5 -A 5 Length of output: 1227 Add hostname validation to The security concern is valid: the Vercel bypass header is currently added to every outbound request without verifying the destination host. If The proposed fix is feasible—both call sites (lines 70 and 120) have access to the URL and can pass it as a parameter. You should:
Clarify which hostnames should be trusted (the proposal suggests |
||
|
||
async fn do_authed_request( | ||
auth: &AuthStore, | ||
build: impl FnOnce(reqwest::Client, String) -> reqwest::RequestBuilder, | ||
url: String, | ||
) -> Result<reqwest::Response, reqwest::Error> { | ||
let client = reqwest::Client::new(); | ||
|
||
let mut req = build(client, url) | ||
.header( | ||
"Authorization", | ||
format!( | ||
"Bearer {}", | ||
match &auth.secret { | ||
AuthSecret::ApiKey { api_key } => api_key, | ||
AuthSecret::Session { token, .. } => token, | ||
} | ||
), | ||
) | ||
.header("X-Cap-Desktop-Version", env!("CARGO_PKG_VERSION")); | ||
|
||
if let Ok(s) = std::env::var("VITE_VERCEL_AUTOMATION_BYPASS_SECRET") { | ||
req = req.header("x-vercel-protection-bypass", s); | ||
} | ||
let req = build(client, url).header( | ||
"Authorization", | ||
format!( | ||
"Bearer {}", | ||
match &auth.secret { | ||
AuthSecret::ApiKey { api_key } => api_key, | ||
AuthSecret::Session { token, .. } => token, | ||
} | ||
), | ||
); | ||
|
||
req.send().await | ||
apply_env_headers(req).send().await | ||
} | ||
|
||
pub trait ManagerExt<R: Runtime>: Manager<R> { | ||
|
@@ -73,6 +77,12 @@ pub trait ManagerExt<R: Runtime>: Manager<R> { | |
build: impl FnOnce(reqwest::Client, String) -> reqwest::RequestBuilder, | ||
) -> Result<reqwest::Response, AuthedApiError>; | ||
|
||
async fn api_request( | ||
&self, | ||
path: impl Into<String>, | ||
build: impl FnOnce(reqwest::Client, String) -> reqwest::RequestBuilder, | ||
) -> Result<reqwest::Response, reqwest::Error>; | ||
|
||
async fn make_app_url(&self, pathname: impl AsRef<str>) -> String; | ||
} | ||
|
||
|
@@ -99,6 +109,17 @@ impl<T: Manager<R> + Emitter<R>, R: Runtime> ManagerExt<R> for T { | |
Ok(response) | ||
} | ||
|
||
async fn api_request( | ||
&self, | ||
path: impl Into<String>, | ||
build: impl FnOnce(reqwest::Client, String) -> reqwest::RequestBuilder, | ||
) -> Result<reqwest::Response, reqwest::Error> { | ||
let url = self.make_app_url(path.into()).await; | ||
let client = reqwest::Client::new(); | ||
|
||
apply_env_headers(build(client, url)).send().await | ||
} | ||
|
||
async fn make_app_url(&self, pathname: impl AsRef<str>) -> String { | ||
let app_state = self.state::<ArcLock<crate::App>>(); | ||
let server_url = &app_state.read().await.server_url; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redact secrets/PII and set a request timeout.
Logs may contain tokens, API keys, emails, etc. Redact before upload. Also set a per-request timeout to avoid hanging.
Helper (place near top of module):
Note: adds regex + once_cell deps if not already present. Based on learnings.
Also applies to: 71-76