Restore encryption logic and finalize secure CI/CD pipelines - #596
Conversation
There was a problem hiding this comment.
Sorry @ikostan, your pull request is larger than the review limit of 150000 diff characters
|
Important Review skippedToo many files! This PR contains 187 files, which is 37 over the limit of 150. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (187)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python | May 8, 2026 3:10a.m. | Review ↗ | |
| JavaScript | May 8, 2026 3:10a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Delete a large set of legacy GDScript files and their .uid counterparts from the scripts/ directory. Removed items include JavaScriptBridgeWrapper, OSWrapper, advanced_settings, audio_constants, audio_manager, audio_settings, audio_web_bridge, HUD/menu/player/gameplay-related scripts, resource_preloader and other related autoload/singleton scripts to clean up obsolete UI/audio/web-bridge code in preparation for refactor or replacement.
Adds a new GUT test suite (test_full_encryption_workflow.gd) that exercises the complete Settings encryption workflow: fresh install encryption/save/load, plaintext fallback and migration, mixed-systems concurrent writes, restart behavior after migration, and corrupted file handling. Tests isolate filesystem state at user://test_encryption_workflow.cfg and use per-test setup/teardown to backup/restore ProjectSettings salt, clear Globals' cached key, and swap Globals.settings to avoid polluting global state. Also adds the corresponding test .uid file.
Enhance encryption-related logging across Globals and Settings: add detailed, emoji-prefixed error/debug messages for save/load/encryption key operations, surface decryption failures, plaintext migration warnings, and confirm cached key generation. Ensure use of the centralized ensure_encryption_key helper when saving, and add logging in ensure_encryption_key itself. Add GUT tests (test_encryption_logging.gd + uid) to exercise the new logging branches (key generation, missing config, successful encrypt/decrypt, plaintext load, and save failures). These changes make encryption issues easier to diagnose during development and CI runs.
Add a project setting placeholder for 'game/security/save_salt' and harden the encryption key generation: if the salt is empty, _get_encryption_key now logs a warning and aborts to plaintext (returns empty) to avoid silent weak-key fallbacks. Adjust the production guard to crash only on the dev_fallback_salt while still allowing automated/browser-driven tests. Add GUT tests (test_encryption_failsafe.gd + .uid) that verify the salt property is registered, that missing salt triggers the plaintext fallback, and that a valid salt results in an encrypted save; tests also isolate ProjectSettings and clear the cached encryption key during setup/teardown.
Create workspace package (workspace/__init__.py) and move various CI/test/lint scripts into the workspace/ directory. Renamed/moved: run_browser_tests.sh, run_gdlint.sh, run_gdunit4_unit_tests.sh, run_gut_unit_tests.sh, run_markdown_lint.sh, run_pipeline.sh, run_unit_tests.sh, and run_yaml_lint.sh to consolidate tooling and improve project organization.
Call Globals.set_test_encryption_key() in several GUT tests and add explicit loading flags to prevent auto-saves during test setup. Modified tests: test_combined_multi_manager_loads.gd, test_globals_resource.gd, test_preserve_other_sections.gd, and test_settings_observer.gd. This ensures the empty project salt is overridden with a valid test key so Globals._save_settings uses encryption (avoiding plaintext fallback) and prevents test actions/signals from triggering unintended saves to production files.
Add *.pyc to .gitignore and adjust tests to tolerate known encryption fallback messages. audio_flow_test now ignores logs containing 'encryption aborted' when checking for unexpected warnings. difficulty_flow_test accepts either an encrypted save log or a plaintext fallback message and improves related assertion messages to avoid false failures on WebGL/plaintext fallbacks.
Add a new how-to document explaining how to locate and read the virtual file system created by Godot web exports (Emscripten IDBFS / IndexedDB) and include a JavaScript snippet to extract file bytes in Chrome DevTools. Also update README to link to the new guide so users can inspect saved files and verify contents before and after encryption.
Add two Milestone 17 docs describing UI audio and config encryption work. Part 1 documents a dedicated, rate-limited slider SFX and centralized AudioManager.play_sfx with stream caching and an AudioStreamPlayer pool; it also details VolumeSlider hardening (interaction guards, programmatic update API, delta/rate gating, invalid-bus protection) and extensive GUT/GdUnit4 test updates to prevent feedback loops and flaky CI. Part 2 documents centralized encryption key management in Globals (deterministic per-device key with test/web overrides), safe encrypted ConfigFile load/save with legacy plaintext migration, CI injection of a PRODUCTION_SALT, and broad test/tooling adjustments to validate migration and preserve unrelated config sections. Both files summarize added tests, CI/deploy changes, and migration/backward-compatibility safeguards.
…SkyLockAssault into fix-encryption-pipeline
Add two Milestone 17 documentation files: Part 1 describes audio/UI changes including a dedicated slider SFX, AudioManager pooling/caching, VolumeSlider hardening and test-suite updates; Part 2 documents centralized encryption key management for config files, plaintext→encrypted migration, CI salt injection and test adaptations. Remove the older duplicate milestone doc files.
Apply markdown style and linting fixes across documentation and the lint runner. Changes include: normalize list markers and spacing, remove trailing/extra blank lines, reflow wrapped lines for readability, convert an inline image to a reference-style link, and standardize table formatting in milestone docs. Also tweak README milestone wording for clarity. Minor shell script formatting added to workspace/run_markdown_lint.sh to improve readability. These edits are purely stylistic to satisfy markdownlint and improve docs consistency.
Cosmetic cleanup across docs and scripts: standardize list markers (replace leading '+' list markers with '-'), fix blank-line/whitespace inconsistencies, and tidy minor punctuation/spacing in README and several docs. Also add a small readability newline in workspace/run_markdown_lint.sh. No functional changes.
…ers_only_during_manual_slider_adjustments.md
…ers_only_during_manual_slider_adjustments.md
Add a new script .github/scripts/inject_salt.sh that injects security/save_salt into Godot project files, properly escaping backslashes and quotes and handling presence/absence of the [game] section. Update .github/workflows/lint_test_deploy.yml to run a CI job that exercises the shared script, and make browser functional tests depend on it. Update tests/ci/test_salt_injection.py to call the shared bash script instead of using an inline AWK blob, simplify the test flow, and adjust expected output for the escaped salt. This centralizes salt injection logic and makes CI/tests use the same implementation.
Clarify escaping behavior in .github/scripts/inject_salt.sh (suggest using printf %q or careful sed and explicitly escape backslashes and quotes). Update tests/ci/test_salt_injection.py to match the real Action script path, use a raw Python string for the complex secret, capture subprocess output and handle FileNotFoundError, and print stderr on script failures. Add additional integration tests to cover injecting into an existing [game] section (ensuring correct insertion order) and overwriting an existing salt, improve failure messages, and ensure cleanup of the dummy file.
Refactor .github/scripts/inject_salt.sh to read the salt from the SALT environment variable and use TARGET_FILE for the input path; the AWK logic was reformatted but retains the same behavior (inject or overwrite security/save_salt in the [game] section, or append the section). Remove the stale dummy.godot fixture. Update tests/ci/test_salt_injection.py to run the AWK logic directly from Python (embed the AWK script), properly escape the test secret, load it via env, and add multiple test cases (no [game] section, existing [game] with following section, and overwrite existing salt). Also improve error messaging for missing awk and ensure temp-file replace/cleanup.
name: Default Pull Request Template
about: Suggesting changes to SkyLockAssault
title: ''
labels: ''
assignees: ''
Description
Here is the accurate summary of the Git/GitHub catastrophe that forced us to do all of this reconstruction:
The reason your last two PRs suddenly disappeared is that a destructive version control operation occurred on your repository—most likely a bad merge resolution, an accidental git reset, or a force push. This operation grabbed the pointer for your main branch and forcibly dragged it backward in time to an older commit.
When that pointer was moved backward, the commits containing your two merged PRs (which held the initial encryption pipeline work) were completely severed from the active timeline. The code wasn't permanently deleted from GitHub's servers—as evidenced by the old GitHub Actions runs and the Git Reflog—but it became "orphaned." Because the commits were no longer part of the active branch history, the code simply vanished from your working directory.
Because the branch was rolled back, your project was thrown into a fractured state. The active codebase reverted to a state before the encryption was fully stabilized, causing the singletons to fall out of sync. We had to do all of this painstaking work today to manually resurrect that orphaned logic, re-integrate the safe_load_config and ensure_encryption_key methods across globals.gd, settings.gd, and audio_manager.gd, and patch the CI/CD failsafes so the game could actually read its own save files again.
Related Issue
Closes #ISSUE_NUMBER (if applicable)
Changes
system")
Testing
works on Win10 with 60 FPS")
Checklist
Additional Notes
Anything else? (e.g., "Tested on Win10 64-bit; needs Linux validation")