Merge pull request #685 from termux/fix/termux-env

Revise environment variables and bindings for different image types

CLAUDE.md

@@ -102,9 +102,15 @@ Distribution type is detected at login:
 (not dir — proot may materialise the bind-mount target during a
 concurrent session) ⇒ `termux`; else `normal`. `termux`: no
 `--link2symlink`, no `--change-id`; hardcoded HOME/PATH/PREFIX/TMPDIR;
-Android system bindings always on; Termux prefix not bound (guest has
-its own at the same path). **Cross-arch is refused** — host and guest
-share `TERMUX_PREFIX`, so host binaries would shadow the container's.
+image Env + Android host vars applied like `normal`; Android system
+bindings + shared storage + Dalvik/ART caches (`/data/app`,
+`/data/dalvik-cache`, `/data/misc/apexdata/com.android.art/dalvik-cache`)
+on when non-isolated (off when isolated/minimal); the host's Termux app
+dirs under `/data/data/com.termux` are **never** bound (the guest ships
+its own, so only its `cache` dir is created inside the rootfs); Termux
+prefix not bound (guest has its own at the same path). **Cross-arch is
+refused** — host and guest share `TERMUX_PREFIX`, so host binaries
+would shadow the container's.
 
 ## Commands and locks
 
@@ -207,13 +213,18 @@ PUT (no chunked, no cross-repo mount, no multi-arch index). 401/403 ⇒
 ## Login env (`commands/login/`)
 
 `child_env` is built explicitly and passed to `os.execvpe` — no
-`env -i` wrapper, host env is **not** propagated. `normal`-type,
-non-minimal precedence (later wins): PATH/MOZ_FAKE_NO_SANDBOX/
-PULSE_SERVER baseline → image `Env` (filtered by `IMAGE_ENV_BLOCKED`:
-Android vars, MOZ/PULSE, TERM/COLORTERM) → Android system vars (Termux
-+ non-isolated) → user `--env` → HOME/USER/TERM/COLORTERM. PATH is not
-blocked but `TERMUX_PREFIX/bin` is deduped + appended after image Env
-(non-isolated).
+`env -i` wrapper, host env is **not** propagated. `normal`-type
+precedence (later wins): PATH/MOZ_FAKE_NO_SANDBOX/PULSE_SERVER baseline
+(non-minimal only) → image `Env` (filtered by `IMAGE_ENV_BLOCKED`:
+Android vars, MOZ/PULSE, TERM/COLORTERM) → Android host vars
+(`ANDROID_HOST_ENV_VARS`, Termux + neither isolated nor minimal) →
+user `--env` → HOME/USER (non-minimal only) → TERM/COLORTERM. Image
+`Env` and `--env` apply in **every** mode (isolated and minimal
+included); only the Android host vars are gated on the default mode.
+On non-Termux hosts no host vars are inherited. PATH is not blocked but
+`TERMUX_PREFIX/bin` is deduped + appended after image Env (non-isolated,
+non-minimal). `termux`-type uses the same image-Env + Android-host-var
+logic on top of its hardcoded HOME/PATH/PREFIX/TMPDIR baseline.
 
 `inject_termux_profile()` writes `/etc/profile.d/termux-profile.sh` so
 `su - other` doesn't drop the proot-distro-set vars: POSIX case-guard
@@ -224,10 +235,11 @@ against the identifier regex `^[A-Za-z_][A-Za-z0-9_]*$`; anything that
 would otherwise corrupt the sourced script (spaces, `;`, quotes …) is
 dropped silently. Legacy `termux-prefix.sh` unlinked first.
 
-`minimal` clears almost everything: only `--env` + `TERM` (default
-`xterm-256color`) + inherited `COLORTERM`. `PROOT_L2S_DIR` pinned to
-`rootfs/.l2s` (created upfront) for `normal` on Termux so concurrent
-sessions agree. `LD_PRELOAD` stripped before exec.
+`minimal` clears almost everything: image `Env` + `--env` + `TERM`
+(default `xterm-256color`) + inherited `COLORTERM`; no baseline PATH,
+no MOZ/PULSE, no Android host vars, no HOME/USER. `PROOT_L2S_DIR`
+pinned to `rootfs/.l2s` (created upfront) for `normal` on Termux so
+concurrent sessions agree. `LD_PRELOAD` stripped before exec.
 
 ## Run / build
 

proot_distro/commands/login/__init__.py

@@ -55,7 +55,8 @@ from proot_distro.names import require_valid_name
 from proot_distro.paths import container_dir, container_rootfs
 
 from proot_distro.commands.login.env import (
-    IMAGE_ENV_BLOCKED, inject_termux_profile, read_manifest_env,
+    ANDROID_HOST_ENV_VARS, IMAGE_ENV_BLOCKED,
+    inject_termux_profile, read_manifest_env,
 )
 from proot_distro.commands.login.migrate import migrate_legacy_rootfs
 from proot_distro.commands.login.passwd import (
@@ -191,15 +192,30 @@ def _resolve_login_user(rootfs: str, container_name: str, user_arg: str) -> dict
     }
 
 
-def _build_termux_env(extra_env, minimal):
+def _build_termux_env(container_path, extra_env, minimal, isolated):
     """Env dict for termux-type containers."""
     env: dict = {}
-    termux_home_inner = TERMUX_HOME
     if not minimal:
-        env["HOME"] = termux_home_inner
+        env["HOME"] = TERMUX_HOME
         env["PATH"] = f"{TERMUX_PREFIX}/bin"
         env["PREFIX"] = TERMUX_PREFIX
         env["TMPDIR"] = f"{TERMUX_PREFIX}/tmp"
+
+    # Image manifest Env applies in every mode (including isolated and
+    # minimal). IMAGE_ENV_BLOCKED still guards host-controlled vars.
+    for entry in read_manifest_env(container_path):
+        key, _, val = entry.partition("=")
+        if key and key not in IMAGE_ENV_BLOCKED:
+            env[key] = val
+
+    # Android system vars are inherited from the host only in the default
+    # mode; isolated and minimal sessions keep just the image's values.
+    if IS_TERMUX and not isolated and not minimal:
+        for var in ANDROID_HOST_ENV_VARS:
+            val = os.environ.get(var, "")
+            if val:
+                env[var] = val
+
     for entry in extra_env:
         key, _, val = entry.partition("=")
         if key:
@@ -218,34 +234,25 @@ def _build_normal_env(container_path, login_user, login_home,
     """Env dict for normal-type containers."""
     env: dict = {}
 
-    if minimal:
-        for entry in extra_env:
-            key, _, val = entry.partition("=")
-            if key:
-                env[key] = val
-        env["TERM"] = os.environ.get("TERM", "") or "xterm-256color"
-        host_colorterm = os.environ.get("COLORTERM", "")
-        if host_colorterm:
-            env["COLORTERM"] = host_colorterm
-        return env
-
-    env["PATH"] = DEFAULT_PATH_ENV
-    if IS_TERMUX:
-        env["MOZ_FAKE_NO_SANDBOX"] = "1"
-        env["PULSE_SERVER"] = "127.0.0.1"
+    # The Termux baseline (PATH, browser/audio hints) is part of the
+    # default container environment, not the stripped-down minimal one.
+    if not minimal:
+        env["PATH"] = DEFAULT_PATH_ENV
+        if IS_TERMUX:
+            env["MOZ_FAKE_NO_SANDBOX"] = "1"
+            env["PULSE_SERVER"] = "127.0.0.1"
 
+    # Image manifest Env applies in every mode (including isolated and
+    # minimal). IMAGE_ENV_BLOCKED still guards host-controlled vars.
     for entry in read_manifest_env(container_path):
         key, _, val = entry.partition("=")
         if key and key not in IMAGE_ENV_BLOCKED:
             env[key] = val
 
-    if IS_TERMUX and not isolated:
-        for var in (
-            "ANDROID_ART_ROOT", "ANDROID_DATA", "ANDROID_I18N_ROOT",
-            "ANDROID_ROOT", "ANDROID_RUNTIME_ROOT",
-            "ANDROID_TZDATA_ROOT",
-            "BOOTCLASSPATH", "DEX2OATBOOTCLASSPATH", "EXTERNAL_STORAGE",
-        ):
+    # Android system vars are inherited from the host only in the default
+    # mode; isolated and minimal sessions keep just the image's values.
+    if IS_TERMUX and not isolated and not minimal:
+        for var in ANDROID_HOST_ENV_VARS:
             val = os.environ.get(var, "")
             if val:
                 env[var] = val
@@ -255,8 +262,9 @@ def _build_normal_env(container_path, login_user, login_home,
         if key:
             env[key] = val
 
-    env["HOME"] = login_home
-    env["USER"] = login_user
+    if not minimal:
+        env["HOME"] = login_home
+        env["USER"] = login_user
     env["TERM"] = os.environ.get("TERM", "") or "xterm-256color"
     host_colorterm = os.environ.get("COLORTERM", "")
     if host_colorterm:
@@ -332,7 +340,9 @@ def _command_login_inner(container_name: str, args) -> None:
     if dist_type == "termux":
         if not login_wd:
             login_wd = TERMUX_HOME
-        child_env = _build_termux_env(extra_env, minimal)
+        child_env = _build_termux_env(
+            container_path, extra_env, minimal, isolated
+        )
 
         if run_inner is not None:
             inner = run_inner

proot_distro/commands/login/env.py

@@ -40,13 +40,24 @@ from proot_distro.constants import TERMUX_PREFIX
 _VALID_ENV_KEY_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
 
 
+# Android system environment variables harvested from the launching
+# Termux process so the guest's ART/dalvik tooling can locate the
+# runtime. Inherited from the host only in the default mode (neither
+# isolated nor minimal); isolated and minimal sessions keep just the
+# image manifest's own values. Shared by both the normal-type and
+# termux-type env builders.
+ANDROID_HOST_ENV_VARS = (
+    "ANDROID_ART_ROOT", "ANDROID_DATA", "ANDROID_I18N_ROOT",
+    "ANDROID_ROOT", "ANDROID_RUNTIME_ROOT", "ANDROID_TZDATA_ROOT",
+    "BOOTCLASSPATH", "DEX2OATBOOTCLASSPATH", "EXTERNAL_STORAGE",
+)
+
+
 # Vars the image Env must not override. Some are proot-distro-defined
 # values; others are host-inherited terminal vars that must remain
 # under the launcher's control regardless of image configuration.
 IMAGE_ENV_BLOCKED = frozenset({
-    "ANDROID_ART_ROOT", "ANDROID_DATA", "ANDROID_I18N_ROOT",
-    "ANDROID_ROOT", "ANDROID_RUNTIME_ROOT", "ANDROID_TZDATA_ROOT",
-    "BOOTCLASSPATH", "DEX2OATBOOTCLASSPATH", "EXTERNAL_STORAGE",
+    *ANDROID_HOST_ENV_VARS,
     "MOZ_FAKE_NO_SANDBOX", "PULSE_SERVER",
     "TERM", "COLORTERM",
 })

proot_distro/commands/login/proot_cmd.py

@@ -142,14 +142,33 @@ def _add_non_minimal_binds(
         _add_termux_dev_binds(args, rootfs)
 
     if IS_TERMUX and not isolated:
-        _add_android_data_binds(args, rootfs, dist_type)
+        # Dalvik/ART caches and shared storage are host-domain Android
+        # paths bound for both distro types in the default mode.
+        _add_dalvik_cache_binds(args)
         args += storage_bindings()
+        # The Termux app's private dirs (apps, cache, $HOME) are bound
+        # only for normal-type containers. A termux-type guest ships its
+        # own /data/data/com.termux and must never see the host's.
+        if dist_type != "termux":
+            _add_termux_app_binds(args)
 
-    if IS_TERMUX and (dist_type == "termux" or not isolated or need_emu):
+    # Android system directories (/apex, /system, /vendor, …). Bound for
+    # normal-type when not isolated, or when emulating (the QEMU loader
+    # needs them), and for termux-type only when not isolated. Fully
+    # isolated sessions of either type get no host directories.
+    if IS_TERMUX and (not isolated or need_emu):
         args += system_bindings()
         if dist_type != "termux":
             args.append(f"--bind={TERMUX_PREFIX}")
 
+    # A termux-type guest still needs its own cache dir to exist; create
+    # it inside the rootfs (never bound from the host).
+    if IS_TERMUX and dist_type == "termux" and not isolated:
+        os.makedirs(
+            os.path.join(rootfs, "data", "data", TERMUX_APP_PACKAGE, "cache"),
+            exist_ok=True,
+        )
+
     if use_shared_home:
         if dist_type == "termux":
             args.append(f"--bind={TERMUX_HOME}:{TERMUX_HOME}")
@@ -186,8 +205,13 @@ def _add_termux_dev_binds(args, rootfs):
     args.append(f"--bind={tmp_dir}:/dev/shm")
 
 
-def _add_android_data_binds(args, rootfs, dist_type):
-    """Bind Android dalvik caches + Termux app cache + Termux home."""
+def _add_dalvik_cache_binds(args):
+    """Bind the host's Dalvik/ART caches (both distro types).
+
+    These are Android-system caches, not the Termux app's private data,
+    so both normal-type and termux-type guests get them. Each dir must
+    carry the world-execute bit to be traversable from the guest user.
+    """
     for data_dir in (
         "/data/app", "/data/dalvik-cache",
         "/data/misc/apexdata/com.android.art/dalvik-cache",
@@ -198,18 +222,19 @@ def _add_android_data_binds(args, rootfs, dist_type):
         if mode in ("1", "5", "7"):
             args.append(f"--bind={data_dir}")
 
+
+def _add_termux_app_binds(args):
+    """Bind the Termux app's private dirs (apps, cache, $HOME).
+
+    Normal-type containers only: a termux-type guest must not see the
+    host's /data/data/com.termux, so this is never called for it.
+    """
     apps_dir = f"/data/data/{TERMUX_APP_PACKAGE}/files/apps"
     if os.path.isdir(apps_dir):
         args.append(f"--bind={apps_dir}")
 
-    if dist_type != "termux":
-        args.append(f"--bind=/data/data/{TERMUX_APP_PACKAGE}/cache")
-        args.append(f"--bind={TERMUX_HOME}")
-    else:
-        os.makedirs(
-            os.path.join(rootfs, "data", "data", TERMUX_APP_PACKAGE, "cache"),
-            exist_ok=True,
-        )
+    args.append(f"--bind=/data/data/{TERMUX_APP_PACKAGE}/cache")
+    args.append(f"--bind={TERMUX_HOME}")
 
 
 def _add_custom_binds(args, custom_binds):

tests/integration/test_login_get_proot_cmd.py

@@ -137,3 +137,102 @@ def test_termux_branch_adds_proot_extensions(tmp_path, monkeypatch):
     assert "--link2symlink" in args
     assert "--kill-on-exit" in args
     assert "--change-id=0:0" in args
+
+
+def _termux_host_proot_args(tmp_path, monkeypatch, **over):
+    """build_proot_args under a faked Termux host with sentinel bind helpers.
+
+    storage/system bindings are stubbed to fixed sentinels so the dispatch
+    logic can be asserted independently of the test host's real /system,
+    /storage, etc. The dalvik-cache and Termux-app bind helpers are
+    replaced by recorders that append a label to the returned list.
+    """
+    monkeypatch.setattr(proot_cmd, "IS_TERMUX", True)
+    monkeypatch.setattr(proot_cmd, "system_bindings", lambda: ["--bind=/system"])
+    monkeypatch.setattr(proot_cmd, "storage_bindings",
+                        lambda: ["--bind=/storage"])
+    calls = []
+    monkeypatch.setattr(proot_cmd, "_add_dalvik_cache_binds",
+                        lambda args: calls.append("dalvik"))
+    monkeypatch.setattr(proot_cmd, "_add_termux_app_binds",
+                        lambda args: calls.append("termux_app"))
+    rootfs = tmp_path / "rootfs"
+    rootfs.mkdir()
+    base = dict(
+        proot_bin="proot", rootfs=str(rootfs), login_wd="/",
+        login_uid=None, login_gid=None, login_home=None,
+        emu_args=[], need_emu=False, target_arch=HOST_ARCH,
+        hostname="localhost", kernel_release="6.0-test",
+        dist_type="termux", minimal=False, isolated=False,
+        no_link2symlink=False, no_sysvipc=False, no_kill_on_exit=False,
+        use_shared_home=False, shared_tmp=False, shared_x11=False,
+        custom_binds=[], redirect_ports=False, inner=["/bin/login"],
+    )
+    base.update(over)
+    args = proot_cmd.build_proot_args(**base)
+    return args, calls
+
+
+def test_termux_type_binds_dalvik_storage_system(tmp_path, monkeypatch):
+    # Termux-type, non-isolated: dalvik caches, shared storage, and Android
+    # system dirs are bound, but the host's /data/data/com.termux app dirs
+    # and the Termux prefix bridge are not.
+    args, calls = _termux_host_proot_args(tmp_path, monkeypatch)
+    assert "--bind=/system" in args
+    assert "--bind=/storage" in args
+    assert calls == ["dalvik"]
+    assert not any(a.startswith(f"--bind={TERMUX_PREFIX}") for a in args)
+
+
+def test_termux_type_isolated_no_host_dirs(tmp_path, monkeypatch):
+    # Termux-type, isolated: no host directories at all.
+    args, calls = _termux_host_proot_args(
+        tmp_path, monkeypatch, isolated=True,
+    )
+    assert "--bind=/system" not in args
+    assert "--bind=/storage" not in args
+    assert calls == []
+
+
+def test_normal_type_binds_android_data_and_storage(tmp_path, monkeypatch):
+    # Normal-type, non-isolated: dalvik caches, Termux app dirs, shared
+    # storage, system dirs, and the Termux prefix bridge are all bound.
+    args, calls = _termux_host_proot_args(
+        tmp_path, monkeypatch, dist_type="normal",
+        login_uid="0", login_gid="0", login_home="/root",
+        inner=["/bin/sh", "-l"],
+    )
+    assert "--bind=/system" in args
+    assert "--bind=/storage" in args
+    assert calls == ["dalvik", "termux_app"]
+    assert f"--bind={TERMUX_PREFIX}" in args
+
+
+def test_minimal_login_keeps_image_env(builders, capsys):
+    # Minimal mode no longer discards the image manifest's Env entries.
+    builders.make_container("box", arch=HOST_ARCH, manifest={
+        "image_config": {"config": {"Env": ["FOO=frommanifest"]}},
+    })
+    with pytest.raises(SystemExit) as exc:
+        command_login(_login_args("box", minimal=True))
+    assert exc.value.code == 0
+    out = capsys.readouterr().out
+    assert "FOO=frommanifest" in out
+
+
+def test_termux_type_login_applies_image_env(builders, capsys):
+    # Termux-type containers now apply the image manifest's Env entries.
+    builders.make_container("tbox", arch=HOST_ARCH, manifest={
+        "image_config": {"config": {"Env": ["FOO=frommanifest"]}},
+    })
+    login_bin = os.path.join(
+        container_rootfs("tbox") + TERMUX_PREFIX, "bin", "login"
+    )
+    os.makedirs(os.path.dirname(login_bin), exist_ok=True)
+    with open(login_bin, "w") as fh:
+        fh.write("#!/bin/sh\n")
+    with pytest.raises(SystemExit) as exc:
+        command_login(_login_args("tbox"))
+    assert exc.value.code == 0
+    out = capsys.readouterr().out
+    assert "FOO=frommanifest" in out