MOB-74 — plugin envelope v2: verify signature before eval

CHANGELOG.md

@@ -2,6 +2,35 @@
 
 ### Fixed
 
+- **Plugin signature verification now runs before `Code.eval_file`** on the
+  manifest (MOB-74). The v1 signature covered the eval'd manifest map, so
+  verifiers needed the eval to run first to rebuild the payload — letting
+  a malicious `priv/mob_plugin.exs` execute arbitrary code on the
+  consumer's machine during plugin activation.
+
+  Envelope v2 puts the signed `file_hashes` list on disk alongside the
+  signature; verification runs entirely off the envelope and the file
+  bytes, no eval required. `priv/mob_plugin.exs` is one of those
+  file_hashes, so tampering with the manifest bytes shifts its hash and
+  fails verification. `MobDev.Plugin.Verify.load_verified/1` is the new
+  safe consumer path: verify → then eval. `MobDev.Plugin.activated/0`,
+  `mix mob.plugins`, `mix mob.audit_plugins`, and `MobDev.Plugin.Report`
+  all migrated.
+
+  **v1 envelopes are refused** — accepting them silently reopens the
+  bug. The error is a distinguished `:envelope_v1_unsupported` reason
+  and the `SignatureGate` message tells the author to re-sign with a
+  mob_dev that produces envelope v2 (`mix mob.plugin.sign`). Every
+  plugin signed with a mob_dev that predates this change must be
+  re-signed on the consumer's next build.
+
+  Follow-up tickets on file for the walk to data-only manifests
+  (safe-by-construction): **MOB-185** (`mix mob.plugin.lint`),
+  **MOB-186** (migrate first-party plugins to the pure-data subset),
+  **MOB-187** (static JSON/TOML manifest at 1.0).
+
+  See `decisions/2026-09-11-plugin-envelope-v2-verify-before-eval.md`.
+
 - **Play upload keystore no longer bakes a trailing newline into the stored
   password** (MOB-71). `Mix.shell().prompt/1` returns the whole input line
   including the trailing `\n`, and `MobDev.GooglePlay.SetupWizard.generate_keystore/2`

decisions/2026-09-11-plugin-envelope-v2-verify-before-eval.md

@@ -0,0 +1,110 @@
+# Plugin envelope v2 — verify signature before eval
+
+- Date: 2026-09-11
+- Status: accepted
+
+## Context
+
+Before this change, `MobDev.Plugin.Manifest.load/1` (which is called from
+`MobDev.Plugin.activated/0` on every `mix mob.deploy`, and from
+`mix mob.plugins` / `mix mob.audit_plugins` / `MobDev.Plugin.Report`)
+called `Code.eval_file("priv/mob_plugin.exs")` **before** any signature
+check. The signature check that ran afterwards (`SignatureGate.check_plugin/4`
+→ `Verify.verify_plugin(dir, manifest)`) covered the *eval'd manifest map*
+plus a list of referenced sources — but NOT the manifest bytes themselves.
+So a malicious plugin could:
+
+```elixir
+# priv/mob_plugin.exs
+File.write!("/tmp/pwned", "evil ran")         # side effect
+%{name: :innocent, mob_version: "~> 0.7", ...}
+```
+
+`Code.eval_file/1` would execute the side effect on the consumer's
+machine, then return the innocent-looking map. The signature covered the
+map (which was untouched) so `Verify` reported clean, and the CVE was
+never surfaced.
+
+Filed as MOB-74 in the July 2026 audit.
+
+## Decision
+
+Move to envelope v2, which flips the trust chain end-to-end:
+
+1. **The manifest bytes join the signed file_hashes list.** `Sign.referenced_files/2`
+   now always prepends `priv/mob_plugin.exs` — the file that used to be
+   trusted implicitly is now cryptographically covered like every other
+   source.
+2. **The v2 envelope on disk carries the file_hashes list alongside the
+   signature.** Payload signed = `%{file_hashes: [...], envelope_version: 2}`.
+   The eval'd manifest map is no longer part of the payload — its
+   authoritative representation is the on-disk bytes, whose hash lives
+   in file_hashes.
+3. **`Verify.verify_plugin/1` no longer takes a manifest arg.** It reads
+   the envelope, rebuilds the payload from the envelope's own file_hashes
+   (proving the author signed *this* list of files), then re-hashes each
+   listed file on disk (proving the disk hasn't been tampered with since
+   signing). Neither step calls `Code.eval_file/1`.
+4. **`Verify.load_verified/1`** is the new consumer entry point. Verifies
+   first; only calls `Manifest.load/1` (which does the eval) after
+   verification passes. This is the actual MOB-74 close: a plugin that
+   fails verification never has its `.exs` bytes evaluated on the
+   consumer's machine.
+5. **v1 envelopes are refused.** They can't verify without the eval'd
+   manifest to rebuild their payload, so accepting them silently reopens
+   the CVE. The refusal is a distinguished `:envelope_v1_unsupported`
+   error with an actionable re-sign hint.
+
+## Consequences
+
+- **Breaking change for signed plugins.** Every plugin published with a
+  mob_dev ≤ 0.7.1 signature must be re-signed with mob_dev 0.7.2+.
+  There is no fallback path; the whole point of the fix is that the
+  fallback path was the bug. First-party plugins get re-signed as part
+  of the 0.7.2 release; third-party authors will see the
+  `:envelope_v1_unsupported` error and follow the re-sign hint in the
+  message. See [`MOB_PLUGIN_SECURITY.md`](../MOB_PLUGIN_SECURITY.md).
+- **Consumer paths that eval a manifest** now go through
+  `MobDev.Plugin.Verify.load_verified/1` in the four places that
+  matter to build- or activation-time: `MobDev.Plugin.activated/0`,
+  `mix mob.plugins`, `mix mob.audit_plugins`, `MobDev.Plugin.Report`.
+- **Author-side paths still eval before verify** on purpose —
+  `mix mob.plugin.sign` and `mix mob.plugin.keygen` operate on plugins
+  the author owns and is about to sign; there is no attack vector.
+  `mix mob.plugin.trust` is the first-trust decision by definition;
+  the user is explicitly reviewing a plugin they don't trust yet.
+  `mix mob.validate_plugin` runs as an author-side lint pass
+  (`priv/mob_plugin.exs` shape checks against `Manifest.validate/1`);
+  no attack vector for the author's own plugin. All four are noted as
+  separate follow-ups (MOB-185/186/187 lay out the longer-term path
+  to data-only manifests, which closes the class entirely).
+- **`SignatureGate.check_activated/1` tier-0 detection** switched from
+  "`is_map(manifest)`" (the old proxy) to
+  "`Manifest.manifest_present?(dir)`". This is necessary because a
+  failed-verification tier-1 plugin now also has `manifest == nil` —
+  distinguishing the two by manifest-file presence surfaces the
+  friendly error for the failed case instead of silently skipping.
+- **`Verify.load_verified/2` accepts `acknowledged_unsafe: true`** as
+  the second-arg option, and `MobDev.Plugin.activated_with_verify/0`
+  passes it for plugins in `:acknowledge_unsafe_plugins`. Without
+  this, an acknowledged unsigned plugin's `{:error, :missing_signature}`
+  would strip its manifest map from the build even though
+  `SignatureGate.check_plugin/4` still lets the plugin through
+  (`if name in acknowledged, do: :ok`). Downstream `Merge`,
+  `AndroidBootstrap`, `RuntimeManifest` all filter on
+  `is_map(manifest)` — the acknowledged plugin would appear activated
+  but contribute nothing: no NIFs, no gradle deps, no permissions. The
+  opt-in escape hatch is scoped to `:missing_signature` only; every
+  other verify failure (`:invalid_signature`, `:missing_pubkey`,
+  `:envelope_v1_unsupported`) still refuses the eval because those
+  are tamper / mis-key / attack signals, not "unsigned during dev".
+- **UX regression when the manifest name isn't derivable from a nil
+  manifest** — the gate falls back to `Path.basename(dir)` for the error
+  label. In real deployments this equals the Hex package name; in tests
+  it's a temp-dir basename. Acceptable — the alternative is a bigger
+  return-shape refactor to thread the dep name through every layer.
+- **Follow-up tickets on file:** MOB-185 (`mix mob.plugin.lint`),
+  MOB-186 (migrate first-party plugins to the pure-data subset the
+  linter accepts), MOB-187 (static JSON/TOML manifest at 1.0). Together
+  they walk the manifest format from "safe if verified" (this ADR)
+  toward "safe by construction" (MOB-187 endstate).

lib/mix/tasks/mob.audit_plugins.ex

@@ -39,7 +39,7 @@ defmodule Mix.Tasks.Mob.AuditPlugins do
   """
 
   alias MobDev.Plugin
-  alias MobDev.Plugin.{Audit, Manifest, Report}
+  alias MobDev.Plugin.{Audit, Report}
 
   @switches [plugin: :string, accept_medium: :boolean]
 
@@ -88,7 +88,7 @@ defmodule Mix.Tasks.Mob.AuditPlugins do
 
     for name <- Plugin.activated_names(), dir = deps[name], not is_nil(dir) do
       manifest =
-        case Manifest.load(dir) do
+        case MobDev.Plugin.Verify.load_verified(dir) do
           {:ok, m} -> m
           {:error, _} -> nil
         end

lib/mix/tasks/mob.plugin.sign.ex

@@ -5,8 +5,13 @@ defmodule Mix.Tasks.Mob.Plugin.Sign do
 
   @moduledoc """
   Signs the plugin in `<dir>` (default: cwd) and writes
-  `priv/mob_plugin.sig`. The signature covers the loaded manifest
-  plus SHA-256 hashes of every file the manifest references.
+  `priv/mob_plugin.sig`. The signature covers the SHA-256 hashes of
+  `priv/mob_plugin.exs` and every source file the manifest references —
+  the manifest bytes are one of those files, so tampering with the
+  manifest fails verification. The v2 envelope on disk carries the
+  signed `file_hashes` list so `MobDev.Plugin.Verify.verify_plugin/1`
+  can check integrity without ever `Code.eval_file`-ing the manifest
+  (see MOB-74).
 
       mix mob.plugin.sign [--plugin <dir>]
 

lib/mix/tasks/mob.plugins.ex

@@ -20,7 +20,7 @@ defmodule Mix.Tasks.Mob.Plugins do
   *activated* — only then are its contributions merged into the build.
   """
 
-  alias MobDev.Plugin.{Manifest, Report, Validator}
+  alias MobDev.Plugin.{Report, Validator}
 
   @impl Mix.Task
   def run(_args) do
@@ -56,7 +56,7 @@ defmodule Mix.Tasks.Mob.Plugins do
   defp load_all_manifests do
     Mix.Project.deps_paths()
     |> Enum.map(fn {app, path} ->
-      case Manifest.load(path) do
+      case MobDev.Plugin.Verify.load_verified(path) do
         {:ok, manifest} ->
           {app, manifest}
 

lib/mob_dev/plugin.ex

@@ -104,17 +104,72 @@ defmodule MobDev.Plugin do
   `MobDev.Plugin.Merge`.
 
   Resolves each activated name to its dependency directory and loads its
-  manifest (nil for a tier-0 plugin). Activated names that don't resolve to a
-  dep are skipped — `mix mob.plugins` is where that mismatch surfaces to users.
+  manifest via `MobDev.Plugin.Verify.load_verified/1` — signature and
+  file-integrity checks run **before** `Code.eval_file` (MOB-74), so a
+  plugin that fails verification never has its `priv/mob_plugin.exs`
+  executed. Failed plugins come back as `{dir, nil}` here; callers that
+  need to distinguish "tier-0 plugin (no manifest)" from "verify failed"
+  should use `activated_with_verify/0` — that's the shape
+  `SignatureGate.check_activated/1` consumes to produce friendly
+  build-blocking errors. Activated names that don't resolve to a dep
+  are skipped — `mix mob.plugins` is where that mismatch surfaces
+  to users.
   """
   @spec activated() :: [{Path.t(), map() | nil}]
   def activated do
+    for {dir, manifest, _status} <- activated_with_verify(), do: {dir, manifest}
+  end
+
+  @typedoc """
+  Verification status returned from `activated_with_verify/0`. `:ok` means the
+  manifest was loaded after a passing signature + tamper check; `:unsigned`
+  means there was no manifest at all (tier-0 plugin — no signature required);
+  `{:error, reason}` means the plugin failed verification and its manifest
+  bytes were never eval'd. Reasons come from `MobDev.Plugin.Verify.verify_error/0`.
+  """
+  @type verify_status ::
+          :ok
+          | :unsigned
+          | {:error, MobDev.Plugin.Verify.verify_error()}
+
+  @typedoc "One entry in `activated_with_verify/0`'s return list."
+  @type activated_entry :: {Path.t(), map() | nil, verify_status()}
+
+  @doc """
+  Same as `activated/0` but also returns the verification status per plugin.
+
+  `SignatureGate.check_activated/1` consumes this shape so it can produce a
+  clear build-blocking error naming the failed plugin — without needing to
+  re-load or re-verify anything. A plugin that failed verification appears as
+  `{dir, nil, {:error, reason}}`; a tier-0 plugin (no `priv/mob_plugin.exs`
+  and no signature required) appears as `{dir, nil, :unsigned}`.
+  """
+  @spec activated_with_verify() :: [activated_entry()]
+  def activated_with_verify do
     deps = Mix.Project.deps_paths()
+    acknowledged = MobDev.Plugin.SignatureGate.acknowledged_unsafe()
 
     for name <- activated_names(), dir = deps[name], not is_nil(dir) do
-      case MobDev.Plugin.Manifest.load(dir) do
-        {:ok, manifest} -> {dir, manifest}
-        {:error, _reason} -> {dir, nil}
+      # Threading acknowledged-unsafe here matters. Without it, an unsigned
+      # plugin the user has explicitly opted into via
+      # :acknowledge_unsafe_plugins would get :missing_signature from
+      # load_verified/2 and its manifest map would be dropped — silently
+      # stripping the plugin's NIFs, gradle deps, swift files, and
+      # permissions from the build. `SignatureGate.check_plugin/4` would
+      # still let it through as :ok (that's what "acknowledged" means),
+      # but downstream Merge/AndroidBootstrap/RuntimeManifest filter on
+      # is_map(manifest) and skip the nil. Result: a build that ships
+      # nothing from the plugin, no error message. See MOB-74 pre-merge
+      # review. Every OTHER verify failure (:invalid_signature,
+      # :missing_pubkey, :envelope_v1_unsupported) still refuses the eval
+      # — those are tamper/mis-key signals, not the "unsigned during dev"
+      # case.
+      opts = if name in acknowledged, do: [acknowledged_unsafe: true], else: []
+
+      case MobDev.Plugin.Verify.load_verified(dir, opts) do
+        {:ok, nil} -> {dir, nil, :unsigned}
+        {:ok, manifest} -> {dir, manifest, :ok}
+        {:error, reason} -> {dir, nil, {:error, reason}}
       end
     end
   end

lib/mob_dev/plugin/manifest.ex

@@ -67,6 +67,17 @@ defmodule MobDev.Plugin.Manifest do
     end
   end
 
+  @doc """
+  Returns `true` when `plugin_dir` has a `priv/mob_plugin.exs`, `false`
+  otherwise. Distinguishes tier-0 plugins (no manifest, no signature
+  needed) from tier-1+ plugins (manifest present, signature required)
+  without evaluating the manifest itself. See `Verify.load_verified/1`.
+  """
+  @spec manifest_present?(Path.t()) :: boolean()
+  def manifest_present?(plugin_dir) do
+    File.exists?(Path.join(plugin_dir, @manifest_path))
+  end
+
   defp eval(path) do
     case Code.eval_file(path) do
       {map, _bindings} when is_map(map) ->

lib/mob_dev/plugin/report.ex

@@ -134,8 +134,12 @@ defmodule MobDev.Plugin.Report do
   defp find_row_name(rows, name), do: Enum.find_value(rows, name, &(&1.name == name && name))
 
   defp load_manifest(dir) do
-    case MobDev.Plugin.Manifest.load(dir) do
-      {:ok, manifest} -> manifest
+    # Verified load per MOB-74 — refuse to eval a manifest whose signature or
+    # file-hashes don't check out. Report-only path; a plugin that fails
+    # verification simply shows up with no capability data instead of running
+    # its code inside our process.
+    case MobDev.Plugin.Verify.load_verified(dir) do
+      {:ok, manifest} when is_map(manifest) -> manifest
       _ -> %{}
     end
   end