WIndows: INSTALLDIR's permission seems not right.

[wyzzoo]

INSTALLDIR's permissions seems not right.

Directory permissions after install swift-5.7-DEVELOPMENT-SNAPSHOT-2022-06-04-a-windows10.exe on Windows 10 x64:

>accesschk.exe -d c:/library

Accesschk v6.12 - Reports effective permissions for securable objects
Copyright (C) 2006-2017 Mark Russinovich
Sysinternals - www.sysinternals.com

c:\Library
  RW BUILTIN\Administrators
  RW NT AUTHORITY\SYSTEM
  R  BUILTIN\Users
  RW NT AUTHORITY\Authenticated Users

I don't think you want Authenticated Users Group has the write permission.

[wjk]

If you move the INSTALLDIR somewhere under C:\Program Files, I understand that Windows would then apply those permissions automatically.

Besides, it looks like this exact change is currently being implemented in #139.

[compnerd]

@wjk hmm, I don't see how that PR changes the permissions or the location (it merely renames swift to Swift). The runtime was moved into ProgramFiles a while ago. The toolchain remains under C:\Library as the default (for now). But, I do agree with this in principle - the toolchain image should not be mutable by NT AUTHORITY\Authenticate Users.

[wjk]

I don't see how that PR changes the permissions or the location

My apologies. I confused what you were changing with what was already there. I would recommend moving the toolchain under ProgramFiles as well.

[compnerd]

I don't see how that PR changes the permissions or the location

My apologies. I confused what you were changing with what was already there.

No worries; that PR is also not mine :)

I would recommend moving the toolchain under ProgramFiles as well.

I'd like to do that some day. However, the name is an issue - if we can rename Program Files to ProgramFiles perhaps - I really would prefer not having the space in the path at this point where the path handling is already not very robust.

[wjk]

Unfortunately, the space in Program Files is non-negotiable.

[compnerd]

Unfortunately, the space in Program Files is non-negotiable.

I know; but that also means that it is currently something that is not worth fighting over. Using the alternate path allows focus on the other larger issue of tool stability and quality. Once those are settled, I think going over the codepaths with a fine tooth comb to find any possible issues where the spaces could be a problem (e.g. VFS computation, name computation, etc).

[compnerd]

Okay, I've played around a little bit with the toolchain. Given that we are already renaming part of the toolchain for version information, I think that we can also reasonably just move the toolchain portion safely as well. The current idea is to move the toolchain from %SystemDrive%\Library\Developer\Toolchains\unknown-Asserts-development.xctoolchain to %ProgramFiles%\Swift\Toolchains\[version]+Asserts.

[compnerd]

So, moving to the per-user install would actually mean that we can be safe for the most part (unless the user has a space in the username). The install should get isolated to %LocalAppData%\Programs which shouldn't have a space and thus should be relatively safe. This would also drop the need for Administrator privileges.