Fix security and reliability issues

[hobostay]

Summary

• Security: Validate URL scheme in open-external-url handler — The shell.openExternal(url) call accepted any URL without validation, allowing file://, javascript:, or other dangerous schemes. Now only http:, https:, and mailto: are permitted.
• Bug: Fix latest video detection using mtime — get-recorded-video-path used lexicographic sort to find the latest recording (videoFiles.sort().reverse()[0]), which is unreliable (e.g. recording-9.webm sorts after recording-10.webm). Now sorts by file modification time.
• Bug: Add null guard for AudioData.format — cloneWithTimestamp used a non-null assertion (src.format!) that could throw an unhelpful error if format is null. Now validates upfront with a descriptive error message.
• Bug: Prevent encodeQueue underflow — The encodeQueue counter in VideoExporter could go negative if the encoder's output callback fires more times than expected. Added Math.max(0, ...) guard.

Test plan

• Verify open-external-url blocks file:///etc/passwd and allows https://example.com
• Create multiple recordings (recording-9, recording-10, etc.) and verify the latest is returned
• Export a project with audio and verify no regression in audio processing
• Export a video project and verify no regression in export pipeline

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved recorded video detection to use actual file modification times when identifying the latest recording, avoiding incorrect picks.
  • Added validation and protocol restrictions for opening external URLs (only http, https, mailto), with clear error responses for invalid or unsupported URLs.
  • Enforced audio format presence in the encoder to prevent silent failures when format data is missing.
  • Prevented video encoder queue underflow by clamping the queue counter to never go negative.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1d7e50d1-ce81-4277-bc65-64e85c5a7247

📥 Commits

Reviewing files that changed from the base of the PR and between cf6dce5 and a20a31f.

📒 Files selected for processing (4)

• electron/ipc/handlers.ts
• src/components/video-editor/VideoEditor.tsx
• src/lib/exporter/audioEncoder.ts
• src/lib/exporter/videoExporter.ts

---

📝 Walkthrough

Walkthrough

Swapped filename-based selection for newest recorded video to mtime-based scanning with stat error handling; tightened external URL handler with parsing and scheme whitelist; audio cloning now requires AudioData.format; video encoder prevents encodeQueue underflow; removed webcamSizePreset from a saveProject dependency.

Changes

Cohort / File(s)	Summary
IPC Handler Updates electron/ipc/handlers.ts	get-recorded-video-path now selects latest .webm by fs.stat().mtimeMs, skips files that fail stat, and returns { success: false, message: "No recorded video found" } if none found. open-external-url parses the URL, rejects invalid input, restricts to http:, https:, mailto:, and calls shell.openExternal(parsed.toString()).
Audio Encoder Validation src/lib/exporter/audioEncoder.ts	cloneWithTimestamp enforces presence of AudioData.format (throws if missing) and checks planar-ness via src.format.includes("planar"); passes src.format directly when constructing cloned AudioData.
Video Encoder Queue Safety src/lib/exporter/videoExporter.ts	Encode callback clamps decrement with this.encodeQueue = Math.max(0, this.encodeQueue - 1) to avoid negative encodeQueue.
Video Editor deps src/components/video-editor/VideoEditor.tsx	Removed webcamSizePreset from saveProject dependency array so callback identity no longer changes when that state updates (callback still reads the value).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• feat: Add webcam size with slider #345 — touches VideoEditor.tsx and webcamSizePreset usage; likely directly related to the save/export/serialization behavior adjusted here.

Poem

files get older, not louder — we pick by mtime,
links get checked, only safe schemes get to rhyme,
audio wants its format, no mystery please,
queues stop sinking below zero with ease,
tiny fixes, lowkey graceful, midnight cleanups 🌙🛠️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately summarizes the main changes: security validation and bug fixes for reliability. Specific enough for scanning history.
Description check	✅ Passed	PR description comprehensively covers all changes with clear motivation, includes testing checklist, but doesn't follow the repo's required template structure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@electron/ipc/handlers.ts`:
- Around line 636-648: The code validates the URL but calls
shell.openExternal(url) with the original string; instead re-serialize the
parsed URL to ensure a normalized, validated value is used—after the
ALLOWED_SCHEMES check and using the parsed variable (e.g., parsed.toString() or
parsed.href) pass that into shell.openExternal rather than the raw url string to
follow Electron's recommended pattern.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0bcf54bc-0dcf-4d1d-a577-ef47abd7f55c

📥 Commits

Reviewing files that changed from the base of the PR and between e7d5f51 and cf6dce5.

📒 Files selected for processing (3)

• electron/ipc/handlers.ts
• src/lib/exporter/audioEncoder.ts
• src/lib/exporter/videoExporter.ts

[marcgabe15]

Looks good on my end tested video encoder/exporer w/ mac

[siddharthvaddem]

@hobostay please fix ci checks

[hobostay]

@siddharthvaddem Fixed! All CI issues addressed in 721e8f4:

1. Type Check — Removed trailing comma in SUPPORTED_LOCALES (src/i18n/config.ts) that caused Locale type to include undefined, fixing all 11 downstream type errors.
2. Lint — Removed unused webcamSizePreset from useMemo dependency array in VideoEditor.tsx.
3. CodeRabbit review — Applied the suggested fix: shell.openExternal(parsed.toString()) instead of raw url string.