2013-08-29 12:00 AEST by Arthur Barrett - Calculating the 'file mode' on windows is a nightmare, and it's gone seriously wrong a few times.  

Glen has been able to recently reproduce this (again) in the lab.

The code tries to be 'clever' and read the persmissions like cygwin does and in a cygwin compatible way.  
But since most people don't use cygwin, it's just (needlessly) complex and surprisingly innacurate.  I've 
adjusted it a bit over the years to default more and more towards using 'native win32 persmissions' and 
having registry options/environment vars to enable the cygwin stuff if people really need that 
compatibility.

GetUnixFileModeNtEA() is determining that the attribs are 0600 - if that is not what the attribs really are 
then we are back to the old problem of this over-complex stuff yielding the incorrect result.

You can use the environment variables to use a more modern routine to determine the mode/attribs:
CYGWIN=ntsec
or
CVSNT=ntsec

GetUnixFileModeNtEA() uses a really old Win32 call pNtQueryEaFile() to get the status, wheras 
GetUnixFileModeNtSec() uses the more modern DACL calls (GetSecurityDescriptorOwner and 
GetSecurityDescriptorGroup).  

The earlier call to GetFileAttributesEx() identifies it as:
- a file (not a directory)
- a read/write file

There should probably be some logic after the call to GetUnixFileModeNtSec()/GetUnixFileModeNtEA() to 
test that either the owner/group/other 'write' bit is set if GetFileAttributesEx() says it is a read/write file...  
eg:
simplemode=GetFileAttributesEx()
mode=GetUnixFileModeNtEA()
if  !(simplemode&FILE_ATTRIBUTE_READONLY)
  if !(mode&0222)
    mode!=200;

Or even better maybe CVS Suite 2010/2.8.02 should use ntsec by default not ntea...

2013-08-30 01:15 AEST by Glen Starrett - Unfortunately experimentation with CYGWIN=ntsec and CVSNT=ntsec show that it
only adds 'x' to the user permission and doesn't change group permissions at
all.  E.g. in this case I've added 3 files.  The first listed is with neither
CYGWIN or CVSNT set to anything, the 2nd is CYGWIN=ntsec and the 3rd is CVSNT=ntsec.


-rw------- 1 gstarret apache     4 Aug 29 07:57 new-08-29a.txt
-rwx------ 1 gstarret apache     4 Aug 29 07:59 new-08-29b.txt
-rwx------ 1 gstarret apache     4 Aug 29 07:52 new-08-29.txt

2013-08-30 01:22 AEST by Glen Starrett - Possible workarounds:

* To permanently fix the permissions:
  * Check them out on a UNIX/Linux host
  * Correct the permissions
  * Commit with -f (force) to set the permissions

The problem with this approach is that it's a manual process that has to be done
after the file has been added & committed on the Win32 system.


* To workaround the problem with postcommit:
  * Create a script that runs chmod to correct the permissions (make sure that
all cvs users have permission to run the script)
  * Add a line in postcommit to run that script after a commit has been made to
the affected module

The problem with the this approach is that it has to change the permissions on
all files added from win32 every time.

Sample script (saved in /usr/local/bin/ directory):
>>>>
[gstarret@pluto bin]$ ll fix-gapt-shadow-perms
-rwxr-xr-- 1 gstarret cvsusers 114 Aug 29 09:13 fix-gapt-shadow-perms
[gstarret@pluto bin]$ cat fix-gapt-shadow-perms
#!/bin/bash

echo Correcting permissions...
chmod -Rf g+r /var/www/html-gapt/*
echo Done correcting permissions!
<<<<

Line in postcommit:
>>>>
^gapt-html /usr/local/bin/fix-gapt-shadow-perms
<<<<

2013-08-30 01:47 AEST by Glen Starrett - Another workaround:

* Set the following environment variable on the client:
CVSNT=nontsec,nontea

This will cause new files to be added with RW for everyone (ugo=rw).  With this
sample z1.txt was added & committed before setting the variable on the client,
z2.txt after:

[gstarret@pluto html-cool]$ ll z*
-rw------- 1 gstarret apache 5 Aug 29 09:35 z1.txt
-rw-rw-rw- 1 gstarret apache 5 Aug 29 09:36 z2.txt

The downside to this workaround is that:
1) The environment variable must be set on each Windows client that could add files
2) It sets global RW permissions on the files

2013-08-30 10:16 AEST by Arthur Barrett - OK - ignore most of what is written above.

Changing to enhancement.

This bug is about: add a file on windows, checkout on linux, get back group / world permissions (mode)

The permissions (mode) is calculated on windows using one of three algorithms:
- ntsec
- ntea
- hardcoded 644

Regardless of how the permissions (mode) is calculated on windows, it is then mangles with this 
algorithm:


  if ( bhfi.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
   buf->st_mode &= ~(S_IWRITE | S_IWGRP | S_IWOTH);
  else
   buf->st_mode |= _S_IWRITE | ((buf->st_mode & S_IRGRP) ? S_IWGRP : 0)
        | ((buf->st_mode & S_IROTH) ? S_IWOTH : 0);


The upshot of that is 'if the file is not read only, then set the group and other bits to write'.

That logic makes sense from a purely windows POV, but when the ntsec algorithm has gone to a bunch of 
trouble to find group and 'everyone' permissions, it's unhelpful.

This specific case occurs when the 'checkout' trigger is used, which highlights that users of 'checkout' 
trigger may expect that the 'checked out shadow copy' has particular permissions.

So there are several things asked for:
- some way to define the calculation mode more easily than setting environment variables 
  (but should environment variables when set override the other setting?)
- some default that is sensible (for the calc method)
- some 'manual' calculation method (eg: be able to set the 644 default in registry)
- some way the server can be configured to restrict the 'stored' permissions (umask)
- some way the server/checkout trig can be configured to set permissions or adjust permissions (umask) 
- fix ntsec so that it doesn't return 700 all the time... (eg: for text files with everyone read set)


I think the client should get the permission scheme from the server if possible - and that would override 
any environment variable.  But the local HKCU or HKLM registry could also be used, and that would be a 
'default' but not override the env. variable.

Lastly I think the 'fixed' ntsec should probably have a new name since some people may be relying on this 
rubbish behaviour (maybe it works properly for cygwin permissions...), eg: ntsec2, ntnew, ntfix, etc...