security-union / security-union/remote-shutter

Photos save races sendResource and can delete the movie mid-transfer

Open
#160 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Swift
Stars
19
Forks
3
Avg merge
28m
Merged PRs (30d)
6

Description

RemoteCam/RecordingPipeline.swift:181-215 starts the peer transfer and the Photos save over the same file, concurrently:

private func saveMovieToPhotosAppAndRemotePeer(_ sendVideoToPeer: Bool) {
    let outputFileURL = movieUrl()
    if sendVideoToPeer {
        sendVideoAsResource(outputFileURL)          // fire-and-forget into the actor
    } ...
    PHPhotoLibrary.requestAuthorization { [weak self] status in
        if status == .authorized {
            PHPhotoLibrary.shared().performChanges({
                options.shouldMoveFile = true       // :197 — MOVES the file being transferred
                ...
            }, completionHandler: { success, error in
                cleanupFileAt(outputFileURL)        // :205 — DELETES it
            })
        } else {
            cleanupFileAt(outputFileURL)            // :212 — DELETES it on denial
        }
    }
}

movieUrl() is a single fixed temp path (RemoteCam/MediaProcessors.swift:13-15), so there is exactly one file and two owners.

If the Photos leg wins — denial fires :212 almost immediately — handleSendVideoResource's FileManager.default.attributesOfItem (SessionCoordinator.swift:1097) throws, producing VideoResourceTransferFailed. Which lands on the :936 case (see the sibling issue) and may be swallowed.

Fix direction: give each recording a unique temp path, and don't hand the file to Photos (shouldMoveFile) or delete it until the peer transfer has finished with it.

Not observed in the wild; found by inspection while diagnosing the Mac recording hang (PR #157).

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with saveMovieToPhotosAppAndRemotePeer in RemoteCam/RecordingPipeline.swift:181-215, then inspect movieUrl() in RemoteCam/MediaProcessors.swift:13-15 and handleSendVideoResource in SessionCoordinator.swift:1097. Trace both ownership paths and the :936 error case; done means each recording has an independent temporary file and neither Photos nor cleanup removes it before peer transfer finishes.

Written by the indexing model from the issue text.

Assessment

Tech stack
swift
Domain
mobile-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
63/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.