From 12a0ca9656a5a7025aa2bbd7171b60c3c2b764be Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 2 Jul 2024 12:24:57 -0400 Subject: [PATCH] assistant: Fix a race condition that could cause a pointer file to get ingested into the annex This was caused by commit fb8ab2469d389e5b1e554831eeb8b7c7a072d5d7 putting an isPointerFile check in the wrong place. So if the file was not a pointer file at that point, but got replaced by one before the file got locked down, the pointer file would be ingested into the annex. The fix is simply to move the isPointerFile check to after safeToAdd locks down the file. Now if the file changes to a pointer file after the isPointerFile check, ingestion will see that it changed after lockdown, and will refuse to add it to the annex. Sponsored-by: the NIH-funded NICEMAN (ReproNim TR&D3) project --- Assistant/Threads/Committer.hs | 21 ++++++++++++++++--- Assistant/Threads/Watcher.hs | 7 ++----- CHANGELOG | 7 +++++++ ..._41___commited_unlocked_link_to_annex.mdwn | 3 +++ ..._44ab02fd027efc56ce8bb701e5513b1d._comment | 15 +++++++++++++ 5 files changed, 45 insertions(+), 8 deletions(-) create mode 100644 doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex/comment_7_44ab02fd027efc56ce8bb701e5513b1d._comment diff --git a/Assistant/Threads/Committer.hs b/Assistant/Threads/Committer.hs index 07013c0486..229ad17d1a 100644 --- a/Assistant/Threads/Committer.hs +++ b/Assistant/Threads/Committer.hs @@ -290,19 +290,34 @@ handleAdds lockdowndir havelsof largefilematcher annexdotfiles delayadd cs = ret refillChanges postponed returnWhen (null toadd) $ do + (addedpointerfiles, toaddrest) <- partitionEithers + <$> mapM checkpointerfile toadd (toaddannexed, toaddsmall) <- partitionEithers - <$> mapM checksmall toadd + <$> mapM checksmall toaddrest addsmall toaddsmall addedannexed <- addaction toadd $ catMaybes <$> addannexed toaddannexed - return $ addedannexed ++ toaddsmall ++ otherchanges + return $ addedannexed ++ toaddsmall ++ addedpointerfiles ++ otherchanges where (incomplete, otherchanges) = partition (\c -> isPendingAddChange c || isInProcessAddChange c) cs returnWhen c a | c = return otherchanges | otherwise = a - + + checkpointerfile change = do + let file = toRawFilePath $ changeFile change + mk <- liftIO $ isPointerFile file + case mk of + Nothing -> return (Right change) + Just key -> do + mode <- liftIO $ catchMaybeIO $ fileMode <$> R.getFileStatus file + liftAnnex $ stagePointerFile file mode =<< hashPointerFile key + return $ Left $ Change + (changeTime change) + (changeFile change) + (LinkChange (Just key)) + checksmall change | not annexdotfiles && dotfile f = return (Right change) diff --git a/Assistant/Threads/Watcher.hs b/Assistant/Threads/Watcher.hs index 2df29ce76c..3a72901087 100644 --- a/Assistant/Threads/Watcher.hs +++ b/Assistant/Threads/Watcher.hs @@ -196,11 +196,8 @@ shouldRestage :: DaemonStatus -> Bool shouldRestage ds = scanComplete ds || forceRestage ds onAddFile :: Bool -> Handler -onAddFile symlinkssupported f fs = do - mk <- liftIO $ isPointerFile $ toRawFilePath f - case mk of - Nothing -> onAddFile' contentchanged addassociatedfile addlink samefilestatus symlinkssupported f fs - Just k -> addlink f k +onAddFile symlinkssupported f fs = + onAddFile' contentchanged addassociatedfile addlink samefilestatus symlinkssupported f fs where addassociatedfile key file = Database.Keys.addAssociatedFile key diff --git a/CHANGELOG b/CHANGELOG index fa9509fe61..9e467cfb12 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,10 @@ +git-annex (10.20240702) UNRELEASED; urgency=medium + + * assistant: Fix a race condition that could cause a pointer file to + get ingested into the annex. + + -- Joey Hess Tue, 02 Jul 2024 12:14:53 -0400 + git-annex (10.20240701) upstream; urgency=medium * git-annex remotes can now act as proxies that provide access to diff --git a/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex.mdwn b/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex.mdwn index 1b084030c1..4c14d1b40f 100644 --- a/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex.mdwn +++ b/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex.mdwn @@ -104,3 +104,6 @@ on laptop where I dive into inception: 10.20240129 [[!meta author=yoh]] [[!tag projects/repronim]] + +[[!meta title="inception: pointer file can be ingested into the annex due to assistant bug or manually"]] + diff --git a/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex/comment_7_44ab02fd027efc56ce8bb701e5513b1d._comment b/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex/comment_7_44ab02fd027efc56ce8bb701e5513b1d._comment new file mode 100644 index 0000000000..3fa5b92a77 --- /dev/null +++ b/doc/bugs/assistant___40__webapp__41___commited_unlocked_link_to_annex/comment_7_44ab02fd027efc56ce8bb701e5513b1d._comment @@ -0,0 +1,15 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 7""" + date="2024-07-02T16:15:28Z" + content=""" +I've fixed this race in the assistant. + +Question now is, can this bug be closed, or does it need to be left open, +and git-annex made to recover from this situation? Given the complexity +of making git-annex notice this, I'm sort of inclined to not have it +auto-recover. Manual recovery seems pretty simple, just delete the file and +re-add it with the right key. + +Thoughts? +"""]]