assistant: Fix a race condition that could cause a pointer file to get ingested into...

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

diff --git a/Assistant/Threads/Committer.hs b/Assistant/Threads/Committer.hs
index 07013c0486492110de807882bc7e9611665b9e7e..229ad17d1aed64d7a866d61df3c340b219ec5044 100644 (file)
--- 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 2df29ce76c4410196760e143f99e84b6cd67c9d2..3a729010877a9d456477384961b051f5059e255d 100644 (file)
--- 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 fa9509fe619d65018530827603a7e401f1dc5500..9e467cfb1257625e0300475d81eff5a7b716eb6a 100644 (file)
--- 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 <id@joeyh.name>  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 1b084030c1acf1af16c681c3f93c0b4b6b942bb7..4c14d1b40f7a6d2d199db8877e15fd262ae017a8 100644 (file)
--- 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 (file)
index 0000000..3fa5b92
--- /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?
+"""]]