Skip to content

fix(export): harden post-merge file handling#444

Merged
meiiie merged 1 commit into
webadderallorg:mainfrom
meiiie:fix/pr410-post-merge-export-hardening
May 8, 2026
Merged

fix(export): harden post-merge file handling#444
meiiie merged 1 commit into
webadderallorg:mainfrom
meiiie:fix/pr410-post-merge-export-hardening

Conversation

@meiiie
Copy link
Copy Markdown
Collaborator

@meiiie meiiie commented May 8, 2026

Description

Follow-up to #410 for two final CodeRabbit export hardening comments that were left after the merge.

Changes Made

  • Treat EEXIST from the first fs.rename(tempPath, destinationPath) as recoverable so the safe copy/replace fallback can run on Windows overwrite cases.
  • Check the normalized path with isAllowedLocalReadPath() before realpath / stat for non-media local read paths, then keep the realpath re-check for symlink protection.
  • Add a regression test for the initial EEXIST fallback path.

Validation

  • npm test -- electron/ipc/register/export.test.ts electron/ipc/export/native-video.test.ts (67 tests)
  • npx biome check --formatter-enabled=false electron/ipc/register/export.ts electron/ipc/register/export.test.ts
  • npx tsc --noEmit
  • git diff --check

Notes

This does not change CUDA/D3D11 routing, browser mic behavior, or exporter feature gates from #410.

Summary by CodeRabbit

  • Bug Fixes

    • Improved export logic to reliably handle destination conflicts (including Windows-specific cases) by using a safer fallback when a rename can't be trusted.
    • Added earlier permission validation for export paths to fail fast on disallowed locations.
  • Tests

    • Added tests for export edge cases and ensured test mocks are cleaned up after each test.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9180e61f-5472-4654-9025-722b2ab48d78

📥 Commits

Reviewing files that changed from the base of the PR and between 20f111e and 81bfd2d.

📒 Files selected for processing (2)
  • electron/ipc/register/export.test.ts
  • electron/ipc/register/export.ts

📝 Walkthrough

Walkthrough

The PR widens rename-failure handling by treating EEXIST like other fallback-triggering errors, adds an early resolved-path permission check, and updates tests to restore mocks and cover the EEXIST fallback scenario.

Changes

Export Robustness

Layer / File(s) Summary
Temp File Move Fallback
electron/ipc/register/export.ts
moveExportedTempFile adds EEXIST to the set of error codes that trigger the cross-device-safe copy-and-rename fallback sequence.
Path Permission Validation
electron/ipc/register/export.ts
resolveAllowedReadableFilePath adds an early guard that rejects paths whose resolved form is not approved for local reads, before calling fs.realpath or fs.stat.
Test Coverage & Cleanup
electron/ipc/register/export.test.ts
Test suite adds afterEach mock restoration and a new EEXIST scenario test verifying the fallback behavior updates the destination and removes the temp file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • webadderallorg/Recordly#419: Both PRs tighten local-read authorization; this PR shifts the early permission check in resolveAllowedReadableFilePath while the referenced PR modifies isAllowedLocalReadPath and related handlers.

Suggested labels

Checked

Poem

🐰
Paths checked before they roam,
Temp files dance to find a home,
When EEXIST bars the gate,
Copy-rename will set things straight,
A hop, a fix, and all is well.

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: hardening file handling in the export module for post-merge fixes.
Description check ✅ Passed The description includes all critical sections: clear purpose, specific changes made, comprehensive validation steps, and relevant notes about scope.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@meiiie meiiie force-pushed the fix/pr410-post-merge-export-hardening branch from 20f111e to 81bfd2d Compare May 8, 2026 06:08
@meiiie meiiie merged commit 71484b1 into webadderallorg:main May 8, 2026
3 checks passed
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.

1 participant