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.

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

os: file write permission ignored on linux trybots #69561

Closed
neild opened this issue Sep 20, 2024 · 6 comments
Closed

os: file write permission ignored on linux trybots #69561

neild opened this issue Sep 20, 2024 · 6 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@neild
Copy link
Contributor

neild commented Sep 20, 2024

https://go.dev/cl/614296 includes a simple test of file permissions:

  • os.WriteFile a file with 0o444 permissions (read-only).
  • os.WriteFile the file again. This should fail, since the file is read-only.

This test passes for me locally. It fails on linux-386 and linux-amd64 trybots. (It passes on wasip1 builders, ignore that, wasip1 doesn't support file permissions.)

Confusingly, it passes on linux gomotes. I can't replicate the trybot failure anywhere.

Logging in the test confirms that the file is being created with mode 0444. We're still able to write to it.

I have no idea what's going on here; I assume it's some trybot weirdness rather than our bug, but I don't know what might cause it.

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 20, 2024
@cagedmantis cagedmantis added this to the Go1.24 milestone Sep 20, 2024
@cagedmantis
Copy link
Contributor

@ianlancetaylor
Copy link
Member

Is it possible that the trybot is running the tests as root?

I'm checking that possibility in https://go.dev/cl/614377.

@ianlancetaylor
Copy link
Member

Yes, that is why.

I think these tests will have to be skipped for root. We have similar skips in, for example, TestRemoveAllButReadOnlyAndPathError.

@dmitshur
Copy link
Contributor

It's likely related that linux-386 and linux-amd64 builders are configured to run with a no-network check:

https://cs.opensource.google/go/x/build/+/luci-config:main.star;l=527-532;drc=c29a1893e55d9ac317ea49eb6a4479b48521662d

Whose current implementation uses --map-root-user in the unshare call, see here.

If so, the key to reproducing locally is to run the test in the same way:

unshare --net --map-root-user -- \
sh -c 'ip link set dev lo up && go test -run=^TestCreateFilePermissions$'

@dmitshur
Copy link
Contributor

In build 8736249311634815793 (a shard of a real linux-amd64 build on CL 614296, whose no_network property is true) TestCreateFilePermissions fails.
In build 8736232595984980273 (a shadow build with identical inputs as the build above, except it has no_network property overridden to false), TestCreateFilePermissions passes with:

=== RUN   TestCreateFilePermissions
    os_test.go:2948: file mode = 444
--- PASS: TestCreateFilePermissions (0.00s)

So it's confirmed as the relevant variable.

Shadow build launch command details
$ led get-build -real-build 8736249311634815793 | led edit -pa no_network=false | led launch
[I 2024-09-20 19:52:52] getting build definition
[I 2024-09-20 19:52:53] Launching the led job as a real build
[I 2024-09-20 19:52:53] LUCI UI: https://ci.chromium.org/b/8736232595984980273

@neild
Copy link
Contributor Author

neild commented Sep 21, 2024

I think these tests will have to be skipped for root.

I had managed to completely forget that root gets to ignore file modes. Thanks for a mystery solved!

@neild neild closed this as completed Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants