From b50efc25305948e1443925a32d50b02b9ddde9de Mon Sep 17 00:00:00 2001 From: harsha balakrishnan Date: Wed, 1 Jul 2026 17:06:09 +0530 Subject: [PATCH] =?UTF-8?q?fix:=20close=20audit=20findings=20=E2=80=94=20U?= =?UTF-8?q?I=20polish,=20security,=20CI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UI/accessibility: - Icon button focus rings, larger header hit targets, checkbox focus - FormatSelector ARIA parity, settings tab panels with aria-controls - panelBlurFlyIn on view transitions, tab/hotkey/banner motion gaps - Token/error CSS tokens (--status-error), tighter section labels Security: - Remove unused copy_file_to_clipboard IPC command - Sanitize save_error_report filenames and confine to output dir - Cap scan_directory depth/files; cap token_stats events at 500 - Gate PARSEKIT_E2E_INSTALL_UPDATE to debug builds only Infra/docs: - Add GitHub Actions CI (npm test, svelte-check, cargo test) - Align PRODUCT.md and brand.md with actual signing/typography --- .github/workflows/ci.yml | 31 ++++++++++ PRODUCT.md | 4 +- brand.md | 6 +- src-tauri/src/lib.rs | 77 +++++++++++++++++++++--- src-tauri/src/token_stats.rs | 21 +++++++ src/App.svelte | 15 ++++- src/components/FormatSelector.svelte | 7 ++- src/components/OutputFolderPicker.svelte | 1 + src/components/ProgressHud.svelte | 17 +++++- src/components/RecentBatches.svelte | 13 +++- src/components/SettingsScreen.svelte | 42 +++++++++++-- src/components/TokenSavingsBanner.svelte | 9 +++ src/index.css | 41 ++++++++----- 13 files changed, 242 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..a6dbd38 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,31 @@ +name: CI + +on: + push: + branches: [master, fix/*] + pull_request: + branches: [master] + +jobs: + test: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: "20" + cache: npm + + - name: Install frontend dependencies + run: npm ci + + - name: Frontend unit tests + run: npm test + + - name: Svelte typecheck + run: npm run check + + - name: Rust unit tests + run: cargo test + working-directory: src-tauri \ No newline at end of file diff --git a/PRODUCT.md b/PRODUCT.md index d86b5c5..f54df16 100644 --- a/PRODUCT.md +++ b/PRODUCT.md @@ -30,6 +30,6 @@ Knowledge workers, researchers, lawyers, consultants, and power users who live i ## Non-Functional Requirements - 100% local (LiteParse v2 native OCR; optional first-run Tesseract language data). - Native macOS look & feel (dark mode, SF Symbols, accessibility, haptics). -- Zero telemetry, zero network except LiteParse necessities. +- Zero telemetry. Network use is limited to optional GitHub release checks (updater) and first-run OCR language data downloads when needed. - Minimal CPU/memory; smart concurrency (max 4-6 workers on M-series). -- Signed & notarized .app build ready. +- Ad-hoc signed `.app` builds (see `docs/RELEASING.md`); notarization is planned for public distribution. diff --git a/brand.md b/brand.md index c5d6a6e..1f7f932 100644 --- a/brand.md +++ b/brand.md @@ -48,13 +48,13 @@ ParseKit uses custom CSS variables (not shadcn). Backup at `src/index.css.bak`. Body, primary CTA, and muted text pairs verified for WCAG AA in light and dark derivations. -## Typography — Instrument Serif + Inter + JetBrains Mono +## Typography — Instrument Serif + System Sans + JetBrains Mono - **Display (app title, about header):** Instrument Serif -- **Body (UI):** Inter +- **Body (UI):** System sans (`-apple-system`, SF Pro Text) for native macOS feel - **Mono (counts, workers):** JetBrains Mono -Loaded via Google Fonts in `src/index.css`. Variables: `--font-serif`, `--font-sans`, `--font-mono`. +Instrument Serif and JetBrains Mono are bundled locally in `src/index.css`. Variables: `--font-serif`, `--font-sans`, `--font-mono`. ## Gradients diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 8037cbe..5c7fed4 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -647,6 +647,26 @@ fn imagemagick_available() -> bool { ) } +#[cfg(test)] +mod security_path_tests { + use super::sanitize_report_file_name; + + #[test] + fn sanitize_report_file_name_accepts_simple_names() { + assert_eq!( + sanitize_report_file_name("parsekit-errors.txt").unwrap(), + "parsekit-errors.txt" + ); + } + + #[test] + fn sanitize_report_file_name_rejects_traversal() { + assert!(sanitize_report_file_name("../secrets.txt").is_err()); + assert!(sanitize_report_file_name("sub/file.txt").is_err()); + assert!(sanitize_report_file_name("").is_err()); + } +} + #[cfg(test)] mod dependency_detect_tests { use super::*; @@ -760,10 +780,41 @@ fn gatekeeper_fix_command() -> String { "xattr -cr /Applications/ParseKit.app && xattr -d com.apple.FinderInfo /Applications/ParseKit.app 2>/dev/null || true".to_string() } +/// Reject path components in report filenames (IPC boundary). +pub(crate) fn sanitize_report_file_name(file_name: &str) -> Result { + let trimmed = file_name.trim(); + if trimmed.is_empty() { + return Err("File name is empty".into()); + } + if trimmed.contains('/') || trimmed.contains('\\') || trimmed.contains("..") { + return Err("Invalid file name".into()); + } + if trimmed.chars().any(|c| c.is_control()) { + return Err("Invalid file name".into()); + } + Ok(trimmed.to_string()) +} + +/// Ensure the parent directory of `target` stays under `base_dir`. +fn path_stays_within_base(base_dir: &std::path::Path, target: &std::path::Path) -> Result<(), String> { + let base = std::fs::canonicalize(base_dir) + .map_err(|e| format!("Could not resolve output folder: {e}"))?; + let parent = target + .parent() + .ok_or_else(|| "Invalid report path".to_string())?; + let parent_resolved = std::fs::canonicalize(parent) + .map_err(|e| format!("Could not resolve report folder: {e}"))?; + if !parent_resolved.starts_with(&base) { + return Err("Report path escapes the output folder".into()); + } + Ok(()) +} + /// Write a text report (error log) into `dir/file_name`, then reveal it in Finder. /// Returns the full path of the written file. #[tauri::command] fn save_error_report(dir: String, file_name: String, contents: String) -> Result { + let safe_name = sanitize_report_file_name(&file_name)?; let mut path = std::path::PathBuf::from(&dir); if !path.is_dir() { // Fall back to ~/Downloads if the original output dir is gone. @@ -776,7 +827,9 @@ fn save_error_report(dir: String, file_name: String, contents: String) -> Result std::path::PathBuf::from(home) }; } - path.push(&file_name); + let base_dir = path.clone(); + path.push(&safe_name); + path_stays_within_base(&base_dir, &path)?; std::fs::write(&path, contents).map_err(|e| format!("Could not write report: {e}"))?; let path_str = path.to_string_lossy().to_string(); #[cfg(target_os = "macos")] @@ -870,6 +923,9 @@ pub(crate) const SUPPORTED_EXTENSIONS: &[&str] = &[ "xlsm", "ods", "csv", "tsv", "png", "jpg", "jpeg", "gif", "bmp", "tiff", "tif", "webp", "svg", ]; +const SCAN_MAX_DEPTH: usize = 32; +const SCAN_MAX_FILES: usize = 10_000; + pub(crate) fn scan_directory_sync(path: String) -> Result, String> { let path = normalize_user_path(path); let dir = Path::new(&path); @@ -878,9 +934,19 @@ pub(crate) fn scan_directory_sync(path: String) -> Result, String> { } let mut files = Vec::new(); - for entry in WalkDir::new(dir).into_iter().filter_map(|e| e.ok()) { + for entry in WalkDir::new(dir) + .max_depth(SCAN_MAX_DEPTH) + .follow_links(false) + .into_iter() + { + let entry = entry.map_err(|e| format!("Could not scan directory: {e}"))?; let entry_path = entry.path(); if entry_path.is_file() { + if files.len() >= SCAN_MAX_FILES { + return Err(format!( + "Too many supported files (max {SCAN_MAX_FILES}). Choose a smaller folder." + )); + } if let Some(ext) = entry_path.extension().and_then(|e| e.to_str()) { let ext_lower = ext.to_lowercase(); if SUPPORTED_EXTENSIONS.contains(&ext_lower.as_str()) { @@ -1003,11 +1069,6 @@ fn open_in_finder(path: String) -> Result<(), String> { Ok(()) } -#[tauri::command] -fn copy_file_to_clipboard(path: String) -> Result, String> { - std::fs::read(&path).map_err(|e| format!("Failed to read file: {}", e)) -} - #[tauri::command] fn show_main_window( app: tauri::AppHandle, @@ -1276,7 +1337,6 @@ pub fn run() { path_is_directory, canonicalize_paths, open_in_finder, - copy_file_to_clipboard, save_error_report, get_system_info, check_dependencies, @@ -1503,6 +1563,7 @@ pub fn run() { } // Test-only: same `install_update` path as the gold banner "Install & Restart" button. + #[cfg(debug_assertions)] if std::env::var("PARSEKIT_E2E_INSTALL_UPDATE").as_deref() == Ok("1") { let handle = app.handle().clone(); tauri::async_runtime::spawn(async move { diff --git a/src-tauri/src/token_stats.rs b/src-tauri/src/token_stats.rs index 0420309..55c7b5c 100644 --- a/src-tauri/src/token_stats.rs +++ b/src-tauri/src/token_stats.rs @@ -6,6 +6,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; const STATS_FILE_NAME: &str = "token-stats.json"; +const MAX_TOKEN_EVENTS: usize = 500; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] pub struct FileTypeStats { @@ -128,6 +129,10 @@ fn apply_record(stats: &mut TokenStats, input: &RecordInput) { tokens_saved, pages_unlocked, }); + if stats.events.len() > MAX_TOKEN_EVENTS { + let overflow = stats.events.len() - MAX_TOKEN_EVENTS; + stats.events.drain(0..overflow); + } } #[cfg(test)] @@ -250,6 +255,22 @@ mod tests { assert_eq!(load(), TokenStats::default()); } + #[test] + fn events_are_capped_at_max() { + let _guard = TestDirGuard::new(); + for i in 0..(MAX_TOKEN_EVENTS + 25) { + record(RecordInput { + file_type: format!("pdf{i}"), + tokens_saved: 1, + pages_unlocked: 0, + documents_unlocked: 0, + }) + .expect("record"); + } + let stats = load(); + assert_eq!(stats.events.len(), MAX_TOKEN_EVENTS); + } + #[test] fn save_load_roundtrip() { let _guard = TestDirGuard::new(); diff --git a/src/App.svelte b/src/App.svelte index 29c22c8..6c70ff2 100644 --- a/src/App.svelte +++ b/src/App.svelte @@ -76,6 +76,8 @@ collapseSlideOut, hintFadeIn, hintFadeOut, + panelBlurFlyIn, + panelBlurFlyInParams, panelBlurFlyOut, panelBlurFlyOutParams, panelFadeIn, @@ -87,6 +89,7 @@ import "./index.css"; const reducedMotion = $derived(prefersReducedMotion.current); + const mainPanelIn = $derived(panelBlurFlyInParams(reducedMotion)); const mainPanelOut = $derived(panelBlurFlyOutParams(reducedMotion)); const mainFadeIn = $derived(panelFadeIn(reducedMotion)); const mainFadeOut = $derived(panelFadeOut(reducedMotion)); @@ -934,13 +937,14 @@
{#if !showSettings && !showHistory} {#key "main"} -
+
{t("app.name")} {#if appVersion}{appVersion}{/if}
{/each} -
+
\ No newline at end of file diff --git a/src/components/OutputFolderPicker.svelte b/src/components/OutputFolderPicker.svelte index f7a80b0..97bd805 100644 --- a/src/components/OutputFolderPicker.svelte +++ b/src/components/OutputFolderPicker.svelte @@ -31,6 +31,7 @@ type="button" class="output-folder-path-btn" title={value || t("config.downloads")} + aria-label="{t('config.outputFolder')}: {value || t('config.downloads')}" onclick={pick} > {displayPath} diff --git a/src/components/ProgressHud.svelte b/src/components/ProgressHud.svelte index 3a8ddd6..278bfd9 100644 --- a/src/components/ProgressHud.svelte +++ b/src/components/ProgressHud.svelte @@ -1,6 +1,9 @@ {#if latestBatch} -
+
{t("recent.title")}
{#if showHistoryButton}
- {#if activeTab === "general"} -
+ {#key activeTab} + {#if activeTab === "general"} +
{t("settings.appLanguageTitle")}

{t("settings.appLanguageHint")}

@@ -302,13 +313,25 @@ {recordingHotkey ? t("settings.hotkeyRecording") : t("settings.hotkeyChange")} {#if globalShortcut !== DEFAULT_GLOBAL_SHORTCUT} - {/if}
{#if hotkeyError} -

{hotkeyError}

+

+ {hotkeyError} +

{/if}
@@ -417,7 +440,13 @@
{:else} -
+
{t("settings.ocrLanguageTitle")}

{t("settings.ocrLanguageHint")}

@@ -447,6 +476,7 @@
{/if} + {/key}
diff --git a/src/components/TokenSavingsBanner.svelte b/src/components/TokenSavingsBanner.svelte index c248627..3adee12 100644 --- a/src/components/TokenSavingsBanner.svelte +++ b/src/components/TokenSavingsBanner.svelte @@ -1,4 +1,7 @@