fix: cannot set mode to false

[develar]

because of nodejs validation: must be a 32-bit unsigned integer or an octal string. Received false

[develar]

It is required to use it in snap, where apparmor is applied (see sindresorhus/conf#82).

[lucyllewy]

The default should be tighter than 666 which will grant write ability to everyone on the system. A better alternative is 644, which grants read-only to everyone on the box and read-write to the owner of the file.

Suggested change

	fd = fs.openSync(tmpfile, 'w', options.mode || 0o666)
	fd = fs.openSync(tmpfile, 'w', options.mode || 0o644)

[develar]

No, because this value before umask — so, 666 will be equal to 644 (the same as 777 for dir will be equal to 755). That's why 666 it is default value not only in NodeJS, but also in Go.

[lucyllewy]

oh I see. In that case then it's fine :-)

[develar]

https://www.cyberciti.biz/tips/understanding-linux-unix-umask-value-usage.html

files are created with the access mode 666 and directories with 777

The default umask 002 used for normal user. With this mask default directory permissions are 775 and default file permissions are 664.

[develar]

Got it — you request to remove "write" permission for group (644 vs 664). Hmm.... yes, probably makes sense, but... if options.mode is set to false, it means according to docs that

chown from being ran you can also pass false, in which case the file will be created with the current user's credentials.

and also means, that write-file-atomic should not affect defaults in any way. So, I still think that we should not specify any custom value and obey defaults — NodeJS uses 666 by default.

[develar]

Yeach... according to my experience, file mode is not a way to set file mode — it heavily depends on system umask, so, you are:

• or need to deal somehow with umask (pre-calculate and so on) and hope that it will work
• or simply specify some default mode and then use chmod directly to set wanted permissions (as it is currently implemented — reliable way).

As the current problem is that our custom logic (additional chown/chmod) conflicts with snap sandboxing rules (as result, app cannot start correctly), and not security, we just need to use defaults and do not do more than we need. In this case security is not even to be considered — since it is snap daemon responsibility to care about security (and dir is sandboxed anyway). write-file-atomic just need to write file atomic :)

(yes, I know, that directory permissions doesn't prevent writing to file and I am not security expert, but purpose of this PR is not to change security of created files, but give an ability to disable custom chmod/chown for whatever reason client of this module can have).

[isaacs]

0o666 is the appropriate default here. If the user has set their umask to 0o2 instead of 0o22, then we must assume there's some kind of reason for that (ie, that they wish files to be group-writable by default, which is not uncommon, and not unreasonable in many contexts). Same for 0o777 for directories.

Node.js programs can alter the umask by calling process.umask().

[brunolemos]

@diddledan what's left to get this merged?

[sindresorhus]

@isaacs ⬆️

[isaacs]

Seems fine to me. Published on 3.0.2.

[isaacs]

0o666 is the appropriate default here. If the user has set their umask to 0o2 instead of 0o22, then we must assume there's some kind of reason for that (ie, that they wish files to be group-writable by default, which is not uncommon, and not unreasonable in many contexts). Same for 0o777 for directories.

Node.js programs can alter the umask by calling process.umask().

[magicdawn]

mode has default value for fs.openSync, use the default 0o666 or just leave it undefined is OK