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

[Refactor]: Remove Unnecessary Binary Code #2

Open
4 of 5 tasks
iwr-redmond opened this issue Jan 3, 2025 · 9 comments
Open
4 of 5 tasks

[Refactor]: Remove Unnecessary Binary Code #2

iwr-redmond opened this issue Jan 3, 2025 · 9 comments
Labels
bug Something isn't working

Comments

@iwr-redmond
Copy link

iwr-redmond commented Jan 3, 2025

Checklist

  • The issue has not been resolved by following the troubleshooting guide
  • The issue exists on a clean installation of Fooocus
  • The issue exists in the current version of Fooocus
  • The issue has not been reported before recently
  • The issue has been reported before but has not been fixed yet

What happened?

The SimpleSDXL2 source package relies extensively on the simpleai_base Python library, which includes compiled Rust code primarily designed to facilitate features irrelevant to this fork, as well as compiled packages of two dependencies that can probably be replaced with their respective mainline equivalents.

This issue contains my suggestions for addressing these transition challenges.

Steps to reproduce the problem

  1. Install FooocusPlus
  2. Note that simpleai_base is installed per the code in launch.py
  3. You may also see a Chinese-language warning in the console originating from compiled Rust code contained within simpleai_base/src/token.rs

What should have happened?

Remove simpleai_base dependency

Calls to simpleai are a frequent feature of the SimpleSDXL code, which relies on this package to, among other things, interface with the ComfyUI backend. You will see in the repository that there are two folders, src for Rust (compiled) and python/simpleai_base for ordinary Python code.

Integrating a clean version of this code into this fork will look something like:

  1. Creating a folder in enhanced, e.g. enhanced/backend to contain the files from python/simpleai_base
  2. Replacing the ComfyTaskParams class in params_mapper.py with both the original non-Rust Python function from the old dev branch and the current parameter table from src/params_mapper.rs (appropriately modified for Python)
  3. Porting the gen_entry_point_id function from Rust to comfyd.py - fortunately it appears to be a simple function that takes the process ID and hashes it
  4. Going through all the code, but particularly webui.py, topbar.py, and simpleai.py to ensure all Python calls are redirected to the new enhanced/backend module, and to clean out any DID-related calls that refer to removed Rust functions.
  5. Remove the base_file installation code from launch.py#48 to ensure the deprecated simpleai_base wheels are no longer installed

You may also wish to rename enhanced/simpleai.py to, for example, enhanced/fooocusplus.py, and then update all references within the code from 'enhanced.simpleai' to 'enhanced.fooocusplus'.

Remove binary packages

The requirements_versions.txt file currently installs two SimpleSDXL-specific packages from ./enhanced/libs. Looking at the diffs for these, it appears the only major changes were to adjust URLs:

As a result, these packages should be reverted to their respective upstream versions hosted on PyPI (per SimpleSDXL issue 97):

# requirements_versions.txt
rembg
rf-groundingdino

Features associated with these packages must then be tested to ensure they work as expected. It may be necessary to pin certain versions in the requirements file to ensure smooth inference. For example, GroundingDINO may need to revert to the Fooocus upstream version:

groundingdino-py==0.4.0

Confirm VRAM sharing code is removed

The rollout of the simpleai_base Rust code in preparation for the recently announced multiuser and networking functions began in August, and may be related to the quality regression you identified in SimpleSDXL issue 77.

In the dev 1215 branch, there is now a configurable variable reserved_vram that admin users (i.e. registered with the TOKEN_TM_URL server in Shenzhen) can use to disable VRAM sharing, while user and guest VRAM sharing settings are remotely managed via the runtime-downloaded global_vars.json.

In the main 0916 branch, on which FooocusPlus is based, the main change made to prepare for VRAM sharing was commit cc338e4, which adjusted the ComfyUI startup parameters as follows:

  • enhanced/simpleai.py#L31: --disable-smart-memory
  • enhanced/simpleai.py#L32: --cuda-malloc (if not disabled in shared.args)
  • shared.py#L7: args = None (i.e. making --cuda-malloc default)

These options will need to be reviewed in the current FooocusPlus code, which includes later changes made upstream in SimpleSDXL, with a view to reverting to their 0820 defaults. Information about these command line options can be reviewed here.

Note that the EXTRA_RESERVED_VRAM code in model_management.py originates from ComfyUI (source) and does not need to be removed.

What browsers do you use to access Fooocus?

No response

Where are you running Fooocus?

Locally

What operating system are you using?

Linux

Console logs

N/A

Additional information

No response

@iwr-redmond iwr-redmond added the bug Something isn't working label Jan 3, 2025
@DavidDragonsage
Copy link
Owner

@iwr-redmond Wow - you have really put a lot of work into this! It is all rather overwhelming. Although I have programming experience, this is the first major project I have done with Python.

Yes, I was wondering where that random Chinese message was coming from in the console. So I see no .rs code in the python_embeded or in the FooocusPlus repo folders. So the code is run directly from the github simpleai_base repo? Is that really possible?

I was wondering whether it might be easier to go back to the 0820 SimpleSDXL release, but it looks like that was already infected with Rust.

@iwr-redmond
Copy link
Author

iwr-redmond commented Jan 4, 2025

Your announcement of this fork happened to coincide with my equally amateur security audit, after I got to wondering how the new lockdown was going to be enforced. A dead bee can still sting, so finding out early is probably for the best.

Rust is compiled code and therefore harder to see on end-user machines, which given the purpose of tamper-resistance makes sense for the SimpleSDXL crew, who sadly appear to be hitched but not churched. At present the simpleai_base package, both standard Python and binary Rust, is installed along with the other requirements when SimpleSDXL runs for the first time. If you find the simpleai_base library folder inside your venv's site-packages, there will be a __pycache__ folder with binary .pyc files, which is normal for every package after installation, a bunch of human-readable .py scripts, and a sole binary file simpleai_base.cp310-win_amd64.pyd that contains the compiled Rust code.

Whether you choose to stick with the main branch or rebase on dev (I see you have reviewed my commentary in Discussion #3), I suggest the priority should be to clean out the unusual code first as suggested in this issue - you'll probably find that most of the other annoyances fade away once this is done. The language changes and whatnot can wait until the end, I reckon.

Python is very helpful during such heart surgery, because its errors while crashing due to failed imports are usually very descriptive ("I can't find xxx.yyy") and therefore easy to run down. At its core, SimpleSDXL is basically the ComfyUI engine + a small connector (simpleai_base) + the Fooocus frontend. Even though the Rust code isn't anything to write home about, the connector code itself works pretty well, meaning you don't need to perform heart surgery on the main interface beyond removing the various did references, and you certainly don't need to do much with the ComfyUI engine.

@DavidDragonsage
Copy link
Owner

@iwr-redmond Thank you, it is really helpful to know the location of the compiled code. I can see that the compiled code was there in the 0820 SimpleSDXL release. So basically for as long as I have been using SimpleSDXL it may have been calling home.

I am treating the removal of this code as of the highest priority. I would like to have it done today - at least that is my (rather unlikely) goal, I also have some important non-computer work I have to complete today, so I need to do that first.

That is an excellent idea to first move the python/simpleai_base code over to the repo. And as you say, Python is really good at announcing problems.

I really appreciate your help in this, and especially in identifying that SimpleSDXL has already been calling home every time you run the program. If you find any further information on what it has been sending, please let me know. The more info. the better, because that will help convince people that they need to switch.

I need to provide a secure alternative to the Fooocus community as soon as possible and hopefully get people switched over to FooocusPlus. I feel a special responsibility in this because I have been encouraging people to use SimpleSDXL for some time now, especially at the Facebook Pure Fooocus group but also at other places and through other Fooocus community leaders.

1 similar comment
@DavidDragonsage
Copy link
Owner

@iwr-redmond Thank you, it is really helpful to know the location of the compiled code. I can see that the compiled code was there in the 0820 SimpleSDXL release. So basically for as long as I have been using SimpleSDXL it may have been calling home.

I am treating the removal of this code as of the highest priority. I would like to have it done today - at least that is my (rather unlikely) goal, I also have some important non-computer work I have to complete today, so I need to do that first.

That is an excellent idea to first move the python/simpleai_base code over to the repo. And as you say, Python is really good at announcing problems.

I really appreciate your help in this, and especially in identifying that SimpleSDXL has already been calling home every time you run the program. If you find any further information on what it has been sending, please let me know. The more info. the better, because that will help convince people that they need to switch.

I need to provide a secure alternative to the Fooocus community as soon as possible and hopefully get people switched over to FooocusPlus. I feel a special responsibility in this because I have been encouraging people to use SimpleSDXL for some time now, especially at the Facebook Pure Fooocus group but also at other places and through other Fooocus community leaders.

@iwr-redmond
Copy link
Author

iwr-redmond commented Jan 5, 2025

As of the system_info.rs in the 0916 release, in addition to operating system data like RAM and VRAM it's been collecting:

  • Local IP Address
  • Public IP address
  • MAC address (unique to each network/WiFi card)
  • Hard drive partition UUID
  • GPU memory

There is also infrastructure in rathole.rs to create a VPN tunnel between two points, which according to the code in token.rs may already have been active, potentially permitting remote access to systems while SimpleSDXL was running.

@DavidDragonsage
Copy link
Owner

DavidDragonsage commented Jan 5, 2025

Well it was a long day, but progress was made. The original code is obsessed with restoring simpleai_base to python_embeded to the point where it disconnected simpleai_base from the repo before reinstalling simpleai_base in python_embedded. At that point I had not done (5) above.

What I am working on now is a replacement for systeminfo.rs - that code is inextricably linked to user tokens. I am in the process of importing some appropriate code from mainline Fooocus. The main point is to determine available VRAM and advise the user, and of course mainline Fooocus has always done that, in a fairly simple way.

@DavidDragonsage
Copy link
Owner

@iwr-redmond Thank you for the further information on the security risks.

Yes, I had just noticed that systeminfo.rs is collecting the MAC address - not good! I had not looked at that rathole yet, but what an ominous name! That does sound like a serious vulnerability. The sooner I get this done the better.

That call home in env_utils.rs, I think it may be sending a GIF of QR code that part of the process of becoming a full user.

@iwr-redmond
Copy link
Author

That call home in env_utils.rs, I think it may be sending a GIF of QR code that part of the process of becoming a full user.

The function is called "logging_launch_info" and in addition to the URL request, you can see that it sends out the variables "did" and "info". Rathole is a NAT traversal package, see here.

@DavidDragonsage
Copy link
Owner

@iwr-redmond Thank you again for the further info. I have posted a security update at Pure Fooocus, with the suggestion that it would be a good time to try out RuinedFooocus.
https://www.facebook.com/groups/fooocus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants