Skip to content

FileRepository volume_mounts joins omit machine_id — cross-device path corruption #195

Description

@utof

Summary

volume_mounts is keyed on (volume_id, machine_id) — the same volume mounts at different paths on different machines. Three SqliteFileRepository queries join it without filtering machine_id, so on a database that has ever seen more than one machine they can pair a local file row with a foreign machine's mount path.

Affected queries

All in crates/db/src/file_repo.rs:

fn join
list_files_needing_backfill_sql LEFT JOIN volume_mounts vm ON vm.volume_id = fl.volume_id AND vm.deleted_at IS NULL
lookup_by_file_uuid SELECT body same shape
list_files_pending_full_hash_sql JOIN volume_mounts vm ON … AND vm.deleted_at IS NULL

None constrain vm.machine_id. Two of them then reduce with MIN(vm.mount_path), so the winner is whichever path sorts first — not the local one.

Contrast: the volume repo gets this right

crates/db/src/volume_repo.rs:100-111 takes the machine as a parameter and filters on it:

LEFT JOIN volume_mounts vm
  ON vm.volume_id = v.volume_id
 AND vm.machine_id = ?1
 AND vm.deleted_at IS NULL

and the schema carries an index built for exactly this access pattern (V001__initial.sql:57):

CREATE INDEX idx_volume_mounts_volume_machine ON volume_mounts(volume_id, machine_id);

So the correct shape is established; these three joins just don't use it.

Impact

Single-machine databases are unaffected — there is only one machine_id, so the unfiltered join is accidentally correct. It goes wrong as soon as a second machine records a mount for the same volume:

  • Backfill worker computes quick_hash from active_path. A foreign mount path either does not exist (row silently skipped — cf. Backfill worker: emit tracing::warn! + count rows with no active mount #162) or, worse, does exist locally and resolves to a different file, writing a wrong quick_hash against a real hash.
  • lookup_by_file_uuid feeds the absolute path used for transcription source resolution.
  • The bug is silent in every case: no error, just a path belonging to another machine.

Fix

Thread DeviceId into the three methods and add AND vm.machine_id = ?n, mirroring VolumeRepository::list. The trait methods currently take only limit / file_uuid:

fn list_files_needing_backfill(&self, limit: u32) -> Result<Vec<BackfillFileRow>, CoreError>;
fn list_files_pending_full_hash(&self, limit: usize) -> Result<Vec<FileUuid>, CoreError>;

Both production call sites already have a device id in hand (crates/cli/src/main.rs:654, crates/desktop/src/lib.rs:349), so the change is mechanical — but it is a public trait-signature change across perima-core, perima-db, and both shells.

Related

Same class as #165 (update_full_hash should take a DeviceId for CRDT correctness) — both are FileRepository methods that need device identity and don't take it. Worth doing as one pass over the trait rather than piecemeal.

Also related to #162 (backfill worker should warn + count rows with no active mount): once machine_id is filtered, "no active mount on this machine" becomes the precise, correct meaning of that count.

Discovery context

Found while designing the location verify-sweep, which needs this join and must not inherit the defect: a sweep that stats foreign mount paths would mark the entire local catalogue Missing, and a prune button would then delete it. The sweep will filter machine_id from the start; this issue tracks the three pre-existing queries.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/cliTouches the perima CLI binarypriority/mediumDefault priority — address in normal flowtype/bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions