Repository: isomorphic-git/isomorphic-git
Pull request #1521 (2022-03-21 11:13:23)

fix(updateIndex): prevent removing directories
I noticed I had a small oversight inside the `updateIndex` API. 

- Removing a directory should throw an error if `force` was not used.
- Removing a directory with `force` should not remove all files inside that directory.

This PR fixes both issues and adds tests for both.

## I'm fixing a bug or typo

- [x] if this is your first time contributing, run `npm run add-contributor` and follow the prompts to add yourself to the README
- [x] squash merge the PR with commit message "fix(updateIndex): prevent removing directories"


Patch:
diff --git a/.releaserc b/.releaserc
index 752147ce..83b48acc 100644
--- a/.releaserc
+++ b/.releaserc
@@ -10,4 +10,4 @@ branches:
 success:
   - '@semantic-release/github'
   - path: '@semantic-release/exec'
-    cmd: "node __tests__/__helpers__/tweet.js 'v${nextRelease.version} has been published! https://github.com/isomorphic-git/isomorphic-git/releases/tag/v${nextRelease.version}'"
+    cmd: "node __tests__/__helpers__/tweet.cjs 'v${nextRelease.version} has been published! https://github.com/isomorphic-git/isomorphic-git/releases/tag/v${nextRelease.version}'"
diff --git a/src/api/updateIndex.js b/src/api/updateIndex.js
index 6dd6de3d..b7e7929e 100644
--- a/src/api/updateIndex.js
+++ b/src/api/updateIndex.js
@@ -78,15 +78,22 @@ export async function updateIndex({
             fileStats = await fs.lstat(join(dir, filepath))
 
             if (fileStats) {
+              if (fileStats.isDirectory()) {
+                // Removing directories should not work
+                throw new InvalidFilepathError('directory')
+              }
+
               // Do nothing if we don't force and the file still exists in the workdir
               return
             }
           }
 
-          // Remove the file from the index if it's forced or the file does not exist
-          index.delete({
-            filepath,
-          })
+          // Directories are not allowed, so we make sure the provided filepath exists in the index
+          if (index.has({ filepath })) {
+            index.delete({
+              filepath,
+            })
+          }
         }
       )
     }


Test patch:
diff --git a/__tests__/__helpers__/tweet.js b/__tests__/__helpers__/tweet.cjs
similarity index 100%
rename from __tests__/__helpers__/tweet.js
rename to __tests__/__helpers__/tweet.cjs
diff --git a/__tests__/test-updateIndex.js b/__tests__/test-updateIndex.js
index 82dee6f8..341de31b 100644
--- a/__tests__/test-updateIndex.js
+++ b/__tests__/test-updateIndex.js
@@ -265,4 +265,74 @@ describe('updateIndex', () => {
       }
     `)
   })
+
+  it('should throw if we try to remove a directory', async () => {
+    // Setup
+    const { fs, dir } = await makeFixture('test-empty')
+    await fs.mkdir(path.join(dir, 'hello-world'))
+    await fs.write(path.join(dir, 'hello-world/a'), 'a')
+    await add({
+      fs,
+      dir,
+      filepath: 'hello-world/a',
+    })
+    // Test
+    let error = null
+    try {
+      await updateIndex({
+        fs,
+        dir,
+        remove: true,
+        filepath: 'hello-world',
+      })
+    } catch (e) {
+      error = e
+    }
+    expect(error).not.toBeNull()
+    expect(error.caller).toEqual('git.updateIndex')
+    error = error.toJSON()
+    delete error.stack
+    expect(error).toMatchInlineSnapshot(`
+      Object {
+        "caller": "git.updateIndex",
+        "code": "InvalidFilepathError",
+        "data": Object {
+          "reason": "directory",
+        },
+        "message": "\\"filepath\\" should not be a directory.",
+      }
+    `)
+  })
+
+  it('should not throw if we force remove a directory', async () => {
+    // Setup
+    const { fs, dir } = await makeFixture('test-empty')
+    await fs.mkdir(path.join(dir, 'hello-world'))
+    await fs.write(path.join(dir, 'hello-world/a'), 'a')
+    await add({
+      fs,
+      dir,
+      filepath: 'hello-world/a',
+    })
+    // Test
+    let fileStatus = await status({
+      fs,
+      dir,
+      filepath: 'hello-world/a',
+    })
+    expect(fileStatus).toBe('added')
+    await updateIndex({
+      fs,
+      dir,
+      remove: true,
+      force: true,
+      filepath: 'hello-world',
+    })
+    fileStatus = await status({
+      fs,
+      dir,
+      filepath: 'hello-world/a',
+    })
+    expect(fileStatus).toBe('added')
+  })
 })