Skip to content

fix: XSS安全修复 + 服务崩溃保护 + 导入导出 + 搜索功能#1

Open
Haaaiawd wants to merge 5 commits intomasterfrom
devin/1776345684-improvements
Open

fix: XSS安全修复 + 服务崩溃保护 + 导入导出 + 搜索功能#1
Haaaiawd wants to merge 5 commits intomasterfrom
devin/1776345684-improvements

Conversation

@Haaaiawd
Copy link
Copy Markdown
Owner

Summary

This PR addresses critical security vulnerabilities and adds key missing features identified during a project analysis.

Security (P0):

  • Fix XSS vulnerability in loadPrompts() — user-supplied prompt.name, prompt.content, and tags were interpolated into HTML without escaping. Now uses the existing escapeHtml() helper.
  • Fix XSS in showEditPromptModal() — replaced string interpolation of user data into value="..." attributes with safe DOM property assignment (el.value = ...).
  • Remove unused windows crate (v0.52) from root Cargo.toml. The service module already has its own windows v0.58 dependency.

Reliability (P1):

  • Wrap service::run_service() in catch_unwind with a 3-second auto-restart loop, so the service thread recovers from panics instead of silently dying.

Features (P1):

  • Export: New export_prompts Tauri command + UI button. Exports all prompts as a JSON file download.
  • Import: New import_prompts Tauri command + file picker UI. Imports prompts from a previously exported JSON file.
  • Search: New search_prompts Tauri command + search bar in the prompts panel header. Searches by name, content, and tags via SQL LIKE.

Minor fixes:

  • Remove duplicate promptList.innerHTML = promptsHtml assignment
  • Remove duplicate logsToolbar.insertBefore(refreshLogsBtn, clearLogsBtn) call

Review & Testing Checklist for Human

  • Verify the XSS fix in showEditPromptModal: The modal now creates elements with empty value="" and sets them via DOM properties after appending. Confirm that prompts containing ", <, >, and <script> tags render safely in both the list view and the edit modal.
  • Test import/export round-trip: Export prompts, then import the same file. Note that importing the same file twice will create duplicate entries — there is no dedup logic. Decide if this is acceptable or if a name-based merge strategy is needed.
  • Verify catch_unwind compatibility: service::run_service() must be UnwindSafe for the panic protection to work correctly. The retry loop also has no max retry limit or backoff — if the service panics on startup repeatedly, it will retry every 3 seconds indefinitely.
  • Verify Tauri argument naming: The import command passes { jsonData: text } from JS, relying on Tauri 2's automatic camelCase→snake_case conversion to match the Rust parameter json_data: String. Confirm this works with the project's Tauri invoke path.
  • Build the project on Windows (or via CI): This PR was not locally compiled because the project requires nightly Rust (edition = "2024") and Windows-specific dependencies.

Suggested test plan:

  1. Build and launch the app
  2. Create a prompt with name <img src=x onerror=alert(1)> — verify it renders as text, not as HTML
  3. Edit that prompt — verify the edit modal displays the raw string correctly
  4. Export prompts → inspect the JSON file → import it back → verify prompts appear (and note duplicates)
  5. Use the search bar to filter prompts by name, content substring, and tag
  6. Kill the service thread (or trigger a panic) and verify it auto-restarts after ~3 seconds

Notes

  • The search result rendering in bindFunctionButtons duplicates the HTML template from loadPrompts(). A future refactor could extract a shared renderPromptCard() helper.
  • search_prompts does not escape SQL LIKE wildcards (%, _) in the user query — entering % will match everything. This is low-risk but worth noting.
  • chrono_now_string() returns a Unix timestamp (seconds), not an ISO 8601 datetime, despite the function name suggesting otherwise.

Link to Devin session: https://app.devin.ai/sessions/ffc0c35c3461413cae659b426b1007a4
Requested by: @Haaaiawd

P0 Security:
- Fix XSS in loadPrompts() - escape prompt.name, content, tags with escapeHtml()
- Fix XSS in showEditPromptModal() - use DOM APIs instead of string interpolation
- Remove unused windows crate from root Cargo.toml (service has its own)

P1 Features:
- Add service thread panic protection with auto-restart (catch_unwind + 3s retry)
- Add prompt export (export_prompts command + download JSON)
- Add prompt import (import_prompts command + file picker UI)
- Add main panel search (search_prompts command + search bar UI)

Quick fixes:
- Remove duplicate insertBefore call
- Remove duplicate innerHTML assignment

Co-Authored-By: Ha AI <1134180104@qq.com>
Copilot AI review requested due to automatic review settings April 16, 2026 13:34
@devin-ai-integration
Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Tauri app against XSS, adds prompt import/export + search capabilities, and attempts to make the embedded service thread more resilient to panics.

Changes:

  • Escape user-controlled prompt fields in prompt list rendering; update edit modal to avoid interpolating user data into HTML attributes.
  • Add UI controls + Tauri commands for prompt search, JSON export, and JSON import.
  • Add a panic-restart loop around service::run_service(); remove the unused root windows crate dependency.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
src/main_simple.js Adds prompt search + import/export UI bindings; applies XSS escaping in list rendering and safer modal value assignment.
src/main.rs Adds export_prompts, import_prompts, search_prompts commands; wraps service thread in catch_unwind restart loop.
src/index.html Adds prompt toolbar (search/export/import) and hidden file input for import.
Cargo.toml Removes root windows dependency entry.
Cargo.lock Removes windows 0.52.0 from the root package dependency list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main.rs Outdated
Comment thread src/main.rs Outdated
Comment thread src/main.rs
Comment on lines +100 to +118
match result {
Ok(_) => {
eprintln!("⚠️ [ENGINE] 服务线程正常退出,尝试重启...");
}
Err(e) => {
let msg = if let Some(s) = e.downcast_ref::<&str>() {
s.to_string()
} else if let Some(s) = e.downcast_ref::<String>() {
s.clone()
} else {
"未知错误".to_string()
};
eprintln!("❌ [ENGINE] 服务线程崩溃: {},3秒后自动重启...", msg);
}
}
// 短暂等待后自动重启
std::thread::sleep(std::time::Duration::from_secs(3));
eprintln!("🔄 [ENGINE] 正在重启服务线程...");
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be careful restarting service::run_service() in-process: service::run_service() currently calls env_logger::init_from_env(...), which panics if the logger is already initialized. After the first panic, the restart loop is likely to immediately panic again during logger init. Consider making logger initialization idempotent (e.g., try_init with ignored error) or moving logger init outside the restart loop.

Copilot uses AI. Check for mistakes.
Comment thread src/main.rs
Comment on lines +1331 to +1336
fn chrono_now_string() -> String {
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap_or_default();
format!("{}", now.as_secs())
}
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chrono_now_string() returns a Unix timestamp in seconds, but the name suggests a formatted datetime string. Consider renaming it to reflect what it returns (e.g., unix_timestamp_secs_string) or changing the implementation to emit an ISO-8601/RFC3339 timestamp if the export format expects a human-readable datetime.

Copilot uses AI. Check for mistakes.
Comment thread src/main_simple.js
Comment on lines +1234 to +1239
// Use DOM APIs instead of innerHTML to prevent XSS
const modal = document.createElement('div');
modal.id = 'edit-prompt-modal';
modal.className = 'modal-overlay';
modal.innerHTML = `
<div class="modal-content">
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says “Use DOM APIs instead of innerHTML to prevent XSS”, but the implementation still uses modal.innerHTML = ... (it’s just no longer interpolating user data). To avoid confusion, either update the comment to reflect the actual mitigation (no user-data interpolation + property assignment), or build the modal DOM tree without innerHTML.

Copilot uses AI. Check for mistakes.
Comment thread src/index.html Outdated
Comment thread src/main.rs
Comment on lines +1306 to +1313
for prompt in &export_data.prompts {
let tags_str = prompt.tags.as_ref().map(|t| t.join(","));
let result = conn.execute(
"INSERT INTO prompts (name, tags, content, content_type, variables_json, app_scopes_json, inject_order, version, updated_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)",
rusqlite::params![
prompt.name,
tags_str,
prompt.content,
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import_prompts writes tags back to the DB as a comma-joined string, but the rest of the app expects the tags column to contain JSON (see create_prompt/update_prompt which serde_json::to_string the tag array). This will cause imported prompts to lose tags in get_all_prompts (JSON parse fails) and makes search/export inconsistent. Persist tags using the same JSON encoding used by the other CRUD paths.

Copilot uses AI. Check for mistakes.
Comment thread src/main.rs Outdated
Comment thread src/main.rs
Comment on lines +96 to +99
loop {
let result = std::panic::catch_unwind(|| {
service::run_service();
});
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This panic-supervision loop uses catch_unwind, but the workspace Cargo.toml sets [profile.release] panic = "abort", which prevents unwinding in release builds—panics will abort the process and never be caught/restarted. If restart-on-panic is required in production, switch release to panic = "unwind" (and audit the cost), or supervise the service via an external process instead of relying on catch_unwind.

Copilot uses AI. Check for mistakes.
Haaaiawd and others added 4 commits April 16, 2026 21:46
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants