feat: Liquid Glass UI + Security Hardening + Bug Fixes#2481
feat: Liquid Glass UI + Security Hardening + Bug Fixes#2481xjwm5685-ui wants to merge 2 commits intofarion1231:mainfrom
Conversation
## UI Overhaul - Liquid Glass design system: translucent frosted glass surfaces with specular highlights - Dynamic Island / macOS Dock app switcher with spring physics - Stacked bar chart usage dashboard with blue-purple gradient - Model breakdown list with color dots and percentage bars - Spring animations throughout (cards, dialogs, buttons, view transitions) - Responsive hover lift, press squish, and stagger entrance effects ## Bug Fixes - fix(farion1231#2471): Strip empty `pages` field from Responses API tool_use input - fix(farion1231#2465): effortLevel "max" not persisting — use deepFill for snippet re-application - fix: Remove dead `update_proxy` function from http_client - fix: body_filter log now shows all removed keys across entire request ## Security - AES-256-GCM encryption for API keys in SQLite (crypto.rs) - WebDAV password encryption in settings.json - Deep-link SSRF protection (block localhost/private IPs) - Input validation on deep-link name, API key, repo fields - SwitchLock TOCTOU race condition fix - cargo-audit CI step ## Refactoring - Extract lib.rs setup into named functions (init_database, import_initial_data, etc.) - Extract forwarder status update helpers (record_success, record_failure) - Named proxy config constants replacing magic numbers - CSP tightened (removed http: from connect-src) ## Build - Release profile: codegen-units=16, lto=false, opt-level=2 (5-8x faster builds)
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc00b82f60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn is_encrypted_str(data: &str) -> bool { | ||
| is_encrypted(data.as_bytes()) |
There was a problem hiding this comment.
encrypt stores ciphertext as base64 (CCS1+nonce+ciphertext encoded), but is_encrypted_str compares the raw UTF-8 string prefix to CCS1, which will always be false for valid ciphertext. As a result, all call sites that gate decryption on this helper (e.g. provider/settings reads) skip decryption, and write paths that avoid double-encrypting can repeatedly encrypt already-encrypted values; this breaks round-tripping of stored secrets and config JSON.
Useful? React with 👍 / 👎.
| || v4.octets()[0] == 0 // 0.0.0.0/8 (current network) | ||
| || v4.octets()[0] == 100 && v4.octets()[1] >= 64 && v4.octets()[1] <= 127 // 100.64.0.0/10 (CGN) | ||
| || v4.octets()[0] == 169 && v4.octets()[1] == 254 // 169.254.0.0/16 (link-local) | ||
| || v4.octets()[0] == 198 && v4.octets()[1] == 18 || v4.octets()[1] == 19 // 198.18.0.0/15 (benchmark) |
There was a problem hiding this comment.
The last clause in is_private_ip has an operator-precedence bug: v4.octets()[0] == 198 && v4.octets()[1] == 18 || v4.octets()[1] == 19 evaluates true for any IPv4 address with second octet 19, even when the first octet is not 198. This causes false SSRF blocking for legitimate literal hosts like 8.19.x.x instead of only blocking 198.18.0.0/15.
Useful? React with 👍 / 👎.
…etection, IPv4 range - fix(ui): replace border-image with background-clip gradient border on .liquid-glass, .liquid-glass-active, .liquid-glass-emerald — border-image ignores border-radius, causing select/dropdown panels to render with rectangular borders - feat(ui): add LiquidGlassFilter SVG component (opt-in displacement refraction filters) - fix(crypto): is_encrypted_str now base64-decodes before checking the magic prefix, fixing false negatives when testing already-stored encrypted values (fixes farion1231#2471-P1) - fix(deeplink): add parentheses around IPv4 198.18/15 range check to fix operator precedence bug where octet[1]==19 was OR'd with the whole expression (fixes SSRF P2) - fix(skills): resolve skills install "目录不存在" on Windows by adding hyphen-bounded suffix matching — handles org-namespaced skill IDs like 'vercel-react-best-practices' mapping to repo dir 'react-best-practices' (fixes farion1231#2477) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@claude review |
|
Claude finished @farion1231's task in 3m 9s —— View job PR Review — feat: Liquid Glass UI + Security Hardening + Bug FixesTasks
OverviewThis is a large PR (3348+/1555−, 45 files) combining a UI overhaul, encryption-at-rest for API keys, deep-link SSRF protection, and several bug fixes. The security work is solid in intent; a few gaps are flagged below.
🔴 P1 — Issues to fix before merging1. IPv4-mapped IPv6 SSRF bypass — The IPv6 branch of // Current IPv6 branch — misses IPv4-mapped addresses
IpAddr::V6(v6) => {
v6.is_loopback() // only ::1
|| v6.is_unique_local() // fc00::/7
|| v6.is_unicast_link_local() // fe80::/10
|| *v6 == Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0) // ::
|| *v6 == Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1) // ::1 (redundant)
}Fix: add an IPv4-mapped check using IpAddr::V6(v6) => {
// Unwrap IPv4-mapped addresses (e.g. ::ffff:127.0.0.1) and check those too
if let Some(v4) = v6.to_ipv4_mapped() {
return is_private_ip(&IpAddr::V4(v4));
}
v6.is_loopback()
|| v6.is_unique_local()
|| v6.is_unicast_link_local()
|| *v6 == Ipv6Addr::UNSPECIFIED
}2. Overly broad empty-string stripping in tool inputs — The fix for issue #2471 ( // strips ANY "" value from ANY tool's input object
obj.retain(|_k, v| !v.as_str().is_some_and(|s| s.is_empty()));This will silently drop legitimate empty-string arguments. For example, a code-editing tool with // More conservative: only strip `pages: ""`
obj.retain(|k, v| k != "pages" || !v.as_str().is_some_and(|s| s.is_empty()));🟡 P2 — Should be addressed3. Silent API key loss on hostname change — When let settings_config = decrypt_settings_config_str(&settings_config_str)
.map(|s| serde_json::from_str(&s).unwrap_or(serde_json::Value::Null))
.unwrap_or(serde_json::Value::Null); // silent null on decrypt failureA user who renames their machine will see all providers appear with empty configuration and no explanation. Recommend at least logging a 4.
if let Some(ref url) = config_url {
if !url.is_empty() {
validate_url(url, "configUrl")?;
}
}5. DNS rebinding not defended —
🔵 P3 — Minor / polish6. Inconsistent encrypt-before-write pattern —
7. Duplicate link-local check —
What's good ✅
|
|
@claude review (Re-running with the updated review prompt for comparison.) |
|
Claude finished @farion1231's task in 2m 18s —— View job PR Re-review (strict prompt) — feat: Liquid Glass UI + Security Hardening + Bug Fixes
Re-reviewed under stricter criteria. Two of the previous review's findings still meet the bar; most others were either speculative, pre-existing, or pedantic and have been dropped. Findings1. IPv4-mapped IPv6 SSRF bypass —
IpAddr::V6(v6) => {
if let Some(v4) = v6.to_ipv4_mapped() {
return is_private_ip(&IpAddr::V4(v4));
}
v6.is_loopback() || v6.is_unique_local() || v6.is_unicast_link_local()
|| *v6 == Ipv6Addr::UNSPECIFIED
}2. Overly broad empty-string stripping in tool inputs — The fix for #2471 strips every empty-string field from every function_call's arguments, not only obj.retain(|_k, v| !v.as_str().is_some_and(|s| s.is_empty()));Concrete failure scenario: a model emits an Edit-style tool call with obj.retain(|k, v| k != "pages" || !v.as_str().is_some_and(|s| s.is_empty()));3. Silent data wipe when decrypt fails — let settings_config = decrypt_settings_config_str(&settings_config_str)
.map(|s| serde_json::from_str(&s).unwrap_or(serde_json::Value::Null))
.unwrap_or(serde_json::Value::Null);Concrete trigger: the encryption key is derived from Notes on dropped findings
Everything else looked solid: SwitchLock TOCTOU fix is correct, AES-GCM nonce framing is correct, WebDAV password redaction before IPC is correct, |
2 participants
Summary
UI Overhaul — Liquid Glass Design System
Bug Fixes
pagesfield from Responses API → Anthropic tool_use conversioneffortLevel: "max"not persisting — usedeepFill(only add missing fields) instead ofdeepMerge(overwrite) when re-applying common config snippetupdate_proxyfunctionSecurity Hardening
crypto.rs) — key derived from machine fingerprinthttp:from connect-srcRefactoring
lib.rssetup extracted into named functions (init_database,import_initial_data,register_deeplink_handler,setup_tray)record_success/record_failurehelpersBuild
codegen-units=16, lto=false, opt-level=2(~5-8x faster builds, slightly larger binary)Test plan
pages: ""no longer errorseffortLevel: "max"persists after save/reopensqlite3that settings_config is not plaintext)ccswitch://import?provider[api_key]=test&provider[base_url]=http://127.0.0.1cargo testandpnpm test