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

syscall: Windows does not respect permission bits passed to Open #35033

Closed
bcmills opened this issue Oct 21, 2019 · 6 comments
Closed

syscall: Windows does not respect permission bits passed to Open #35033

bcmills opened this issue Oct 21, 2019 · 6 comments
Labels
FrozenDueToAge help wanted modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 21, 2019

In the course of testing #31481, I found the following failure on the windows builders at patch set 3 (https://storage.googleapis.com/go-build-log/096e97bd/windows-386-2008_7b478c28.log):

--- FAIL: TestScript (0.00s)
    --- FAIL: TestScript/mod_cache_rw (0.29s)
        script_test.go:191: 
            # Regression test for golang.org/issue/31481. (0.000s)
            # golang.org/issue/31481: an explicit flag should make directories in the module
            # cache writable in order to work around the historical inability of 'rm -rf' to
            # forcibly remove files in unwritable directories. (0.210s)
            # After adding an extraneous file, 'go mod verify' should fail. (0.046s)
            # However, files within those directories should still be read-only to avoid
            # accidental mutations. (0.001s)
            > [!root] ! cp $WORK/extraneous.txt $GOPATH/pkg/mod/rsc.io/[email protected]/go.mod
            FAIL: testdata\script\mod_cache_rw.txt:16: unexpected command success
            
FAIL
FAIL	cmd/go	75.878s

This suggests that either the root condition in TestScript is failing to detect a user acting in some sort of root-like role on Windows, or else we are failing to mark the contents of the module cache read-only on Windows.

CC @jayconrod @alexbrainman @zx2c4
@bcmills bcmills added help wanted OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. modules labels Oct 21, 2019
@bcmills bcmills added this to the Backlog milestone Oct 21, 2019
@zx2c4
Copy link
Contributor

zx2c4 commented Oct 21, 2019

Suggested issue title change: "syscall: Windows does not respect permission bits passed to Open"

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/202439 mentions this issue: syscall: respect permission bits on file opening on Windows

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/202440 mentions this issue: windows: respect permission bits on file opening

@bcmills bcmills changed the title cmd/go: files in module cache may be writable on Windows syscall: Windows does not respect permission bits passed to Open Oct 21, 2019
@bcmills
Copy link
Contributor Author

bcmills commented Oct 21, 2019

Thanks! I wasn't sure what the root cause would turn out to be, so I didn't want to make assumptions in the initial title. 😉

gopherbot pushed a commit to golang/sys that referenced this issue Oct 22, 2019
@zx2c4
windows: respect permission bits on file opening
742c48e 
Chmod toggles the FILE_ATTRIBUTES_READONLY flag depending on the
permission bits. That's a bit odd but I guess some compromises were made
at some point and this is what was chosen to map to a Unix concept that
Windows doesn't really have in the same way. That's fine. However, the
logic used in Chmod was forgotten from Open, which then manifested
itself in various places, most recently, go modules' read-only behavior.

This corresonds with the syscall CL 202439.

Updates golang/go#35033

Change-Id: Id8e74c5205057a74a35eda213516780b79a2aed2
Reviewed-on: https://go-review.googlesource.com/c/sys/+/202440
Run-TryBot: Jason A. Donenfeld <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
@erifan erifan mentioned this issue Nov 25, 2019
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/223957 mentions this issue: doc/go1.14: mention Windows change for Open permissions

bjackman added a commit to bjackman/fleetspeak that referenced this issue Mar 18, 2020
@bjackman
Fix permission error with wnix sockets.
0a3781d 
In the fix for golang/go#35033 Go started creating
read-only files on Windows when no read permission bit was requested in the
OpenFile call. This was not documented in the release notes (but see
https://go-review.googlesource.com/c/go/+/223957/).

Deal with the immediate issue by adding owner-read and owner-write permissions
in our WriteFile which creates the file.

Also add a workaround for the fact that previous version of Fleetspeak have left
behind read-only files - simply clear the read-only attribute if the file
already exists.
@bjackman bjackman mentioned this issue Mar 18, 2020
Merged
gopherbot pushed a commit that referenced this issue Mar 18, 2020
@ianlancetaylor
doc/go1.14: mention Windows change for Open permissions
e39de05 
For #35033

Change-Id: Ie15353322d5cfe7320199103ad9543fb89a842ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/223957
Reviewed-by: Brendan Jackman <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/223962 mentions this issue: [release-branch.go1.14] doc/go1.14: mention Windows change for Open permissions

gopherbot pushed a commit that referenced this issue Mar 18, 2020
@ianlancetaylor
[release-branch.go1.14] doc/go1.14: mention Windows change for Open p…
b620f6f 
…ermissions

For #35033
For #36878

Change-Id: Ie15353322d5cfe7320199103ad9543fb89a842ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/223957
Reviewed-by: Brendan Jackman <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
(cherry picked from commit e39de05)
Reviewed-on: https://go-review.googlesource.com/c/go/+/223962
Reviewed-by: Dmitri Shuralyov <[email protected]>
@mattn mattn mentioned this issue Apr 7, 2020
Open
@golang golang locked and limited conversation to collaborators Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted modules 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
@zx2c4 @bcmills @gopherbot