Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,24 @@ jobs:
name: OpenGameBoost-Windows
path: dist/OpenGameBoost.exe
retention-days: 30

release:
needs: build
runs-on: ubuntu-latest
if: startsWith(github.ref, 'refs/tags/')
permissions:
contents: write

steps:
- name: Download artifact
uses: actions/download-artifact@v4
with:
name: OpenGameBoost-Windows

- name: Create Release
uses: softprops/action-gh-release@v2
with:
files: OpenGameBoost.exe
draft: false
prerelease: false
token: ${{ secrets.GITHUB_TOKEN }}
14 changes: 9 additions & 5 deletions config.py
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 _merge_defaults uses shallow copy of DEFAULT_CONFIG, allowing mutation of module-level defaults

The PR correctly fixed load() and reset_to_defaults() to use _deep_copy_defaults(), but _merge_defaults() still passes DEFAULT_CONFIG directly to merge_dict() as the base parameter. Inside merge_dict, result = base.copy() performs only a shallow copy, so nested dicts in DEFAULT_CONFIG are shared by reference.

Root Cause and Impact

When a user's config file is missing an entire section (e.g., "registry"), the shallow copy at config.py:101 means result["registry"] is the same dict object as DEFAULT_CONFIG["registry"]. After self.config = result at line 109, calling Config.set("registry", "enabled", False) at config.py:124 mutates self.config["registry"]["enabled"], which is the same object as DEFAULT_CONFIG["registry"]["enabled"].

This corrupts the module-level DEFAULT_CONFIG for the lifetime of the process. Subsequent calls to reset_to_defaults() or _deep_copy_defaults() will serialize the mutated DEFAULT_CONFIG, producing incorrect "defaults". For example, if the user toggles a service off and then resets to defaults, the reset would preserve the toggled-off state instead of restoring the true default.

(Refers to line 109)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"check_updates": True,
},
"game_detector": {
"enabled": True,
"enabled": False,
"auto_optimize": True,
"check_interval": 5,
},
Expand Down Expand Up @@ -60,6 +60,10 @@ def __init__(self, config_path: str = None):
self.config: Dict[str, Any] = {}
self.load()

def _deep_copy_defaults(self) -> Dict[str, Any]:
"""Create a deep copy of DEFAULT_CONFIG to prevent mutation."""
return json.loads(json.dumps(DEFAULT_CONFIG))

def load(self) -> bool:
"""Load configuration from file."""
try:
Expand All @@ -71,13 +75,13 @@ def load(self) -> bool:
logger.info(f"Configuration loaded from {self.config_path}")
return True
else:
self.config = DEFAULT_CONFIG.copy()
self.config = self._deep_copy_defaults()
self.save()
logger.info("Created default configuration")
return True
except Exception as e:
logger.error(f"Error loading configuration: {e}")
self.config = DEFAULT_CONFIG.copy()
self.config = self._deep_copy_defaults()
return False

def save(self) -> bool:
Expand All @@ -102,7 +106,7 @@ def merge_dict(base: dict, override: dict) -> dict:
result[key] = value
return result

self.config = merge_dict(DEFAULT_CONFIG, self.config)
self.config = merge_dict(self._deep_copy_defaults(), self.config)

def get(self, section: str, key: str = None, default: Any = None) -> Any:
"""Get a configuration value."""
Expand All @@ -121,5 +125,5 @@ def set(self, section: str, key: str, value: Any):

def reset_to_defaults(self):
"""Reset all configuration to defaults."""
self.config = DEFAULT_CONFIG.copy()
self.config = self._deep_copy_defaults()
self.save()
Loading