From 062f09a000426c9d03451be1553f00f89ff9e81f Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 9 Jun 2026 19:39:27 -0700 Subject: [PATCH] fix: prevent concurrent transcription races and mask API key --- app.go | 112 ++++++++++++++++++++------- internal/audio/recorder/recorder.go | 16 +++- internal/config/config.go | 16 +++- internal/logger/logger.go | 3 +- internal/services/whisper/whisper.go | 15 +++- internal/ui/tray/tray.go | 10 ++- internal/windows/overlay.go | 14 ++++ linux_press_daemon.go | 5 ++ linux_press_daemon_stub.go | 2 + text_insert_nonlinux.go | 12 +-- 10 files changed, 159 insertions(+), 46 deletions(-) diff --git a/app.go b/app.go index b1026bf..3729974 100644 --- a/app.go +++ b/app.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "runtime" + "sort" "strings" "sync/atomic" "time" @@ -25,18 +26,19 @@ import ( // App struct type App struct { - ctx context.Context - audioRecorder *recorder.AudioRecorder - hotkeyListener *hotkey.Listener - transcriber *transcriber.Client - config *config.Config - overlay platform.Overlay - recordingPath string - recording int32 - isQuitting bool - wasMediaPlaying bool - whisperManager *whisper.Manager - tempDir string + ctx context.Context + audioRecorder *recorder.AudioRecorder + hotkeyListener *hotkey.Listener + transcriber *transcriber.Client + config *config.Config + overlay platform.Overlay + recordingPath string + recording int32 + isQuitting bool + wasMediaPlaying bool + whisperManager *whisper.Manager + tempDir string + transcribing int32 // atomic: 1 = transcription in progress, prevents concurrent } // NewApp creates a new App application struct @@ -236,6 +238,8 @@ func (a *App) StartRecording() { err = a.audioRecorder.Start(a.recordingPath) } if err != nil { + // Clean up the orphaned temp file since recording failed to start + os.Remove(a.recordingPath) if a.overlay != nil { a.overlay.Hide() } @@ -268,23 +272,25 @@ func (a *App) StopRecording() { } if a.audioRecorder == nil { - atomic.StoreInt32(&a.recording, 0) return } + // Capture the path BEFORE stopping the recorder, so it can't be + // overwritten by a concurrent StartRecording (which sets a.recordingPath). + pathToProcess := a.recordingPath + err := a.audioRecorder.Stop() if err != nil { logger.Error("Failed to stop recording: %v", err) return } - // Capture the path before it can be overwritten by another immediate start - pathToProcess := a.recordingPath // Transcribe in a goroutine to avoid blocking go a.processRecording(pathToProcess) } -// processRecording handles transcription and pasting +// processRecording handles transcription and pasting. +// Uses an atomic guard to prevent concurrent transcriptions. func (a *App) processRecording(recordingPath string) { if recordingPath == "" { logger.Error("No recording path set") @@ -295,6 +301,15 @@ func (a *App) processRecording(recordingPath string) { return } + // Prevent concurrent transcriptions: only one goroutine can process at a time. + // If another transcription is already in progress, discard this recording. + if !atomic.CompareAndSwapInt32(&a.transcribing, 0, 1) { + logger.Info("Another transcription already in progress, discarding recording: %s", recordingPath) + os.Remove(recordingPath) + return + } + defer atomic.StoreInt32(&a.transcribing, 0) + // IDIOT-PROOFING: Ignore extremely short recordings (less than ~100ms or ~3KB) // that are likely accidental clicks or hardware glitches. stat, statErr := os.Stat(recordingPath) @@ -407,7 +422,17 @@ func (a *App) processRecording(recordingPath string) { // GetSettings returns the current configuration func (a *App) GetSettings() map[string]interface{} { conf := make(map[string]interface{}) - conf["api_key"] = a.config.APIKey + // Mask the API key: only reveal the last 4 characters so the user can verify + // which key is configured without exposing the full secret to the frontend. + if a.config.APIKey != "" { + key := a.config.APIKey + if len(key) > 4 { + key = "****" + key[len(key)-4:] + } + conf["api_key"] = key + } else { + conf["api_key"] = "" + } conf["shortcut"] = a.config.Shortcut conf["whisper_model"] = a.config.WhisperModel conf["ai_model"] = a.config.AIModel @@ -433,7 +458,12 @@ func (a *App) GetSettings() map[string]interface{} { // SaveSettings updates the configuration func (a *App) SaveSettings(settings map[string]interface{}) string { if val, ok := settings["api_key"].(string); ok { - a.config.APIKey = val + // Only update the API key if it's not the masked value returned by GetSettings. + // GetSettings masks the key as "****abcd" so the frontend can show the last 4 chars. + // If the user didn't change it and sent back the masked value, preserve the real key. + if !strings.HasPrefix(val, "****") { + a.config.APIKey = val + } } // PLATFORM NOTE: Shortcut saving is disabled on Linux because Linux uses // the `--press` daemon approach (GNOME custom shortcuts) instead of the @@ -502,7 +532,9 @@ func (a *App) SaveSettings(settings map[string]interface{}) string { return fmt.Sprintf("Error saving settings: %v", err) } - // Re-init transcriber with new settings + // Re-init transcriber with new settings. + // Note: a.config.APIKey is already updated by the api_key field above. + // Since GetSettings masks the key, we only update if it's not still the masked value. a.transcriber = transcriber.NewClient( a.config.APIKey, a.config.WhisperModel, @@ -668,6 +700,8 @@ func (a *App) Shutdown(ctx context.Context) { if a.overlay != nil { a.overlay.Close() } + // Gracefully shut down the Linux press daemon HTTP server (no-op on Windows) + stopLinuxPressDaemon() logger.Close() } @@ -681,20 +715,30 @@ func (a *App) CheckOnline() bool { return whisper.CheckOnline() } -// IsWhisperInstalled checks if offline whisper is installed +// IsWhisperInstalled checks if offline whisper is installed. +// Reuses the cached whisperManager if available. func (a *App) IsWhisperInstalled() bool { - mgr, err := whisper.NewManager() - if err != nil { - return false + mgr := a.whisperManager + if mgr == nil { + var err error + mgr, err = whisper.NewManager() + if err != nil { + return false + } } return mgr.IsInstalled() } -// GetWhisperInfo returns information about installed whisper +// GetWhisperInfo returns information about installed whisper. +// Reuses the cached whisperManager if available. func (a *App) GetWhisperInfo() map[string]interface{} { - mgr, err := whisper.NewManager() - if err != nil { - return map[string]interface{}{"installed": false} + mgr := a.whisperManager + if mgr == nil { + var err error + mgr, err = whisper.NewManager() + if err != nil { + return map[string]interface{}{"installed": false} + } } if !mgr.IsInstalled() { @@ -741,10 +785,18 @@ func (a *App) UninstallWhisper() string { return "Whisper uninstalled successfully" } -// GetAvailableWhisperModels returns list of available whisper models +// GetAvailableWhisperModels returns list of available whisper models, +// sorted by name for consistent UI display. func (a *App) GetAvailableWhisperModels() []map[string]string { - var models []map[string]string - for name, info := range whisper.Models { + var names []string + for name := range whisper.Models { + names = append(names, name) + } + sort.Strings(names) + + models := make([]map[string]string, 0, len(names)) + for _, name := range names { + info := whisper.Models[name] models = append(models, map[string]string{ "name": name, "size": info.Size, diff --git a/internal/audio/recorder/recorder.go b/internal/audio/recorder/recorder.go index 856ae4e..bac9e22 100644 --- a/internal/audio/recorder/recorder.go +++ b/internal/audio/recorder/recorder.go @@ -262,14 +262,22 @@ func (r *AudioRecorder) Cleanup() { } // onAudioData is called by miniaudio when audio data is available. -// Safe to call without r.mu because atomic writing flag prevents writes -// after Stop() clears the flag. +// Safe to call without r.mu: the atomic writing flag prevents writes after +// Stop() clears it, and r.device.Stop() waits for in-flight callbacks to +// complete before returning, so r.outputFile is guaranteed valid here. func (r *AudioRecorder) onAudioData(_, inputSamples []byte, _ uint32) { - if atomic.LoadInt32(&r.writing) == 0 || r.outputFile == nil || len(inputSamples) == 0 { + if atomic.LoadInt32(&r.writing) == 0 || len(inputSamples) == 0 { return } - n, err := r.outputFile.Write(inputSamples) + // Double-check outputFile under the assumption that Stop() has already + // set writing=0 before touching the file handle. + f := r.outputFile + if f == nil { + return + } + + n, err := f.Write(inputSamples) if err == nil { atomic.AddUint32(&r.dataSize, uint32(n)) } diff --git a/internal/config/config.go b/internal/config/config.go index a567925..7fc8009 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -6,6 +6,7 @@ import ( "encoding/json" "os" "path/filepath" + "sync" ) // Application defaults @@ -43,6 +44,9 @@ type Config struct { const CurrentConfigVersion = 1 const MaxHistoryItems = 100 +// saveMu protects concurrent writes to the config file. +var saveMu sync.Mutex + // DefaultConfig returns a new configuration with sensible default values. func DefaultConfig() *Config { return &Config{ @@ -91,7 +95,11 @@ func (c *Config) migrate() { // Save writes the configuration to the specified file path. // If configPath is empty, it uses the default configuration path. +// It uses atomic writes (write-to-temp then rename) to prevent corruption. func Save(c *Config, configPath string) error { + saveMu.Lock() + defer saveMu.Unlock() + if configPath == "" { var err error configPath, err = GetConfigPath() @@ -111,7 +119,13 @@ func Save(c *Config, configPath string) error { return err } - return os.WriteFile(configPath, data, 0600) + // Atomic write: write to temp file, then rename to prevent corruption + // if the app crashes mid-write. + tmpPath := configPath + ".tmp" + if err := os.WriteFile(tmpPath, data, 0600); err != nil { + return err + } + return os.Rename(tmpPath, configPath) } // GetConfigPath returns the default configuration file path. diff --git a/internal/logger/logger.go b/internal/logger/logger.go index ebb9720..27d8a0e 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -54,12 +54,13 @@ func Init() error { return nil } -// Close closes the log file handle. +// Close flushes buffered writes and closes the log file handle. func Close() { logMutex.Lock() defer logMutex.Unlock() if logFile != nil { + logFile.Sync() // Flush buffered writes so the last log entries are not lost logFile.Close() logFile = nil } diff --git a/internal/services/whisper/whisper.go b/internal/services/whisper/whisper.go index b434ce6..d145d31 100644 --- a/internal/services/whisper/whisper.go +++ b/internal/services/whisper/whisper.go @@ -2,6 +2,7 @@ package whisper import ( + "context" "encoding/json" "fmt" "io" @@ -295,7 +296,10 @@ func (m *Manager) Transcribe(audioPath string, language string) (string, error) // Run whisper with simple arguments: ./main -m model.bin -l language -f audio.wav // Vulkan build uses GPU by default, no need for -ngl - cmd := exec.Command(binaryPath, "-m", modelPath, "-l", language, "-f", audioPath) + // Use a 5-minute timeout to prevent infinite hangs on long audio or stuck processes + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, binaryPath, "-m", modelPath, "-l", language, "-f", audioPath) // Set working directory to the binary's location so it can find DLLs cmd.Dir = filepath.Dir(binaryPath) @@ -419,7 +423,10 @@ func CheckOnline() bool { } defer resp.Body.Close() - return true + // Drain the response body to allow connection reuse and prevent resource leak + io.Copy(io.Discard, resp.Body) + + return resp.StatusCode == http.StatusOK } // DownloadProgress represents download progress @@ -437,6 +444,10 @@ func downloadFile(url, dest string, progress chan<- DownloadProgress) error { } defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("download failed: server returned HTTP %d", resp.StatusCode) + } + out, err := os.Create(dest) if err != nil { return err diff --git a/internal/ui/tray/tray.go b/internal/ui/tray/tray.go index a809b6d..6dc65a4 100644 --- a/internal/ui/tray/tray.go +++ b/internal/ui/tray/tray.go @@ -13,6 +13,7 @@ import ( "runtime" "strings" "sync" + "sync/atomic" "wis-free-v3/internal/config" "wis-free-v3/internal/logger" @@ -47,7 +48,7 @@ var trayLabel = "wis-free-v3" // Menu item references for dynamic updates var statusMenuItem *systray.MenuItem var triggerCountItem *systray.MenuItem -var triggerCount int +var triggerCount int32 var iconsInitOnce sync.Once var startupMenuItem *systray.MenuItem @@ -233,10 +234,13 @@ func icoToPNG(data []byte) ([]byte, error) { return nil, fmt.Errorf("no PNG image found in ico") } +var statusMu sync.Mutex var lastStatus string // UpdateStatus updates the status text displayed in the tray menu. func UpdateStatus(status string) { + statusMu.Lock() + defer statusMu.Unlock() if statusMenuItem != nil && status != lastStatus { lastStatus = status statusMenuItem.SetTitle("Status: " + status) @@ -255,9 +259,9 @@ func UpdateStatus(status string) { // IncrementTriggerCount increments the troubleshooting counter in the tray. func IncrementTriggerCount() { - triggerCount++ + count := atomic.AddInt32(&triggerCount, 1) if triggerCountItem != nil { - triggerCountItem.SetTitle(fmt.Sprintf("Shortcut detected: %d times", triggerCount)) + triggerCountItem.SetTitle(fmt.Sprintf("Shortcut detected: %d times", count)) } } diff --git a/internal/windows/overlay.go b/internal/windows/overlay.go index 65ebe29..f8be081 100644 --- a/internal/windows/overlay.go +++ b/internal/windows/overlay.go @@ -99,6 +99,8 @@ type Overlay struct { mu sync.RWMutex stopCh chan struct{} bgBrush syscall.Handle + barBrushWhite syscall.Handle // Pre-created GDI brush for white bars (recording animation) + barBrushOrange syscall.Handle // Pre-created GDI brush for orange bars (transcribing animation) hFont syscall.Handle volume uint64 // atomic: float64 stored via math.Float64bits smoothedVolume uint64 // atomic: float64 stored via math.Float64bits @@ -171,6 +173,12 @@ func (o *Overlay) Close() { if o.hFont != 0 { procDeleteObject.Call(uintptr(o.hFont)) } + if o.barBrushWhite != 0 { + procDeleteObject.Call(uintptr(o.barBrushWhite)) + } + if o.barBrushOrange != 0 { + procDeleteObject.Call(uintptr(o.barBrushOrange)) + } close(o.stopCh) } @@ -193,6 +201,12 @@ func (o *Overlay) run() { brushRec, _, _ := procCreateSolidBrush.Call(COLOR_BG_DARK) o.bgBrush = syscall.Handle(brushRec) + whiteRec, _, _ := procCreateSolidBrush.Call(uintptr(COLOR_WHITE)) + o.barBrushWhite = syscall.Handle(whiteRec) + + orangeRec, _, _ := procCreateSolidBrush.Call(uintptr(COLOR_MIC_ORANGE)) + o.barBrushOrange = syscall.Handle(orangeRec) + fontName := syscall.StringToUTF16Ptr("Segoe UI") fontRec, _, _ := procCreateFontW.Call( 18, 0, 0, 0, 600, diff --git a/linux_press_daemon.go b/linux_press_daemon.go index d139daa..4de6c20 100644 --- a/linux_press_daemon.go +++ b/linux_press_daemon.go @@ -76,6 +76,9 @@ func sendLinuxPressPing() error { return nil } +// linuxPressServer holds the HTTP server reference for graceful shutdown. +var linuxPressServer *http.Server + func (a *App) startLinuxPressDaemon() { mux := http.NewServeMux() mux.HandleFunc("/press", func(w http.ResponseWriter, r *http.Request) { @@ -100,7 +103,9 @@ func (a *App) startLinuxPressDaemon() { Addr: linuxPressAddr, Handler: mux, ReadHeaderTimeout: 2 * time.Second, + IdleTimeout: 5 * time.Second, } + linuxPressServer = server if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed { logger.Error("Linux press daemon stopped: %v", err) } diff --git a/linux_press_daemon_stub.go b/linux_press_daemon_stub.go index f73435d..8356625 100644 --- a/linux_press_daemon_stub.go +++ b/linux_press_daemon_stub.go @@ -9,3 +9,5 @@ package main func (a *App) startLinuxPressDaemon() {} + +func stopLinuxPressDaemon() {} diff --git a/text_insert_nonlinux.go b/text_insert_nonlinux.go index 8a1e2fc..2353fa9 100644 --- a/text_insert_nonlinux.go +++ b/text_insert_nonlinux.go @@ -28,13 +28,15 @@ func (a *App) insertTranscription(text string) { // Paste a.pasteText() - // Restore old clipboard after a short delay, but ONLY if the user - // hasn't manually copied something else or another burst hasn't finished. + // Restore old clipboard after a delay, but ONLY if the clipboard still + // contains our transcribed text (i.e. user hasn't copied something else). if clipErr == nil && oldClip != "" { go func() { - time.Sleep(1000 * time.Millisecond) - current, _ := wailsruntime.ClipboardGetText(a.ctx) - if current == text { + time.Sleep(1500 * time.Millisecond) + current, currentErr := wailsruntime.ClipboardGetText(a.ctx) + // Only restore if no error reading AND clipboard still holds our text + // AND it hasn't been modified by another goroutine + if currentErr == nil && current == text { wailsruntime.ClipboardSetText(a.ctx, oldClip) logger.Info("Clipboard history restored") }