Bugzilla

(In reply to Bob Owen (:bobowen) from comment #9)
> (In reply to :Gijs (he/him) from comment #8)
> > (In reply to andrej.wolkow from comment #7)
> > > I disable the inheritance in H:\\test2. This is just to simplify the test to be able to remove the permission. In the default scenario I have 2 different permissions and enabled inheritance.
> > 
> > In that case it sounds like bug 1816694 would have broken the behaviour here, as we avoid manually (re-)applying permissions if permission inheritance is turned on on the parent folder (at least, that seems to be what the patch description says). It's also possible that there's a difference between moves and copies here. Bob, can you take a look at this bug, please? :-)
> 
> The change in bug 1816694 was to not set the file to inherit when it is already inheriting.
> This was for performance and to prevent audit messages in certain configurations.
> I don't see how that could cause this issue, from a permissions point of view the patch shouldn't have changed anything (unless there is a bug).

narrator: there was a bug

Looks setting to inherit in the way we do removes previous access as well.
So, the detection of the existing settings need to check all ACEs.

(In reply to Bob Owen (:bobowen) from comment #9)
> (In reply to :Gijs (he/him) from comment #8)
> > (In reply to andrej.wolkow from comment #7)
> > > I disable the inheritance in H:\\test2. This is just to simplify the test to be able to remove the permission. In the default scenario I have 2 different permissions and enabled inheritance.
> > 
> > In that case it sounds like bug 1816694 would have broken the behaviour here, as we avoid manually (re-)applying permissions if permission inheritance is turned on on the parent folder (at least, that seems to be what the patch description says). It's also possible that there's a difference between moves and copies here. Bob, can you take a look at this bug, please? :-)
> 
> The change in bug 1816694 was to not set the file to inherit when it is already inheriting.
> This was for performance and to prevent audit messages in certain configurations.
> I don't see how that could cause this issue, from a permissions point of view the patch shouldn't have changed anything (unless there is a bug).

narrator: there was a bug

Looks like setting to inherit in the way we do removes previous access as well.
So, the detection of the existing settings needs to check all ACEs.