feat(ui): Add delete button to session history list view

[alexhayes-block]

Overview

This PR adds a delete button feature to the session history list view, allowing users to permanently delete session history files directly from the UI. This improves user experience by providing a way to manage and clean up session history without having to manually delete files from the filesystem.

If you delete an active Goose session the app for that session doesn't become corrupted (or the app for that session break or anything like that) but unless you create another message in the session, it's lost. If you add another message in the active session after deleting it, the full session is saved again on the file system.

Changes

• Added a delete button to each session item in the history list
• Implemented a confirmation modal with appropriate styling to prevent accidental deletions
• Added loading state during deletion for better user feedback
• Implemented success/error notifications to confirm deletion status
• Added secure file deletion with validation checks to prevent path traversal attacks
• Created IPC handlers for communication between the UI and Electron main process

Screenshots

Session History Screen

Confirmation Dialog

Success Toast

Testing

• Tested session deletion functionality with various session files
• Verified that the confirmation modal appears when the delete button is clicked
• Confirmed that success notifications appear after successful deletion
• Verified that the session is removed from the list after deletion
• Tested what occurs if you delete an active session

Security Considerations

• Added validation for session ID format to prevent path traversal attacks
• Implemented checks to ensure deletion only occurs within the designated sessions directory
• Added proper error handling for all file operations

[alexhayes-block]

I chose to do this all in the UI but it maybe it should live in the goose crate, perhaps it belongs in crates/goose/src/session/storage.rs?

[zanesq]

@alexhayes-block yeah ideally it would be a feature in both cli and desktop also if possible and moving it to the backend would help with that. Also need to be extra careful with security, heres what copilot said:

Yes, there are a few important security considerations regarding these changes, especially related to file deletion via Electron IPC. Here’s a review of the main risks and mitigations:

---

## 1. **Path Traversal and File Deletion**

### **What’s Happening**
- You added an Electron IPC handler (`delete-session-file`) that deletes a file with a name based on a user-provided `sessionId`.
- There is a regex validation step: `/^\d{8}_\d{6}$/` (expects something like `YYYYMMDD_HHMMSS`).
- There’s also a path traversal check:
  ```js
  if (!resolvedPath.startsWith(resolvedSessionDir + path.sep)) {
      // ...
  }

Potential Issues

a. Path Traversal:

• The regex is strict and should prevent arbitrary file paths, but always be cautious with user input.
• The check resolvedPath.startsWith(resolvedSessionDir + path.sep) is good, but if resolvedSessionDir does not have a trailing slash, startsWith could match unintended paths (e.g., /tmp/foo vs /tmp/foobar).
  • The use of resolvedSessionDir + path.sep is correct, but ensure that resolvedSessionDir never ends with a slash already, or you could end up with a double slash (which may or may not matter depending on the platform).
• The check does not account for symbolic links inside sessionDir—a symlink could point outside the directory.

b. File Existence and Race Conditions:

• There is a TOCTOU (time-of-check to time-of-use) race between await fs.access(sessionFilePath); and await fs.unlink(sessionFilePath);. Malicious processes could swap the file between these calls, potentially causing unintended file deletion.
• Mitigation: Use fs.unlink directly and handle errors, or in node v14+, use fs.rm with {force: true} and avoid checking existence first.

---

2. IPC Exposure

• The IPC handler is exposed to the renderer process. If the renderer is ever compromised, an attacker can request deletion of any session file (but not arbitrary files, due to validation).
• This is a standard risk in Electron apps; always assume the renderer can be compromised and limit the IPC surface.

---

3. Other Considerations

• Logging: Logging the full file path on errors can leak sensitive user information (like home directory paths) to logs that may be sent to developers. Redact or sanitize logs if sensitive.
• Return Value: The function returns false on error, but the renderer code does not distinguish between "file didn’t exist" and "file couldn’t be deleted," which may be a minor UX or audit concern.
• Hardcoded Path: The session directory is hardcoded to ~/.local/share/goose/sessions. If the app ever migrates or supports multiple users/locations, this could become a maintenance/security issue.

---

4. General Review of the Frontend

• The frontend never exposes the sessionId directly other than via user actions, so the attack surface is limited to what a user can do in the UI or via a compromised renderer.
• No obvious XSS or privilege escalation issues from the frontend changes.

---

Summary of Recommendations

1. Path Traversal
   • The regex and startsWith check are good, but consider using path.relative to ensure the target file is a child of the session directory:

     const relative = path.relative(resolvedSessionDir, resolvedPath);
     if (relative.startsWith('..') || path.isAbsolute(relative)) {
         // Not a child
     }

2. Race Condition
   • Avoid fs.access before fs.unlink. Just fs.unlink and catch the error.
3. Symbolic Links
   • If possible, prevent storing symlinks in the session directory, or use fs.lstat and ensure the file is not a symlink.
4. IPC Surface
   • Restrict the IPC handler as much as possible; document and audit all IPC endpoints.
5. Logging
   • Avoid logging full file paths in production logs.

[zanesq]

@alexhayes-block would like to get this in can you pull main resolve conflicts and address the comments?

[DOsinga]

closes #3747

[zanesq]

related #3839

[michaelneale]

sorry @alexhayes-block looks like a bit more conflict resolving now as some bigger changes landed. I think it is valid to have something like this.

[taniandjerry]

@Abhijay007 wanted to create a new PR for this (mentioned on Discord), since this PR falls back on a much older version. Are they okay to take this up?

Abhijay mentioned that they don’t want to create duplicate efforts or add unnecessary friction for the dev team, and they're happy to give it a shot 🙂

They also mentioned we could add support for deleting individual chats, but some may also want the option to delete all chats at once. Should we consider including that too?

@DOsinga @michaelneale @alexhancock

[zanesq]

@alexhayes-block any objection to ^ or do you have time to wrap this up?

[michaelneale]

I believe @Abhijay007 was going to take a run at getting this one fixed up

[taniandjerry]

Thank you so much @Abhijay007 ! #4480

[alexhayes-block]

This has now been implemented in another PR.