chore(gui-client/linux): export all logs, not just app logs #4830
Conversation
…dev.firezone.client`
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
github-actions
bot
added
the
kind/chore
Terraform Cloud Plan Output |
Performance Test ResultsTCP
UDP
|
| # Allow users in `firezone` group to delete log files | ||
| LogsDirectoryMode=775 |
There was a problem hiding this comment.
This would allow users in firezone write bogus log files. I think the threat from that is small, and the only alternative I had in mind is issuing a "ClearLogs" command to the IPC service and waiting for a response, which would add a lot of complexity to both the IPC protocol and the log clearing code.
So we could do that later if we really need.
But since the GUI decides what goes into the zip file in the end, it may not even matter if it can modify the original logs.
There was a problem hiding this comment.
but we don't only use those for the zip right? also for streaming them upstream
There was a problem hiding this comment.
Yeah, if we re-enable log uploading.
Depending how tight we want the security sandbox, I was thinking the tunnel process shouldn't be allowed to read the user's files, so it would be safer to let the user read the tunnel's files.
|
|
||
| #[cfg(not(debug_assertions))] | ||
| /// Does nothing in release mode | ||
| pub async fn fault_msleep(&self, _millis: u64) {} |
There was a problem hiding this comment.
The fault injection tests weren't used anywhere. I really like the idea but the code is simpler without them. If anyone has a slow hard disk and runs into issues with that, we can deal with it then.
| ctlr_tx | ||
| .send(ControllerRequest::ClearLogs) | ||
| .await | ||
| .context("Failed to send ClearLogs request")?; |
There was a problem hiding this comment.
The results are not checked anywhere, but the code is exercised.
| Ok(()) | ||
| } | ||
|
|
||
| /// Pops up the "Save File" dialog | ||
| pub(crate) fn export_logs_inner(ctlr_tx: CtlrTx) -> Result<()> { | ||
| let now = chrono::Local::now(); | ||
| let datetime_string = now.format("%Y_%m_%d-%H-%M"); | ||
| let stem = PathBuf::from(format!("connlib-{datetime_string}")); | ||
| let stem = PathBuf::from(format!("firezone_logs_{datetime_string}")); |
There was a problem hiding this comment.
Changed the default zip name to match the macOS Client.
I could also make it default to the Documents folder if that's worth it for UX.
| @@ -154,7 +154,7 @@ async function clearLogs() { | |||
| console.error(e); | |||
| }) | |||
| .finally(() => { | |||
| logCountOutput.innerText = "0 files, 0 MB"; | |||
| countLogs(); | |||
There was a problem hiding this comment.
If the permissions are wrong, the GUI may fail to clear all the logs, but it won't show an error because there's no error handling in this part of the code. In that case, I want the display to be accurate.
| # Allow users in `firezone` group to delete log files | ||
| LogsDirectoryMode=775 |
There was a problem hiding this comment.
but we don't only use those for the zip right? also for streaming them upstream
17ac1eb looks good on both Linux and Windows
Before merging