-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Added Nuitka artifacts via gh-actions #330
base: build
Are you sure you want to change the base?
Conversation
…fferentiated for mac and windows
Thanks for the PR. It's a simple enough thing to switch to a different packaging system, but I originally chose PyInstaller as it seemed to be the most popular choice in other projects. This popularity of course can lead to issues (e.g., the virus scanner false positives), but since I don't use the pre-built versions myself I didn't really see this as a major problem. As it happens, I did some work a while ago to make the proxy compatible with Nuitka builds, and the instructions are already in the readme. Can you explain why the version check change is needed? I didn't run into any issues here in my own testing. |
You're absolutely right! It's just that many users might be discouraged if they need to compile things themselves. Also it's very very unfortunate that many AI-based antivirus programs tend to flag PyInstaller by default. I'm planning to review the version checking again, probably next week when I have a bit more time. You might be right that the changes aren't necessary anymore. A few months ago, when I was using Nuitka, it had issues with these checks. I noticed you've since switched from Thanks a lot for the excellent tool, by the way! But would you generally be open to the idea of publishing the Nuitka release as well? |
@simonrob This has unfortunately become a major issue that is preventing many users from updating past v2024-9-12. Hopefully, this PR can be merged before any critical bug fixes or security updates are released for email-oauth2-proxy. |
I'm happy to switch if it turns out Nuitka packaging is preferable. Could you and/or any other commenters confirm that this option definitely doesn't trigger virus warnings? At the moment, rather than merging this PR, I'd probably just make the changes in the existing build script to use Nuitka instead since I'm not convinced the other changes are needed. In addition, if PyInstaller is causing significant problems, as noted above, you can already build this yourself – this has been possible since 50d07c9 (July 2024) which made changes specifically to support Nuitka. |
I have confirmed that the emailproxy.exe binary in the emailproxy-2024-11-11_nuitka-Windows.zip file linked by @sommerf-lf can be successfully downloaded on a Windows 11 system, and comes up clear when scanned with Windows Defender. In contrast, I should clarify that the issue with the official (PyInstaller) version of the 2024-11-11 release is not merely that it triggers "warnings", but rather that it cannot even be downloaded from GitHub without being blocked or immediately quarantined/deleted by Windows Defender and Windows Advanced Threat Protection.
Since this is beyond my personal expertise, it would be helpful to see a response from @sommerf-lf to confirm that the two of you have come to a meeting of the minds on the proposed solution. |
Well talking from my experience (@bwbug, I am no security expert either but have some years experience in deploying python executibles on systems without a python env on it) pyinstaller is often (nearly always) detected as a false positive on windows environments, mainly by Windows Defender itself. Running other AV ourselves (which I wouldn't like to disclose openly, but nothing from VirusTotal's list) pyinstaller is also flagged every time. Nuitka on the other hand offered better compatibility (with the tradeoff of sometimes beeing more hard to get to work if importing nieche libraries). But I think the biggest positive/differentiating factor is Microsoft Defender not flagging Nuitka, as in most environments Microsoft Defender is still working in the background even when using another managed service on top (which should outweight the numerically larger number of detections) We can see Nuitka is no saints either (even worse on VIrusTotal by numbers), but in the past years it worked better in all environments I had access to. Also I suggest to just add Nuitka to the releases and still keep the notice about (For interested ones maybe have a look at this small article: https://medium.com/@markhank/how-to-stop-your-python-programs-being-seen-as-malware-bfd7eb407a7; they also mention that Windows Defender should be the primary goal) |
Thanks for the detailed summary. Like I said, I'm happy to switch if it's a better option. I don't think I'll keep both Nuitka and PyInstaller though - I can imagine this leading to questions about which one to use, and potential interoperability issues. There are also PyInstaller-specific things in the proxy at the moment that I'd be happy to remove. Let me know about the other changes and I'll take a look when I next get chance. |
@simonrob As a possible alternative, it might be worthwhile trying the approach outlined in the article that @sommerf-lf linked. Here is the relevant section:
@simonrob If you have the time and inclination to perform the above ritual after each new release, then it may be feasible to stay with the PyInstaller builds for Windows. The best way to test if this would work as advertised by the author of the article, would be to go ahead and submit a false-positive report for the emailproxy-2024-11-11_pyinstaller-Windows.zip file. What do you think? |
I had a brief look again at what I did when I earlier (April 2023; on commit 779e70e) made a quickly thrown together way to run using Nuitka. I had to remove the usages of
|
As our company's AV did always flag the pyinstaller version (as also noted in releases text and several discussions) and it doesn't quite fit our environment to run venvs barebone, I decided to add Nuitka compilation. (Nuitka converts python code to C/C++ and then compiles it, which makes the release standalone.)
I'd appreciate it, if you would give me feedback and if possible merge my changes into the main repo. I was not quite sure what the relationship between main and build is, but as the workflow files are in build, I worked on this branch. (It should be able to be merged into main without any conflicts.)
My changes:
see for artifacts: https://github.com/sommerf-lf/email-oauth2-proxy/releases/tag/build-1fcb1933
emailproxy-2024-11-11_nuitka-macOS.zip
emailproxy-2024-11-11_nuitka-Windows.zip