'bzr push' does not preserve sgid bit on newly created directories over sftp
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Bazaar |
Confirmed
|
Medium
|
Unassigned | ||
Breezy |
Triaged
|
Low
|
Unassigned |
Bug Description
It seems that the chmod through sftp strips sgid and suid bits. So even if a directory is created with sgid, trying to 'chmod 2775' sets the mode to 775 on the remote side.
Steps to reproduce:
$ bzr init a
$ echo a > a/a
$ bzr add a/a
$ bzr commit -m a a/a
$ bzr branch a b
$ chown -R .users b/.bzr
$ find b/.bzr -type d -print0 | xargs -0 chmod 2770
$ find b/.bzr -type f -print0 | xargs -0 chmod 660
# Branch 'b' should now be set up properly as a shared repository.
$ echo b > a/b
$ bzr add a/b
$ bzr commit -m b a/b
$ cd a
$ bzr push sftp://
$ cd ../b/.bzr/
$ ll
If you look closely, you will see that the directory that holds 'a-*.knit' will have the sticky group bit set, but the directory that holds 'b-*.knit' does not. It has permissions 770, but not 2770.
Simon Ekstrand has this comment:
Unless this has been fixed recently, it's broken, atleast for
open-ssh(d). Openssh's serverside sftp implementation masks out any suid/sgid bits on chmod operations for regular users. The sftp transport mkdir method does a mode-less mkdir then a chmod, which doesn't work. Including the directory mode with the mkdir call would work fine (tested), but there's a comment in the mkdir method about this breaking with the sftp server used for tests.
So it seems simply updating SftpTransport to use mode with mkdir, and then fix the stub server.
We still have to worry a little bit about if the remote umask prevents us from creating the bits we want.
tags: | added: sftp |
summary: |
- 'bzr push' does not preserve sgid bit on newly created directories + 'bzr push' does not preserve sgid bit on newly created directories over + sftp |
tags: | added: check-for-breezy |
Changed in brz: | |
status: | New → Triaged |
importance: | Undecided → Medium |
tags: | removed: check-for-breezy |
Changed in brz: | |
importance: | Medium → Low |
shouldn't os.mkdir() honor the umask anyway? it seems like we should just be able to use sftp.mkdir(path, mode) and stub_sftp will "just work".