Repository navigation
Conversation
Local backend (127.0.0.1:6767):
- Routes now match on HTTP method. A plain GET (e.g. <img src> on any
web page, no CORS needed) could reach POST routes like /remove/{appid}.
- Reject requests whose Host is not 127.0.0.1/localhost:6767 (DNS
rebinding) or whose Origin is not a Steam surface.
- CORS echoes the trusted origin instead of "*".
- Non-numeric appids are a 404 instead of a 500 from long.Parse.
- netsh urlacl is reserved for the current user, not Everyone.
Settings:
- The "nothing set, delete the file" check is now derived from every
AppSettings property. DnsMode was missing from the hand-kept list, so a
user whose only change was the DNS mode lost it on the next save.
- Save is serialized (all saves share one .tmp path) and IO failures are
logged instead of thrown out of property setters.
PluginAddService: Busy is entered atomically, so two quick picks can no
longer both start a download.
Adds tests for all of the above and a CI workflow running build + test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Local backend (
127.0.0.1:6767) could be driven by any web page the user opens.MatchPostwasMatchGet), so a plain GET reached state-changing routes.<img src="http://127.0.0.1:6767/remove/730">on any site deleted that app's lua file, no CORS involved.Access-Control-Allow-Origin: *with no Origin/Host check let any site call the other endpoints too, and nothing stopped DNS rebinding.Changes in
HttpServerService:Match(method, expected, path, pattern, out long id)).Hostis127.0.0.1:6767/localhost:6767andOriginis absent (the app's ownCefInjectorServiceHttpClient) or an https Steam surface (store.steampowered.com,steamcommunity.com,steamloopback.host).*.long.Parse.netsh urlaclfallback reserves the URL for the current user instead ofEveryone.The plugin's existing calls are unaffected:
CefInjectorService.CallBackendMethodalready uses the correct method for every route and sends no Origin.Settings silently lost.
SettingsService.Savedeletessettings.jsonwhen nothing is set, using a hand-kept list of fields.DnsModewas missing from it (the same bug the comment describes forFixesPageSize), so a user whose only change was the DNS mode lost it on the next save. The check is now derived from everyAppSettingsproperty.Saveis also serialized (all saves share one.tmppath, so concurrent saves raced onFile.Move), and IO failures are logged instead of thrown out of property setters.PluginAddService:Busywas a read-then-write bool on the thread pool, so two quick picks could both start. It is now entered withInterlocked.CompareExchange.CI: adds
build-test.ymlrunningdotnet build+dotnet testonwindows-latest. Previously only the i18n check ran.Testing
dotnet buildclean, 0 errors.HttpServerSecurityTests(trusted/untrusted Host+Origin, method mismatch, malformed ids) andSettingsEmptinessTests(everyAppSettingsproperty counts as non-empty — would have caughtDnsMode). All 34 pass.LuaVaultTests.Rename_*) are pre-existing and unrelated: they assert the English string "Build …" and fail on a machine with a non-English UI culture (they get "Сборка …"). They should pass on the CI runner.