search

nim-lang / RFCs

A repository for your Nim proposals.
135 stars 26 forks source link

Add isWindowsDeveloperModeActive #343

Closed rominf closed 3 years ago

rominf commented 3 years ago

On Windows 10, symlink creation works when the user is an Administrator (see https://github.com/nim-lang/Nim/pull/17012) or if the developer mode is activated (see: https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/). I propose to add the isWindowsDeveloperModeActive function to the winlean module for handling the latter.

The implementation is relatively straightforward; see https://github.com/golang/go/blob/5a8fbb0d2d339fa87a02c0794f5a92c1ce121631/src/os/os_windows_test.go#L1001 for example.

timotheecour commented 3 years ago

@rominf now that https://github.com/nim-lang/Nim/pull/17012 was merged (thanks!) PR welcome for this, and then we need to think what's the proper way to support symlinks for os.copyDir on windows.

One possiblity would be:

proc copyDir(...):
  for ...
    when defined(windows):
      if isAdmin() or isWindowsDeveloperModeActive():
        call copyFile with {`cfSymlinkAsIs`}
    else:
        call copyFile with {`cfSymlinkIgnore`}

Another possibility would be:

proc copyDir(...):
  for ...
    when defined(windows):
      call copyFile with `{cfSymlinkAsIs, cfSymlinkIgnoreErrors}`
    else:
      call copyFile with `{cfSymlinkAsIs}`

with some new cfSymlinkIgnoreErrors

And yet another would be:

proc copyDir(..., onError = onErrorDefault):
  for ...
    call copyFile with `{cfSymlinkAsIs}, onError(path, kind)

which would allow users to pass a callback that would be called on error (with a sane default)

Varriount commented 3 years ago

As I pointed out in the PR regarding isAdmin, this kind of procedure has little value. Furthermore, it promotes the kind of logic that leads to bugs and vulnerabilities.

Using the assumption that a user is an administrator, a developer, etc., to assume that some set of functionality can be used, is fundamentally flawed when you are dealing with modern operating systems. This is because such the conditions on which such assumptions are based can change at a moment's notice (TOCTOU). Not only that, but the assumptions aren't even guaranteed to hold true in any time but the present - for example, what happens if the user is a developer, but certain capabilities which are normally enabled have been disabled via group policy?

Nobody has given a good purpose yet for these kinds of functions that both outweigh their inherent disadvantages, and can't already be done through basic error handling.

timotheecour commented 3 years ago

just because a feature can be misused doesn't mean it's not useful. If we don't provide it, users will re-implement, most likely poorly

The OS will still check permissions when priviledges are needed for an OS operation, so the TOCTOU concerns don't apply here unless you're obviously misusing the feature by granting access to some resource just on the basis of isAdmin / isWindowsDeveloperModeActive, which is not what the feature is about.

example use cases:

fail fast

when we want a script to only run as root, or only run as non-root; eg, see https://electrictoolbox.com/check-user-root-sudo-before-running/; eg, allows to not do some initial tasks if later tasks would fail because of that

improved error messages

giving relevant context, eg: (After a common failed with a permission error): report whether user was admin or had developper mode enabled

relying on EPERM errno doesn't always work

eg if calling a third party script or a command (eg apt get install); you'll get a non-zero exit code but won't know that it's an EPERM error

API's whose semantics is affected by develooper mode

eg: https://docs.microsoft.com/en-us/windows/win32/api/d3d12/nf-d3d12-id3d12device-setstablepowerstate

Do not call this method in normal execution for a shipped application. This method only works while the machine is in developer mode. If developer mode is not enabled, then device removal will occur. Instead, call this method in response to an off-by-default, developer-facing switch. Calling it in response to command line parameters, config files, registry keys, and developer console commands are reasonable usage scenarios.

see also https://stackoverflow.com/questions/41231586/how-to-detect-if-developer-mode-is-active-on-windows-10

ID3D12Device::SetStablePowerState function call is only available if developer mode is turned on in the system. If not, it triggers a device removal.

testing

eg in go: see https://golang.org/src/os/os_windows_test.go

// TestSymlinkCreation verifies that creating a symbolic link
// works on Windows when developer mode is active.
// This is supported starting Windows 10 (1703, v10.0.14972).
func TestSymlinkCreation(t *testing.T) {
    if !testenv.HasSymlink() && !isWindowsDeveloperModeActive() {
        t.Skip("Windows developer mode is not active")
    }

other OS, eg android:

setting developper mode on windows

Araq commented 3 years ago

just because a feature can be misused doesn't mean it's not useful. If we don't provide it, users will re-implement, most likely poorly

Yes and then these are their bugs, not ours. That can be a pretty important thing when you lack infinite resources to develop a project and also when you don't want to sit on millions of lines of code you have to maintain for good.

Varriount commented 3 years ago

fail fast when we want a script to only run as root, or only run as non-root; eg, see https://electrictoolbox.com/check-user-root-sudo-before-running/; eg, allows to not do some initial tasks if later tasks would fail because of that

improved error messages giving relevant context, eg: (After a common failed with a permission error): report whether user was admin or had developper mode enabled

What are some common tasks (on Windows, or on both Windows and Linux) where these points apply, and can't be satisfied by a quick test call (such as attempting to create an empty file)?

testing eg in go: see https://golang.org/src/os/os_windows_test.go

That is a standard library test, testing low-level, OS-specific functionality. Furthermore, the function isWindowsDeveloperModeActive is a private function unavailable for use in the standard library.
Aside from testing in a rather domain-specific area of programming (a cross-platform standard library), are there any actual common scenarios where this function will be needed?

other OS, eg android

Android and iOS are very, very, very different from desktop operating systems, especially from a permissions standpoint. They do not allow anywhere near the kind of flexibility in changing user permissions and system state, and the significance of being rooted vs non-rooted is a quality that is very unique to those systems.

When considering the inclusion of something into the standard library, a number of points have to be considered (roughly, from most to least important):

If something can't pass the above questions, then it might be better to place it in a module. Keep in mind that, once things are added to the standard library, it can be difficult to remove them (and even if they are removed, the identifiers used can't be reclaimed).