search

natcap / invest

InVEST®: models that map and value the goods and services from nature that sustain and fulfill human life.
Apache License 2.0
159 stars 65 forks source link

Validation is allowing read-only directories as workspaces #1599

Closed davemfish closed 1 week ago

davemfish commented 1 month ago

I just tried this directory: C:\\Program Files\\InVEST 3.14.1 Workbench\\runs via the workbench. There was no validation warning, so this is what happened when I clicked run:

[1]   File "C:\Users\dmf\projects\invest\env\lib\site-packages\natcap\invest\utils.py", line 145, in prepare_workspace
[1]     os.makedirs(workspace)
[1]   File "C:\Users\dmf\projects\invest\env\lib\os.py", line 215, in makedirs
[1]
[1] [09:25:33.665] [undefined]     makedirs(head, exist_ok=exist_ok)
[1]   File "C:\Users\dmf\projects\invest\env\lib\os.py", line 225, in makedirs
[1]
[1] [09:25:33.666] [undefined]     mkdir(name, mode)
[1] PermissionError: [WinError 5] Access is denied: 'C:\\Program Files\\InVEST 3.14.1 Workbench\\runs'

This is especially important because a relative path entered in the Workbench UI will be relative to the installation path (C:\\Program Files\\InVEST 3.14.1 Workbench\\) where the user may not have write permission.

emlys commented 1 month ago

It looks like the necessary validation code is already there to perform this check. Might just need to find where we call check_directory on the workspace and set permissions='rwx'.

emilyanndavis commented 3 weeks ago

This seems to be a Windows-specific issue, since workspace permissions validation works as expected on macOS.

The cause appears to be a limitation of Python's os.access method. From the docs:

Note: I/O operations may fail even when access() indicates that they would succeed, particularly for operations on network filesystems which may have permissions semantics beyond the usual POSIX permission-bit model.

A more reliable approach may well be to attempt the operation (e.g., write to the workspace directory) and throw an error if it fails, rather than try to determine in advance whether the operation is allowed.