Rebuild Privacy around BlueStacks' own ad switches#63
Conversation
The guest hosts block could never stop BlueStacks' advertising. Those ads are served by HD-Player.exe on Windows, not by the Android guest: a live capture on 5.22.250.1015 showed the player holding connections to googlesyndication, inmobi, rubiconproject and adnxs while the guest was completely powered off. Applying the block left the player's ad endpoints unchanged (40 before, 40 after). BlueStacks ships explicit switches for this in bluestacks.conf, and turning them off measured 40 endpoints to 0 on the same rig, with only Play, GMS, Firebase and fonts left. Add ad_settings.py to drive those switches, and make the Privacy tab a toggle for them. To survive version updates it discovers keys by concept pattern (programmatic_ads, send_*_stats, auto_upload) instead of a fixed list, so a renamed or newly added switch is still covered. It found 23 switches where a hand-written list had 19. Three gates keep that safe: the key must match a concept pattern, must not match an exclusion (root, adb, *_preference, and the inverted disable/skip keys), and must be a boolean 0/1 value, which is what keeps ids such as android_google_ad_id out of scope no matter how they are named. Originals are recorded per key, verified live as a byte-identical restore of the real config. BlueStacks selectively reverts keys on start: the bst.enable_* ones stick while most bst.feature.* are put back, including four stats beacons. status() reports that drift so the tab can offer a re-apply, and an optional read-only pin holds them (verified: 23/23 still off after a real start, instance boots normally). Also fix the hosts block itself, which stays as the in-guest tracker control: - Gate it on a patched engine. Editing the guest system image trips BlueStacks' tamper check and shuts the instance down mid-session; root is not required, any modification does it. The Magisk tab already gated on this, Privacy did not. - Enumerate subdomains explicitly. A hosts file has no wildcards, so an entry for doubleclick.net did nothing for cm.g.doubleclick.net or pagead2.googlesyndication.com, which is most real ad traffic. - Drop rtbhouse.net, which does not resolve, for rtbhouse.com and the esp.rtbhouse.com endpoint actually seen. - Stop the docstring claiming it blocks the player's ads. 247 tests green.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8f5b29f6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for key in sorted(switches): | ||
| if config_handler.modify_config_file(config_path, key, OFF): | ||
| changed += 1 | ||
|
|
||
| _write_state(config_path, originals) |
There was a problem hiding this comment.
If any per-key modify_config_file() call fails after an earlier key was changed, or if _write_state() itself fails, the operation exits without a sidecar even though the config is already partially or fully modified. A subsequent apply then records those new "0" values as the originals, so Restore can no longer recover the user's actual settings. Persist the original-value state atomically before the first config mutation, or update the config and state as one recoverable transaction.
Useful? React with 👍 / 👎.
| for data in self._window.instance_data.values(): | ||
| path = data.get("config_path") | ||
| if path: | ||
| return path |
There was a problem hiding this comment.
When more than one supported edition is installed, this returns the first config from instance_data, so only that edition is changed while the confirmation and status claim the operation covers every instance; registry_handler.get_all_bluestacks_installations() explicitly supports separate Normal, China, and MSI config paths. It also returns nothing when BlueStacks is installed but no instance survives the instance_data filtering, making this global feature unavailable even though the config exists. Derive and operate on all distinct config paths in window.installations instead.
Useful? React with 👍 / 👎.
| if checked: | ||
| ad_settings.lock(config_path) | ||
| else: | ||
| ad_settings.unlock(config_path) |
There was a problem hiding this comment.
If bluestacks.conf was already read-only for root persistence when the ad switches were applied, status() presents that shared attribute as this checkbox's pin; unchecking it then unconditionally clears the attribute here. In that upgrade/classic-root scenario, using the Privacy control silently removes root's persistence protection and allows BlueStacks to reset the root keys. Track whether the Privacy feature acquired the shared lock, or refuse to release a lock owned by another feature.
Useful? React with 👍 / 👎.
| try: | ||
| with open(_state_path(config_path), encoding="utf-8") as f: | ||
| state = json.load(f) | ||
| except (OSError, ValueError): | ||
| return None |
There was a problem hiding this comment.
After a user pins the config and chooses Restore, remove() deletes the sidecar but leaves the file read-only; this early return then prevents the lock state from reaching the page, which programmatically shows an unchecked box. Clicking that unchecked box only attempts to lock an already-locked file and immediately refreshes it back to unchecked, so the user cannot unpin the config through the UI and BlueStacks remains unable to save settings. Either release an ad-owned pin during restore or report the live lock independently of the sidecar.
Useful? React with 👍 / 👎.
Definition-list bullets in Features and Project Structure used a hyphen where a colon belongs, so all 42 now read as labels. Fixes the comma splices left behind by the earlier wording pass, pairing clauses with semicolons and expanding with colons. Tightens a few run-ons, drops the exclamation in Contributing, and rewrites the Modules-tab and companion-guide paragraphs that had run together. Version range reads "5.20.x to 5.21.x" instead of using a dash. No content changes.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
A blocklist entry has to be its own exact hostname, so comparing with == rather than `in` is the stricter check: a substring sitting inside some longer domain can no longer satisfy the test. This also clears two high-severity CodeQL incomplete-URL-sanitization alerts, which flag "domain" in x without being able to tell tuple membership from a substring check on a URL.
2 participants
Why
The guest hosts block could never stop BlueStacks' advertising. Those ads are served by
HD-Player.exeon Windows, not by the Android guest.Evidence, measured live on 5.22.250.1015 with a fresh instance:
BlueStacksAppplayerWeb,BstkSVCandBlueStacksServiceshad 0 each. The guest's ownnetstatshowed ~19, all Google/GMS/cloud.bluestacks.com.BlueStacks ships explicit switches for this in
bluestacks.conf. Turning them off, same rig, same instance:What remained was purely Play, GMS, Firebase and fonts. The emulator stayed healthy.
What changed
New
ad_settings.py, and the Privacy tab becomes a toggle for it.Version resilience was the design constraint, since a hardcoded key list silently rots across builds. It discovers keys by concept pattern (
programmatic_ads,send_*_stats,auto_upload) against whatever config is present, so a renamed or newly added switch is covered with no code change. On this build it found 23 switches where a hand-written list had 19, catchingsend_auto_record_stats,send_usage_state_statsandshow_nowbux_rewards_red_dot_onboarding.Three gates keep pattern matching safe, so a rename can never make it flip something harmful:
root,adb,*_preference, and the inverteddisable/skipkeys),"0"/"1"value, which is what keeps ids likeandroid_google_ad_idout of scope regardless of naming.Originals are recorded per key. BlueStacks selectively reverts on start (
bst.enable_*stick, mostbst.feature.*are put back including four stats beacons), sostatus()reports that drift and the tab offers a re-apply, plus an optional read-only pin.The hosts block stays as the in-guest tracker control, with its real defects fixed:
doubleclick.netdid nothing forcm.g.doubleclick.netorpagead2.googlesyndication.com, which is most real ad traffic.rtbhouse.netdropped (it does not resolve) forrtbhouse.comand theesp.rtbhouse.comendpoint actually observed.Verification
Live on a fresh BlueStacks + fresh Android 13 instance, not just unit tests:
remove()247 tests green, ruff clean. New tests use the real observed key set as a fixture, including the near-misses that must not be touched (
bst.enable_adb_accesscontains "ad";android_google_ad_idis an id;*_preferencecontrols whether the user's own toggle is visible;skipNowggLoginis inverted).