From d3f7224b2480a961e7ec5f2094134f58cf23d052 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Tue, 7 Jul 2026 13:04:08 -0600 Subject: [PATCH 1/2] MOB-39: plugins can contribute AndroidManifest components + res files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two optional android: manifest keys: - manifest_application_snippets: XML fragments spliced into the app's (a //), idempotent per android:name. - res_files: plugin-relative paths copied into the app res/ tree at their derived res// destination. Closes the gap that forced a plugin needing a manifest component + resource (e.g. mob_nfc's HCE HostApduService + apduservice.xml) to make it a manual host_requirement. - Merge: android_manifest_snippets/1 + android_res_files/1 gatherers. - Validator: both classified in the conflict surface — duplicate component names / res destinations across plugins are build errors. - Manifest: schema validation (lists of strings; res_files need a res segment). - NativeBuild: splice components before + copy res files, both ledger-pruned like bridge_kt; build-time res-dest collision guard. Tests across merge/manifest/conflict_surface/native_build (307 in those files). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 15 +++ lib/mob_dev/native_build.ex | 120 +++++++++++++++++- lib/mob_dev/plugin/manifest.ex | 56 ++++++++ lib/mob_dev/plugin/merge.ex | 52 ++++++++ lib/mob_dev/plugin/validator.ex | 35 ++++- test/mob_dev/native_build_test.exs | 45 +++++++ test/mob_dev/plugin/conflict_surface_test.exs | 25 ++++ test/mob_dev/plugin/manifest_test.exs | 42 ++++++ test/mob_dev/plugin/merge_test.exs | 67 ++++++++++ 9 files changed, 452 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca3f827..4beb46f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,21 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ## [Unreleased] +### Added +- **Plugins can contribute AndroidManifest `` components and `res/` + files.** Two new optional `android:` manifest keys — + `manifest_application_snippets` (XML fragments spliced into the app's + `` block, idempotent per `android:name`) and `res_files` + (plugin-relative paths copied into the app `res/` tree at their derived + `res//` destination). This closes the gap that forced plugins + needing a ``/``/`` + resource (e.g. `mob_nfc`'s + HCE `HostApduService` + `apduservice.xml`) to make it a manual + `host_requirement`. New `MobDev.Plugin.Merge.android_manifest_snippets/1` + + `android_res_files/1` gatherers (classified in the cross-plugin conflict + surface — duplicate component names / res destinations are build errors), + manifest-schema validation, and `NativeBuild` splice + copy (ledger-pruned + like `bridge_kt`). (MOB-39) + ### Fixed - **`mix mob.new_plugin` no longer scaffolds plugins pinned to the abandoned `mob ~> 0.6`.** `MobDev.Plugin.Scaffold` hard-coded `{:mob, "~> 0.6"}` in the diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index 5759687..f462be6 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -162,6 +162,7 @@ defmodule MobDev.NativeBuild do :ok <- apply_plugin_android_manifest!(), :ok <- apply_plugin_gradle_deps!(), :ok <- apply_plugin_android_kotlin!(), + :ok <- apply_plugin_android_res!(), :ok <- apply_fonts_to_android!(), :ok <- gradle_assemble(), :ok <- adb_install_all(apk, bundle_id, device_id), @@ -4470,19 +4471,25 @@ defmodule MobDev.NativeBuild do # Layer 2. MobDev.Plugin.Validator.raise_on_capability_drift!(activated) + permissions = MobDev.Plugin.Merge.android_permissions(activated) + snippets = for s <- MobDev.Plugin.Merge.android_manifest_snippets(activated), do: s.snippet + case File.read(@android_manifest_path) do {:error, :enoent} -> - if MobDev.Plugin.Merge.android_permissions(activated) != [] do + if permissions != [] or snippets != [] do IO.puts( - " [plugin android] #{@android_manifest_path} not found — skipping plugin permissions." + " [plugin android] #{@android_manifest_path} not found — skipping plugin " <> + "permissions + manifest components." ) end :ok {:ok, content} -> - permissions = MobDev.Plugin.Merge.android_permissions(activated) - patched = merge_android_permissions(content, permissions) + patched = + content + |> merge_android_permissions(permissions) + |> merge_android_manifest_components(snippets) if patched != content, do: File.write!(@android_manifest_path, patched) @@ -4814,6 +4821,60 @@ defmodule MobDev.NativeBuild do :ok end + @android_res_root "android/app/src/main" + + # Copies each activated plugin's `android.res_files` into the app's `res/` + # tree (at its declared `res//` destination), so a manifest + # component's `@xml/…` reference resolves at build time. Ledger-pruned like + # bridge_kt (a res file left by a since-removed plugin is deleted). Raises on + # two plugins targeting the same destination with different sources — the + # cross-plugin validator catches this at activation, this is the build-time + # backstop. No-op (with a notice) when the res root is missing. + defp apply_plugin_android_res! do + res_files = MobDev.Plugin.Merge.android_res_files(MobDev.Plugin.activated()) + + cond do + res_files == [] -> + :ok + + not File.dir?(@android_res_root) -> + IO.puts(" [plugin android] #{@android_res_root} not found — skipping plugin res files.") + :ok + + true -> + raise_on_res_dest_collision!(res_files) + + written = + for %{src: src, dest: dest} <- res_files do + target = Path.join(@android_res_root, dest) + File.mkdir_p!(Path.dirname(target)) + File.cp!(src, target) + IO.puts(" ✓ android res → #{Path.relative_to_cwd(target)}") + target + end + + __prune_plugin_artifacts__(:android_res, written) + :ok + end + end + + defp raise_on_res_dest_collision!(res_files) do + res_files + |> Enum.group_by(& &1.dest, & &1.src) + |> Enum.each(fn {dest, srcs} -> + case Enum.uniq(srcs) do + [_single] -> + :ok + + many -> + Mix.raise( + "Android res collision: multiple plugins target #{dest}:\n " <> + Enum.join(many, "\n ") <> "\nRename one so plugin res files don't clash." + ) + end + end) + end + # Copies each activated plugin's `bridge_kt` into the app's Kotlin sourceSet # (at its own package path, read from the file's `package` line) so Gradle # compiles it, and (re)generates `io.mob.plugin.MobPluginBootstrap` whose @@ -5050,6 +5111,57 @@ defmodule MobDev.NativeBuild do @spec __merge_gradle_deps__(String.t(), [String.t()]) :: String.t() def __merge_gradle_deps__(content, deps), do: merge_gradle_deps(content, deps) + @doc false + @spec __merge_android_manifest_components__(String.t(), [String.t()]) :: String.t() + def __merge_android_manifest_components__(manifest, snippets), + do: merge_android_manifest_components(manifest, snippets) + + # Pure transform: splice each plugin `` snippet (a , + # , …) in just before ``. Idempotent per component: + # skips any snippet whose `android:name` (or, lacking one, whose trimmed body) + # is already present, so re-runs and hand-added copies don't duplicate. Each + # snippet's own indentation is preserved and shifted 8 spaces to sit inside + # . No `` → returns the manifest untouched rather + # than risk corrupting it. + defp merge_android_manifest_components(manifest, []), do: manifest + + defp merge_android_manifest_components(manifest, snippets) when is_binary(manifest) do + missing = Enum.reject(snippets, &manifest_component_present?(manifest, &1)) + + case missing do + [] -> + manifest + + _ -> + block = Enum.map_join(missing, "\n\n", &indent_manifest_snippet/1) + + if String.contains?(manifest, "") do + String.replace(manifest, "", "#{block}\n ", + global: false + ) + else + manifest + end + end + end + + defp manifest_component_present?(manifest, snippet) do + case Regex.run(~r/android:name="([^"]+)"/, snippet) do + [_, name] -> String.contains?(manifest, ~s(android:name="#{name}")) + _ -> String.contains?(manifest, String.trim(snippet)) + end + end + + defp indent_manifest_snippet(snippet) do + snippet + |> String.trim("\n") + |> String.split("\n") + |> Enum.map_join("\n", fn + "" -> "" + line -> " " <> line + end) + end + # Pure transform: insert new `` tags for each permission not # already declared. Insertion point is right after the last existing # `` line; failing that (no permissions declared yet), diff --git a/lib/mob_dev/plugin/manifest.ex b/lib/mob_dev/plugin/manifest.ex index fb563a9..a0e194d 100644 --- a/lib/mob_dev/plugin/manifest.ex +++ b/lib/mob_dev/plugin/manifest.ex @@ -83,6 +83,8 @@ defmodule MobDev.Plugin.Manifest do |> check_mob_version(manifest) |> check_spec_version(manifest) |> check_permissions(manifest) + |> check_android_manifest_snippets(manifest) + |> check_android_res_files(manifest) |> check_nifs(manifest) |> check_screens(manifest) |> check_screens_generator(manifest) @@ -172,6 +174,60 @@ defmodule MobDev.Plugin.Manifest do defp check_permission_entry(errors, other, i), do: ["permissions entry ##{i} must be a map, got: #{inspect(other)}" | errors] + # `android.manifest_application_snippets` (optional) is a list of XML strings + # spliced into the app manifest's `` block (a ``, + # ``, …). Each must be a non-empty string; content is the plugin + # author's responsibility (the native build inserts verbatim). + defp check_android_manifest_snippets(errors, manifest) do + case get_in(manifest, [:android, :manifest_application_snippets]) do + nil -> + errors + + list when is_list(list) -> + if Enum.all?(list, &(is_binary(&1) and &1 != "")), + do: errors, + else: [ + "android.manifest_application_snippets must be a list of non-empty XML strings" + | errors + ] + + other -> + [ + "android.manifest_application_snippets must be a list of XML strings, got: #{inspect(other)}" + | errors + ] + end + end + + # `android.res_files` (optional) is a list of plugin-relative paths copied into + # the app's `res/` tree. Each must be a non-empty string containing a `res` + # path segment (the build derives the `res//` destination from it). + defp check_android_res_files(errors, manifest) do + case get_in(manifest, [:android, :res_files]) do + nil -> + errors + + list when is_list(list) -> + cond do + not Enum.all?(list, &(is_binary(&1) and &1 != "")) -> + ["android.res_files must be a list of non-empty path strings" | errors] + + not Enum.all?(list, &("res" in Path.split(&1))) -> + [ + "android.res_files paths must contain a \"res\" segment (e.g. " <> + "priv/native/android/res/xml/foo.xml) so the build can place them" + | errors + ] + + true -> + errors + end + + other -> + ["android.res_files must be a list of path strings, got: #{inspect(other)}" | errors] + end + end + # `:nifs` is optional. When present, each entry's optional `:platform` (used by # cross-platform plugins that ship a separate iOS + Android source for the same # module) must be `:ios` or `:android`. For single-source C/ObjC/zig NIFs the diff --git a/lib/mob_dev/plugin/merge.ex b/lib/mob_dev/plugin/merge.ex index 160a3c7..d0bde91 100644 --- a/lib/mob_dev/plugin/merge.ex +++ b/lib/mob_dev/plugin/merge.ex @@ -42,6 +42,58 @@ defmodule MobDev.Plugin.Merge do @spec gradle_deps([plugin()]) :: [String.t()] def gradle_deps(plugins), do: collect_uniq(plugins, [:android, :gradle_deps]) + @doc """ + AndroidManifest `` snippets each plugin contributes (a ``, + ``, ``, …), tagged with `:plugin`. `native_build` splices + each into the app manifest's `` block (idempotent on the + component's `android:name`). Ships the manifest half of a component whose class + rides in the plugin's `bridge_kt`; see also `android_res_files/1` for the + `res/` half (e.g. an `apduservice.xml`). + """ + @spec android_manifest_snippets([plugin()]) :: [%{plugin: atom(), snippet: String.t()}] + def android_manifest_snippets(plugins) do + for {_dir, manifest} <- with_manifests(plugins), + snippet <- List.wrap(get_in(manifest, [:android, :manifest_application_snippets])), + is_binary(snippet), + do: %{plugin: manifest[:name], snippet: snippet} + end + + @doc """ + Android `res/` files each plugin contributes, resolved to `{plugin, src, dest}`. + `src` is absolute (against the plugin dir); `dest` is the path under + `android/app/src/main/` derived from the declared path's last `res/` segment + (`priv/native/android/res/xml/foo.xml` → `res/xml/foo.xml`). `native_build` + copies each into the app res tree. Pairs with `android_manifest_snippets/1` + (a `` needs its `res/xml/foo.xml`). + """ + @spec android_res_files([plugin()]) :: [%{plugin: atom(), src: String.t(), dest: String.t()}] + def android_res_files(plugins) do + for {dir, manifest} <- with_manifests(plugins), + rel <- List.wrap(get_in(manifest, [:android, :res_files])), + is_binary(rel) do + %{plugin: manifest[:name], src: Path.join(dir, rel), dest: res_dest(rel)} + end + end + + # Android res destination for a plugin-relative path: everything from the last + # `res` path segment onward, so it lands correctly typed under the app's + # `res/` tree. Falls back to `res/` when no `res` segment is present + # (the manifest validator rejects that case up front). + defp res_dest(rel) do + parts = Path.split(rel) + + case last_index(parts, "res") do + nil -> Path.join("res", Path.basename(rel)) + idx -> Path.join(Enum.drop(parts, idx)) + end + end + + defp last_index(list, value) do + list + |> Enum.with_index() + |> Enum.reduce(nil, fn {v, i}, acc -> if v == value, do: i, else: acc end) + end + @doc "Unique iOS framework names across plugins." @spec ios_frameworks([plugin()]) :: [String.t()] def ios_frameworks(plugins), do: collect_uniq(plugins, [:ios, :frameworks]) diff --git a/lib/mob_dev/plugin/validator.ex b/lib/mob_dev/plugin/validator.ex index 1ef8e82..c2eb3de 100644 --- a/lib/mob_dev/plugin/validator.ex +++ b/lib/mob_dev/plugin/validator.ex @@ -15,7 +15,7 @@ defmodule MobDev.Plugin.Validator do is `File.exists?/1` for path checks, isolated in `validate_plugin/3`). """ - alias MobDev.Plugin.Manifest + alias MobDev.Plugin.{Manifest, Merge} @type result :: %{errors: [String.t()], warnings: [String.t()]} @@ -38,6 +38,7 @@ defmodule MobDev.Plugin.Validator do [ nif_dirs, [android[:bridge_kt], android[:jni_source]], + List.wrap(android[:res_files]), List.wrap(ios[:swift_files]) ] |> List.flatten() @@ -251,6 +252,14 @@ defmodule MobDev.Plugin.Validator do {:collision, [{"Android JNI source basename (android.jni_source)", &jni_basenames/1}]}, bridge_classes: {:collision, [{"Android bridge class (android.bridge_class)", &bridge_class_names/1}]}, + android_manifest_snippets: + {:collision, + [ + {"AndroidManifest component (android.manifest_application_snippets android:name)", + &manifest_component_names/1} + ]}, + android_res_files: + {:collision, [{"Android res destination (android.res_files)", &res_file_dests/1}]}, plist_keys: {:collision, [{"iOS Info.plist key (ios.plist_keys)", &plist_key_names/1}]}, lifecycle: {:collision, [{"supervised worker (lifecycle.supervised)", &supervised_workers/1}]}, @@ -624,6 +633,30 @@ defmodule MobDev.Plugin.Validator do end end + # Two plugins contributing a manifest component with the same android:name + # would duplicate it in the app's (the merge is idempotent per + # name, so the second plugin's would be dropped — a silent loss). Key the + # collision on the component name. Reuses the Merge gatherer for the snippet + # set so the two can't drift. + defp manifest_component_names(manifest) do + Merge.android_manifest_snippets([{".", manifest}]) + |> Enum.flat_map(fn %{snippet: s} -> component_name(s) end) + end + + defp component_name(snippet) do + case Regex.run(~r/android:name="([^"]+)"/, snippet) do + [_, name] -> [name] + _ -> [] + end + end + + # Two plugins copying a res file to the same destination (e.g. both ship + # res/xml/apduservice.xml) would clobber each other. Key on the derived dest; + # reuses the Merge gatherer so the dest derivation stays single-sourced. + defp res_file_dests(manifest) do + Merge.android_res_files([{".", manifest}]) |> Enum.map(& &1.dest) + end + # Two plugins setting the same Info.plist key silently last-write-wins in the # merged plist (Map.merge in Merge.plist_keys/1). defp plist_key_names(manifest) do diff --git a/test/mob_dev/native_build_test.exs b/test/mob_dev/native_build_test.exs index 9aa45e6..da743d2 100644 --- a/test/mob_dev/native_build_test.exs +++ b/test/mob_dev/native_build_test.exs @@ -120,6 +120,51 @@ defmodule MobDev.NativeBuildTest do end end + describe "__merge_android_manifest_components__/2" do + @manifest """ + + + + + + """ + + test "splices a component in just before " do + out = + NativeBuild.__merge_android_manifest_components__(@manifest, [ + ~s() + ]) + + assert out =~ ~s( (before its close, after the activity) + assert out =~ ~r/MainActivity.*MobNfcApduService.*<\/application>/s + end + + test "is idempotent on the component's android:name" do + snippet = ~s() + once = NativeBuild.__merge_android_manifest_components__(@manifest, [snippet]) + twice = NativeBuild.__merge_android_manifest_components__(once, [snippet]) + assert once == twice + assert length(String.split(once, "MobNfcApduService")) == 2 + end + + test "no snippets → manifest unchanged" do + assert NativeBuild.__merge_android_manifest_components__(@manifest, []) == @manifest + end + + test "no → returns manifest untouched rather than corrupting it" do + weird = "" + assert NativeBuild.__merge_android_manifest_components__(weird, [""]) == weird + end + + test "preserves nested indentation, shifted into " do + snippet = "\n \n" + out = NativeBuild.__merge_android_manifest_components__(@manifest, [snippet]) + assert out =~ " " + assert out =~ " " + end + end + describe "__notify_hub_kotlin__/0" do test "generated hub is the stable io.mob.plugin seam with the three members" do src = NativeBuild.__notify_hub_kotlin__() diff --git a/test/mob_dev/plugin/conflict_surface_test.exs b/test/mob_dev/plugin/conflict_surface_test.exs index aa04be7..6875d01 100644 --- a/test/mob_dev/plugin/conflict_surface_test.exs +++ b/test/mob_dev/plugin/conflict_surface_test.exs @@ -99,6 +99,31 @@ defmodule MobDev.Plugin.ConflictSurfaceTest do assert Enum.any?(errs, &(&1 =~ "Info.plist key")) end + test "duplicate AndroidManifest component name across plugins" do + plugins = + same(%{ + android: %{ + manifest_application_snippets: [ + ~s() + ] + } + }) + + assert %{errors: errs} = Validator.cross_validate(plugins) + assert Enum.any?(errs, &(&1 =~ "AndroidManifest component")) + end + + test "duplicate Android res destination across plugins" do + plugins = + two( + %{android: %{res_files: ["priv/a/res/xml/svc.xml"]}}, + %{android: %{res_files: ["priv/b/res/xml/svc.xml"]}} + ) + + assert %{errors: errs} = Validator.cross_validate(plugins) + assert Enum.any?(errs, &(&1 =~ "Android res destination")) + end + test "duplicate supervised worker across plugins" do plugins = same(%{lifecycle: %{supervised: [MyApp.Worker]}}) assert %{errors: errs} = Validator.cross_validate(plugins) diff --git a/test/mob_dev/plugin/manifest_test.exs b/test/mob_dev/plugin/manifest_test.exs index 325b336..5ca68c3 100644 --- a/test/mob_dev/plugin/manifest_test.exs +++ b/test/mob_dev/plugin/manifest_test.exs @@ -37,6 +37,48 @@ defmodule MobDev.Plugin.ManifestTest do end end + describe "validate/1 android.manifest_application_snippets + res_files" do + test "accepts well-formed snippets and res_files" do + m = + Map.put(@valid, :android, %{ + manifest_application_snippets: [~s()], + res_files: ["priv/native/android/res/xml/svc.xml"] + }) + + assert {:ok, ^m} = Manifest.validate(m) + end + + test "rejects non-list / non-string snippets" do + assert {:error, errs} = + Manifest.validate( + Map.put(@valid, :android, %{manifest_application_snippets: ""}) + ) + + assert Enum.any?(errs, &(&1 =~ "manifest_application_snippets")) + + assert {:error, errs2} = + Manifest.validate( + Map.put(@valid, :android, %{manifest_application_snippets: ["", :nope]}) + ) + + assert Enum.any?(errs2, &(&1 =~ "manifest_application_snippets")) + end + + test "rejects res_files without a res segment (dest can't be derived)" do + assert {:error, errs} = + Manifest.validate(Map.put(@valid, :android, %{res_files: ["priv/xml/svc.xml"]})) + + assert Enum.any?(errs, &(&1 =~ "res")) + end + + test "rejects a non-list res_files" do + assert {:error, errs} = + Manifest.validate(Map.put(@valid, :android, %{res_files: "res/xml/x.xml"})) + + assert Enum.any?(errs, &(&1 =~ "res_files")) + end + end + describe "validate/1" do test "nil (no manifest) is valid" do assert {:ok, nil} = Manifest.validate(nil) diff --git a/test/mob_dev/plugin/merge_test.exs b/test/mob_dev/plugin/merge_test.exs index 147ce56..4e04ec7 100644 --- a/test/mob_dev/plugin/merge_test.exs +++ b/test/mob_dev/plugin/merge_test.exs @@ -544,4 +544,71 @@ defmodule MobDev.Plugin.MergeTest do ] = Merge.host_requirements(plugins) end end + + describe "android_manifest_snippets/1" do + test "collects snippets across plugins, tagged with the plugin" do + plugins = [ + {"/a", base(%{name: :a, android: %{manifest_application_snippets: [""]}})}, + {"/z", nil}, + {"/b", + base(%{ + name: :b, + android: %{manifest_application_snippets: ["", ""]} + })} + ] + + assert [ + %{plugin: :a, snippet: ""}, + %{plugin: :b, snippet: ""}, + %{plugin: :b, snippet: ""} + ] = Merge.android_manifest_snippets(plugins) + end + + test "absent / malformed snippets contribute nothing" do + plugins = [ + {"/a", base(%{name: :a})}, + {"/b", base(%{name: :b, android: %{manifest_application_snippets: [:oops]}})} + ] + + assert Merge.android_manifest_snippets(plugins) == [] + end + end + + describe "android_res_files/1" do + test "resolves src absolutely and derives the res// dest" do + plugins = [ + {"/plug", + base(%{ + name: :plug, + android: %{res_files: ["priv/native/android/res/xml/foo_apduservice.xml"]} + })} + ] + + assert [ + %{ + plugin: :plug, + src: "/plug/priv/native/android/res/xml/foo_apduservice.xml", + dest: "res/xml/foo_apduservice.xml" + } + ] = Merge.android_res_files(plugins) + end + + test "uses the LAST res segment when the path has more than one" do + plugins = [{"/p", base(%{android: %{res_files: ["res/drawable/res/icon.xml"]}})}] + assert [%{dest: "res/icon.xml"}] = Merge.android_res_files(plugins) + end + + test "combines across plugins and ignores tier-0 manifests" do + plugins = [ + {"/a", base(%{name: :a, android: %{res_files: ["x/res/values/strings.xml"]}})}, + {"/z", nil}, + {"/b", base(%{name: :b, android: %{res_files: ["y/res/xml/svc.xml"]}})} + ] + + assert [ + %{plugin: :a, dest: "res/values/strings.xml"}, + %{plugin: :b, dest: "res/xml/svc.xml"} + ] = Merge.android_res_files(plugins) + end + end end From 81f442d2b7555a9f14f09e06ece278be9033b8c6 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Tue, 7 Jul 2026 13:48:36 -0600 Subject: [PATCH 2/2] =?UTF-8?q?MOB-39:=20harden=20res=5Ffiles=20=E2=80=94?= =?UTF-8?q?=20path-traversal=20guard,=20host-clobber=20guard,=20sign=20byt?= =?UTF-8?q?es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review of #32: - F1 (blocker): res_files path traversal. Manifest validation now rejects a '..' segment, and native_build's copy resolves via __res_target__/2 which Path.expands and requires the destination stay under the app res/ dir — so a '..'-bearing path can't make File.cp! write plugin bytes anywhere on the host. - F2: host-resource clobber + prune-deletes-host-file. The copy now refuses to overwrite a file this build didn't write on a prior run (ledger-tracked), so a plugin can't replace a host-owned resource — which also stops the prune from ever deleting a host file. - F3: res_files bytes are now in the signed payload (Sign.compute_file_hashes), same as bridge_kt/jni_source — copied resource bytes are tamper-evident. Tests: '..' rejection, __res_target__ containment (incl. res-vs-resx boundary), res_files signing. 322 in the affected files; credo --strict clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/mob_dev/native_build.ex | 69 ++++++++++++++++++++++++--- lib/mob_dev/plugin/manifest.ex | 13 ++++- lib/mob_dev/plugin/sign.ex | 8 +++- test/mob_dev/native_build_test.exs | 24 ++++++++++ test/mob_dev/plugin/manifest_test.exs | 9 ++++ test/mob_dev/plugin/sign_test.exs | 18 +++++++ 6 files changed, 132 insertions(+), 9 deletions(-) diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index f462be6..2b69ca8 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -4541,16 +4541,21 @@ defmodule MobDev.NativeBuild do # (previous − current), then persists `current`. Per-scope and only called # when that concern's merge runs, so an iOS-only build never prunes Android # artifacts. Returns the pruned paths (for tests). + # The relative paths a prior build recorded for a plugin-artifact `scope` + # (empty when none). Shared by the prune and the res host-clobber guard. + defp read_plugin_artifact_ledger(scope) do + case File.read(Path.join(@plugin_artifact_ledger_dir, to_string(scope))) do + {:ok, body} -> String.split(body, "\n", trim: true) + _ -> [] + end + end + @spec __prune_plugin_artifacts__(atom(), [Path.t()]) :: [Path.t()] def __prune_plugin_artifacts__(scope, current) do ledger = Path.join(@plugin_artifact_ledger_dir, to_string(scope)) current = current |> Enum.map(&Path.relative_to_cwd/1) |> Enum.uniq() - previous = - case File.read(ledger) do - {:ok, body} -> String.split(body, "\n", trim: true) - _ -> [] - end + previous = read_plugin_artifact_ledger(scope) pruned = for stale <- previous -- current, File.exists?(stale) do @@ -4830,6 +4835,17 @@ defmodule MobDev.NativeBuild do # two plugins targeting the same destination with different sources — the # cross-plugin validator catches this at activation, this is the build-time # backstop. No-op (with a notice) when the res root is missing. + # + # Two safety guards (the manifest validator enforces the first at activation + # too; these are the build-time backstop, since `activated/0` can feed + # unvalidated manifests): + # * containment — the resolved destination must stay under the app `res/` + # dir, so a `..`-bearing path can't make `File.cp!` write plugin bytes + # anywhere on the build host (path traversal). + # * no host clobber — refuse to overwrite a file this build didn't write on + # a previous run (tracked in the ledger); otherwise a plugin could replace + # a host-owned resource (e.g. res/values/styles.xml) and, worse, the + # ledger prune would later delete the host's file on plugin removal. defp apply_plugin_android_res! do res_files = MobDev.Plugin.Merge.android_res_files(MobDev.Plugin.activated()) @@ -4843,13 +4859,22 @@ defmodule MobDev.NativeBuild do true -> raise_on_res_dest_collision!(res_files) + prior = read_plugin_artifact_ledger(:android_res) written = for %{src: src, dest: dest} <- res_files do - target = Path.join(@android_res_root, dest) + target = safe_res_target!(dest) + rel = Path.relative_to_cwd(target) + + if File.exists?(target) and rel not in prior do + Mix.raise( + "plugin res file #{dest} would overwrite host-owned #{rel} — rename it in the plugin" + ) + end + File.mkdir_p!(Path.dirname(target)) File.cp!(src, target) - IO.puts(" ✓ android res → #{Path.relative_to_cwd(target)}") + IO.puts(" ✓ android res → #{rel}") target end @@ -4858,6 +4883,36 @@ defmodule MobDev.NativeBuild do end end + # Resolve a plugin res destination to a copy target, raising if it escapes the + # app `res/` dir. The hard security boundary behind the manifest validator's + # `..` rejection. + defp safe_res_target!(dest) do + case __res_target__(@android_res_root, dest) do + {:ok, target} -> + target + + {:error, :escapes_res_dir} -> + Mix.raise( + "plugin res file destination escapes the app res/ dir: #{dest} " <> + "(path traversal — declared res_files must not contain \"..\")" + ) + end + end + + @doc false + # Pure: {:ok, copy_target} when `dest` (joined onto `root`) stays inside + # `root/res`, else {:error, :escapes_res_dir}. `..` and absolute escapes are + # normalised by Path.expand before the containment check. + @spec __res_target__(String.t(), String.t()) :: {:ok, String.t()} | {:error, :escapes_res_dir} + def __res_target__(root, dest) do + res_dir = Path.expand(Path.join(root, "res")) + target_abs = Path.expand(Path.join(root, dest)) + + if target_abs == res_dir or String.starts_with?(target_abs, res_dir <> "/"), + do: {:ok, Path.join(root, dest)}, + else: {:error, :escapes_res_dir} + end + defp raise_on_res_dest_collision!(res_files) do res_files |> Enum.group_by(& &1.dest, & &1.src) diff --git a/lib/mob_dev/plugin/manifest.ex b/lib/mob_dev/plugin/manifest.ex index a0e194d..b44ee75 100644 --- a/lib/mob_dev/plugin/manifest.ex +++ b/lib/mob_dev/plugin/manifest.ex @@ -201,7 +201,11 @@ defmodule MobDev.Plugin.Manifest do # `android.res_files` (optional) is a list of plugin-relative paths copied into # the app's `res/` tree. Each must be a non-empty string containing a `res` - # path segment (the build derives the `res//` destination from it). + # path segment (the build derives the `res//` destination from it) + # and must NOT contain a `..` segment — otherwise the derived destination could + # escape the app `res/` dir and `File.cp!` would write plugin bytes anywhere on + # the build host (path traversal). The native build enforces containment again + # at copy time as defense in depth. defp check_android_res_files(errors, manifest) do case get_in(manifest, [:android, :res_files]) do nil -> @@ -212,6 +216,13 @@ defmodule MobDev.Plugin.Manifest do not Enum.all?(list, &(is_binary(&1) and &1 != "")) -> ["android.res_files must be a list of non-empty path strings" | errors] + Enum.any?(list, &(".." in Path.split(&1))) -> + [ + "android.res_files paths must not contain a \"..\" segment " <> + "(path traversal — the copy destination must stay under the app res/ dir)" + | errors + ] + not Enum.all?(list, &("res" in Path.split(&1))) -> [ "android.res_files paths must contain a \"res\" segment (e.g. " <> diff --git a/lib/mob_dev/plugin/sign.ex b/lib/mob_dev/plugin/sign.ex index 3742049..ece9030 100644 --- a/lib/mob_dev/plugin/sign.ex +++ b/lib/mob_dev/plugin/sign.ex @@ -45,6 +45,8 @@ defmodule MobDev.Plugin.Sign do - `manifest.ios.swift_files` — single files (list of paths). - `manifest.android.bridge_kt` and `manifest.android.jni_source` — single paths each. + - `manifest.android.res_files` — the resource files copied verbatim + into the app `res/` tree (list of paths). - `manifest.nifs[].native_dir` — recursive over `.c`, `.h`, `.cpp`, `.zig` files inside. This is the only case where a directory is expanded. @@ -153,9 +155,13 @@ defmodule MobDev.Plugin.Sign do ] |> Enum.filter(&is_binary/1) + # res_files are copied verbatim into the app res/ tree, so their bytes must + # be tamper-evident too (same as bridge_kt / jni_source). + res = list_of_strings(get_in(manifest, [:android, :res_files])) + nifs = nif_files(manifest, plugin_dir) - swift ++ android ++ nifs + swift ++ android ++ res ++ nifs end defp nif_files(manifest, plugin_dir) do diff --git a/test/mob_dev/native_build_test.exs b/test/mob_dev/native_build_test.exs index da743d2..b887db7 100644 --- a/test/mob_dev/native_build_test.exs +++ b/test/mob_dev/native_build_test.exs @@ -165,6 +165,30 @@ defmodule MobDev.NativeBuildTest do end end + describe "__res_target__/2 (plugin res path containment)" do + @root "android/app/src/main" + + test "a normal res destination resolves to a copy target under res/" do + assert {:ok, "android/app/src/main/res/xml/svc.xml"} = + NativeBuild.__res_target__(@root, "res/xml/svc.xml") + end + + test "a .. traversal that escapes res/ is rejected" do + assert {:error, :escapes_res_dir} = + NativeBuild.__res_target__(@root, "res/../../../build.gradle") + end + + test "a deeper traversal to an arbitrary host path is rejected" do + assert {:error, :escapes_res_dir} = + NativeBuild.__res_target__(@root, "res/../../../../../../etc/hosts") + end + + test "the res dir itself is allowed but a sibling of res/ is not" do + assert {:ok, _} = NativeBuild.__res_target__(@root, "res") + assert {:error, :escapes_res_dir} = NativeBuild.__res_target__(@root, "res/../resx/x") + end + end + describe "__notify_hub_kotlin__/0" do test "generated hub is the stable io.mob.plugin seam with the three members" do src = NativeBuild.__notify_hub_kotlin__() diff --git a/test/mob_dev/plugin/manifest_test.exs b/test/mob_dev/plugin/manifest_test.exs index 5ca68c3..97e8864 100644 --- a/test/mob_dev/plugin/manifest_test.exs +++ b/test/mob_dev/plugin/manifest_test.exs @@ -77,6 +77,15 @@ defmodule MobDev.Plugin.ManifestTest do assert Enum.any?(errs, &(&1 =~ "res_files")) end + + test "rejects a res_files path containing .. (path traversal)" do + assert {:error, errs} = + Manifest.validate( + Map.put(@valid, :android, %{res_files: ["x/res/../../../build.gradle"]}) + ) + + assert Enum.any?(errs, &(&1 =~ ".." or &1 =~ "traversal")) + end end describe "validate/1" do diff --git a/test/mob_dev/plugin/sign_test.exs b/test/mob_dev/plugin/sign_test.exs index 2ac3959..a2c519c 100644 --- a/test/mob_dev/plugin/sign_test.exs +++ b/test/mob_dev/plugin/sign_test.exs @@ -59,6 +59,24 @@ defmodule MobDev.Plugin.SignTest do ] end + test "hashes android.res_files so copied resource bytes are signed", %{dir: dir} do + write_file(dir, "android/res/xml/svc.xml", "") + write_file(dir, "android/res/values/strings.xml", "") + + manifest = %{ + name: :mob_x, + mob_version: "~> 0.6", + plugin_spec_version: 1, + android: %{ + res_files: ["android/res/xml/svc.xml", "android/res/values/strings.xml"] + } + } + + paths = Sign.compute_file_hashes(dir, manifest) |> Enum.map(&elem(&1, 0)) + assert "android/res/xml/svc.xml" in paths + assert "android/res/values/strings.xml" in paths + end + test "is independent of the order swift_files appear in the manifest", %{dir: dir} do write_file(dir, "ios/A.swift", "alpha") write_file(dir, "ios/B.swift", "beta")