fix: harden tauri local access and stabilize task lifecycle#50
fix: harden tauri local access and stabilize task lifecycle#50ShellMonster merged 3 commits intomainfrom
Conversation
|
CodeAnt AI is reviewing your PR. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on bolstering the application's security posture in its Tauri desktop environment, enhancing the reliability of background data migration, and refining the task management lifecycle. It also brings consistency to how image aspect ratios are presented, ensuring a more uniform user experience. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Nitpicks 🔍
|
There was a problem hiding this comment.
Code Review
This pull request introduces important security hardening for the Tauri desktop application by implementing a strict CORS policy and validating local file paths to prevent directory traversal attacks. It also includes a critical fix for a pagination issue in the database migration logic and improves the worker pool shutdown sequence for faster and more reliable application exit, along with refactoring to unify aspect ratio formatting. However, it's crucial to note that the introduced security protections are explicitly bypassed when the application is not running in Tauri mode, leaving it vulnerable to authenticated CORS attacks and arbitrary file reads via path traversal in non-Tauri environments. Additionally, there are suggestions to improve code maintainability by reducing duplication and simplifying logic.
| targetPath := path | ||
| if isTauriSidecarMode() { | ||
| validatedPath, validateErr := validateRefPathForTauri(path) | ||
| if validateErr != nil { | ||
| log.Printf("[API] 非法本地参考图路径: %s, err: %v\n", path, validateErr) | ||
| Error(c, http.StatusBadRequest, 400, "参考图路径不在允许目录内") | ||
| return | ||
| } | ||
| targetPath = validatedPath | ||
| } | ||
| content, err := os.ReadFile(targetPath) |
There was a problem hiding this comment.
The GenerateWithImagesHandler function allows reading arbitrary files from the server's file system when not running in Tauri mode. The path parameter from the request is used directly in os.ReadFile without any validation or sanitization. An attacker can exploit this to read sensitive files such as configuration files, credentials, or system files. While validation was added for Tauri mode, other modes (like Docker) remain unprotected. Strict path validation should be implemented for all environments.
backend/cmd/server/main.go
Outdated
| if origin != "" { | ||
| c.Writer.Header().Set("Access-Control-Allow-Origin", origin) |
There was a problem hiding this comment.
The application reflects the Origin header directly into the Access-Control-Allow-Origin response header when not running in Tauri mode. Combined with Access-Control-Allow-Credentials: true (line 187), this configuration allows any malicious website to make authenticated cross-origin requests to the backend API, leading to unauthorized data access or actions. It is recommended to use a strict allowlist of origins instead of reflecting the Origin header. Additionally, the logic in this else block for handling non-Tauri environments can be simplified to improve readability and reduce nesting.
backend/internal/api/handlers.go
Outdated
| func isTauriSidecarMode() bool { | ||
| return os.Getenv("TAURI_PLATFORM") != "" || os.Getenv("TAURI_FAMILY") != "" | ||
| } |
There was a problem hiding this comment.
The function isTauriSidecarMode is a duplicate of the isTauriSidecar function in backend/cmd/server/main.go. This introduces code duplication, which can make future maintenance more difficult.
To avoid duplication, this function should be moved to a shared utility package (e.g., internal/util or internal/platform) and be called from both main.go and handlers.go.
References
- Avoid duplicated code in the codebase; it should be abstracted into reusable functions or components. (link)
|
CodeAnt AI finished reviewing your PR. |
|
已根据本轮 CodeReview 补充修复并推送
本地验证:
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This PR significantly strengthens security for Tauri desktop applications, particularly in CORS handling and local file access validation, and also optimizes worker pool shutdown logic and fixes a database migration bug. However, the CORS implementation contains two significant issues: a loose prefix check for Tauri origins and an insecure handling of wildcard allowlists that allows reflected origins with credentials. These issues could allow unauthorized cross-origin or cross-app access to the backend API. Additionally, while overall code quality is high, there are areas for improvement in code clarity and security.
| } else if originInAllowlist(trimmedOrigin, corsAllowlist) { | ||
| c.Writer.Header().Set("Access-Control-Allow-Origin", trimmedOrigin) | ||
| c.Writer.Header().Set("Access-Control-Allow-Credentials", "true") |
There was a problem hiding this comment.
The CORS implementation incorrectly handles the wildcard (*) in the CORS_ALLOW_ORIGINS allowlist. If the allowlist contains *, the originInAllowlist function returns true for any origin, which is then reflected in the Access-Control-Allow-Origin header with Access-Control-Allow-Credentials set to true. This effectively bypasses browser security, allowing malicious websites to make authenticated requests if the allowlist is misconfigured with a wildcard. The logic should be updated to ensure that if a wildcard is used, credentials are not permitted, or the origin is not reflected unless it matches a specific trusted domain. Furthermore, the non-Tauri mode logic in this CORS middleware is complex and repetitive, especially where if trimmedOrigin == "" and else if len(corsAllowlist) == 0 branches perform the same c.Writer.Header().Set("Access-Control-Allow-Origin", "*") operation. Reorganizing this logic to prioritize invalid origins and then handle corsAllowlist presence separately would improve clarity and maintainability.
backend/internal/api/handlers.go
Outdated
| real := abs | ||
| if resolved, err := filepath.EvalSymlinks(abs); err == nil && strings.TrimSpace(resolved) != "" { | ||
| real = filepath.Clean(resolved) | ||
| } |
There was a problem hiding this comment.
在 validateRefPathForTauri 函数中,静默忽略 filepath.EvalSymlinks 的错误存在安全隐患。
当前逻辑是:如果 EvalSymlinks 执行失败,代码会回退到检查未经符号链接解析的原始路径 (abs)。如果这个原始路径本身是一个指向允许目录之外的符号链接,os.ReadFile 在后续操作中仍然会跟随这个链接,从而绕过了路径限制,导致潜在的目录遍历漏洞。
例如,攻击者可以创建一个符号链接 /app/tmp/malicious_link 指向 /etc/passwd。如果由于某些原因(如权限问题)EvalSymlinks 失败,验证逻辑会检查 /app/tmp/malicious_link 本身,发现它在允许的 /app/tmp 目录内,从而错误地通过验证。但随后的 os.ReadFile 会读取到 /etc/passwd 的内容。
为了修复此漏洞,当 filepath.EvalSymlinks 返回错误时,应该直接拒绝该路径。
real, err := filepath.EvalSymlinks(abs)
if err != nil {
// 如果符号链接解析失败,出于安全考虑应直接拒绝。
// 失败可能意味着链接损坏,或存在权限问题,这可能掩盖恶意路径。
return "", fmt.Errorf("ref path could not be resolved: %w", err)
}
real = filepath.Clean(real)References
- 规范 feat: integrate folder features into HistoryPanel (Phase 4) #29 “本地路径读取需防止路径遍历攻击 (验证路径合法性)”。当前实现虽然尝试进行验证,但存在漏洞,未能完全防止路径遍历攻击。 (link)
backend/cmd/server/main.go
Outdated
| if strings.HasPrefix(strings.ToLower(origin), "tauri://") { | ||
| return true | ||
| } |
There was a problem hiding this comment.
The isAllowedTauriOrigin function uses a loose prefix check (strings.HasPrefix(strings.ToLower(origin), "tauri://")) to validate the Origin header in Tauri mode. This allows any origin starting with tauri:// to pass validation. When validation passes, the server reflects the origin in the Access-Control-Allow-Origin header and sets Access-Control-Allow-Credentials to true. This allows any application on the system that can use the tauri:// scheme (such as another malicious Tauri app) to make authenticated requests to the backend API. It is recommended to strictly validate the origin against a known, trusted value (e.g., tauri://localhost).
|
已继续处理 Gemini 最新一轮 review,并推送
本地验证已通过:
|
User description
Summary
Validation
Notes
CodeAnt-AI Description
Harden Tauri desktop access, stabilize task lifecycle, and normalize image aspect labels
What Changed
Impact
✅ Block unauthorized local origins in desktop mode✅ Fewer missed migration records during upgrades✅ Faster app exit when tasks are running💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.