Autoupdater #111
Autoupdater #111
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Steam Auto Cracker GUI autoupdater with improved error handling, frozen executable support, and a repository ownership transition from BigBoiCJ to SquashyHydra. The version in latestversion.json is bumped from 2.2.1 to 2.2.4.
Key changes:
- Refactored request retry logic from recursion to iteration with better exception handling
- Added support for frozen executables using temporary directory extraction
- Enhanced file removal and movement logic with symlink handling
- Improved error logging to write next to the executable/script location
Reviewed changes
Copilot reviewed 2 out of 5 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| steam_auto_cracker_gui_autoupdater.py | Core autoupdater logic refactored with iterative retries, tempfile-based extraction for frozen executables, improved path handling, and better error logging |
| requirements.txt | Added auto-py-to-exe dependency for packaging |
| latestversion.json | Updated version to 2.2.4 and changed repository URLs from BigBoiCJ to SquashyHydra |
| .gitignore | Added standard Python ignore patterns for build artifacts, cache, logs, and config files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tempdir = tempfile.mkdtemp() | ||
| with zipfile.ZipFile(io.BytesIO(req.content)) as z: | ||
| namelist = z.namelist() | ||
| if not namelist: | ||
| raise Exception("Downloaded archive is empty") | ||
|
|
||
| # Determine if archive has a single top-level folder | ||
| tops = [n.split('/')[0] for n in namelist if n and not n.startswith('/')] | ||
| top_folder = tops[0] if tops and all(t == tops[0] for t in tops) else None | ||
|
|
||
| z.extractall(tempdir) | ||
|
|
||
| print("Finished extracting the archive!\nRemoving the old installation... (DO NOT CLOSE THE AUTOUPDATER!)\n") | ||
|
|
||
| # Determine base directory where the running single-file exe or the script is located | ||
| if getattr(sys, 'frozen', False): | ||
| base_dir = os.path.dirname(sys.executable) | ||
| else: | ||
| base_dir = os.path.dirname(os.path.abspath(__file__)) | ||
|
|
||
| # Remove old installation files/folders in the base directory (be careful!) | ||
| cwd_files = os.listdir(base_dir) | ||
| for file in cwd_files: | ||
| if file in IGNORE_FILES: # Ignore specific files/directories | ||
| continue | ||
|
|
||
| skip_file = False | ||
| for ext in IGNORE_EXTS: # Ignore specific file extensions | ||
| if file.endswith(ext): | ||
| skip_file = True | ||
| break | ||
| if skip_file: | ||
| continue | ||
|
|
||
| if file == folder_name[:-1]: # Ignore the new installation (without the last "/") | ||
| continue | ||
|
|
||
| if os.path.isfile(file): # If file | ||
| os.remove(file) | ||
|
|
||
| path = os.path.join(base_dir, file) | ||
|
|
||
| if os.path.isfile(path): # If file | ||
| os.remove(path) | ||
| print(f" Removed file {file}") | ||
| else: # If folder/directory | ||
| shutil.rmtree(file) | ||
| print(f" Removed folder {file} and its content") | ||
|
|
||
| if os.path.islink(path): # If it's a symlink, remove the link only | ||
| os.unlink(path) | ||
| print(f" Removed symlink {file}") | ||
| if os.path.isdir(path): | ||
| shutil.rmtree(path) | ||
| print(f" Removed folder {file} and its content") | ||
|
|
||
| print("\nFinished removing the old installation.\nMoving the new installation... (DO NOT CLOSE THE AUTOUPDATER!)") | ||
| files = os.listdir(folder_name) | ||
|
|
||
| # Source directory inside tempdir: either the top folder or the tempdir itself | ||
| if top_folder: | ||
| extracted_root = os.path.join(tempdir, top_folder) | ||
| else: | ||
| extracted_root = tempdir | ||
|
|
||
| files = os.listdir(extracted_root) | ||
| for file in files: | ||
| shutil.move(folder_name + file, "./") # Move to the current directory | ||
| os.rmdir(folder_name[:-1]) # Now remove the extracted directory | ||
|
|
||
| src = os.path.join(extracted_root, file) | ||
| dst = os.path.join(base_dir, file) | ||
| shutil.move(src, dst) | ||
| # Cleanup temporary extraction directory | ||
| try: | ||
| shutil.rmtree(tempdir) | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
If an exception occurs between lines 68-87 (during extraction or base directory determination), the temporary directory created by tempfile.mkdtemp() will not be cleaned up because the cleanup code on lines 129-132 won't be reached. Consider wrapping the tempdir creation and usage in a try-finally block to ensure cleanup happens even if an error occurs.
| for file in files: | ||
| shutil.move(folder_name + file, "./") # Move to the current directory | ||
| os.rmdir(folder_name[:-1]) # Now remove the extracted directory | ||
|
|
||
| src = os.path.join(extracted_root, file) | ||
| dst = os.path.join(base_dir, file) | ||
| shutil.move(src, dst) |
There was a problem hiding this comment.
The code moves files from the extracted archive to the base directory without validating file names or paths. A malicious archive could potentially contain path traversal sequences (e.g., '../../../') in filenames, allowing files to be written outside the intended directory. Consider validating that destination paths remain within the base_dir before moving files.
| tops = [n.split('/')[0] for n in namelist if n and not n.startswith('/')] | ||
| top_folder = tops[0] if tops and all(t == tops[0] for t in tops) else None | ||
|
|
||
| z.extractall(tempdir) |
There was a problem hiding this comment.
The zipfile.extractall() method is called without path validation, which could be vulnerable to zip slip attacks where malicious archives contain files with path traversal sequences. While extraction is done to a temporary directory, this still poses a security risk. Consider validating each member's path before extraction or using a safer extraction method that validates paths.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
The temporary directory cleanup is wrapped in a try-except block that silently catches all exceptions. While this prevents the update from failing due to cleanup issues, it could hide real problems like permission errors or disk space issues. Consider logging the exception or at least printing a warning message to inform users that cleanup failed.
| except Exception: | |
| pass | |
| except Exception as e: | |
| print(f"Warning: Failed to remove temporary directory '{tempdir}': {e}") | |
| traceback.print_exc() |
| try: | ||
| subprocess.Popen([os.path.join(base_dir, "steam_auto_cracker_gui.exe")], cwd=base_dir) | ||
| except Exception: | ||
| subprocess.Popen("steam_auto_cracker_gui.exe") # fallback |
There was a problem hiding this comment.
The fallback subprocess call silently catches all exceptions, which could hide errors from the user. If the primary subprocess call fails and the fallback also fails, the user won't know that the GUI didn't launch successfully. Consider either logging the exception or removing the fallback if the primary method with full path is more reliable.
| subprocess.Popen("steam_auto_cracker_gui.exe") # fallback | |
| # If launching with full path fails, log the error and try a simpler fallback. | |
| print("Warning: Failed to launch steam_auto_cracker_gui.exe using full path. Trying fallback...", file=sys.stderr) | |
| try: | |
| subprocess.Popen("steam_auto_cracker_gui.exe") # fallback | |
| except Exception: | |
| print("Error: Failed to launch steam_auto_cracker_gui.exe using fallback as well.", file=sys.stderr) | |
| traceback.print_exc() |
|
Replace repository ownership SquashyHydra with BigBoiCJ, and Correct version from 2.2.4 to 2.2.1 |
|
|
||
| print("This program will automatically update Steam Auto Cracker GUI, please DO NOT CLOSE IT.\nRetrieving the latest version...") | ||
| req = SACRequest("https://raw.githubusercontent.com/BigBoiCJ/SteamAutoCracker/autoupdater/latestversion.json", "RetrieveLatestVersionJson").req | ||
| req = SACRequest("https://raw.githubusercontent.com/SquashyHydra/SteamAutoCracker/autoupdater/latestversion.json", "RetrieveLatestVersionJson").req |
There was a problem hiding this comment.
This should be changed to "https://raw.githubusercontent.com/BigBoiCJ/SteamAutoCracker/autoupdater/latestversion.json"
| @@ -1 +1 @@ | |||
| {"version": "2.2.1", "link": "https://github.com/BigBoiCJ/SteamAutoCracker/releases/download/[VERSION]-gui/Steam.Auto.Cracker.GUI.v[VERSION].zip", "release": "https://github.com/BigBoiCJ/SteamAutoCracker/releases/tag/[VERSION]-gui"} | |||
| {"version": "2.2.4", "link": "https://github.com/SquashyHydra/SteamAutoCracker/releases/download/[VERSION]-gui/Steam.Auto.Cracker.GUI.v[VERSION].zip", "release": "https://github.com/SquashyHydra/SteamAutoCracker/releases/tag/[VERSION]-gui"} | |||
There was a problem hiding this comment.
this should be {"version": "2.2.1", "link": "https://github.com/BigBoiCJ/SteamAutoCracker/releases/download/[VERSION]-gui/Steam.Auto.Cracker.GUI.v[VERSION].zip", "release": "https://github.com/BigBoiCJ/SteamAutoCracker/releases/tag/[VERSION]-gui"}
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
The url must be changed on line 54