Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

os: Mkdir ignores perm on Windows #65377

Open
xiaq opened this issue Jan 30, 2024 · 6 comments
Open

os: Mkdir ignores perm on Windows #65377

xiaq opened this issue Jan 30, 2024 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Backlog

Comments

@xiaq
Copy link

xiaq commented Jan 30, 2024 •
edited

Go version

go version go1.21.6 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\xiaqq\AppData\Local\go-build
set GOENV=C:\Users\xiaqq\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\xiaqq\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\xiaqq\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.21.6
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\xiaqq\AppData\Local\Temp\go-build2957556198=/tmp/go-build -gno-record-gcc-switches

What did you do?

package main

import (
	"fmt"
	"os"
)

func main() {
	file, err := os.OpenFile("f", os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0o444)
	if err != nil {
		panic(err)
	}
	file.Close()
	stat("f")

	os.Mkdir("d", 0o555)
	stat("d")
}

func stat(name string) {
	info, err := os.Stat(name)
	if err != nil {
		panic(err)
	}
	fmt.Println(info.Mode())
}

What did you see happen?

Command output:

-r--r--r--
dr-xr-xr-x

What did you expect to see?

Command output:

-r--r--r--
drwxrwxrwx
@mknyszek
Copy link
Contributor

According to the OS package's documentation on FileMode:

A FileMode represents a file's mode and permission bits. The bits have the same definition on all systems, so that information about files can be moved from one system to another portably. Not all bits apply to all systems. The only required bit is ModeDir for directories.

This may just be a situation where the write bit doesn't have any meaning on Windows for directories.

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 30, 2024
@mknyszek mknyszek added this to the Backlog milestone Jan 30, 2024
@mknyszek
Copy link
Contributor

Actually, I don't know what the write behavior here is. I can tell that os.Mkdir just ends up calling CreateDirectory on Windows and skips passing the SECURITY_DESCRIPTOR. However, that kind of makes sense, since the descriptor looks nothing like file modes.

But, I don't know if the correct thing to do here is to just set the mode afterwards on Windows. I'm guessing since os.Mkdir has, AFAICT, operated this way forever, I'm not sure we can change it now without breaking backwards compatibility either anyway.

CC @qmuntal ... maybe? I'm not sure if this specifically is really your area of expertise with Windows, so apologies for the noise if not.

@xiaq
Copy link
Author

xiaq commented Jan 30, 2024

According to the OS package's documentation on FileMode:

A FileMode represents a file's mode and permission bits. The bits have the same definition on all systems, so that information about files can be moved from one system to another portably. Not all bits apply to all systems. The only required bit is ModeDir for directories.

This may just be a situation where the write bit doesn't have any meaning on Windows for directories.

It does, if you call os.Chmod with 0o555 it will set the read-only bit on directories.

Both os.OpenFile and os.Chmod treats absence of the write bit to mean read-only on Windows, it would make sense for os.Mkdir to be consistent with them.

@qmuntal
Copy link
Contributor

qmuntal commented Jan 31, 2024 •
edited

It does, if you call os.Chmod with 0o555 it will set the read-only bit on directories.

We could use os.Chmod to set the read-only bit on directories, but os.Chmod uses SetFileAttributesW under the hood, and its documentation says that FILE_ATTRIBUTE_READONLY is not honored for directories:

A file that is read-only. Applications can read the file, but cannot write to it or delete it. This attribute is not honored on directories.

This limitation doesn't seem specific to SetFileAttributesW, other sources contain the same warning, e.g.: https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants.

IMO there is nothing we can do here. os.Mkdir ignores the passed permission bits, but so does Windows.

@xiaq
Copy link
Author

xiaq commented Jan 31, 2024

It does, if you call os.Chmod with 0o555 it will set the read-only bit on directories.

We could use os.Chmod to set the read-only bit on directories, but os.Chmod uses SetFileAttributesW under the hood, and it's documentation says that FILE_ATTRIBUTE_READONLY is not honored for directories:

A file that is read-only. Applications can read the file, but cannot write to it or delete it. This attribute is not honored on directories.

This limitation doesn't seem specific to SetFileAttributesW, other sources contain the same warning, e.g.: https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants.

IMO there is nothing we can do here. os.Mkdir ignores the passed permission bits, but so does Windows.

Interesting! Didn't know that setting read-only on a directory doesn't actually make it read-only.

However, contrary to documentation, setting read-only on a directory does do something: it prevents deletion:

md d
attrib +r d
REM will fail with "access denied"
rd d
attrib -r d
REM will succeed
rd d

@qmuntal
Copy link
Contributor

qmuntal commented Mar 7, 2024

However, contrary to documentation, setting read-only on a directory does do something: it prevents deletion:

Good catch. Then it is ok for me to honor the permissions by calling Chmod just after CreateDirectory. Do you want to submit the fix yourself?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@xiaq @mknyszek @qmuntal