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

Request to change working directory from Windows/System32 to another directory #609

Closed
vpenades opened this issue Feb 20, 2019 · 8 comments · Fixed by #637
Closed

Request to change working directory from Windows/System32 to another directory #609

vpenades opened this issue Feb 20, 2019 · 8 comments · Fixed by #637
Labels
closed:done is:feature resolution:known We have a resolution for this, which is described in the issue, under heading Resolution
Milestone

Comments

@vpenades
Copy link

Hi

This is not yet another "why the working directory points to Windows/System32" , I've read the previous issues about the issue and I understand the reasons behind why the working directory does not point to where the test is located.

My concern is that, pointing the working directory to Windows/System32 by default is dangerous, since a naive test can inadvertently write files to that directory, or any third party application, expecting to output to the working directory, writes files to it.

Which is exactly what just happened to me. I was pointing my tests to use TestDirectory and WorkDirectory as NUnit requires.... but by using a third party assembly which happens to write log files into the current working directory, I've just found that my Windows/System32 directory is full of trash.

Could it be possible to change this behaviour by default, so, the current working directory points to a safer sink directory? for example, the Temp Files directory?

@CharliePoole
Copy link
Contributor

The framework does not set the working directory at all. It leaves it wherever it is set initially by the runner or the user. In the case of the console runner, it is also not changed so it's ultimately up to the user. In the case of the nunit3 VS adapter, it is again unchanged from what VS sets it to.

What runner are you using? That's where this issue needs to go.

@vpenades
Copy link
Author

In here they say setting the current path is adapter's responsability.

Clearly, this is still an open issue, and several parties involved seem to pass the ball between each other.

Assuming a beginner's level developer who has no previous knowledge of this issue, the expected, and safest behaviour would be to ensure that at the very least, when the tests run, the working directory does not point to the most dangerous directory in the whole computer.

So my recomendation for the NUnit adapter would be that, before running the tests, if the working directory points to Windows\ or Windows\System32 or any other directory considered harmful, to change the directory to TEMP\ or any other harmless directory.

@CharliePoole
Copy link
Contributor

I agree completely WRT the adapter. That's what I guessed might be the case when I asked what runner you were using.

I don't think it's safe behavior for any library linked by users like the framework to change the current directory. The caller may have some reason to set the directory before calling the library. If that library changes the current directory, that's very surprising behavior.

It sounds like you want this to be an adapter issue and I'm of the same mind. Should I transfer it there for you?

@vpenades
Copy link
Author

vpenades commented Feb 20, 2019

@CharliePoole sure, you can transfer the issue to the adapter section.

I really don't know who would be responsible of fixing this.. And I really don't care where working directory is pointing to, as long as it is not System32'

@CharliePoole
Copy link
Contributor

The symptom of using System32 can only occur under the adapter, so I think it should be fixed in the adapter. But @OsirisTerje, the project lead for the adapter would be the one to decide how to fix.

@CharliePoole CharliePoole transferred this issue from nunit/nunit Feb 20, 2019
@OsirisTerje
Copy link
Member

@CharliePoole Fair request.
@vpenades Anything under \windows is good, but "any other directory considered harmful," any other that comes to mind?

@OsirisTerje OsirisTerje added this to the 3.14 milestone Feb 20, 2019
@vpenades
Copy link
Author

vpenades commented Feb 20, 2019

@OsirisTerje It comes to mind pretty much most SpecialFolders:

  • Windows
  • Program Files (x86)
  • ProgramsData
  • Users (but not AppData\Local\Temp, which is inside the current user's path)

I can't imagine a reason why a test might require to run pointing to these directories. While doing so inadvertently might cause serious harm to the computer.

So I believe these directories need to be opted out by default, and maybe allow to manually opt them in, but under the full knowledge of the developer.

OsirisTerje pushed a commit that referenced this issue Jul 10, 2019
@OsirisTerje OsirisTerje added the resolution:known We have a resolution for this, which is described in the issue, under heading Resolution label Jul 29, 2019
OsirisTerje added a commit that referenced this issue Jul 30, 2019
@OsirisTerje
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed:done is:feature resolution:known We have a resolution for this, which is described in the issue, under heading Resolution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants