✨ [desktop,core]: Add hostname to User-Agent for device identification#94
✨ [desktop,core]: Add hostname to User-Agent for device identification#94yuler wants to merge 3 commits into
Conversation
Include system hostname in the User-Agent header so devices can be identified during authorization. Update Session model to parse and display hostname in session summaries.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the collection and display of the local hostname in the desktop application's user agent. It adds a Rust command to retrieve the hostname, includes it in the user agent string generated in the frontend, and parses it in the backend to display in the session summary. The review feedback highlights three key areas for improvement: a regression in the backend user agent formatting when the client is missing, a potential frontend failure where a rejected hostname promise could discard successfully retrieved system info, and a robustness concern regarding non-ASCII or control characters in hostnames that could break HTTP headers.
- Fix user agent summary regression when client is not present - Handle get_hostname promise rejection individually - Sanitize hostname to safe ASCII characters
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds the system hostname to the desktop application's User-Agent string and updates the backend session model to parse and display it. The feedback focuses on standardizing the User-Agent format by enclosing the hostname within the comment parentheses (e.g., (platform; hostname)) rather than appending it to the end. This includes suggestions to sanitize parentheses from the hostname in Rust, adjust the User-Agent construction in TypeScript, update the parsing regex and exclude "Unknown" hostnames in Ruby, and align the test cases accordingly.
- Filter parentheses from hostname in Rust - Include hostname in comment parentheses (platform; hostname) - Update regex to parse new format, skip Unknown hostname - Update test to match new format
|
/ |
yuler
changed the title
There was a problem hiding this comment.
Code Review
This pull request adds the system hostname to the desktop app's User-Agent string and updates the Rails backend to parse and display it in the session summary. The changes include adding a new get_hostname Tauri command in Rust, updating the frontend API to fetch and append the hostname, and modifying the Session model and its tests in Ruby. The reviewer feedback suggests filtering out backslashes from the hostname in Rust to prevent malformed User-Agent headers, and using a case-insensitive comparison for the "Unknown" fallback string in Ruby.
| let sanitized: String = h | ||
| .to_string_lossy() | ||
| .chars() | ||
| .filter(|c| c.is_ascii() && !c.is_control() && *c != '(' && *c != ')') |
There was a problem hiding this comment.
The backslash character is a special character in User-Agent comments (used as an escape character). If a hostname contains a backslash, it can escape the closing parenthesis or other characters, leading to malformed User-Agent headers and potential parsing issues on the server. We should filter out backslashes along with ( and ) to ensure the hostname is safe for the User-Agent comment block.
| .filter(|c| c.is_ascii() && !c.is_control() && *c != '(' && *c != ')') | |
| .filter(|c| c.is_ascii() && !c.is_control() && *c != '(' && *c != ')' && *c != '\\') |
| user_agent.truncate(35) | ||
| parts << (client || os) | ||
| end | ||
| parts << "(#{hostname})" if hostname.present? && hostname != "Unknown" |
There was a problem hiding this comment.
To make the check more robust against different casing of the "Unknown" fallback (e.g., if a client or future version sends "unknown" or "UNKNOWN"), we should perform a case-insensitive comparison using casecmp?.
parts << "(#{hostname})" if hostname.present? && !hostname.casecmp?("unknown")1 participant
Summary
hostnamecrate to Rust backend and exposeget_hostnameTauri commandTest plan