fix(lfs): require proof of possession for cross-repo objects

services/lfs/server.go

@@ -263,22 +263,13 @@ func BatchHandler(ctx *context.Context) {
 			}
 
 			if exists && meta == nil {
-				accessible, err := git_model.LFSObjectAccessible(ctx, ctx.Doer, p.Oid)
-				if err != nil {
-					log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err)
-					writeStatus(ctx, http.StatusInternalServerError)
-					return
-				}
-				if accessible {
-					_, err := git_model.NewLFSMetaObject(ctx, repository.ID, p)
-					if err != nil {
-						log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err)
-						writeStatus(ctx, http.StatusInternalServerError)
-						return
-					}
-				} else {
-					exists = false
-				}
+				// The object exists in the content store but is not linked to this
+				// repo. Do not auto-link it based on cross-repo access: the token
+				// only authorizes this repo, and for deploy keys ctx.Doer is the
+				// repo owner, so trusting it here would let a single-repo key pull
+				// objects from any repo the owner can see. Require proof of
+				// possession by making the client upload the (hash-verified) bytes.
+				exists = false
 			}
 
 			responseObject = buildObjectResponse(rc, p, false, !exists, err)
@@ -337,13 +328,17 @@ func UploadHandler(ctx *context.Context) {
 
 	uploadOrVerify := func() error {
 		if exists {
-			accessible, err := git_model.LFSObjectAccessible(ctx, ctx.Doer, p.Oid)
-			if err != nil {
-				log.Error("Unable to check if LFS MetaObject [%s] is accessible. Error: %v", p.Oid, err)
+			// The bytes already exist in the content store. Only skip proof of
+			// possession when the object is already linked to *this* repo; never
+			// trust cross-repo access (ctx.Doer is the repo owner for deploy keys),
+			// which would let a caller link an object it cannot produce.
+			meta, err := git_model.GetLFSMetaObjectByOid(ctx, repository.ID, p.Oid)
+			if err != nil && err != git_model.ErrLFSObjectNotExist {
+				log.Error("Unable to get LFS MetaObject [%s]. Error: %v", p.Oid, err)
 				return err
 			}
-			if !accessible {
-				// The file exists but the user has no access to it.
+			if meta == nil {
+				// The file exists but is not linked to this repo.
 				// The upload gets verified by hashing and size comparison to prove access to it.
 				hash := sha256.New()
 				written, err := io.Copy(hash, ctx.Req.Body)

tests/integration/api_repo_lfs_test.go

@@ -240,16 +240,22 @@ func TestAPILFSBatch(t *testing.T) {
 			assert.Equal(t, "Size must be less than or equal to 2", br.Objects[0].Error.Message)
 		})
 
-		t.Run("AddMeta", func(t *testing.T) {
+		t.Run("CrossRepoObjectRequiresUpload", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 
+			// An object whose bytes already exist in the store but which is not
+			// linked to this repo must not be silently linked, even when the
+			// caller can access it in another repo. Auto-linking let a deploy key
+			// (whose token carries the repo owner's identity) exfiltrate objects
+			// across repos without proving possession. The client must upload.
 			p := lfs.Pointer{Oid: "05eeb4eb5be71f2dd291ca39157d6d9effd7d1ea19cbdc8a99411fe2a8f26a00", Size: 6}
 
 			contentStore := lfs.NewContentStore()
 			exist, err := contentStore.Exists(p)
 			assert.NoError(t, err)
 			assert.True(t, exist)
 
+			// The object is linked to another repo owned by the same user.
 			repo2 := createLFSTestRepository(t, "lfs-batch2-repo")
 			storeObjectInRepo(t, repo2.ID, "dummy0")
 
@@ -266,11 +272,13 @@ func TestAPILFSBatch(t *testing.T) {
 			br := decodeResponse(t, resp.Body)
 			assert.Len(t, br.Objects, 1)
 			assert.Nil(t, br.Objects[0].Error)
-			assert.Empty(t, br.Objects[0].Actions)
+			// The client is told to upload instead of the object being linked.
+			assert.Contains(t, br.Objects[0].Actions, "upload")
 
+			// No meta object may have been created for this repo.
 			meta, err = git_model.GetLFSMetaObjectByOid(t.Context(), repo.ID, p.Oid)
-			assert.NoError(t, err)
-			assert.NotNil(t, meta)
+			assert.Nil(t, meta)
+			assert.Equal(t, git_model.ErrLFSObjectNotExist, err)
 
 			// Cleanup
 			err = contentStore.Delete(p.RelativePath())