Privileges escalate by default for Windows on CI

mrclary commented 5 months ago

Checklist

[X] I added a descriptive title
[X] I searched open reports and couldn't find a duplicate

What happened?

@jaimergp, when testing the Windows installers for Spyder, menuinst creates the shortcut for _mode=system rather than _mode=user, but only on CI. This does not occur for macOS or Linux.

The installer appears to build correctly, but when testing the install, it creates the shortcut in the wrong place when run on CI. (I'm using /InstallationType=JustMe /NoRegistry=1 flags). See this run. When I ssh into the workflow, I see that .nonadmin is present in both the base and target prefixes.
When I bypass the install test on CI, the workflow succeeds and I can download the artifact and test the install on my local machine. This works perfectly: shortcut is created for the user, not system. I've tried both the GUI and and the command prompt (even using /NoRegistry=1) with the same successful result. See this run without test.

What could be causing the problem on CI?

Additional Context

No response

mrclary commented 5 months ago

@jaimergp, also, do you know of a way to display or save the install log from NSIS when executing the installer from the command line?

jaimergp commented 5 months ago

GHA on Windows runs as an administrator account, so constructor, conda-standalone and menuinst will run in sudo mode, believing it's an all-users install. I've observed the same behavior in napari and gave up because it does work locally. Maybe we can allow overrides at some point but for now that's the known behavior.

For the logs, you need to build your installers with the logging build of nsis. See https://github.com/conda/constructor/blob/d934f13d103359d0d208108fa53ccf529d143cd7/dev/extra-requirements-windows.txt#L1.

mrclary commented 5 months ago

GHA on Windows runs as an administrator account, so constructor, conda-standalone and menuinst will run in sudo mode, believing it's an all-users install. I've observed the same behavior in napari and gave up because it does work locally. Maybe we can allow overrides at some point but for now that's the known behavior.

I see, thank you. So menuinst will install the shortcut based on its current privileges; .nonadmin only tells menuinst whether or not to elevate those privileges. I'll update our workflow accordingly. I'm just glad to know that there wasn't a bug 😁.

For the logs, you need to build your installers with the logging build of nsis. See https://github.com/conda/constructor/blob/d934f13d103359d0d208108fa53ccf529d143cd7/dev/extra-requirements-windows.txt#L1.

Thank you! 🙏 😊 I was not aware of that! Is there a flag I need to use on the command line? Or will it automatically send logs to stdout? to log file?

jaimergp commented 5 months ago

You will find a install.log file in the target location. You don't have to do anything else. It's not perfect but it's something. This is how we use it in the constructor test suite.

mrclary commented 5 months ago

You will find a install.log file in the target location. You don't have to do anything else.

start /wait Spyder-6.0.0a4.dev150-Windows-x86_64.exe /S

Hmm... I can't find the log file... What am I missing?

Installer Build Environment

``` # packages in environment at C:\Users\rclary\.conda\envs\spy-inst: # # Name Version Build Channel anyio 3.7.1 pyhd8ed1ab_0 conda-forge archspec 0.2.2 pyhd8ed1ab_0 conda-forge attrs 23.2.0 pyh71513ae_0 conda-forge beautifulsoup4 4.12.3 pyha770c72_0 conda-forge boa 0.16.0 pyhd8ed1ab_1 conda-forge boltons 23.1.1 pyhd8ed1ab_0 conda-forge brotli-python 1.1.0 py310h00ffb61_1 conda-forge bzip2 1.0.8 hcfcfb64_5 conda-forge ca-certificates 2023.11.17 h56e8100_0 conda-forge certifi 2023.11.17 pyhd8ed1ab_0 conda-forge cffi 1.16.0 py310h8d17308_0 conda-forge chardet 5.2.0 py310h5588dad_1 conda-forge charset-normalizer 3.3.2 pyhd8ed1ab_0 conda-forge click 8.1.7 win_pyh7428d3b_0 conda-forge colorama 0.4.6 pyhd8ed1ab_0 conda-forge conda 23.11.0 py310h5588dad_1 conda-forge conda-build 3.28.4 py310h5588dad_0 conda-forge conda-index 0.3.0 pyhd8ed1ab_1 conda-forge conda-libmamba-solver 23.12.0 pyhd8ed1ab_0 conda-forge conda-package-handling 2.2.0 pyh38be061_0 conda-forge conda-package-streaming 0.9.0 pyhd8ed1ab_0 conda-forge conda-standalone 23.11.0 h57928b3_1 conda-forge constructor 3.6.0 pyh31a35d3_0 conda-forge distro 1.9.0 pyhd8ed1ab_0 conda-forge exceptiongroup 1.2.0 pyhd8ed1ab_2 conda-forge filelock 3.13.1 pyhd8ed1ab_0 conda-forge fmt 10.2.1 h181d51b_0 conda-forge freetype 2.12.1 hdaf720e_2 conda-forge gitdb 4.0.11 pyhd8ed1ab_0 conda-forge gitpython 3.1.41 pyhd8ed1ab_0 conda-forge idna 3.6 pyhd8ed1ab_0 conda-forge importlib_resources 6.1.1 pyhd8ed1ab_0 conda-forge jinja2 3.1.3 pyhd8ed1ab_0 conda-forge joblib 1.3.2 pyhd8ed1ab_0 conda-forge json5 0.9.14 pyhd8ed1ab_0 conda-forge jsonpatch 1.33 pyhd8ed1ab_0 conda-forge jsonpointer 2.4 py310h5588dad_3 conda-forge jsonschema 4.21.1 pyhd8ed1ab_0 conda-forge jsonschema-specifications 2023.12.1 pyhd8ed1ab_0 conda-forge krb5 1.21.2 heb0366b_0 conda-forge lcms2 2.16 h67d730c_0 conda-forge lerc 4.0.0 h63175ca_0 conda-forge libarchive 3.7.2 h313118b_1 conda-forge libcurl 8.5.0 hd5e4a3a_0 conda-forge libdeflate 1.19 hcfcfb64_0 conda-forge libffi 3.4.2 h8ffe710_5 conda-forge libiconv 1.17 hcfcfb64_2 conda-forge libjpeg-turbo 3.0.0 hcfcfb64_1 conda-forge liblief 0.12.3 h63175ca_0 conda-forge libmamba 1.5.6 h3f09ed1_0 conda-forge libmambapy 1.5.6 py310h04f2035_0 conda-forge libpng 1.6.39 h19919ed_0 conda-forge libsolv 0.7.27 h12be248_0 conda-forge libsqlite 3.44.2 hcfcfb64_0 conda-forge libssh2 1.11.0 h7dfc565_0 conda-forge libtiff 4.6.0 h6e2ebb7_2 conda-forge libwebp-base 1.3.2 hcfcfb64_0 conda-forge libxcb 1.15 hcd874cb_0 conda-forge libxml2 2.12.4 hc3477c8_1 conda-forge libzlib 1.2.13 hcfcfb64_5 conda-forge lz4-c 1.9.4 hcfcfb64_0 conda-forge lzo 2.10 he774522_1000 conda-forge m2-msys2-runtime 2.5.0.17080.65c939c 3 conda-forge m2-patch 2.7.5 2 conda-forge m2w64-gcc-libgfortran 5.3.0 6 conda-forge m2w64-gcc-libs 5.3.0 7 conda-forge m2w64-gcc-libs-core 5.3.0 7 conda-forge m2w64-gmp 6.1.0 2 conda-forge m2w64-libwinpthread-git 5.0.0.4634.697f757 2 conda-forge mamba 1.5.6 py310hd9d798f_0 conda-forge markdown-it-py 3.0.0 pyhd8ed1ab_0 conda-forge markupsafe 2.1.4 py310h8d17308_0 conda-forge mdurl 0.1.2 pyhd8ed1ab_0 conda-forge menuinst 2.0.2 py310h00ffb61_0 conda-forge more-itertools 10.2.0 pyhd8ed1ab_0 conda-forge msys2-conda-epoch 20160418 1 conda-forge nsis 3.08 h9ead6bc_log_2 conda-forge openjpeg 2.5.0 h3d672ee_3 conda-forge openssl 3.2.0 hcfcfb64_1 conda-forge packaging 23.2 pyhd8ed1ab_0 conda-forge pillow 10.2.0 py310h1e6a543_0 conda-forge pip 23.3.2 pyhd8ed1ab_0 conda-forge pkginfo 1.9.6 pyhd8ed1ab_0 conda-forge pkgutil-resolve-name 1.3.10 pyhd8ed1ab_1 conda-forge platformdirs 4.1.0 pyhd8ed1ab_0 conda-forge pluggy 1.4.0 pyhd8ed1ab_0 conda-forge prompt-toolkit 3.0.42 pyha770c72_0 conda-forge prompt_toolkit 3.0.42 hd8ed1ab_0 conda-forge psutil 5.9.8 py310h8d17308_0 conda-forge pthread-stubs 0.4 hcd874cb_1001 conda-forge py-lief 0.12.3 py310h00ffb61_0 conda-forge pybind11-abi 4 hd8ed1ab_3 conda-forge pycosat 0.6.6 py310h8d17308_0 conda-forge pycparser 2.21 pyhd8ed1ab_0 conda-forge pygments 2.17.2 pyhd8ed1ab_0 conda-forge pysocks 1.7.1 pyh0701188_6 conda-forge python 3.10.13 h4de0772_1_cpython conda-forge python-libarchive-c 5.0 py310h5588dad_2 conda-forge python_abi 3.10 4_cp310 conda-forge pytz 2023.3.post1 pyhd8ed1ab_0 conda-forge pyyaml 6.0.1 py310h8d17308_1 conda-forge referencing 0.32.1 pyhd8ed1ab_0 conda-forge reproc 14.2.4.post0 hcfcfb64_1 conda-forge reproc-cpp 14.2.4.post0 h63175ca_1 conda-forge requests 2.31.0 pyhd8ed1ab_0 conda-forge rich 13.7.0 pyhd8ed1ab_0 conda-forge ripgrep 14.1.0 h7f3b576_0 conda-forge rpds-py 0.17.1 py310h87d50f1_0 conda-forge ruamel.yaml 0.18.5 py310h8d17308_0 conda-forge ruamel.yaml.clib 0.2.7 py310h8d17308_2 conda-forge ruamel.yaml.jinja2 0.2.4 py_1 conda-forge setuptools 69.0.3 pyhd8ed1ab_0 conda-forge setuptools-scm 8.0.4 pyhd8ed1ab_0 conda-forge setuptools_scm 8.0.4 hd8ed1ab_0 conda-forge smmap 5.0.0 pyhd8ed1ab_0 conda-forge sniffio 1.3.0 pyhd8ed1ab_0 conda-forge soupsieve 2.5 pyhd8ed1ab_1 conda-forge tk 8.6.13 h5226925_1 conda-forge tomli 2.0.1 pyhd8ed1ab_0 conda-forge tqdm 4.66.1 pyhd8ed1ab_0 conda-forge truststore 0.8.0 pyhd8ed1ab_0 conda-forge typing-extensions 4.9.0 hd8ed1ab_0 conda-forge typing_extensions 4.9.0 pyha770c72_0 conda-forge tzdata 2023d h0c530f3_0 conda-forge ucrt 10.0.22621.0 h57928b3_0 conda-forge urllib3 2.1.0 pyhd8ed1ab_0 conda-forge vc 14.3 hcf57466_18 conda-forge vc14_runtime 14.38.33130 h82b7239_18 conda-forge vs2015_runtime 14.38.33130 hcb4865c_18 conda-forge watchgod 0.8.2 pyhd8ed1ab_0 conda-forge wcwidth 0.2.13 pyhd8ed1ab_0 conda-forge wheel 0.42.0 pyhd8ed1ab_0 conda-forge win_inet_pton 1.1.0 pyhd8ed1ab_6 conda-forge xorg-libxau 1.0.11 hcd874cb_0 conda-forge xorg-libxdmcp 1.1.3 hcd874cb_0 conda-forge xz 5.2.6 h8d14728_0 conda-forge yaml 0.2.5 h8ffe710_2 conda-forge yaml-cpp 0.8.0 h63175ca_0 conda-forge zipp 3.17.0 pyhd8ed1ab_0 conda-forge zstandard 0.22.0 py310h0009e47_0 conda-forge zstd 1.5.5 h12be248_0 conda-forge ```

construct.yaml

```yaml name: Spyder company: Spyder-IDE reverse_domain_identifier: org.spyder-ide.Spyder version: 6.0.0a4.dev150 channels: - local - conda-forge/label/spyder_kernels_rc - conda-forge conda_default_channels: - conda-forge - defaults specs: - python=3.10.13 - conda >=23.11.0 - menuinst >=2.0.2 - mamba installer_filename: Spyder-6.0.0a4.dev150-Windows-x86_64.exe installer_type: exe initialize_by_default: false initialize_conda: false register_python: false register_envs: false extra_envs: spyder-runtime: specs: - python=3.10.13 - spyder=6.0.0a4.dev150 - cython - matplotlib - numpy - openpyxl - pandas - scipy - sympy - python-lsp-server=1.9.1.dev6 - qtconsole=5.5.1.dev1 - spyder-kernels=3.0.0b4.dev3 extra_files: - C:\Users\rclary\Documents\Repos\spyder\installers-conda\resources\bundle_readme.md: README.txt - C:\Users\rclary\Documents\Repos\spyder\installers-conda\build\condarc: .condarc license_file: C:\Users\rclary\Documents\Repos\spyder\installers-conda\resources\bundle_license.rtf welcome_image: C:\Users\rclary\Documents\Repos\spyder\installers-conda\build\welcome_img_win.png header_image: C:\Users\rclary\Documents\Repos\spyder\installers-conda\build\header_img_win.png icon_image: C:\Users\rclary\Documents\Repos\spyder\img_src\spyder.ico default_prefix: '%LOCALAPPDATA%\spyder-6' default_prefix_domain_user: '%LOCALAPPDATA%\spyder-6' default_prefix_all_users: '%ALLUSERSPROFILE%\spyder-6' check_path_length: false pre_install: C:\Users\rclary\Documents\Repos\spyder\installers-conda\resources\pre-install.bat post_install: C:\Users\rclary\Documents\Repos\spyder\installers-conda\resources\post-install.bat ```

jaimergp commented 5 months ago

That should be enough, I think?

This is how we run it in CI, in case it helps. Maybe specify /D=some-path to make sure where to expect the file.

mrclary commented 5 months ago

This is how we run it in CI, in case it helps. Maybe specify /D=some-path to make sure where to expect the file.

I tried that as well. However, I'm running it locally right now, so maybe it needs admin privileges for some reason? I'll test that also...

mrclary commented 5 months ago

That should be enough, I think?

Found the issue. The environment variable NSIS_USING_LOG_BUILD=1 also needs to be set prior to the build. https://github.com/conda/constructor/blob/d934f13d103359d0d208108fa53ccf529d143cd7/constructor/winexe.py#L352

@jaimergp, thanks for all your help on this! I really appreciate all the work you and others have done on upgrading constructor and menuinst. It is a great help to the Spyder project.

jaimergp commented 5 months ago

Aaah, yes, I remember now. Sorry about that obscure detail 😬 I'll add it to the documentation.

mrclary commented 4 months ago

@jaimergp, when uninstalling, the uninstaller requests the user to restart now or later. This seems to be a result of failing to remove the install.log from the base environment directory. Is this a bug?

marcoesters commented 3 months ago

I can add some more perspective about that log file and why we (on the Anaconda side) don't use it in production:

If you install into an empty directory (which the current implementation of constructor allows), it will write the log file before the installer checks if the directory is empty, which makes the installer fail. That can of course be fixed by excluding that file.
When you hit "Browse" in the GUI to change the destination path, the installer will write the log file into %USERPROFILE%, which is not ideal (and would be a red flag for me as a user).

@jaimergp, when uninstalling, the uninstaller requests the user to restart now or later. This seems to be a result of failing to remove the install.log from the base environment directory. Is this a bug?

This is not a bug. NSIS, which is the backend of the installers, calls all logs install.log. So, this is the log of the uninstaller. It is marked for deletion and should be removed on reboot.

mrclary commented 3 months ago

I can add some more perspective about that log file and why we (on the Anaconda side) don't use it in production:

If you install into an empty directory (which the current implementation of constructor allows), it will write the log file before the installer checks if the directory is empty, which makes the installer fail. That can of course be fixed by excluding that file.

When you hit "Browse" in the GUI to change the destination path, the installer will write the log file into %USERPROFILE%, which is not ideal (and would be a red flag for me as a user).

@jaimergp, when uninstalling, the uninstaller requests the user to restart now or later. This seems to be a result of failing to remove the install.log from the base environment directory. Is this a bug?

This is not a bug. NSIS, which is the backend of the installers, calls all logs install.log. So, this is the log of the uninstaller. It is marked for deletion and should be removed on reboot.

Hmm...thanks @marcoesters, I will reconsider using the NSIS log version. My motivation was to provide some debugging information for developers, but if it will cause more problems for end users it may be counterproductive.

marcoesters commented 3 months ago

For CIs, enabling logging could be okay because these corner cases don't apply there/are easily fixable.

What I do is enable logging for dev builds of installers and integration testing. I only disable logging for production.

conda / menuinst