From ed30bef7c73d9c3964e0989f58c284ca19621a12 Mon Sep 17 00:00:00 2001 From: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Date: Sat, 15 Aug 2026 11:00:49 -0700 Subject: [PATCH] fix: isolate iOS native review environment Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Jude Edwards --- tools/native-review/README.md | 9 ++++-- tools/native-review/ios_review.py | 15 ++++++++-- tools/native-review/tests/test_ios_review.py | 29 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/tools/native-review/README.md b/tools/native-review/README.md index 5e13377bd23..3a5dc84008b 100644 --- a/tools/native-review/README.md +++ b/tools/native-review/README.md @@ -25,9 +25,12 @@ keys never enter the reviewed process. Production bundle IDs, keyring services, and remote relays fail closed. iOS runs erase the selected simulator before and after the journey. Do not point -the runner at a simulator containing state you need. The review-only environment -suppresses the launch notification prompt because Flutter cannot actuate -SpringBoard; normal app launches retain production permission behavior. +the runner at a simulator containing state you need. The Flutter child receives +only an allowlist of host tool settings plus review-only flags; inherited tokens, +production keys, and cloud credentials do not enter the reviewed process. The +review-only environment suppresses the launch notification prompt because +Flutter cannot actuate SpringBoard; normal app launches retain production +permission behavior. These controls protect reviewer state from accidents. They are **not** containment for hostile code. Use a dedicated macOS user, disposable simulator, diff --git a/tools/native-review/ios_review.py b/tools/native-review/ios_review.py index 84fac1f9683..b56e8994fbe 100755 --- a/tools/native-review/ios_review.py +++ b/tools/native-review/ios_review.py @@ -19,6 +19,10 @@ ROOT = pathlib.Path(__file__).resolve().parents[2] DEFAULT_TEST = ROOT / "mobile/integration_test/native_review_pairing_test.dart" +FLUTTER_ENV_ALLOWLIST = { + "PATH", "HOME", "TMPDIR", "LANG", "LC_ALL", "SHELL", "USER", "LOGNAME", + "TERM", "__CF_USER_TEXT_ENCODING", "DEVELOPER_DIR", +} class ReviewError(RuntimeError): @@ -54,6 +58,13 @@ def provenance() -> dict[str, Any]: return {"head_sha": git("rev-parse", "HEAD"), "dirty": bool(status), "status": status.splitlines()} +def flutter_environment() -> dict[str, str]: + """Return only host settings required to launch Flutter and Xcode tooling.""" + env = {key: value for key, value in os.environ.items() if key in FLUTTER_ENV_ALLOWLIST} + env.update({"BUZZ_NATIVE_REVIEW": "1", "SIMCTL_CHILD_BUZZ_NATIVE_REVIEW": "1"}) + return env + + def wait_for_recording(recorder: subprocess.Popen[str], timeout_seconds: float = 15) -> None: if recorder.stderr is None: raise ReviewError("simulator recorder has no diagnostic stream") @@ -101,12 +112,10 @@ def run_review(test: pathlib.Path, device_name: str, output_root: pathlib.Path) stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, text=True) wait_for_recording(recorder) receipt["artifacts"]["video"] = "video.mp4" - flutter_env = {**os.environ, "BUZZ_NATIVE_REVIEW": "1", - "SIMCTL_CHILD_BUZZ_NATIVE_REVIEW": "1"} result = run(["flutter", "drive", "--driver", "test_driver/integration_test.dart", "--target", str(test.relative_to(ROOT / "mobile")), "-d", udid, "--keep-app-running", "--dart-define=BUZZ_NATIVE_REVIEW=true"], - cwd=ROOT / "mobile", check=False, env=flutter_env) + cwd=ROOT / "mobile", check=False, env=flutter_environment()) (run_dir / "flutter.log").write_text(result.stdout + result.stderr) receipt["artifacts"]["log"] = "flutter.log" screenshot = run_dir / "final.png" diff --git a/tools/native-review/tests/test_ios_review.py b/tools/native-review/tests/test_ios_review.py index 969d14fd4d6..4eee9e2ff6e 100644 --- a/tools/native-review/tests/test_ios_review.py +++ b/tools/native-review/tests/test_ios_review.py @@ -35,6 +35,30 @@ def kill(self): class IosReviewTests(unittest.TestCase): + def test_flutter_environment_allowlists_tool_settings_and_scrubs_secrets(self): + inherited = { + "PATH": "/repo/bin:/usr/bin", + "HOME": "/Users/reviewer", + "TMPDIR": "/tmp/review", + "DEVELOPER_DIR": "/Applications/Xcode.app/Contents/Developer", + "BUZZ_PRIVATE_KEY": "production-key", + "BUZZ_AUTH_TAG": "production-auth", + "AWS_SECRET_ACCESS_KEY": "cloud-secret", + "GOOGLE_APPLICATION_CREDENTIALS": "/secrets/google.json", + "GH_TOKEN": "github-token", + } + with mock.patch.dict(ios_review.os.environ, inherited, clear=True): + child = ios_review.flutter_environment() + + self.assertEqual(child, { + "PATH": inherited["PATH"], + "HOME": inherited["HOME"], + "TMPDIR": inherited["TMPDIR"], + "DEVELOPER_DIR": inherited["DEVELOPER_DIR"], + "BUZZ_NATIVE_REVIEW": "1", + "SIMCTL_CHILD_BUZZ_NATIVE_REVIEW": "1", + }) + def test_device_selection_prefers_latest_runtime_and_records_it(self): payload = {"devices": { "com.apple.CoreSimulator.SimRuntime.iOS-18-5": [ @@ -59,10 +83,12 @@ def test_missing_device_fails_clearly(self): def test_flutter_failure_finalizes_recording_writes_receipt_and_cleans_device(self): recorder = FakeRecorder() commands = [] + flutter_environments = [] def fake_run(command, **kwargs): commands.append(command) if command[:2] == ["flutter", "drive"]: + flutter_environments.append(kwargs["env"]) return subprocess.CompletedProcess(command, 1, "journey failed", "diagnostic") if "screenshot" in command: pathlib.Path(command[-1]).write_bytes(b"png") @@ -89,6 +115,9 @@ def fake_run(command, **kwargs): self.assertEqual(receipt["artifacts"], { "video": "video.mp4", "log": "flutter.log", "screenshot": "final.png" }) + self.assertEqual(len(flutter_environments), 1) + self.assertEqual(flutter_environments[0]["BUZZ_NATIVE_REVIEW"], "1") + self.assertEqual(flutter_environments[0]["SIMCTL_CHILD_BUZZ_NATIVE_REVIEW"], "1") self.assertIn(["xcrun", "simctl", "shutdown", "device"], commands) self.assertIn(["xcrun", "simctl", "erase", "device"], commands)