Overwriting rename is not atomic

[wmertens]

• Operating System: Linux 5
• Node.js version: 12
• fs-extra version: latest

As you can see in

node-fs-extra/lib/move/move.js

Lines 34 to 37 in 6bffcd8

	return remove(dest, err => {
	if (err) return cb(err)
	return rename(src, dest, overwrite, cb)
	})

, when overwriting, the target is first removed. This is not an atomic operation and can leave the target removed but the source in place.

However, the rename call already removes the target, and this is an atomic operation.

So IMHO, move should only check if the target exists when not overwriting, it should not delete the file itself. (and if the source and target have the same inode, it should not count as already existing, #801)

[RyanZim]

Ah, good point; PR welcome to fix this.

[RyanZim]

Set out to fix this today; I didn't read the docs close enough:

In the case that newPath already exists, it will be overwritten. If there is a directory at newPath, an error will be raised instead.

We support moving dirs, and at this stage in the code, we know nothing about the state of the destination. We're gonna have to be non-atomic for dirs anyhow; so I'm going to elect to leave this as-is. If you're working with files only, and you need perfect atomic behavior, use fs.rename directly.