daemon: Set umask to 0000 #52892
Conversation
| // setDefaultUmask sets the umask to 0022 to avoid problems | ||
| // setDefaultUmask sets the umask to 0 to avoid problems | ||
| // caused by custom umask | ||
| func setDefaultUmask() error { |
There was a problem hiding this comment.
After successfully setting the umask, we should probably also update that global var in (fsutil.UmaskIsZero = true) #52817 (comment)
164b31c to
6417f92
Compare
6417f92 to
99535ea
Compare
Previously the daemon forced its umask to 0022, which silently stripped the group/other-write bits from any explicitly requested file mode. This caused e.g. `COPY --chmod=777` to result in 0755 rather than 0777. Clear the umask so explicit modes are honored. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
99535ea to
9384dea
Compare
|
FWIW, we can still suppress the CodeQL warning through the UI, but I generally prefer it to be captured in code where possible (think it's cleaner to have a comment in version control that indicates it was evaluated and decided on). |
Clearing the daemon umask means creation sites can no longer rely on umask 0022 to remove group/other-write bits from permissive modes. Adjust modes for daemon-owned files that previously depended on the old umask: - fuse-overlayfs lower file - goroutine stack dump file - layer migration tar-data file - network namespace mount file Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Managed containerd inherits the daemon's cleared umask, so files it creates are no longer filtered by the historical 0022 umask. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
9384dea to
533ae76
Compare
|
I left the suppression commit, but dropped the graphql workflow change. Let's defer to a follow up |
|
Yeah, let's suppress it manually. I'm still wondering if there's 2 separate CodeQL workflows running; a default (on the repository config) and one defined as GitHub action. There's also still a warning that there's a config issue that we still need to look at (CodeQL is really confusing, more so due to our DIND flows I guess) |
| "github.com/moby/moby/v2/daemon/libnetwork/portallocator" | ||
| "github.com/moby/moby/v2/pkg/homedir" | ||
| "github.com/pkg/errors" | ||
| fsutilcopy "github.com/tonistiigi/fsutil/copy" //nolint:depguard // Needed to keep fsutil copy behavior aligned with the daemon umask. |
There was a problem hiding this comment.
Ah, yes, I forgot about the depguard.
Perhaps the workaround can be removed now that docker sets the proper umask as well, but not sure if it's used elsewhere.
|
Probably flaky test; |
Summary
Clear the daemon umask so explicitly requested file modes are preserved instead of being filtered by the previous
0022umask.This fixes cases such as
COPY --chmod=777producing0755permissions, and adjusts daemon-owned file creation sites that previously relied on the old umask to remove group/other-write bits.Also document that managed containerd inherits the daemon's cleared umask.
Release notes (optional)
A picture of a cute animal (not mandatory but encouraged)