Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isWindowsDeveloperModeActive #343

Closed
rominf opened this issue Feb 13, 2021 · 5 comments
Closed

Add isWindowsDeveloperModeActive #343

rominf opened this issue Feb 13, 2021 · 5 comments

Comments

@rominf
Copy link

rominf commented Feb 13, 2021

On Windows 10, symlink creation works when the user is an Administrator (see nim-lang/Nim#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
Copy link
Member

timotheecour commented Mar 8, 2021

@rominf now that nim-lang/Nim#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
Copy link

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
Copy link
Member

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
Copy link
Member

Araq commented Mar 12, 2021

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
Copy link

Varriount commented Mar 12, 2021

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):

  • Does this solve a problem that developers from a variety of domains (science, web, GUI, etc.) face on a regular basis?
  • Does the utility provided outweigh the complexity introduced in terms of usability (and proper vs improper usage)? Keep in mind that flexibility does not always equal usefulness, and that simplicity can be a good thing.
  • Does the complexity of the implementation merit dedicated resources from the development team?
  • Does this addition make its module's API more useful, more cohesive, and easier to understand?
  • Is a proper implementation of this functionality so difficult and/or complex, that it would be difficult for developers to write their own solution?

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).

@Araq Araq closed this as completed Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants