diff --git a/CHANGELOG.md b/CHANGELOG.md index 21254d2..efc3fe3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -630,6 +630,7 @@ are absent), so shell regressions are caught locally before the push. ### Bug fixes +- **The Magisk denylist now hides root in the Play-Integrity/DroidGuard process (#170).** Denylist entries take a `package[/process]` shape, and `magisk-config.sh` inserts the real package into `package_name` and the process into `process` — never copying one value into both columns. The default enrolls `com.google.android.gms.unstable` (DroidGuard) as a **process** of the `com.google.android.gms` package instead of a bogus `package_name` that matched no installed app, so vanilla (non-Shamiko) Magisk actually hides root in the attestation process. The grammar validates both halves (SQL-injection prophylaxis), the health check matches enrolment by the package half, and the example configs demonstrate the corrected form. - **`registry._read()` no longer silently drops an instance row that fails row-level validation (#252).** A row with a known backend kind but a rejected payload is now preserved opaquely so its port index stays reserved and it round-trips; a row too broken to salvage (bad `created_at`, missing/non-int index) is surfaced loudly (backed up to `.bak` with a hint) instead of being dropped and having its index silently reused. - **`create`/`register` now refuse a directory that nests inside — or contains — another registered instance (#255).** Previously the overlap guard existed only for `restore`, so a nested `create` could later be wiped out by a `destroy` of the outer instance. The guard runs before any `mkdir`/registry write, so a refused operation is a no-op. - **`beetroot adopt ` (the help's own example) now auto-derives a valid instance name (#257).** `_adopt_default_name` collapses every non-alphanumeric run (including the dots in an IP) to a hyphen, so a network serial no longer fails the name-grammar guard and demands `--name`. diff --git a/docker/magisk-config.sh b/docker/magisk-config.sh index ec69c71..1065926 100755 --- a/docker/magisk-config.sh +++ b/docker/magisk-config.sh @@ -18,10 +18,17 @@ # internally. Echoed in the waiting log so the user knows which DB # we're targeting when v0.4 stealth-posture work randomises it. # BEETROOT_DENYLIST_PACKAGES= -# Comma-separated list of Android package ids to enrol in Magisk's -# denylist (per-package SQL-injection prophylaxis lives in pydantic; -# see Magisk._check_packages in src/beetroot/config.py). Empty by -# default — the helper SQL'es nothing extra. +# Comma-separated list of ``package[/process]`` entries to enrol in +# Magisk's denylist. Each entry is a package, optionally followed by a +# slash and a process that belongs to it; no slash means the process +# equals the package. The helper splits on the first '/' and INSERTs the +# package into package_name and the process into process — Magisk keys the +# denylist on (package_name, process), so the DroidGuard process +# com.google.android.gms.unstable must be enrolled under its real package +# com.google.android.gms, never as a package of its own (issue #170). +# Per-half SQL-injection prophylaxis lives in pydantic; see +# Magisk._check_packages in src/beetroot/config.py. Empty by default — +# the helper SQL'es nothing extra. # BEETROOT_MAGISK_WAIT_SECS=120 # Upper bound (in 1-second probe attempts) on the Magisk daemon wait. # Conservative because a first boot of redroid+Magisk can legitimately @@ -129,6 +136,21 @@ if [ -n "$DENYLIST_PACKAGES" ]; then if [ -z "$pkg" ]; then continue fi - magisk --sqlite "INSERT OR IGNORE INTO denylist (package_name, process) VALUES ('$pkg', '$pkg');" + # Split the ``package[/process]`` entry (issue #170). The package is + # everything before the first '/', the process everything after it; + # an entry with no '/' has the process default to the package. Magisk + # keys the denylist on (package_name, process), so the DroidGuard + # process com.google.android.gms.unstable must land in the process + # column under its real package com.google.android.gms — never copied + # into package_name, where it matches no installed package and vanilla + # (non-Shamiko) Magisk never hides root. Toybox sh has no arrays; use + # POSIX parameter expansion (``%%/*`` = before first '/', ``#*/`` = + # after first '/') rather than a bash-ism. + pkg_name="${pkg%%/*}" + pkg_proc="${pkg#*/}" + if [ "$pkg_proc" = "$pkg" ]; then + pkg_proc="$pkg_name" + fi + magisk --sqlite "INSERT OR IGNORE INTO denylist (package_name, process) VALUES ('$pkg_name', '$pkg_proc');" done fi diff --git a/docs/guides/examples.md b/docs/guides/examples.md index bd42d4e..16bc6b8 100644 --- a/docs/guides/examples.md +++ b/docs/guides/examples.md @@ -27,11 +27,23 @@ android: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable ``` Use this when you're testing something that doesn't perform anti-root checks, or when you want the lightest-weight setup with sensible defaults. +!!! note "Denylist entries are `package[/process]`" + Each `magisk.denylist` entry is a package, optionally followed by a slash and a **process** that belongs to it (`package/process`). No slash means the process is the package itself. This shape matters because Magisk keys the denylist on `(package_name, process)`: DroidGuard (Play Integrity) runs as the `com.google.android.gms.unstable` **process** of the `com.google.android.gms` package — it is not an installed package of its own. So the schema default enrols it as: + + ```yaml + magisk: + denylist: + - com.google.android.gms + - com.google.android.gms/com.google.android.gms.unstable # DroidGuard process + ``` + + Enrolling the bare `com.google.android.gms.unstable` as if it were a package matches no installed app, and vanilla (non-Shamiko) Magisk then never hides root in DroidGuard. See the [`magisk` config reference](../reference/config.md#magisk). + ### `stealth.yaml` A wider Magisk denylist suitable for use with a root-hider like [Shamiko](https://github.com/LSPosed/LSPosed.github.io). Shamiko turns Magisk's denylist mode into a true allowlist-based hide — processes on the denylist can't detect Magisk at all. The denylist below covers all GMS variants and the Play Store. @@ -54,8 +66,8 @@ android: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable - - com.google.android.gms.persistent + - com.google.android.gms/com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.persistent - com.android.vending ``` @@ -90,7 +102,7 @@ frida: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable ``` Drop the `frida:` block (or copy `examples/default.yaml`) to turn Frida back off. diff --git a/docs/reference/config.md b/docs/reference/config.md index 7e9d711..c2fec04 100644 --- a/docs/reference/config.md +++ b/docs/reference/config.md @@ -292,17 +292,19 @@ The variable-length port list is written to a per-instance `compose.override.yam ## `magisk` -Magisk configuration, including the boot-time denylist. Processes listed here are denylisted in the Magisk SQLite database at boot time, before any app launches. +Magisk configuration, including the boot-time denylist. Entries listed here are enrolled in the Magisk SQLite database at boot time, before any app launches. + +Each entry is encoded as `package[/process]` — a package, optionally followed by a slash and a **process** that belongs to it. With no slash, the process is the package itself (root is hidden from the package's main process). With a slash, the package goes into the denylist's `package_name` column and the process into its `process` column. This matters because Magisk keys the denylist on `(package_name, process)`: Play Integrity's DroidGuard runs as the `com.google.android.gms.unstable` **process** of the `com.google.android.gms` package — it is *not* an installed package of its own, so it must be enrolled as `com.google.android.gms/com.google.android.gms.unstable`. Enrolling the bare `com.google.android.gms.unstable` as if it were a package matches no installed app, and vanilla (non-Shamiko) Magisk then never hides root there. | Field | Type | Default | Description | |-------|------|---------|-------------| -| `denylist` | list[string] | `["com.google.android.gms", "com.google.android.gms.unstable"]` | Package names to add to Magisk's denylist. Each entry must match the Android package-id grammar (`[a-zA-Z0-9._]+`) — validated at load time as SQL-injection prophylaxis. | +| `denylist` | list[string] | `["com.google.android.gms", "com.google.android.gms/com.google.android.gms.unstable"]` | `package[/process]` entries to enrol in Magisk's denylist. Both halves must match the Android package-id grammar (`[a-zA-Z0-9._]+`), separated by at most one `/` — validated at load time as SQL-injection prophylaxis. The default hides root in the GMS main process **and** its `.unstable` DroidGuard process, both under the real `com.google.android.gms` package. | ```yaml magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable # DroidGuard process of GMS - com.google.android.gms.persistent - com.android.vending ``` @@ -440,7 +442,7 @@ modules: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable # DroidGuard process of GMS - com.google.android.gms.persistent - com.android.vending - com.target.app diff --git a/examples/default.yaml b/examples/default.yaml index 25b004f..319f147 100644 --- a/examples/default.yaml +++ b/examples/default.yaml @@ -15,4 +15,4 @@ android: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable diff --git a/examples/lsposed.yaml b/examples/lsposed.yaml index fcc4266..c5c56bb 100644 --- a/examples/lsposed.yaml +++ b/examples/lsposed.yaml @@ -26,4 +26,4 @@ modules: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable diff --git a/examples/stealth.yaml b/examples/stealth.yaml index 1953637..b69e0f5 100644 --- a/examples/stealth.yaml +++ b/examples/stealth.yaml @@ -34,6 +34,6 @@ android: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable - - com.google.android.gms.persistent + - com.google.android.gms/com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.persistent - com.android.vending diff --git a/examples/with-frida.yaml b/examples/with-frida.yaml index 56797bb..f0280f0 100644 --- a/examples/with-frida.yaml +++ b/examples/with-frida.yaml @@ -14,4 +14,4 @@ frida: magisk: denylist: - com.google.android.gms - - com.google.android.gms.unstable + - com.google.android.gms/com.google.android.gms.unstable diff --git a/src/beetroot/api.py b/src/beetroot/api.py index 01e2d75..0fc49a4 100644 --- a/src/beetroot/api.py +++ b/src/beetroot/api.py @@ -1487,10 +1487,15 @@ def health(self) -> dict[str, CheckResult]: checks["magisk.zygisk"] = _check_magisk_zygisk_over_adb(f"localhost:{adb_port}") denylist = self._cfg.magisk.denylist gms_pkg = "com.google.android.gms" + # Denylist entries are ``package[/process]`` (issue #170); match by the + # PACKAGE half so an entry that only writes ``gms/`` still counts + # as enrolled (the SQL check keys on package_name, which is now the real + # package regardless of the process the entry targets). + enrolled = any(entry.split("/", 1)[0] == gms_pkg for entry in denylist) checks[f"magisk.denylist.{gms_pkg}"] = _check_magisk_denylist_over_adb( f"localhost:{adb_port}", gms_pkg, - enrolled=gms_pkg in denylist, + enrolled=enrolled, ) return checks diff --git a/src/beetroot/config.py b/src/beetroot/config.py index 299a363..1a090c0 100644 --- a/src/beetroot/config.py +++ b/src/beetroot/config.py @@ -106,11 +106,17 @@ def validate_android_version(v: int) -> int: _MIN_PORT: Final = 1 _MAX_PORT: Final = 65535 -# Magisk/stealth denylist packages must look like a normal Android package -# id: alphanumerics, dots, and underscores only. Pre-validated at -# config-load time as SQL-injection prophylaxis for the wire-up of -# the denylist through ``magisk-config.sh``'s sqlite REPLACE INTO. -_DENYLIST_PKG_RE: Final = re.compile(r"^[a-zA-Z0-9._]+$") +# Magisk/stealth denylist entries encode a package and an optional process it +# belongs to as ``package[/process]``. Both halves must look like a normal +# Android package id: alphanumerics, dots, and underscores only (the ``/`` is +# only the separator, never inside a half). Magisk's denylist keys on +# (package_name, process), and a process like ``com.google.android.gms.unstable`` +# (DroidGuard) is NOT an installed package — it must be enrolled under its real +# package ``com.google.android.gms`` or vanilla (non-Shamiko) Magisk never hides +# root there (issue #170). Pre-validated at config-load time — on BOTH halves — +# as SQL-injection prophylaxis for the wire-up of the denylist through +# ``magisk-config.sh``'s sqlite INSERT. +_DENYLIST_PKG_RE: Final = re.compile(r"^[a-zA-Z0-9._]+(/[a-zA-Z0-9._]+)?$") # Frida release tags follow the major.minor.patch shape upstream. # Pre-validated so a typo in ``frida.version`` (e.g. ``"16.4"`` or @@ -474,7 +480,7 @@ def model_post_init(self, _ctx: object) -> None: _DEFAULT_DENYLIST: Final = ( "com.google.android.gms", - "com.google.android.gms.unstable", + "com.google.android.gms/com.google.android.gms.unstable", ) @@ -483,17 +489,27 @@ class Magisk(BaseModel): Magisk configuration, including the boot-time denylist. Attributes: - denylist: Package names added to Magisk's denylist at boot. Each - entry must match the Android package-id grammar - (``[a-zA-Z0-9._]+``) — see :data:`_DENYLIST_PKG_RE`. The - grammar is enforced at validation time so - ``magisk-config.sh`` can compose the entries into a SQLite - REPLACE-INTO statement without escaping; any shape that - wouldn't be a valid package name today is assumed to be - either a typo or an injection attempt. - Defaults to the GMS package pair (the v0.3 helper enrolled - these unconditionally; the config move keeps the default - behaviour identical while putting the user in control). + denylist: Entries added to Magisk's denylist at boot, each encoded + as ``package[/process]`` — a package, optionally followed by a + slash and a process that belongs to it (no slash means the + process equals the package). Both halves must match the Android + package-id grammar (``[a-zA-Z0-9._]+``) — see + :data:`_DENYLIST_PKG_RE`. The grammar is enforced on BOTH halves + at validation time so ``magisk-config.sh`` can split the entry + and compose the package + process into a SQLite INSERT without + escaping; any shape that wouldn't be a valid package name today + is assumed to be either a typo or an injection attempt. The + ``package/process`` form matters because Magisk's denylist keys + on ``(package_name, process)``: DroidGuard runs as the + ``com.google.android.gms.unstable`` *process* of the + ``com.google.android.gms`` package — not an installed package of + its own — so it must be enrolled under its real package or + vanilla (non-Shamiko) Magisk never hides root there (issue #170). + Defaults to the GMS main process plus the ``.unstable`` + DroidGuard process, both under the real ``com.google.android.gms`` + package (the v0.3 helper enrolled the GMS pair unconditionally; + the config move keeps the intent identical while putting the user + in control and fixing the process it targets). """ denylist: list[str] = Field(default_factory=lambda: list(_DEFAULT_DENYLIST)) @@ -501,11 +517,12 @@ class Magisk(BaseModel): @field_validator("denylist") @classmethod def _check_packages(cls, value: list[str]) -> list[str]: - for pkg in value: - if not _DENYLIST_PKG_RE.match(pkg): + for entry in value: + if not _DENYLIST_PKG_RE.match(entry): raise ValueError( - f"magisk.denylist entry {pkg!r} is not a valid Android " - "package id (must match [a-zA-Z0-9._]+)" + f"magisk.denylist entry {entry!r} is not a valid " + "package[/process] id (both halves must match [a-zA-Z0-9._]+, " + "with an optional single '/' separating package and process)" ) return value @@ -1313,9 +1330,11 @@ def render_env( f"DISPLAY_FPS={cfg.display.fps}", f"DISPLAY_GPU={resolve_rendering(cfg.display.rendering)}", # Encoded as a comma-separated list because toybox sh has no array - # support — the helper iterates over ``IFS=,``. Per-package shape is - # already validated by the ``Magisk._check_packages`` regex, so we - # can safely join with a delimiter that's not in the package-id grammar. + # support — the helper iterates over ``IFS=,``, then splits each + # ``package[/process]`` entry on the ``/``. Per-entry shape (both + # halves + the single optional ``/``) is already validated by the + # ``Magisk._check_packages`` regex, so we can safely join with a comma + # (which is not in the entry grammar) and split on ``/`` in the helper. f"BEETROOT_DENYLIST_PACKAGES={','.join(cfg.magisk.denylist)}", # v0.4 stealth-posture overrides — emitted with the known-safe # defaults. render_env is the single source of truth instead of the diff --git a/tests/test_config.py b/tests/test_config.py index 7a01a1c..2b8bb45 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -610,32 +610,55 @@ def test_valid_packages_accepted(self) -> None: cfg = Magisk(denylist=["com.google.android.gms", "com.app_id", "com.x.y.z123"]) assert cfg.denylist[0] == "com.google.android.gms" + def test_package_with_process_accepted(self) -> None: + # issue #170: the ``package/process`` shape enrols a process of a + # package (e.g. the DroidGuard process of GMS) under its REAL + # package. Both halves are validated against the package grammar. + cfg = Magisk( + denylist=["com.google.android.gms/com.google.android.gms.unstable"] + ) + assert cfg.denylist[0] == "com.google.android.gms/com.google.android.gms.unstable" + + def test_package_with_malformed_process_rejected(self) -> None: + # The process half is validated against the same grammar; a dash in + # it (not part of the Android package-id grammar) is refused. + with pytest.raises(ValidationError, match=r"package\[/process\] id"): + Magisk(denylist=["com.google.android.gms/bad-proc"]) + + def test_package_with_two_slashes_rejected(self) -> None: + # Only a single optional '/' is allowed; a second slash is neither a + # valid package nor process half. + with pytest.raises(ValidationError, match=r"package\[/process\] id"): + Magisk(denylist=["com.a/com.b/com.c"]) + def test_gms_denylist_default(self) -> None: - # The GMS pair is the default so a bare ``beetroot create`` - # denylists root from GMS out of the box. + # issue #170: the default hides root in the GMS main process AND its + # ``.unstable`` DroidGuard process, both under the REAL package + # ``com.google.android.gms`` — the ``.unstable`` string is a PROCESS, + # not a package, so it must ride on the package/process form. assert Magisk().denylist == [ "com.google.android.gms", - "com.google.android.gms.unstable", + "com.google.android.gms/com.google.android.gms.unstable", ] def test_package_with_space_rejected(self) -> None: - with pytest.raises(ValidationError, match="package id"): + with pytest.raises(ValidationError, match=r"package\[/process\] id"): Magisk(denylist=["com.bad package"]) def test_package_with_semicolon_rejected(self) -> None: # SQL-injection probe: a literal "; DROP TABLE settings;" must # be rejected by the validator before the helper ever sees it. - with pytest.raises(ValidationError, match="package id"): + with pytest.raises(ValidationError, match=r"package\[/process\] id"): Magisk(denylist=["com.app'; DROP TABLE settings;--"]) def test_package_with_dash_rejected(self) -> None: # Dashes are not part of the Android package-id grammar; refuse # them so the validator can't drift to a looser shape later. - with pytest.raises(ValidationError, match="package id"): + with pytest.raises(ValidationError, match=r"package\[/process\] id"): Magisk(denylist=["com.bad-package"]) def test_empty_package_rejected(self) -> None: - with pytest.raises(ValidationError, match="package id"): + with pytest.raises(ValidationError, match=r"package\[/process\] id"): Magisk(denylist=[""]) def test_stealth_key_rejected_with_migration_hint(self, tmp_path: Path) -> None: @@ -1016,12 +1039,14 @@ def test_memswap_limit_emitted_when_set(self) -> None: assert "MEMSWAP_LIMIT=4g" in result def test_emits_default_denylist_packages(self) -> None: - # D1: the default Magisk model carries the GMS pair so a bare - # ``beetroot create`` keeps the historical behaviour intact. + # issue #170: the default carries the GMS main package plus the + # ``.unstable`` DroidGuard process under its real package, joined with + # a comma; the ``/`` inside the second entry survives the CSV join. cfg = InstanceConfig() result = render_env("alpha", cfg) assert ( - "BEETROOT_DENYLIST_PACKAGES=com.google.android.gms,com.google.android.gms.unstable" + "BEETROOT_DENYLIST_PACKAGES=com.google.android.gms," + "com.google.android.gms/com.google.android.gms.unstable" ) in result def test_emits_custom_denylist_packages_as_csv(self) -> None: diff --git a/tests/test_doctor.py b/tests/test_doctor.py index a9e8493..e115388 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -66,10 +66,53 @@ def test_healthy_instance_exits_0(self, cli_root: Path) -> None: assert "host.binder: pass" in result.stdout assert "adb.connect: pass" in result.stdout assert "magisk.zygisk: pass" in result.stdout + # issue #170: the default denylist keeps a plain ``com.google.android.gms`` + # entry alongside the ``package/process`` DroidGuard entry, so the redroid + # health check still matches by PACKAGE and enrols the GMS row (pass, not + # skip). The SQL keys on package_name, which is now the real package. + assert "magisk.denylist.com.google.android.gms: pass" in result.stdout # Frida is opt-out by default since v0.3 — minimal-default # InstanceConfig has frida=None so frida.handshake skips. assert "frida.handshake: skip" in result.stdout + def test_denylist_process_only_entry_still_enrolls_gms(self, cli_root: Path) -> None: + # issue #170 regression: a config whose ONLY GMS entry is the + # ``package/process`` form (no bare ``com.google.android.gms``) must + # still count as enrolled — the health check matches by the PACKAGE + # half, and the SQL keys on package_name (the real package). The + # ``.unstable`` string is a PROCESS of com.google.android.gms, never a + # package_name of its own. + runner.invoke(cli.app, ["create", "alpha"]) + root = registry.instance_path("alpha") + (root / "beetroot.yaml").write_text( + "api_version: 8\n" + "android:\n version: 14\n" + "magisk:\n" + " denylist:\n" + " - com.google.android.gms/com.google.android.gms.unstable\n" + ) + with patch("subprocess.run", side_effect=_healthy_subprocess): + result = runner.invoke(cli.app, ["doctor", "alpha"]) + assert result.exit_code == 0, (result.stdout, result.stderr) + assert "magisk.denylist.com.google.android.gms: pass" in result.stdout + + def test_denylist_without_gms_reports_skip(self, cli_root: Path) -> None: + # issue #170: a denylist that names no GMS package leaves the GMS row + # unenrolled → ``skip`` (the user opted out), never a phantom ``fail``. + runner.invoke(cli.app, ["create", "alpha"]) + root = registry.instance_path("alpha") + (root / "beetroot.yaml").write_text( + "api_version: 8\n" + "android:\n version: 14\n" + "magisk:\n" + " denylist:\n" + " - com.example.other\n" + ) + with patch("subprocess.run", side_effect=_healthy_subprocess): + result = runner.invoke(cli.app, ["doctor", "alpha"]) + assert result.exit_code == 0, (result.stdout, result.stderr) + assert "magisk.denylist.com.google.android.gms: skip" in result.stdout + def test_zygisk_disabled_exits_with_fail_count(self, cli_root: Path) -> None: runner.invoke(cli.app, ["create", "alpha"]) diff --git a/tests/test_magisk_config_helper.py b/tests/test_magisk_config_helper.py index 5f33edb..cfab8dd 100644 --- a/tests/test_magisk_config_helper.py +++ b/tests/test_magisk_config_helper.py @@ -100,16 +100,65 @@ def _run_helper( def test_default_denylist_enrols_gms_pair(tmp_path: Path) -> None: + # issue #170: the default denylist enrols the GMS main process plus its + # ``.unstable`` DroidGuard process, both under the REAL package + # ``com.google.android.gms``. The helper must split ``package/process`` and + # NEVER copy ``com.google.android.gms.unstable`` into package_name (col 1), + # where it matches no installed package and vanilla Magisk hides nothing. code, _out, queries = _run_helper( tmp_path, env={ - "BEETROOT_DENYLIST_PACKAGES": ("com.google.android.gms,com.google.android.gms.unstable") + "BEETROOT_DENYLIST_PACKAGES": ( + "com.google.android.gms," + "com.google.android.gms/com.google.android.gms.unstable" + ) }, ) assert code == 0 gms_inserts = [q for q in queries if "INSERT OR IGNORE INTO denylist" in q] - assert any("com.google.android.gms'" in q for q in gms_inserts) - assert any("com.google.android.gms.unstable" in q for q in gms_inserts) + # The plain package enrols both columns as the package itself. + assert sum( + 1 for q in gms_inserts if "('com.google.android.gms', 'com.google.android.gms')" in q + ) == 1 + # The DroidGuard entry: real package in package_name, unstable in process. + assert sum( + 1 + for q in gms_inserts + if "('com.google.android.gms', 'com.google.android.gms.unstable')" in q + ) == 1 + # Regression guard: the .unstable string must NEVER land in package_name. + for q in gms_inserts: + assert "('com.google.android.gms.unstable'," not in q, ( + f"DroidGuard process wrongly enrolled as a package_name: {q!r}" + ) + + +def test_no_slash_entry_copies_package_into_both_columns(tmp_path: Path) -> None: + # An entry with no ``/`` has the process default to the package (the + # historical single-process behaviour), so both columns carry the package. + code, _out, queries = _run_helper( + tmp_path, + env={"BEETROOT_DENYLIST_PACKAGES": "com.example.app"}, + ) + assert code == 0 + inserts = [q for q in queries if "INSERT OR IGNORE INTO denylist" in q] + assert sum(1 for q in inserts if "('com.example.app', 'com.example.app')" in q) == 1 + + +def test_slash_entry_splits_package_and_process(tmp_path: Path) -> None: + # A ``package/process`` entry splits on the first slash: the package goes + # into package_name, the process into process — never the whole entry. + code, _out, queries = _run_helper( + tmp_path, + env={"BEETROOT_DENYLIST_PACKAGES": "com.pkg/com.pkg.proc"}, + ) + assert code == 0 + inserts = [q for q in queries if "INSERT OR IGNORE INTO denylist" in q] + assert sum(1 for q in inserts if "('com.pkg', 'com.pkg.proc')" in q) == 1 + for q in inserts: + assert "com.pkg/com.pkg.proc" not in q, ( + f"unsplit package/process entry leaked into the INSERT: {q!r}" + ) def test_custom_denylist_csv_parsed(tmp_path: Path) -> None: diff --git a/tests/test_port_collisions.py b/tests/test_port_collisions.py index f1bbb88..9147529 100644 --- a/tests/test_port_collisions.py +++ b/tests/test_port_collisions.py @@ -210,7 +210,7 @@ def test_two_instances_at_unrelated_paths_each_get_distinct_env( b"DISPLAY_FPS=3\n" b"DISPLAY_GPU=guest\n" b"BEETROOT_DENYLIST_PACKAGES=com.google.android.gms," - b"com.google.android.gms.unstable\n" + b"com.google.android.gms/com.google.android.gms.unstable\n" b"BEETROOT_MAGISK_DB=/data/adb/magisk.db\n" b"BEETROOT_MODULES_DIR=/data/adb/modules_update\n" b"BEETROOT_FRIDA_BIN=/data/local/tmp/frida-server\n" @@ -227,7 +227,7 @@ def test_two_instances_at_unrelated_paths_each_get_distinct_env( b"DISPLAY_FPS=3\n" b"DISPLAY_GPU=guest\n" b"BEETROOT_DENYLIST_PACKAGES=com.google.android.gms," - b"com.google.android.gms.unstable\n" + b"com.google.android.gms/com.google.android.gms.unstable\n" b"BEETROOT_MAGISK_DB=/data/adb/magisk.db\n" b"BEETROOT_MODULES_DIR=/data/adb/modules_update\n" b"BEETROOT_FRIDA_BIN=/data/local/tmp/frida-server\n" diff --git a/tests/test_property_render_env.py b/tests/test_property_render_env.py index 62382cb..cc55d51 100644 --- a/tests/test_property_render_env.py +++ b/tests/test_property_render_env.py @@ -116,3 +116,23 @@ def test_render_env_emits_required_keys(cfg: config.InstanceConfig) -> None: } missing = required - keys assert not missing, f"render_env missing required keys: {missing}" + + +def test_render_env_denylist_slash_entry_is_shell_safe() -> None: + """The ``package/process`` denylist entry survives as a shell-safe value. + + issue #170: the default denylist now carries a ``package/process`` entry + whose ``/`` rides through the CSV join into ``BEETROOT_DENYLIST_PACKAGES``. + ``/`` is not a shell-injection vector, so the invariant above holds; this + focused case pins that the slash-bearing entry is actually emitted (not + dropped) and stays free of the forbidden characters. + """ + cfg = config.InstanceConfig() + out = config.render_env("alpha", cfg) + denylist_line = next( + line for line in out.splitlines() if line.startswith("BEETROOT_DENYLIST_PACKAGES=") + ) + value = denylist_line.partition("=")[2] + assert "com.google.android.gms/com.google.android.gms.unstable" in value + for bad in _INJECTION_CHARS: + assert bad not in value