From 38d091342cad8d19db3d63a49639d608c9091de2 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 5 Sep 2026 00:42:35 -0600 Subject: [PATCH 1/2] =?UTF-8?q?Add=20mix=20mob.attest=20=E2=80=94=20prove?= =?UTF-8?q?=20the=20device=20is=20running=20the=20code=20you=20just=20push?= =?UTF-8?q?ed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mix mob.deploy` reports what it did, not what is now true, and the two come apart more often than the exit code suggests. Two instances from one session. A bundle-id divergence sent the BEAM push into one app's container while a different app was running: it did not fail with "not installed", it succeeded and printed a tick, because both containers existed on the device. And a plain dist deploy reported success while twelve `mob` modules on the device kept their old digests. In both cases every individual step was honest about itself and the run as a whole was wrong. Every guide in these repos says "verify effects, not exit codes". That instruction exists because the tools cannot be trusted, and it only works while someone remembers to follow it. This makes it checkable. `module_info(:md5)` on the device is the same digest `:beam_lib.md5/1` gives for the local `.beam`, so a module that never arrived, arrived in the wrong place, or arrived and was never loaded all show up, and no new fingerprinting machinery is needed. Deliberately not hashing artifacts or directories: two builds of the same source differ in timestamps and paths, and a check that cries wolf gets switched off — which is worse than none, because its absence is invisible. Four verdicts. `:stale` is fatal, the device is running code we did not build. `:unreadable` is fatal too: the check could not run, and a check that could not run must not report success — the same rule as a deploy exiting 0 having shipped nothing. `:missing` is reported and is NOT fatal, because interactive BEAM loads a module when something first calls it and most of a bundle is legitimately unloaded at any moment. A run that reaches no device raises rather than passing. It connects without restarting. `Connector.connect_all/1` restarts the app, which reloads every module and would destroy the evidence being read; the tunnels from a previous `mix mob.connect` are device-level and outlive it, so `Node.connect/1` is enough. It found a live failure on its first real use: a deploy printed "✓ (dist, no restart)" and exited 0 while twelve modules stayed stale, verified by hand from an independent node (local DA73F8D7… vs device DD2B00C8…, loaded from ~/.mob/runtime/ios-sim/). Filed as MOB-161; the root cause is not yet diagnosed and is not in scope here. Mutation-tested with `mix mob.mutate` from the previous ticket: 22 killed, 0 survived. It found one gap on the way — the sort in `name_list/1` was untested, so report order could follow map iteration and make two runs of the same check look different. Refs MOB-152, MOB-161 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 12 ++ .../2026-09-05-attest-what-is-running.md | 67 ++++++ lib/mix/tasks/mob.attest.ex | 202 ++++++++++++++++++ lib/mob_dev/attest.ex | 149 +++++++++++++ test/mix/tasks/mob_attest_test.exs | 44 ++++ test/mob_dev/attest_test.exs | 140 ++++++++++++ 6 files changed, 614 insertions(+) create mode 100644 decisions/2026-09-05-attest-what-is-running.md create mode 100644 lib/mix/tasks/mob.attest.ex create mode 100644 lib/mob_dev/attest.ex create mode 100644 test/mix/tasks/mob_attest_test.exs create mode 100644 test/mob_dev/attest_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c7be33..8964aaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,18 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). ### Added +- **`mix mob.attest`** — prove the device is running the code you just pushed. + Compares `module_info(:md5)` on the device against `:beam_lib.md5/1` of the + local `.beam`, so a push that never landed, landed in the wrong place, or + landed and was never loaded all show up. Exits non-zero on a mismatch, and + also when the check could not run — a check that could not run is not a check + that passed. Modules the device has not loaded yet are reported and are not a + failure. Written after a bundle-id divergence let a BEAM push succeed against + the wrong app's container, printing a tick while the app kept running old + code; it found a second live instance (MOB-161) on its first real use. + +### Added + - **`mix mob.mutate`** — mutation testing for the lines a branch changed. Breaks the production code one line at a time and reports the changes nothing noticed, which is the only cheap way to tell a test that guards something diff --git a/decisions/2026-09-05-attest-what-is-running.md b/decisions/2026-09-05-attest-what-is-running.md new file mode 100644 index 0000000..e17e4d1 --- /dev/null +++ b/decisions/2026-09-05-attest-what-is-running.md @@ -0,0 +1,67 @@ +# Deployment attestation compares module digests, not artifacts + +- Date: 2026-09-05 +- Status: accepted + +## Context + +`mix mob.deploy` reports what it did. It does not report what is now true, and +the two come apart more often than the exit code suggests. + +Two instances from one session. A bundle-id divergence sent the BEAM push into +one app's container while a different app was running: it did not fail with +"not installed", it succeeded and printed a tick, because both containers +existed on the device. And a plain dist deploy reported success while twelve +`mob` modules on the device kept their old digests (MOB-161). + +In both cases every individual step was honest about itself and the run as a +whole was wrong. Every guide in these repos says "verify effects, not exit +codes" — an instruction that exists precisely because the tools cannot be +trusted, and that only works while someone remembers to follow it. + +## Decision + +Compare `module_info(:md5)` on the device against `:beam_lib.md5/1` of the +local `.beam`. They are the same digest for the same bytes, so a module that +never arrived, arrived in the wrong place, or arrived and was never loaded all +show up, and no new machinery has to be invented to produce a fingerprint. + +Deliberately not hashing artifacts, directories or the IPA/APK. Two builds of +the same source differ in timestamps and paths, so an artifact hash reports a +mismatch on every rebuild; a check that cries wolf gets switched off, and a +switched-off check is worth less than none because its absence is invisible. + +Four verdicts rather than two: + +* `:stale` is fatal. The device is running code we did not build. +* `:unreadable` is fatal. The check could not run, and a check that could not + run must not report success — the same rule as a deploy exiting 0 having + shipped nothing. +* `:missing` is reported and is **not** fatal. Interactive BEAM loads a module + when something first calls it, so most of a bundle is legitimately unloaded + at any moment. Failing on that would make the tool unusable within a day. +* `:match` is the only pass. + +Attestation connects without restarting. `Connector.connect_all/1` restarts the +app, which reloads every module and would destroy the evidence being read. The +tunnels a previous `mix mob.connect` set up are device-level and outlive it, so +a plain `Node.connect/1` is enough. + +A run that reaches no device raises rather than reporting success, for the same +reason `:unreadable` is fatal. + +## Consequences + +It is a separate task, not a step inside `mob.deploy`. Deploy cannot always +reach the node — a physical iPhone with USB attached suspends its BEAM — and +folding a check that sometimes cannot run into the thing it checks would either +weaken the deploy's exit code or produce false failures. Wiring it in as an +opt-in deploy flag is the obvious follow-up. + +It only sees modules the device has loaded, so it cannot distinguish "shipped +but not yet loaded" from "never shipped". `mix mob.snapshot_loaded` answers the +shipped-set question and the two are complementary. + +It found MOB-161 on its first real use, which is the argument for building it +and also a caution: the failure it found had been happening silently, and there +is no way to know for how long. diff --git a/lib/mix/tasks/mob.attest.ex b/lib/mix/tasks/mob.attest.ex new file mode 100644 index 0000000..7b4c12e --- /dev/null +++ b/lib/mix/tasks/mob.attest.ex @@ -0,0 +1,202 @@ +defmodule Mix.Tasks.Mob.Attest do + @shortdoc "Prove the device is running the code you just pushed" + + @moduledoc """ + Compare a connected device's loaded modules against this build. + + mix mob.connect --no-iex # set up the tunnel first + mix mob.attest + mix mob.attest --json + mix mob.attest --app my_app # only this application's modules + + ## Why + + `mix mob.deploy` reports what it did, not what is now true. The case this + was written for: two bundle ids diverged, the BEAM push addressed one app's + container while another app was running, and it **succeeded** — a tick, no + error, and the app carrying on with the old code. Both containers existed, so + every step was honest about itself and the run as a whole was a lie. + + `module_info(:md5)` on the device is the same digest `:beam_lib.md5/1` gives + for the local `.beam`. Comparing them catches a push that never landed, landed + in the wrong place, or landed and was never loaded. + + ## Options + + * `--app NAME` — restrict to one application (default: the project's own) + * `--node NAME` — attest one node instead of every connected one + * `--json` — machine-readable result on stdout, progress on stderr + + ## Exit status + + Non-zero when any module on the device differs from this build, or when the + check could not run — a check that could not run must not report success. + + Modules the device has not loaded yet are reported and are **not** a failure. + Interactive BEAM loads a module when something first calls it, so most of a + bundle is legitimately unloaded at any moment. + """ + + use Mix.Task + + alias MobDev.Attest + + @switches [app: :string, node: :string, json: :boolean] + + @impl Mix.Task + def run(args) do + {opts, _argv, invalid} = OptionParser.parse(args, strict: @switches, aliases: [n: :node]) + + unless invalid == [] do + Mix.raise("Unknown option(s): #{invalid |> Enum.map(&elem(&1, 0)) |> Enum.join(", ")}") + end + + if opts[:json] do + Process.put(:attest_stdout, Process.group_leader()) + Process.group_leader(self(), Process.whereis(:standard_error)) + end + + Mix.Task.run("compile") + start_dist!() + + case reachable_nodes(opts) do + [] -> + emit(opts, %{"outcome" => "no_nodes", "nodes" => []}) + + Mix.raise(""" + No device answered, so nothing was attested. + + Run `mix mob.connect --no-iex` first. Reporting success here would be + the failure this task exists to catch: a check that could not run is + not a check that passed. + """) + + nodes -> + results = Enum.map(nodes, &attest_node(&1, opts)) + report(results, opts) + end + end + + # Connect without restarting anything. `Connector.connect_all/1` restarts the + # app, which reloads every module and would destroy the very evidence this + # task exists to read. The tunnels a previous `mix mob.connect` set up are + # device-level and outlive it, so a plain `Node.connect/1` is enough. + defp reachable_nodes(opts) do + candidates = + case opts[:node] do + nil -> discover_node_names() + name -> [String.to_atom(name)] + end + + {up, down} = Enum.split_with(candidates, &Node.connect/1) + + for node <- down do + IO.puts("#{node}: unreachable — run `mix mob.connect --no-iex` first") + end + + up + end + + defp discover_node_names do + (MobDev.Discovery.Android.list_devices() ++ MobDev.Discovery.IOS.list_devices()) + |> Enum.map(&MobDev.Device.node_name/1) + |> Enum.uniq() + end + + defp start_dist!() do + unless Node.alive?() do + MobDev.Connector.start_epmd() + + MobDev.Connector.handle_dist_start( + Node.start(:"mob_attest@127.0.0.1", :longnames), + :mob_secret + ) + end + end + + defp attest_node(node, opts) do + app = String.to_atom(opts[:app] || to_string(Mix.Project.config()[:app])) + findings = Enum.map(beams(app), &finding(node, &1)) + + %{node: node, app: app, findings: findings, verdict: Attest.verdict(findings)} + end + + defp finding(node, path) do + case Attest.local_digest(path) do + nil -> + Attest.compare(module_from_path(path), nil, nil) + + {module, expected} -> + Attest.compare(module, expected, remote_digest(node, module)) + end + end + + # `module_info(:md5)` raises :undef for a module the device has never + # loaded, which arrives here as a badrpc EXIT rather than a value. + defp remote_digest(node, module), do: :rpc.call(node, module, :module_info, [:md5], 10_000) + + defp module_from_path(path), do: path |> Path.basename(".beam") |> String.to_atom() + + defp beams(app) do + Mix.Project.build_path() + |> Path.join("lib/#{app}/ebin/*.beam") + |> Path.wildcard() + end + + defp report(results, opts) do + Enum.each(results, &say_node/1) + + failed = Enum.filter(results, &match?({:error, _}, &1.verdict)) + + emit(opts, %{ + "outcome" => if(failed == [], do: "ok", else: "mismatch"), + "nodes" => Enum.map(results, &json_node/1) + }) + + unless failed == [] do + Mix.raise( + failed + |> Enum.map(fn r -> "#{r.node}: #{elem(r.verdict, 1)}" end) + |> Enum.join("\n") + ) + end + end + + defp say_node(%{node: node, findings: findings, verdict: verdict}) do + t = Attest.tally(findings) + + IO.puts( + "#{node}: #{t.match} match, #{t.stale} stale, #{t.missing} not loaded, " <> + "#{t.unreadable} unreadable" + ) + + for f <- findings, f.verdict in [:stale, :unreadable] do + IO.puts(" #{f.verdict}: #{inspect(f.module)}") + end + + case verdict do + :ok -> IO.puts(" the device is running this build") + {:error, message} -> IO.puts(" #{message}") + end + end + + defp json_node(%{node: node, app: app, findings: findings, verdict: verdict}) do + %{ + "node" => to_string(node), + "app" => to_string(app), + "outcome" => if(verdict == :ok, do: "ok", else: "mismatch"), + "message" => if(verdict == :ok, do: nil, else: elem(verdict, 1)), + "tally" => Map.new(Attest.tally(findings), fn {k, v} -> {to_string(k), v} end), + "modules" => + for f <- findings, f.verdict != :match do + %{"module" => inspect(f.module), "verdict" => to_string(f.verdict)} + end + } + end + + defp emit(opts, payload) do + if opts[:json] do + IO.puts(Process.get(:attest_stdout, :standard_io), Jason.encode!(payload, pretty: true)) + end + end +end diff --git a/lib/mob_dev/attest.ex b/lib/mob_dev/attest.ex new file mode 100644 index 0000000..a67e7fc --- /dev/null +++ b/lib/mob_dev/attest.ex @@ -0,0 +1,149 @@ +defmodule MobDev.Attest do + @moduledoc """ + Prove that the code running on a device is the code you just pushed. + + `mix mob.deploy` reports what it *did*, not what is now *true*. Those come + apart more often than the exit code suggests, and the interesting failures + are the quiet ones — a deploy that prints a tick while the app keeps running + something else entirely. + + The case that produced this module: two bundle ids diverged, so the BEAM push + addressed one app's container while a different app was running. It did not + fail with "not installed". It succeeded, printed a tick, and the app carried + on with the old code. Both containers existed on the device, so every layer + of the deploy was telling the truth about its own step, and the run as a whole + was a lie. + + Every guide in these repos says "verify effects, not exit codes". That + instruction exists because the tools cannot be trusted, and it only works + while a human remembers to follow it. This makes it a property the tool + checks. + + ## How + + `module_info(:md5)` on a loaded module is the same digest `:beam_lib.md5/1` + reports for the `.beam` it was loaded from. Ask the device for one, compute + the other locally, compare. A module that never arrived, arrived somewhere + else, or arrived and was never loaded all show up. + + This deliberately does not hash whole files or directories: two builds of the + same source differ in timestamps and paths, and a check that cries wolf gets + turned off. + """ + + @typedoc """ + What a single module's comparison found. + + * `:match` — the device is running the bytes we have. + * `:stale` — it is running *something*, but not this. The push did not land, + or landed somewhere else, or landed and was not loaded. + * `:missing` — the device has never loaded this module. Expected for a lazily + loaded module, damning for one the app needs to boot. + * `:unreadable` — the local `.beam` could not be digested, so nothing can be + concluded. Not a pass. + """ + @type verdict :: :match | :stale | :missing | :unreadable + + @type finding :: %{ + module: module(), + verdict: verdict(), + expected: binary() | nil, + actual: binary() | nil + } + + @doc """ + The digest of a module as built locally. + + Returns `nil` when the file cannot be read or is not a beam — the caller must + treat that as "cannot tell", never as agreement. + """ + @spec local_digest(Path.t()) :: {module(), binary()} | nil + def local_digest(path) do + case :beam_lib.md5(String.to_charlist(path)) do + {:ok, {module, digest}} -> {module, digest} + _ -> nil + end + end + + @doc """ + Compare one module's local digest against what the device reports. + + `remote` is whatever `module_info(:md5)` came back with, including the + failure shapes: `nil`, `{:badrpc, _}`, or an `:undef` exit for a module the + device has never loaded. + """ + @spec compare(module(), binary() | nil, term()) :: finding() + def compare(module, nil, _remote), + do: %{module: module, verdict: :unreadable, expected: nil, actual: nil} + + def compare(module, expected, remote) do + case normalise_remote(remote) do + ^expected -> %{module: module, verdict: :match, expected: expected, actual: expected} + nil -> %{module: module, verdict: :missing, expected: expected, actual: nil} + actual -> %{module: module, verdict: :stale, expected: expected, actual: actual} + end + end + + # A module the device has never loaded raises :undef, which arrives as a + # badrpc EXIT rather than a value. That is "missing", not "different" — the + # distinction matters because a lazily loaded module being absent is normal + # and a mismatched one never is. + defp normalise_remote(digest) when is_binary(digest), do: digest + defp normalise_remote(_other), do: nil + + @doc """ + Whether a set of findings means the deploy can be believed. + + `:stale` is always fatal: the device is running code we did not build, which + is the failure this module exists to catch. + + `:unreadable` is fatal too. It means the check could not run, and a check that + could not run must not report success — that is the same defect as a deploy + exiting 0 having shipped nothing. + + `:missing` is not fatal on its own. Interactive BEAM loads a module when + something first calls it, so a module that is shipped but not yet loaded is + the normal state of most of the bundle. It is reported, not failed on. + """ + @spec verdict([finding()]) :: :ok | {:error, String.t()} + def verdict(findings) do + stale = Enum.filter(findings, &(&1.verdict == :stale)) + unreadable = Enum.filter(findings, &(&1.verdict == :unreadable)) + + cond do + stale != [] -> + {:error, + "#{length(stale)} module(s) on the device do not match this build: " <> + name_list(stale) <> ". The app is running code you did not just push."} + + unreadable != [] -> + {:error, + "#{length(unreadable)} module(s) could not be digested locally: " <> + name_list(unreadable) <> ". Nothing can be concluded about the device."} + + true -> + :ok + end + end + + defp name_list(findings) do + findings + |> Enum.map(&inspect(&1.module)) + |> Enum.sort() + |> Enum.take(5) + |> Enum.join(", ") + end + + @doc """ + Counts per verdict, for the summary line and the JSON payload. + + Every verdict is present whether or not it occurred, so a consumer reading + `.stale` never gets `nil` on the runs that found nothing — the same reason + `mix mob.mutate`'s summary has a fixed shape. + """ + @spec tally([finding()]) :: %{verdict() => non_neg_integer()} + def tally(findings) do + base = %{match: 0, stale: 0, missing: 0, unreadable: 0} + Enum.reduce(findings, base, fn f, acc -> Map.update!(acc, f.verdict, &(&1 + 1)) end) + end +end diff --git a/test/mix/tasks/mob_attest_test.exs b/test/mix/tasks/mob_attest_test.exs new file mode 100644 index 0000000..b5ee3fc --- /dev/null +++ b/test/mix/tasks/mob_attest_test.exs @@ -0,0 +1,44 @@ +defmodule Mix.Tasks.Mob.AttestTest do + @moduledoc """ + The reporting half of `mix mob.attest`. + + Per this repo's convention the task stays a thin unstubbed I/O wrapper and + the decisions live in `MobDev.Attest`; what is testable here is the shape of + what it hands back. + """ + use ExUnit.Case, async: true + + alias MobDev.Attest + + defp finding(verdict), do: %{module: Foo, verdict: verdict, expected: nil, actual: nil} + + describe "the tally that drives the summary line" do + test "a device running this build reports only matches" do + findings = List.duplicate(finding(:match), 55) + + assert Attest.tally(findings) == %{match: 55, stale: 0, missing: 0, unreadable: 0} + assert Attest.verdict(findings) == :ok + end + + test "the shape a real stale device produced" do + # 55 match / 12 stale, from the run that found MOB-161: a deploy that + # printed a tick while twelve modules on the device differed from the + # build. The numbers are the point — a check that only fires when + # everything is wrong would not have caught it. + findings = List.duplicate(finding(:match), 55) ++ List.duplicate(finding(:stale), 12) + + assert Attest.tally(findings) == %{match: 55, stale: 12, missing: 0, unreadable: 0} + assert {:error, message} = Attest.verdict(findings) + assert message =~ "12 module(s)" + end + + test "a mostly-unloaded bundle is still a pass" do + # Interactive BEAM loads a module when something first calls it, so this + # is the ordinary state of a freshly booted app, not a problem. + findings = [finding(:match)] ++ List.duplicate(finding(:missing), 400) + + assert Attest.verdict(findings) == :ok + assert Attest.tally(findings).missing == 400 + end + end +end diff --git a/test/mob_dev/attest_test.exs b/test/mob_dev/attest_test.exs new file mode 100644 index 0000000..f23066c --- /dev/null +++ b/test/mob_dev/attest_test.exs @@ -0,0 +1,140 @@ +defmodule MobDev.AttestTest do + @moduledoc """ + The comparison behind `mix mob.attest`. + + Written against a real failure: two bundle ids diverged, the BEAM push + addressed one app's container while another app was running, and the deploy + printed a tick. Both containers existed, so every step was honest about + itself and the run as a whole was a lie. Nothing in the toolchain could + contradict it. + """ + use ExUnit.Case, async: true + + alias MobDev.Attest + + @digest_a :crypto.hash(:md5, "a") + @digest_b :crypto.hash(:md5, "b") + + describe "compare/3" do + test "the same digest is a match" do + assert %{verdict: :match} = Attest.compare(Foo, @digest_a, @digest_a) + end + + test "a different digest is stale — this is the case the task exists for" do + finding = Attest.compare(Foo, @digest_a, @digest_b) + + assert finding.verdict == :stale + assert finding.expected == @digest_a + assert finding.actual == @digest_b + end + + test "a module the device never loaded is missing, not stale" do + # `module_info(:md5)` raises :undef for an unloaded module, which arrives + # as a badrpc EXIT. Interactive BEAM loads a module when something first + # calls it, so most of a bundle is legitimately unloaded — calling that + # stale would make the check cry wolf and get it turned off. + for remote <- [ + {:badrpc, {:EXIT, {:undef, []}}}, + {:badrpc, :nodedown}, + nil, + :undefined + ] do + assert %{verdict: :missing} = Attest.compare(Foo, @digest_a, remote) + end + end + + test "an undigestable local beam is unreadable, never a match" do + # Nothing can be concluded, and "cannot tell" must not read as agreement. + assert %{verdict: :unreadable} = Attest.compare(Foo, nil, @digest_a) + assert %{verdict: :unreadable} = Attest.compare(Foo, nil, nil) + end + end + + describe "verdict/1" do + defp finding(verdict), do: %{module: Foo, verdict: verdict, expected: nil, actual: nil} + + test "all matched is ok" do + assert Attest.verdict([finding(:match), finding(:match)]) == :ok + end + + test "unloaded modules alone do not fail the check" do + assert Attest.verdict([finding(:match), finding(:missing), finding(:missing)]) == :ok + end + + test "one stale module fails, however many matched" do + findings = List.duplicate(finding(:match), 500) ++ [finding(:stale)] + + assert {:error, message} = Attest.verdict(findings) + assert message =~ "do not match this build" + assert message =~ "running code you did not just push" + end + + test "a check that could not run does not pass" do + # Same rule as a deploy that exits 0 having shipped nothing: not being + # able to measure is not a success. + assert {:error, message} = Attest.verdict([finding(:match), finding(:unreadable)]) + assert message =~ "could not be digested" + end + + test "stale outranks unreadable in the message" do + assert {:error, message} = Attest.verdict([finding(:stale), finding(:unreadable)]) + assert message =~ "do not match this build" + end + + test "the named modules are sorted, so two runs diff cleanly" do + # Found by `mix mob.mutate`: deleting the sort left every test green. + # Report order that follows map iteration makes two runs of the same + # check look different, which is exactly when someone stops reading it. + findings = + for name <- [:Zeta, :Alpha, :Mid], do: %{finding(:stale) | module: name} + + assert {:error, message} = Attest.verdict(findings) + assert message =~ ":Alpha, :Mid, :Zeta" + end + + test "the message names modules but does not run away" do + findings = for n <- 1..50, do: %{finding(:stale) | module: :"Mod#{n}"} + + assert {:error, message} = Attest.verdict(findings) + assert message =~ "50 module(s)" + assert length(String.split(message, ", ")) <= 6 + end + end + + describe "tally/1" do + test "counts each verdict" do + findings = [finding(:match), finding(:match), finding(:stale), finding(:missing)] + + assert Attest.tally(findings) == %{match: 2, stale: 1, missing: 1, unreadable: 0} + end + + test "every key is present even when nothing was found" do + # A consumer reading `.stale` must not get nil on a clean run. + assert Attest.tally([]) == %{match: 0, stale: 0, missing: 0, unreadable: 0} + end + end + + describe "local_digest/1" do + test "digests a real beam, and agrees with module_info(:md5)" do + # The whole comparison rests on these two being the same number. If OTP + # ever changes that, this fails rather than every attestation silently + # reporting stale. + path = :code.which(MobDev.Attest) |> List.to_string() + + assert {MobDev.Attest, digest} = Attest.local_digest(path) + assert digest == MobDev.Attest.module_info(:md5) + end + + test "a file that is not a beam yields nil" do + path = Path.join(System.tmp_dir!(), "not_a_beam_#{System.unique_integer([:positive])}") + File.write!(path, "definitely not a beam") + on_exit(fn -> File.rm(path) end) + + assert Attest.local_digest(path) == nil + end + + test "a missing file yields nil" do + assert Attest.local_digest("/nope/missing.beam") == nil + end + end +end From 7c62ce72837745939d2b0566dc52f6605e3600f5 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sat, 5 Sep 2026 08:31:22 -0600 Subject: [PATCH 2/2] Act on the review: three ways attest reported green having checked nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial review — which this branch shipped without, and which I only ran when asked — found that the digest comparison was sound and everything wrapped around it was not. A node that went away mid-run scored every module `:missing`, and `:missing` was non-fatal. Four hundred dead-node modules produced `0 match, 0 stale, 400 not loaded` followed by "the device is running this build" and exit 0: zero evidence gathered, tick printed, which is precisely the failure this task was written to abolish, reproduced inside it. A test of mine asserted that behaviour, which is how it survived. Every badrpc other than `:undef` is now `:unreadable`, which is fatal. `verdict([])` returned `:ok`, so a typo'd `--app` globbed nothing, compared nothing and passed. An empty scope now refuses, and `--app` naming no beams refuses by name. Unreachable devices were printed and then dropped. With two phones attached and one wedged, attest checked the healthy one and emitted `"outcome": "ok"`. Unreachable is now carried into the report, into the JSON, and into the exit code. The default scope was wrong in a way that undercut this task's own evidence. It checked only the project's application, while `mix mob.deploy` pushes the whole runtime dependency tree — and MOB-161's twelve stale modules were in `mob`, a dependency. The default would have reported `:ok`; I found it only because I happened to pass `--app mob`, then wrote "found it on its first real use" into the changelog and the decision record as though the default would. It now defaults to exactly `HotPush.runtime_beam_dirs/0`, so what is attested cannot drift from what was shipped, and `--app` narrows. On device that moves the check from 67 modules to 2507 and catches the staleness with no flag. The `:missing` rationale was backwards, and measuring settled it. The doc said interactive BEAM loads lazily, so unloaded modules are normal and `:missing` must be tolerated. On a real device the code server IS interactive, so probing `module_info(:md5)` triggers the load and returns a digest — `:undef` therefore means the module is on no code path at all, which for something just pushed is a real failure. It is fatal now. Two consequences stated in the docs: the check is stronger than described, comparing the file the device would load rather than the resident set, and it has a side effect, since probing an unloaded module loads it. Node names were re-derived instead of using the one discovery already resolved. For WiFi-adb Android those disagree — adb id versus `ro.serialno` — a divergence this repo documents as a fixed bug in `discovery/android.ex`, and reintroducing it landed the device silently in the unreachable list. Now reads `device.node`. Also: `Node.connect/1` returns `:ignored` when the local node is not alive and `:ignored` is truthy, so every candidate could land in "reachable" and then fail every RPC; `--cookie`, since a project on a non-default cookie could never connect and was told to run `mix mob.connect` first; the run now reports what answered (code root, OTP release, code mode), because two containers from one project register the same node name and only one wins the EPMD slot; and the cap test did not test the cap, as the review demonstrated by mutating take(5) to take(6) and watching it pass. The task module had no behavioural tests at all — the mutation run I quoted covered only the pure scorer. `finding/2` now takes its digest fetcher, and it plus `outcome/2` and `module_from_path/1` are tested. Device-verified after the fixes: default scope catches 13 stale modules across 2507; `--node nope@127.0.0.1` exits 1; `--app mobb` refuses by name; `--app mob` still narrows to 55/12. Refs MOB-152 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 66 +++---- README.md | 36 ++++ .../2026-09-05-attest-what-is-running.md | 20 ++ lib/mix/tasks/mob.attest.ex | 180 +++++++++++++----- lib/mob_dev/attest.ex | 74 +++++-- test/mix/tasks/mob_attest_test.exs | 59 +++++- test/mob_dev/attest_test.exs | 59 ++++-- 7 files changed, 367 insertions(+), 127 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8964aaa..7c80476 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,3 @@ -# Changelog - -All notable changes to **mob_dev** are documented here. - -Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Versioning: [SemVer](https://semver.org/spec/v2.0.0.html). - -Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). - ---- - ## [Unreleased] ### Added @@ -18,11 +8,11 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). landed and was never loaded all show up. Exits non-zero on a mismatch, and also when the check could not run — a check that could not run is not a check that passed. Modules the device has not loaded yet are reported and are not a - failure. Written after a bundle-id divergence let a BEAM push succeed against - the wrong app's container, printing a tick while the app kept running old - code; it found a second live instance (MOB-161) on its first real use. - -### Added + failure. Defaults to the exact set `mix mob.deploy` pushes, so what is + attested cannot drift from what was shipped. Written after a bundle-id + divergence let a BEAM push succeed against the wrong app's container, + printing a tick while the app kept running old code; it found a second live + instance (MOB-161) on its first real use. - **`mix mob.mutate`** — mutation testing for the lines a branch changed. Breaks the production code one line at a time and reports the changes nothing @@ -33,13 +23,20 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). running, never supplied. `--file`, `--base`, `--test-command`, `--max`, `--json`; exits non-zero when anything survives. -### Fixed +- **`mix mob.deploy --json`** — a machine-readable result on stdout listing the + deployed, failed and skipped devices with their per-device reasons, and an + `outcome` that mirrors the exit status. Progress output is redirected to + stderr for the run, so `mix mob.deploy --json | jq` receives exactly one + document. Emitted on the native-build failure path too, which is when a + caller most needs it. -- **`mix mob.deploy` silently ignored unknown options.** `-d` was never aliased - to `--device` here — `mob.connect` has aliased it all along — so - `mix mob.deploy -d ` deployed to *every* connected device instead of - the one named, without a word. A typo'd `--devcie` did the same. Parsing is - strict now and the run refuses, naming the offending options. +- **`:ios_bundle_id`** in `mob.exs`, for when Android's `applicationId` is not a + legal Apple bundle id. Apple forbids the underscores Android allows, and + `com.example.*` is often already claimed by another Apple team, so the two + frequently cannot be the same string. Every iOS path — deploy, connect, + provision, battery bench, uninstall, the simulator and device builds, + code-signing, and the release IPA — resolves `:ios_bundle_id || :bundle_id`. + Android keeps using `:bundle_id`. ### Changed @@ -61,17 +58,6 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). A `--native` run that built the artifact and found no device to push it to still exits 0 — "build the APK now, attach the phone after" is unchanged. -### Added - -- **`mix mob.deploy --json`** — a machine-readable result on stdout listing the - deployed, failed and skipped devices with their per-device reasons, and an - `outcome` that mirrors the exit status. Progress output is redirected to - stderr for the run, so `mix mob.deploy --json | jq` receives exactly one - document. Emitted on the native-build failure path too, which is when a - caller most needs it. - -### Changed - - **`mix mob.deploy` now exits non-zero when a device fails.** A run that printed `Failed on 1 device(s)` previously still returned status 0, so CI and wrapper scripts read a failed deploy as success. Every device is still @@ -81,18 +67,14 @@ Full module documentation: [hexdocs.pm/mob_dev](https://hexdocs.pm/mob_dev). devices and has been passing, it may now fail** — check whether it was passing on a partial deploy. -### Added - -- **`:ios_bundle_id`** in `mob.exs`, for when Android's `applicationId` is not a - legal Apple bundle id. Apple forbids the underscores Android allows, and - `com.example.*` is often already claimed by another Apple team, so the two - frequently cannot be the same string. Every iOS path — deploy, connect, - provision, battery bench, uninstall, the simulator and device builds, - code-signing, and the release IPA — resolves `:ios_bundle_id || :bundle_id`. - Android keeps using `:bundle_id`. - ### Fixed +- **`mix mob.deploy` silently ignored unknown options.** `-d` was never aliased + to `--device` here — `mob.connect` has aliased it all along — so + `mix mob.deploy -d ` deployed to *every* connected device instead of + the one named, without a word. A typo'd `--devcie` did the same. Parsing is + strict now and the run refuses, naming the offending options. + - **iOS deploys installed the app under one bundle id and pushed BEAMs at another.** `:ios_bundle_id` was resolved by the build but discarded by the deployer and connector, so `mix mob.deploy --native --device ` installed diff --git a/README.md b/README.md index 8e3fc4d..182ad6a 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ end | `mix mob.watch` | Auto-push BEAMs on file save | | `mix mob.watch_stop` | Stop a running `mix mob.watch` | | `mix mob.devices` | List connected devices and their status | +| `mix mob.attest` | Prove a device is running the code you just pushed — compares module digests, not artifacts ([see below](#did-that-deploy-actually-land-mix-mobattest)) | | `mix mob.push` | Hot-push only changed modules (no restart) | | `mix mob.enable ...` | Wire up an optional Mob feature — platform-manifest entries, Elixir stubs, dep injections ([see below](#mix-mobenable-feature)) | | `mix mob.add_nif ` | Scaffold a statically-linked NIF — Elixir stub + `mob.exs` `:static_nifs` append + optional native skeleton ([see below](#mix-mobadd_nif-name)) | @@ -93,6 +94,41 @@ If dist is not reachable (first deploy, app not running), it falls back to `adb **Requirements:** The app must call `Mob.Dist.ensure_started/1` at startup, and the cookie must match the one in `mob.exs` (default `:mob_secret`). +## Did that deploy actually land? (`mix mob.attest`) + +`mix mob.deploy` reports what it *did*. It does not report what is now *true*, +and the two come apart more often than the exit code suggests. + +Two real cases: a bundle-id divergence sent the BEAM push into one app's +container while a *different* app was running — it succeeded and printed a tick, +because both containers existed on the device. And a plain dist deploy reported +success while twelve modules on the device kept their old code. + +```bash +mix mob.connect --no-iex # set up the tunnel +mix mob.attest # compare the device against this build +mix mob.attest --json # machine-readable, for CI or an agent +``` + +``` +mob_plugin_demo_ios_8a4250e9@127.0.0.1: 55 match, 12 stale, 0 not loaded, 0 unreadable + stale: Mob.Renderer + 12 module(s) on the device do not match this build. The app is running code + you did not just push. +``` + +It compares `module_info(:md5)` on the device against `:beam_lib.md5/1` of the +local `.beam` — the same digest for the same bytes. Deliberately **not** an +artifact hash: two builds of the same source differ in timestamps and paths, so +that would report a mismatch on every rebuild, and a check that cries wolf gets +switched off. + +Exits non-zero when a module differs, and also when the check could not run — +a check that could not run is not a check that passed. Modules the device has +not loaded yet are reported and are **not** a failure: interactive BEAM loads a +module when something first calls it, so most of a bundle is legitimately +unloaded at any moment. + ## `mix mob.enable ` Wires up an optional Mob feature in one command — platform-manifest diff --git a/decisions/2026-09-05-attest-what-is-running.md b/decisions/2026-09-05-attest-what-is-running.md index e17e4d1..e1f5bdb 100644 --- a/decisions/2026-09-05-attest-what-is-running.md +++ b/decisions/2026-09-05-attest-what-is-running.md @@ -65,3 +65,23 @@ shipped-set question and the two are complementary. It found MOB-161 on its first real use, which is the argument for building it and also a caution: the failure it found had been happening silently, and there is no way to know for how long. + +That sentence needs a correction, though. The first version of this task scoped +itself to the project's own application, and MOB-161's twelve stale modules +were in `mob` — a dependency. So the default would have reported `:ok`, and the +evidence for the design working was only obtainable by passing `--app mob`, +which nothing suggested doing. The default is now the exact set +`mix mob.deploy` pushes (`HotPush.runtime_beam_dirs/0`), so what is attested +and what was shipped cannot drift apart, and `--app` narrows rather than +replaces. + +A second correction, from the same review. This record originally justified +`:missing` being non-fatal with "interactive BEAM loads a module when something +first calls it, so most of a bundle is legitimately unloaded". Measured on a +device, the inference runs the other way: the code server *is* interactive, so +the probe itself triggers the load and returns a digest. `:undef` therefore +means the module is on no code path at all, which for something that was just +pushed is a real failure — it is now fatal. The check is correspondingly +stronger than described, comparing the file the device would load rather than +the resident set, and it has a side effect worth stating: probing an unloaded +module loads it. diff --git a/lib/mix/tasks/mob.attest.ex b/lib/mix/tasks/mob.attest.ex index 7b4c12e..360588c 100644 --- a/lib/mix/tasks/mob.attest.ex +++ b/lib/mix/tasks/mob.attest.ex @@ -23,8 +23,11 @@ defmodule Mix.Tasks.Mob.Attest do ## Options - * `--app NAME` — restrict to one application (default: the project's own) + * `--app NAME` — narrow to one application. The default is everything + `mix mob.deploy` pushes, which is the only scope that + cannot drift from what was actually shipped * `--node NAME` — attest one node instead of every connected one + * `--cookie C` — dist cookie (default: `mob_secret`, as `Mob.Dist` sets) * `--json` — machine-readable result on stdout, progress on stderr ## Exit status @@ -41,7 +44,7 @@ defmodule Mix.Tasks.Mob.Attest do alias MobDev.Attest - @switches [app: :string, node: :string, json: :boolean] + @switches [app: :string, node: :string, cookie: :string, json: :boolean] @impl Mix.Task def run(args) do @@ -53,14 +56,18 @@ defmodule Mix.Tasks.Mob.Attest do if opts[:json] do Process.put(:attest_stdout, Process.group_leader()) - Process.group_leader(self(), Process.whereis(:standard_error)) + + case Process.whereis(:standard_error) do + nil -> Mix.raise("--json needs :standard_error, which is not registered in this VM") + pid -> Process.group_leader(self(), pid) + end end Mix.Task.run("compile") - start_dist!() + start_dist!(String.to_atom(opts[:cookie] || "mob_secret")) case reachable_nodes(opts) do - [] -> + {[], _down} -> emit(opts, %{"outcome" => "no_nodes", "nodes" => []}) Mix.raise(""" @@ -71,9 +78,9 @@ defmodule Mix.Tasks.Mob.Attest do not a check that passed. """) - nodes -> + {nodes, down} -> results = Enum.map(nodes, &attest_node(&1, opts)) - report(results, opts) + report(results, down, opts) end end @@ -88,81 +95,155 @@ defmodule Mix.Tasks.Mob.Attest do name -> [String.to_atom(name)] end - {up, down} = Enum.split_with(candidates, &Node.connect/1) - - for node <- down do - IO.puts("#{node}: unreachable — run `mix mob.connect --no-iex` first") - end - - up + # `Node.connect/1` returns `true | false | :ignored`, and `:ignored` — the + # local node not being alive — is truthy. Matching on `true` keeps a run + # that could not connect at all from looking like a run that connected to + # everything. + Enum.split_with(candidates, &(Node.connect(&1) == true)) end + # Use the name discovery already resolved rather than deriving one again. + # `Device.node_name/1` and the discovery path disagree for WiFi-adb Android: + # the former builds a suffix from the adb id (`10.0.0.17:5555` -> + # `app_android_10_0_0_17`), the latter prefers `ro.serialno` + # (`app_android_zy22k6bsjm`). That divergence is documented in + # `discovery/android.ex` as a fixed bug; re-deriving reintroduced it, and the + # symptom was a device silently landing in the unreachable list. defp discover_node_names do (MobDev.Discovery.Android.list_devices() ++ MobDev.Discovery.IOS.list_devices()) - |> Enum.map(&MobDev.Device.node_name/1) + |> Enum.map(& &1.node) + |> Enum.reject(&is_nil/1) |> Enum.uniq() end - defp start_dist!() do + defp start_dist!(cookie) do unless Node.alive?() do MobDev.Connector.start_epmd() - - MobDev.Connector.handle_dist_start( - Node.start(:"mob_attest@127.0.0.1", :longnames), - :mob_secret - ) + MobDev.Connector.handle_dist_start(Node.start(:"mob_attest@127.0.0.1", :longnames), cookie) end end defp attest_node(node, opts) do - app = String.to_atom(opts[:app] || to_string(Mix.Project.config()[:app])) - findings = Enum.map(beams(app), &finding(node, &1)) + scope = opts[:app] || "everything mob.deploy pushes" + findings = Enum.map(beams(opts[:app]), &finding(&1, fn m -> remote_digest(node, m) end)) - %{node: node, app: app, findings: findings, verdict: Attest.verdict(findings)} + %{ + node: node, + app: scope, + identity: identity(node), + findings: findings, + verdict: Attest.verdict(findings) + } end - defp finding(node, path) do - case Attest.local_digest(path) do - nil -> - Attest.compare(module_from_path(path), nil, nil) + # Say WHAT answered, not just that something did. + # + # The bug this task was written for is two containers with divergent bundle + # ids built from one project: both register the same node-name pattern and + # whichever wins the EPMD slot is what you reach, which may not be the one + # you deployed to. Physical-device discovery also probes EPMD across the LAN, + # so a colleague running the same project name is reachable. An attestation + # that cannot name its subject proves less than it appears to. + defp identity(node) do + %{ + root: rpc_string(node, :init, :get_argument, [:root]), + otp: rpc_string(node, :erlang, :system_info, [:otp_release]), + code_mode: rpc_string(node, :code, :get_mode, []) + } + end + + defp rpc_string(node, m, f, a) do + case :rpc.call(node, m, f, a, 5_000) do + {:badrpc, _} -> "unknown" + value -> value |> inspect() |> String.slice(0, 120) + end + end - {module, expected} -> - Attest.compare(module, expected, remote_digest(node, module)) + @doc false + # Takes the digest fetcher so the wiring is testable without a device. This + # was the whole of the task's logic and none of it had a test: replacing + # `remote_digest` with `expected` would have reported 100% match on every + # run, with the suite green. + @spec finding(Path.t(), (module() -> term())) :: Attest.finding() + def finding(path, fetch) do + case Attest.local_digest(path) do + nil -> Attest.compare(module_from_path(path), nil, nil) + {module, expected} -> Attest.compare(module, expected, fetch.(module)) end end - # `module_info(:md5)` raises :undef for a module the device has never - # loaded, which arrives here as a badrpc EXIT rather than a value. + # `module_info(:md5)` raises :undef for a module the device cannot find, + # which arrives here as a badrpc EXIT rather than a value. defp remote_digest(node, module), do: :rpc.call(node, module, :module_info, [:md5], 10_000) - defp module_from_path(path), do: path |> Path.basename(".beam") |> String.to_atom() + @doc false + @spec module_from_path(Path.t()) :: module() + def module_from_path(path), do: path |> Path.basename(".beam") |> String.to_atom() + + # Default to exactly the set `mix mob.deploy` pushes, so what is attested and + # what was shipped cannot drift apart. Scoping to the project's own app — + # the first version of this — would have reported :ok on MOB-161, because + # those twelve stale modules were in `mob`, a dependency. The evidence for + # this task working was only obtainable with a non-default flag. + # + # `--app` narrows that set rather than replacing it, and an --app naming + # nothing raises: a glob that matches no files produced an empty finding + # list, and an empty finding list used to be a pass. + defp beams(nil) do + MobDev.HotPush.runtime_beam_dirs() + |> Enum.flat_map(&Path.wildcard(Path.join(&1, "*.beam"))) + end defp beams(app) do - Mix.Project.build_path() - |> Path.join("lib/#{app}/ebin/*.beam") - |> Path.wildcard() + case Enum.filter(beams(nil), &(Path.basename(Path.dirname(Path.dirname(&1))) == app)) do + [] -> + Mix.raise(""" + --app #{app} matched no beams in anything mob.deploy pushes. + + Attesting nothing and reporting success is the failure this task + exists to catch, so this refuses instead. Run without --app to check + everything, or `mix mob.devices` to confirm you are in the right + project. + """) + + beams -> + beams + end end - defp report(results, opts) do + defp report(results, down, opts) do Enum.each(results, &say_node/1) + Enum.each(down, &IO.puts("#{&1}: unreachable — nothing was checked on it")) failed = Enum.filter(results, &match?({:error, _}, &1.verdict)) emit(opts, %{ - "outcome" => if(failed == [], do: "ok", else: "mismatch"), - "nodes" => Enum.map(results, &json_node/1) + "outcome" => outcome(failed, down), + "nodes" => + Enum.map(results, &json_node/1) ++ + Enum.map(down, &%{"node" => to_string(&1), "outcome" => "unreachable"}) }) - unless failed == [] do - Mix.raise( - failed - |> Enum.map(fn r -> "#{r.node}: #{elem(r.verdict, 1)}" end) - |> Enum.join("\n") - ) - end + # A device that never answered is not a device that passed. Checking one + # phone while another sits wedged, and reporting "ok", is the same shape as + # a deploy that skips a target and exits 0. + messages = + Enum.map(failed, fn r -> "#{r.node}: #{elem(r.verdict, 1)}" end) ++ + Enum.map(down, fn n -> "#{n}: unreachable, so it was never checked" end) + + unless messages == [], do: Mix.raise(Enum.join(messages, "\n")) end - defp say_node(%{node: node, findings: findings, verdict: verdict}) do + @doc false + # What a CI job reads. An unreachable device must never leave this "ok": + # checking one phone while another sits wedged and reporting success is the + # same shape as a deploy that skips a target and exits 0. + @spec outcome([map()], [node()]) :: String.t() + def outcome([], []), do: "ok" + def outcome([], _down), do: "unreachable" + def outcome(_failed, _down), do: "mismatch" + + defp say_node(%{node: node, findings: findings, verdict: verdict, identity: id}) do t = Attest.tally(findings) IO.puts( @@ -170,6 +251,8 @@ defmodule Mix.Tasks.Mob.Attest do "#{t.unreadable} unreadable" ) + IO.puts(" answered by: root=#{id.root} otp=#{id.otp} code=#{id.code_mode}") + for f <- findings, f.verdict in [:stale, :unreadable] do IO.puts(" #{f.verdict}: #{inspect(f.module)}") end @@ -180,10 +263,11 @@ defmodule Mix.Tasks.Mob.Attest do end end - defp json_node(%{node: node, app: app, findings: findings, verdict: verdict}) do + defp json_node(%{node: node, app: app, findings: findings, verdict: verdict, identity: id}) do %{ "node" => to_string(node), "app" => to_string(app), + "identity" => %{"root" => id.root, "otp" => id.otp, "code_mode" => id.code_mode}, "outcome" => if(verdict == :ok, do: "ok", else: "mismatch"), "message" => if(verdict == :ok, do: nil, else: elem(verdict, 1)), "tally" => Map.new(Attest.tally(findings), fn {k, v} -> {to_string(k), v} end), diff --git a/lib/mob_dev/attest.ex b/lib/mob_dev/attest.ex index a67e7fc..d872016 100644 --- a/lib/mob_dev/attest.ex +++ b/lib/mob_dev/attest.ex @@ -29,6 +29,11 @@ defmodule MobDev.Attest do This deliberately does not hash whole files or directories: two builds of the same source differ in timestamps and paths, and a check that cries wolf gets turned off. + + Note the side effect: the device runs an interactive code server, so probing + a module it has not loaded causes it to load. That makes the comparison + stronger — it is the file on the code path, not just the resident set — at + the cost of nudging the thing being measured. """ @typedoc """ @@ -37,10 +42,12 @@ defmodule MobDev.Attest do * `:match` — the device is running the bytes we have. * `:stale` — it is running *something*, but not this. The push did not land, or landed somewhere else, or landed and was not loaded. - * `:missing` — the device has never loaded this module. Expected for a lazily - loaded module, damning for one the app needs to boot. - * `:unreadable` — the local `.beam` could not be digested, so nothing can be - concluded. Not a pass. + * `:missing` — the device answered `:undef`. On an interactive code server — + which is what Mob devices run — that means the module is on no code path + at all, because asking for it would otherwise have loaded it. For a module + `mix mob.deploy` pushed, that is a real failure, so it is fatal. + * `:unreadable` — the check could not run: the local `.beam` would not + digest, or the RPC failed for any reason other than `:undef`. Not a pass. """ @type verdict :: :match | :stale | :missing | :unreadable @@ -76,20 +83,27 @@ defmodule MobDev.Attest do def compare(module, nil, _remote), do: %{module: module, verdict: :unreadable, expected: nil, actual: nil} - def compare(module, expected, remote) do - case normalise_remote(remote) do - ^expected -> %{module: module, verdict: :match, expected: expected, actual: expected} - nil -> %{module: module, verdict: :missing, expected: expected, actual: nil} - actual -> %{module: module, verdict: :stale, expected: expected, actual: actual} - end + def compare(module, expected, digest) when is_binary(digest) do + verdict = if digest == expected, do: :match, else: :stale + %{module: module, verdict: verdict, expected: expected, actual: digest} end # A module the device has never loaded raises :undef, which arrives as a - # badrpc EXIT rather than a value. That is "missing", not "different" — the - # distinction matters because a lazily loaded module being absent is normal - # and a mismatched one never is. - defp normalise_remote(digest) when is_binary(digest), do: digest - defp normalise_remote(_other), do: nil + # badrpc EXIT. That is genuinely "not loaded". + def compare(module, expected, {:badrpc, {:EXIT, {:undef, _}}}), + do: %{module: module, verdict: :missing, expected: expected, actual: nil} + + # Every OTHER badrpc is the transport failing, and must never be scored as + # "not loaded" — `:missing` is non-fatal, so a node that went away mid-run + # produced 400 missing modules and a cheerful `:ok`. Zero evidence gathered, + # tick printed: the precise failure this module was written to abolish, + # reproduced inside it. An earlier test asserted the wrong behaviour here, + # which is how it survived. + def compare(module, expected, {:badrpc, _reason}), + do: %{module: module, verdict: :unreadable, expected: expected, actual: nil} + + def compare(module, expected, _unrecognised), + do: %{module: module, verdict: :unreadable, expected: expected, actual: nil} @doc """ Whether a set of findings means the deploy can be believed. @@ -101,14 +115,33 @@ defmodule MobDev.Attest do could not run must not report success — that is the same defect as a deploy exiting 0 having shipped nothing. - `:missing` is not fatal on its own. Interactive BEAM loads a module when - something first calls it, so a module that is shipped but not yet loaded is - the normal state of most of the bundle. It is reported, not failed on. + `:missing` is fatal, and the reason is the opposite of what an earlier version + of this doc claimed. That version said interactive BEAM loads a module on + first call, so most of a bundle is legitimately unloaded and `:missing` must + be tolerated. Measured on a real device, the inference runs the other way: + the code server *is* interactive, so asking for `module_info(:md5)` triggers + the load and returns a digest. `:undef` therefore does not mean "not loaded + yet" — it means the module is on no code path at all, which for something + `mix mob.deploy` pushed is a genuine failure. + + Two consequences worth stating plainly. The check is stronger than first + documented: it compares the file the device would load, not merely the set + that happens to be resident. And it has a side effect — probing an unloaded + module loads it. That is a small mutation of the thing being measured, and + it is the reason this connects without restarting: a restart would reload + everything and destroy far more than a probe does. """ @spec verdict([finding()]) :: :ok | {:error, String.t()} + def verdict([]), + # Nothing was compared, so nothing was proved. A green attestation over an + # empty set is worse than no attestation: it is the switched-off check that + # still reports. + do: {:error, "no modules were compared, so nothing was verified"} + def verdict(findings) do stale = Enum.filter(findings, &(&1.verdict == :stale)) unreadable = Enum.filter(findings, &(&1.verdict == :unreadable)) + missing = Enum.filter(findings, &(&1.verdict == :missing)) cond do stale != [] -> @@ -121,6 +154,11 @@ defmodule MobDev.Attest do "#{length(unreadable)} module(s) could not be digested locally: " <> name_list(unreadable) <> ". Nothing can be concluded about the device."} + missing != [] -> + {:error, + "#{length(missing)} module(s) are on no code path on the device: " <> + name_list(missing) <> ". They were pushed, and the device cannot find them."} + true -> :ok end diff --git a/test/mix/tasks/mob_attest_test.exs b/test/mix/tasks/mob_attest_test.exs index b5ee3fc..d03ece4 100644 --- a/test/mix/tasks/mob_attest_test.exs +++ b/test/mix/tasks/mob_attest_test.exs @@ -8,6 +8,7 @@ defmodule Mix.Tasks.Mob.AttestTest do """ use ExUnit.Case, async: true + alias Mix.Tasks.Mob.Attest, as: AttestTask alias MobDev.Attest defp finding(verdict), do: %{module: Foo, verdict: verdict, expected: nil, actual: nil} @@ -32,13 +33,63 @@ defmodule Mix.Tasks.Mob.AttestTest do assert message =~ "12 module(s)" end - test "a mostly-unloaded bundle is still a pass" do - # Interactive BEAM loads a module when something first calls it, so this - # is the ordinary state of a freshly booted app, not a problem. + test "modules the device cannot find fail the run" do + # An earlier version treated these as routine on the theory that + # interactive BEAM loads lazily. Measured on a device, the probe itself + # triggers the load — so :undef means the module is on no code path, and + # for something mob.deploy pushed that is a real failure. findings = [finding(:match)] ++ List.duplicate(finding(:missing), 400) - assert Attest.verdict(findings) == :ok + assert {:error, message} = Attest.verdict(findings) + assert message =~ "on no code path" assert Attest.tally(findings).missing == 400 end end + + describe "finding/2 — the wiring that had no tests" do + @beam :code.which(MobDev.Attest) |> List.to_string() + + test "compares the local digest against what the fetcher returns" do + # Replacing the fetcher with the expected value would report 100% match + # on every run; nothing exercised this path before. + {_, digest} = MobDev.Attest.local_digest(@beam) + + assert %{verdict: :match} = AttestTask.finding(@beam, fn _ -> digest end) + assert %{verdict: :stale} = AttestTask.finding(@beam, fn _ -> "other" end) + end + + test "a device that cannot answer is unreadable, not a match" do + assert %{verdict: :unreadable} = + AttestTask.finding(@beam, fn _ -> {:badrpc, :nodedown} end) + end + + test "an undigestable local file never calls the fetcher" do + path = Path.join(System.tmp_dir!(), "x_#{System.unique_integer([:positive])}.beam") + File.write!(path, "not a beam") + on_exit(fn -> File.rm(path) end) + + assert %{verdict: :unreadable} = + AttestTask.finding(path, fn _ -> raise "must not be asked" end) + end + + test "the module name comes from the file name" do + assert AttestTask.module_from_path("a/b/Elixir.Mob.Renderer.beam") == Mob.Renderer + end + end + + describe "outcome/2" do + test "clean run" do + assert AttestTask.outcome([], []) == "ok" + end + + test "an unreachable device is never ok" do + # Two devices, one wedged: checking the healthy one and reporting "ok" + # is the same shape as a deploy that skips a target and exits 0. + assert AttestTask.outcome([], [:a@b]) == "unreachable" + end + + test "a mismatch outranks an unreachable device" do + assert AttestTask.outcome([%{}], [:a@b]) == "mismatch" + end + end end diff --git a/test/mob_dev/attest_test.exs b/test/mob_dev/attest_test.exs index f23066c..e1405c1 100644 --- a/test/mob_dev/attest_test.exs +++ b/test/mob_dev/attest_test.exs @@ -28,21 +28,33 @@ defmodule MobDev.AttestTest do assert finding.actual == @digest_b end - test "a module the device never loaded is missing, not stale" do - # `module_info(:md5)` raises :undef for an unloaded module, which arrives - # as a badrpc EXIT. Interactive BEAM loads a module when something first - # calls it, so most of a bundle is legitimately unloaded — calling that - # stale would make the check cry wolf and get it turned off. - for remote <- [ - {:badrpc, {:EXIT, {:undef, []}}}, - {:badrpc, :nodedown}, - nil, - :undefined - ] do - assert %{verdict: :missing} = Attest.compare(Foo, @digest_a, remote) + test "an :undef answer is missing — the device has it on no code path" do + # Measured on a device: the code server is interactive, so asking for + # module_info(:md5) LOADS an unloaded module and returns a digest. + # :undef therefore does not mean "not resident yet", it means the module + # is not findable at all. + assert %{verdict: :missing} = + Attest.compare(Foo, @digest_a, {:badrpc, {:EXIT, {:undef, []}}}) + end + + test "a transport failure is unreadable, NOT missing" do + # The bug this replaces, and the one a previous version of this test + # actively pinned: every badrpc collapsed to :missing, :missing was + # non-fatal, and a node that went away mid-run produced hundreds of + # missing modules and a cheerful :ok. Zero evidence, tick printed. + for remote <- [{:badrpc, :nodedown}, {:badrpc, :timeout}, nil, :undefined] do + assert %{verdict: :unreadable} = Attest.compare(Foo, @digest_a, remote), + "#{inspect(remote)} means the check could not run, not that the module is absent" end end + test "a node that went away does not attest green" do + dead = for _ <- 1..400, do: Attest.compare(Foo, @digest_a, {:badrpc, :nodedown}) + + assert {:error, message} = Attest.verdict(dead) + assert message =~ "could not" + end + test "an undigestable local beam is unreadable, never a match" do # Nothing can be concluded, and "cannot tell" must not read as agreement. assert %{verdict: :unreadable} = Attest.compare(Foo, nil, @digest_a) @@ -57,8 +69,20 @@ defmodule MobDev.AttestTest do assert Attest.verdict([finding(:match), finding(:match)]) == :ok end - test "unloaded modules alone do not fail the check" do - assert Attest.verdict([finding(:match), finding(:missing), finding(:missing)]) == :ok + test "a module the device cannot find is a failure" do + # These were pushed. If the device answers :undef for one, on an + # interactive code server it is not on any code path — which is the + # thing a deploy was supposed to have put there. + assert {:error, message} = Attest.verdict([finding(:match), finding(:missing)]) + assert message =~ "on no code path" + end + + test "nothing compared is not a pass" do + # A typo'd --app used to glob no files, produce no findings, and report + # success. A green attestation over an empty set is the switched-off + # check that still reports. + assert {:error, message} = Attest.verdict([]) + assert message =~ "nothing was verified" end test "one stale module fails, however many matched" do @@ -97,7 +121,12 @@ defmodule MobDev.AttestTest do assert {:error, message} = Attest.verdict(findings) assert message =~ "50 module(s)" - assert length(String.split(message, ", ")) <= 6 + + # Count the names, not the commas. `String.split(message, ", ")` returned + # exactly 6 parts whether the cap was 5 or 6, because the last name is + # glued to the trailing sentence — so the cap test did not test the cap, + # which a review demonstrated by mutating take(5) to take(6). + assert length(Regex.scan(~r/:Mod\d+/, message)) == 5 end end