From 0f3fd9e886f1ff4e8ffdec30acea4ff9504ce9d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 17:24:01 +0000 Subject: [PATCH 1/5] Initial plan From 062e3f57c7b4c1706d3b1ef692e27f8a6973df82 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 17:34:06 +0000 Subject: [PATCH 2/5] Add support for changing the fwup task via SSH Co-authored-by: fhunleth <64669+fhunleth@users.noreply.github.com> --- README.md | 62 ++++++++++++++ lib/mix/tasks/firmware.gen.script.ex | 76 +++++++++++++++-- lib/mix/tasks/upload.ex | 21 ++++- lib/ssh_subsystem_fwup.ex | 72 ++++++++++++++++- test/ssh_subsystem_fwup_test.exs | 117 ++++++++++++++++++++++++++- 5 files changed, 338 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 3a87417..4f9ef9e 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,66 @@ equivalent: cat $firmware | ssh -s $nerves_device fwup ``` +To run a specific task (like `complete` instead of the default `upgrade`), use +the `fwup:` subsystem name: + +```shell +cat $firmware | ssh -s $nerves_device fwup:complete +``` + +## Running different fwup tasks + +By default, uploads run the `upgrade` fwup task. For advanced use cases, you +may want to run different tasks like: + +* `complete` - Completely re-image the device (useful for recovering or initial setup) +* `ops` - Run operations firmware for partition validation, data erasure, or U-Boot updates + +### Configuring the server + +To support multiple tasks, register additional subsystem specs on the device: + +```elixir +:ssh.daemon(@port, [ + {:subsystems, [ + SSHSubsystemFwup.subsystem_spec(devpath: devpath), + SSHSubsystemFwup.subsystem_spec(devpath: devpath, name: "fwup:complete", task: "complete"), + SSHSubsystemFwup.subsystem_spec(devpath: devpath, name: "fwup:ops", task: "ops") + ]} +]) +``` + +Or use the convenience function to register multiple tasks at once: + +```elixir +:ssh.daemon(@port, [ + {:subsystems, SSHSubsystemFwup.subsystem_specs( + devpath: devpath, + tasks: ["upgrade", "complete", "ops"] + )} +]) +``` + +### Uploading with a specific task + +Using `mix upload`: + +```shell +mix upload nerves.local --task complete +``` + +Using `upload.sh`: + +```shell +./upload.sh --task complete nerves.local +``` + +Using raw ssh: + +```shell +cat my_firmware.fw | ssh -s nerves.local fwup:complete +``` + ## Configuration The default options should satisfy most use cases, but it's possible to alter @@ -130,6 +190,8 @@ The following options are available: * `:fwup_env` - a list of name,value tuples to be passed to the OS environment for fwup * `:fwup_extra_options` - additional options to pass to fwup like for setting public keys +* `:name` - the subsystem name. Defaults to `"fwup"`. Set to `"fwup:"` to + create a subsystem that runs a specific task when clients connect using that name. * `:precheck_callback` - an MFArgs to call when there's a connection. If specified, the callback will be passed the username and the current set of options. If allowed, it should return `{:ok, new_options}`. Any other return diff --git a/lib/mix/tasks/firmware.gen.script.ex b/lib/mix/tasks/firmware.gen.script.ex index 2b6def8..202db0e 100644 --- a/lib/mix/tasks/firmware.gen.script.ex +++ b/lib/mix/tasks/firmware.gen.script.ex @@ -25,7 +25,11 @@ defmodule Mix.Tasks.Firmware.Gen.Script do # Upload new firmware to a device running ssh_subsystem_fwup # # Usage: - # upload.sh [destination IP] [Path to .fw file] + # upload.sh [options] [destination IP] [Path to .fw file] + # + # Options: + # --task Specify the fwup task to run (e.g., "upgrade", "complete", "ops") + # Default: No task specified (uses server default, typically "upgrade") # # If unspecified, the destination is nerves.local and the .fw file is naively # guessed @@ -52,12 +56,65 @@ defmodule Mix.Tasks.Firmware.Gen.Script do set -e - DESTINATION=$1 - FILENAME="$2" + TASK="" + DESTINATION="" + FILENAME="" + + # Parse arguments + while [ $# -gt 0 ]; do + case "$1" in + --task) + if [ -n "$2" ] && [ "${2#-}" = "$2" ]; then + TASK="$2" + shift 2 + else + echo "Error: --task requires a value" + exit 1 + fi + ;; + --help|-h) + echo "Usage: upload.sh [options] [destination IP] [Path to .fw file]" + echo + echo "Options:" + echo " --task Specify the fwup task to run (e.g., upgrade, complete, ops)" + echo " Default: No task specified (uses server default)" + echo " --help Show this help message" + echo + echo "Default destination IP is 'nerves.local'" + echo "Default firmware bundle is the first .fw file in '_build/\\${MIX_TARGET}_\\${MIX_ENV}/nerves/images'" + echo + echo "Examples:" + echo " ./upload.sh # Upload to nerves.local" + echo " ./upload.sh 192.168.1.100 # Upload to specific IP" + echo " ./upload.sh --task complete nerves.local # Run complete task" + echo " ./upload.sh --task ops 192.168.1.100 my.fw # Run ops task with specific firmware" + exit 0 + ;; + -*) + echo "Error: Unknown option $1" + exit 1 + ;; + *) + # Positional arguments + if [ -z "$DESTINATION" ]; then + DESTINATION="$1" + elif [ -z "$FILENAME" ]; then + FILENAME="$1" + else + echo "Error: Too many arguments" + exit 1 + fi + shift + ;; + esac + done help() { echo - echo "upload.sh [destination IP] [Path to .fw file]" + echo "upload.sh [options] [destination IP] [Path to .fw file]" + echo + echo "Options:" + echo " --task Specify the fwup task to run" echo echo "Default destination IP is 'nerves.local'" echo "Default firmware bundle is the first .fw file in '_build/\\${MIX_TARGET}_\\${MIX_ENV}/nerves/images'" @@ -108,10 +165,19 @@ defmodule Mix.Tasks.Firmware.Gen.Script do echo "Product: $FIRMWARE_PRODUCT $FIRMWARE_VERSION" echo "UUID: $FIRMWARE_UUID" echo "Platform: $FIRMWARE_PLATFORM" + + # Determine the subsystem name based on task + if [ -n "$TASK" ]; then + SUBSYSTEM="fwup:$TASK" + echo "Task: $TASK" + else + SUBSYSTEM="fwup" + fi + echo echo "Uploading to $DESTINATION..." - cat "$FILENAME" | ssh -s $SSH_OPTIONS $DESTINATION fwup + cat "$FILENAME" | ssh -s $SSH_OPTIONS $DESTINATION "$SUBSYSTEM" """ @spec run(keyword()) :: :ok diff --git a/lib/mix/tasks/upload.ex b/lib/mix/tasks/upload.ex index e8a0445..aaf263c 100644 --- a/lib/mix/tasks/upload.ex +++ b/lib/mix/tasks/upload.ex @@ -26,6 +26,10 @@ defmodule Mix.Tasks.Upload do * `--firmware` - The path to a fw file * `--port` - An alternative TCP port to use for the upload (defaults to 22) + * `--task` - The fwup task to run on the device (defaults to "upgrade"). + Use this to run alternative tasks like "complete" or custom tasks defined + in your firmware. The device must be configured to support the specified + task (see `SSHSubsystemFwup.subsystem_specs/1`). ## Examples @@ -37,11 +41,16 @@ defmodule Mix.Tasks.Upload do mix upload 192.168.1.120 --firmware _build/rpi0_prod/nerves/images/app.fw + Run the complete task instead of upgrade: + + MIX_TARGET=rpi0 mix upload nerves.local --task complete + """ @switches [ firmware: :string, - port: :integer + port: :integer, + task: :string ] @doc false @@ -66,18 +75,21 @@ defmodule Mix.Tasks.Upload do port = opts[:port] || 22 validate_port!(port) + task = opts[:task] + subsystem = if task, do: "fwup:#{task}", else: "fwup" + firmware_path = firmware(opts) Mix.shell().info(""" Path: #{firmware_path} #{maybe_print_firmware_uuid(firmware_path)} - Uploading to #{ip}:#{port}... + #{maybe_print_task(task)}Uploading to #{ip}:#{port}... """) # LD_LIBRARY_PATH is unset to avoid errors with host ssl (see commit 9b1df471) {_, status} = InteractiveCmd.shell( - "cat #{shell_quote(firmware_path)} | ssh -p #{port} -s -- #{shell_quote(ip)} fwup", + "cat #{shell_quote(firmware_path)} | ssh -p #{port} -s -- #{shell_quote(ip)} #{shell_quote(subsystem)}", env: [{"LD_LIBRARY_PATH", false}] ) @@ -178,5 +190,8 @@ defmodule Mix.Tasks.Upload do _, _ -> "" end + defp maybe_print_task(nil), do: "" + defp maybe_print_task(task), do: "Task: #{task}\n" + defp shell_quote(str), do: "'" <> String.replace(str, "'", "'\"'\"'") <> "'" end diff --git a/lib/ssh_subsystem_fwup.ex b/lib/ssh_subsystem_fwup.ex index 349c5b9..27e45f0 100644 --- a/lib/ssh_subsystem_fwup.ex +++ b/lib/ssh_subsystem_fwup.ex @@ -50,6 +50,8 @@ defmodule SSHSubsystemFwup do * `:fwup_env` - a list of name,value tuples to be passed to the OS environment for fwup * `:fwup_extra_options` - additional options to pass to fwup like for setting public keys + * `:name` - the subsystem name. Defaults to `"fwup"`. Can be set to `"fwup:"` + to create a subsystem that runs a specific task when clients connect to it. * `:precheck_callback` - an MFArgs to call when there's a connection. If specified, the callback will be passed the username and the current set of options. If allowed, it should return `{:ok, new_options}`. Any other @@ -63,6 +65,7 @@ defmodule SSHSubsystemFwup do fwup_path: Path.t(), fwup_env: [{String.t(), String.t()}], fwup_extra_options: [String.t()], + name: String.t(), precheck_callback: mfargs() | nil, task: String.t(), success_callback: mfargs() @@ -74,10 +77,77 @@ defmodule SSHSubsystemFwup do @doc """ Helper for creating the SSH subsystem spec + + This creates a single subsystem spec. By default, the subsystem is named + `"fwup"`. Use the `:name` option to set a custom name. A common pattern is + to create multiple subsystem specs for different tasks: + + ```elixir + :ssh.daemon([ + {:subsystems, [ + SSHSubsystemFwup.subsystem_spec(), + SSHSubsystemFwup.subsystem_spec(name: "fwup:complete", task: "complete"), + SSHSubsystemFwup.subsystem_spec(name: "fwup:ops", task: "ops") + ]} + ]) + ``` + + Then clients can connect using `ssh -s device fwup:complete` to run a + specific task. """ @spec subsystem_spec(options()) :: :ssh.subsystem_spec() def subsystem_spec(options \\ []) do - {~c"fwup", {__MODULE__, options}} + name = Keyword.get(options, :name, "fwup") + {to_charlist(name), {__MODULE__, options}} + end + + @doc """ + Helper for creating multiple SSH subsystem specs for different tasks + + This is a convenience function that creates subsystem specs for multiple + tasks. The first task in the list will be registered as both `fwup` (the + default subsystem name) and `fwup:`. Subsequent tasks will only + be registered as `fwup:`. + + Example: + + ```elixir + :ssh.daemon([ + {:subsystems, SSHSubsystemFwup.subsystem_specs( + devpath: "/dev/mmcblk0", + tasks: ["upgrade", "complete", "ops"] + )} + ]) + ``` + + This creates subsystem specs for: + - `fwup` and `fwup:upgrade` - both run the "upgrade" task + - `fwup:complete` - runs the "complete" task + - `fwup:ops` - runs the "ops" task + + Options are the same as `subsystem_spec/1`, except `:tasks` replaces `:task` + and `:name` is automatically set based on the task name. + """ + @spec subsystem_specs(Keyword.t()) :: [:ssh.subsystem_spec()] + def subsystem_specs(options \\ []) do + {tasks, base_options} = Keyword.pop(options, :tasks, ["upgrade"]) + base_options = Keyword.delete(base_options, :name) + + tasks + |> Enum.with_index() + |> Enum.flat_map(fn {task, index} -> + task_options = Keyword.put(base_options, :task, task) + + if index == 0 do + # First task gets both the default "fwup" name and "fwup:" + [ + subsystem_spec(Keyword.put(task_options, :name, "fwup")), + subsystem_spec(Keyword.put(task_options, :name, "fwup:#{task}")) + ] + else + [subsystem_spec(Keyword.put(task_options, :name, "fwup:#{task}"))] + end + end) end @impl :ssh_client_channel diff --git a/test/ssh_subsystem_fwup_test.exs b/test/ssh_subsystem_fwup_test.exs index 27ec684..7ac7159 100644 --- a/test/ssh_subsystem_fwup_test.exs +++ b/test/ssh_subsystem_fwup_test.exs @@ -42,12 +42,31 @@ defmodule SSHSubsystemFwupTest do end) end + def start_sshd_with_subsystems(subsystem_specs, devpath) do + {:ok, ref} = + :ssh.daemon(@port, [ + {:max_sessions, 1}, + {:user_passwords, [{~c"user", ~c"password"}]}, + {:system_dir, ~c"test/fixtures"}, + {:subsystems, subsystem_specs} + ]) + + on_exit(fn -> + :ssh.stop_daemon(ref) + devpath && File.rm!(devpath) + end) + end + def do_ssh(payload) do + do_ssh(payload, ~c"fwup") + end + + def do_ssh(payload, subsystem_name) do connect_opts = [silently_accept_hosts: true, user: ~c"user", password: ~c"password"] {:ok, connection_ref} = :ssh.connect(:localhost, @port, connect_opts) {:ok, channel_id} = :ssh_connection.session_channel(connection_ref, 500) - :success = :ssh_connection.subsystem(connection_ref, channel_id, ~c"fwup", 500) + :success = :ssh_connection.subsystem(connection_ref, channel_id, subsystem_name, 500) # Sending data can fail if the remote side closes first. That's what happens # when the remote reports a fatal error and that's expected. @@ -290,4 +309,100 @@ defmodule SSHSubsystemFwupTest do # Check that the update was applied assert match?(<<"Hello, world!", _::binary>>, File.read!(options[:devpath])) end + + test "using named subsystem with fwup:task syntax", context do + devpath = Path.join(@tmpdir, "#{context.test}.img") + options = [ + success_callback: {Kernel, :send, [self(), :success]}, + devpath: devpath + ] + + File.touch!(devpath) + + # Register subsystems using the name: option + subsystems = [ + SSHSubsystemFwup.subsystem_spec(options ++ [name: "fwup"]), + SSHSubsystemFwup.subsystem_spec(options ++ [name: "fwup:myupgrade", task: "myupgrade"]) + ] + + start_sshd_with_subsystems(subsystems, devpath) + fw_contents = Fwup.create_firmware(task: "myupgrade") + + capture_log(fn -> + {output, exit_status} = do_ssh(fw_contents, ~c"fwup:myupgrade") + + assert exit_status == 0 + assert output =~ "Success!" + end) + + # Check that the success function was called + assert_receive :success + + # Check that the update was applied + assert match?(<<"Hello, world!", _::binary>>, File.read!(devpath)) + end + + test "subsystem_specs generates multiple subsystem specs", context do + devpath = Path.join(@tmpdir, "#{context.test}.img") + + File.touch!(devpath) + + subsystems = SSHSubsystemFwup.subsystem_specs( + devpath: devpath, + success_callback: {Kernel, :send, [self(), :success]}, + tasks: ["upgrade", "complete"] + ) + + # Check that we get the expected number of subsystems + # First task generates 2 (fwup and fwup:upgrade), second task generates 1 (fwup:complete) + assert length(subsystems) == 3 + + # Check subsystem names + names = Enum.map(subsystems, fn {name, _} -> to_string(name) end) + assert "fwup" in names + assert "fwup:upgrade" in names + assert "fwup:complete" in names + + start_sshd_with_subsystems(subsystems, devpath) + + # Test the default fwup subsystem (runs upgrade task) + fw_contents = Fwup.create_firmware(task: "upgrade") + + capture_log(fn -> + {output, exit_status} = do_ssh(fw_contents) + + assert exit_status == 0 + assert output =~ "Success!" + end) + + assert_receive :success + assert match?(<<"Hello, world!", _::binary>>, File.read!(devpath)) + end + + test "subsystem_specs allows using named task via fwup:taskname", context do + devpath = Path.join(@tmpdir, "#{context.test}.img") + + File.touch!(devpath) + + subsystems = SSHSubsystemFwup.subsystem_specs( + devpath: devpath, + success_callback: {Kernel, :send, [self(), :success]}, + tasks: ["upgrade", "mytask"] + ) + + start_sshd_with_subsystems(subsystems, devpath) + + # Test using fwup:mytask subsystem + fw_contents = Fwup.create_firmware(task: "mytask") + + capture_log(fn -> + {output, exit_status} = do_ssh(fw_contents, ~c"fwup:mytask") + + assert exit_status == 0 + assert output =~ "Success!" + end) + + assert_receive :success + assert match?(<<"Hello, world!", _::binary>>, File.read!(devpath)) + end end From c1a1a158dca8930502a6db8a93f31b6eaff3d7f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 17:37:49 +0000 Subject: [PATCH 3/5] Address code review feedback for shell script Co-authored-by: fhunleth <64669+fhunleth@users.noreply.github.com> --- lib/mix/tasks/firmware.gen.script.ex | 80 ++++++++++++++-------------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/lib/mix/tasks/firmware.gen.script.ex b/lib/mix/tasks/firmware.gen.script.ex index 202db0e..0ca7971 100644 --- a/lib/mix/tasks/firmware.gen.script.ex +++ b/lib/mix/tasks/firmware.gen.script.ex @@ -60,34 +60,51 @@ defmodule Mix.Tasks.Firmware.Gen.Script do DESTINATION="" FILENAME="" + show_help() { + echo + echo "Usage: upload.sh [options] [destination IP] [Path to .fw file]" + echo + echo "Options:" + echo " --task Specify the fwup task to run (e.g., upgrade, complete, ops)" + echo " Default: No task specified (uses server default)" + echo " --help Show this help message" + echo + echo "Default destination IP is 'nerves.local'" + echo "Default firmware bundle is the first .fw file in '_build/\\${MIX_TARGET}_\\${MIX_ENV}/nerves/images'" + echo + echo "Examples:" + echo " ./upload.sh # Upload to nerves.local" + echo " ./upload.sh 192.168.1.100 # Upload to specific IP" + echo " ./upload.sh --task complete nerves.local # Run complete task" + echo " ./upload.sh --task ops 192.168.1.100 my.fw # Run ops task with specific firmware" + echo + echo "Environment:" + echo " MIX_TARGET=$MIX_TARGET" + echo " MIX_ENV=$MIX_ENV" + } + # Parse arguments while [ $# -gt 0 ]; do case "$1" in --task) - if [ -n "$2" ] && [ "${2#-}" = "$2" ]; then - TASK="$2" - shift 2 - else - echo "Error: --task requires a value" - exit 1 - fi + # Check that $2 exists and doesn't start with a dash + case "$2" in + ""|-) + echo "Error: --task requires a value" + exit 1 + ;; + -*) + echo "Error: --task requires a value" + exit 1 + ;; + *) + TASK="$2" + shift 2 + ;; + esac ;; --help|-h) - echo "Usage: upload.sh [options] [destination IP] [Path to .fw file]" - echo - echo "Options:" - echo " --task Specify the fwup task to run (e.g., upgrade, complete, ops)" - echo " Default: No task specified (uses server default)" - echo " --help Show this help message" - echo - echo "Default destination IP is 'nerves.local'" - echo "Default firmware bundle is the first .fw file in '_build/\\${MIX_TARGET}_\\${MIX_ENV}/nerves/images'" - echo - echo "Examples:" - echo " ./upload.sh # Upload to nerves.local" - echo " ./upload.sh 192.168.1.100 # Upload to specific IP" - echo " ./upload.sh --task complete nerves.local # Run complete task" - echo " ./upload.sh --task ops 192.168.1.100 my.fw # Run ops task with specific firmware" + show_help exit 0 ;; -*) @@ -109,21 +126,6 @@ defmodule Mix.Tasks.Firmware.Gen.Script do esac done - help() { - echo - echo "upload.sh [options] [destination IP] [Path to .fw file]" - echo - echo "Options:" - echo " --task Specify the fwup task to run" - echo - echo "Default destination IP is 'nerves.local'" - echo "Default firmware bundle is the first .fw file in '_build/\\${MIX_TARGET}_\\${MIX_ENV}/nerves/images'" - echo - echo "MIX_TARGET=$MIX_TARGET" - echo "MIX_ENV=$MIX_ENV" - exit 1 - } - [ -n "$DESTINATION" ] || DESTINATION=nerves.local if [ -z "$FILENAME" ]; then [ -n "$MIX_TARGET" ] || MIX_TARGET=rpi0 @@ -150,10 +152,10 @@ defmodule Mix.Tasks.Firmware.Gen.Script do fi FILENAME=$(ls "$FIRMWARE_PATH/"*.fw 2> /dev/null | head -n 1) - [ -n "$FILENAME" ] || (echo "Error: error determining firmware bundle."; help) + [ -n "$FILENAME" ] || (echo "Error: error determining firmware bundle."; show_help; exit 1) fi - [ -f "$FILENAME" ] || (echo "Error: can't find '$FILENAME'"; help) + [ -f "$FILENAME" ] || (echo "Error: can't find '$FILENAME'"; show_help; exit 1) FIRMWARE_METADATA=$(fwup -m -i "$FILENAME" || echo "meta-product=Error reading metadata!") FIRMWARE_PRODUCT=$(echo "$FIRMWARE_METADATA" | grep -E "^meta-product=" -m 1 2>/dev/null | cut -d '=' -f 2- | tr -d '"') From fbfa351b27268b73f9938b6c7aaa7ef53e657255 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 20:18:39 +0000 Subject: [PATCH 4/5] Use SSH SendEnv option instead of multiple subsystems for task selection Co-authored-by: fhunleth <64669+fhunleth@users.noreply.github.com> --- README.md | 38 ++------- lib/mix/tasks/firmware.gen.script.ex | 9 ++- lib/mix/tasks/upload.ex | 8 +- lib/ssh_subsystem_fwup.ex | 111 +++++++++---------------- test/ssh_subsystem_fwup_test.exs | 116 +++++++-------------------- 5 files changed, 86 insertions(+), 196 deletions(-) diff --git a/README.md b/README.md index 4f9ef9e..a169615 100644 --- a/README.md +++ b/README.md @@ -96,10 +96,10 @@ cat $firmware | ssh -s $nerves_device fwup ``` To run a specific task (like `complete` instead of the default `upgrade`), use -the `fwup:` subsystem name: +SSH's `SendEnv` option to pass the `FWUP_TASK` environment variable: ```shell -cat $firmware | ssh -s $nerves_device fwup:complete +FWUP_TASK=complete cat $firmware | ssh -o SendEnv=FWUP_TASK -s $nerves_device fwup ``` ## Running different fwup tasks @@ -110,30 +110,8 @@ may want to run different tasks like: * `complete` - Completely re-image the device (useful for recovering or initial setup) * `ops` - Run operations firmware for partition validation, data erasure, or U-Boot updates -### Configuring the server - -To support multiple tasks, register additional subsystem specs on the device: - -```elixir -:ssh.daemon(@port, [ - {:subsystems, [ - SSHSubsystemFwup.subsystem_spec(devpath: devpath), - SSHSubsystemFwup.subsystem_spec(devpath: devpath, name: "fwup:complete", task: "complete"), - SSHSubsystemFwup.subsystem_spec(devpath: devpath, name: "fwup:ops", task: "ops") - ]} -]) -``` - -Or use the convenience function to register multiple tasks at once: - -```elixir -:ssh.daemon(@port, [ - {:subsystems, SSHSubsystemFwup.subsystem_specs( - devpath: devpath, - tasks: ["upgrade", "complete", "ops"] - )} -]) -``` +No special server configuration is needed to support different tasks. The +client specifies the task using SSH's `SendEnv` option. ### Uploading with a specific task @@ -152,7 +130,7 @@ Using `upload.sh`: Using raw ssh: ```shell -cat my_firmware.fw | ssh -s nerves.local fwup:complete +FWUP_TASK=complete cat my_firmware.fw | ssh -o SendEnv=FWUP_TASK -s nerves.local fwup ``` ## Configuration @@ -190,15 +168,15 @@ The following options are available: * `:fwup_env` - a list of name,value tuples to be passed to the OS environment for fwup * `:fwup_extra_options` - additional options to pass to fwup like for setting public keys -* `:name` - the subsystem name. Defaults to `"fwup"`. Set to `"fwup:"` to - create a subsystem that runs a specific task when clients connect using that name. * `:precheck_callback` - an MFArgs to call when there's a connection. If specified, the callback will be passed the username and the current set of options. If allowed, it should return `{:ok, new_options}`. Any other return value closes the connection. * `:success_callback` - an MFArgs to call when a firmware update completes successfully. Defaults to `{Nerves.Runtime, :reboot, []}`. -* `:task` - the task to run in the firmware update. Defaults to `"upgrade"` +* `:task` - the task to run in the firmware update. Defaults to `"upgrade"`. + This can be overridden by clients using SSH's `SendEnv` option to pass the + `FWUP_TASK` environment variable. ## License diff --git a/lib/mix/tasks/firmware.gen.script.ex b/lib/mix/tasks/firmware.gen.script.ex index 0ca7971..e41dea6 100644 --- a/lib/mix/tasks/firmware.gen.script.ex +++ b/lib/mix/tasks/firmware.gen.script.ex @@ -168,18 +168,19 @@ defmodule Mix.Tasks.Firmware.Gen.Script do echo "UUID: $FIRMWARE_UUID" echo "Platform: $FIRMWARE_PLATFORM" - # Determine the subsystem name based on task + # Set up task environment variable and SSH options if task is specified if [ -n "$TASK" ]; then - SUBSYSTEM="fwup:$TASK" + export FWUP_TASK="$TASK" + SEND_ENV_OPT="-o SendEnv=FWUP_TASK" echo "Task: $TASK" else - SUBSYSTEM="fwup" + SEND_ENV_OPT="" fi echo echo "Uploading to $DESTINATION..." - cat "$FILENAME" | ssh -s $SSH_OPTIONS $DESTINATION "$SUBSYSTEM" + cat "$FILENAME" | ssh -s $SEND_ENV_OPT $SSH_OPTIONS $DESTINATION fwup """ @spec run(keyword()) :: :ok diff --git a/lib/mix/tasks/upload.ex b/lib/mix/tasks/upload.ex index aaf263c..7ceb3bd 100644 --- a/lib/mix/tasks/upload.ex +++ b/lib/mix/tasks/upload.ex @@ -28,8 +28,7 @@ defmodule Mix.Tasks.Upload do * `--port` - An alternative TCP port to use for the upload (defaults to 22) * `--task` - The fwup task to run on the device (defaults to "upgrade"). Use this to run alternative tasks like "complete" or custom tasks defined - in your firmware. The device must be configured to support the specified - task (see `SSHSubsystemFwup.subsystem_specs/1`). + in your firmware. ## Examples @@ -76,7 +75,8 @@ defmodule Mix.Tasks.Upload do validate_port!(port) task = opts[:task] - subsystem = if task, do: "fwup:#{task}", else: "fwup" + task_env = if task, do: "FWUP_TASK=#{task} ", else: "" + send_env_opt = if task, do: "-o SendEnv=FWUP_TASK ", else: "" firmware_path = firmware(opts) @@ -89,7 +89,7 @@ defmodule Mix.Tasks.Upload do # LD_LIBRARY_PATH is unset to avoid errors with host ssl (see commit 9b1df471) {_, status} = InteractiveCmd.shell( - "cat #{shell_quote(firmware_path)} | ssh -p #{port} -s -- #{shell_quote(ip)} #{shell_quote(subsystem)}", + "#{task_env}cat #{shell_quote(firmware_path)} | ssh #{send_env_opt}-p #{port} -s -- #{shell_quote(ip)} fwup", env: [{"LD_LIBRARY_PATH", false}] ) diff --git a/lib/ssh_subsystem_fwup.ex b/lib/ssh_subsystem_fwup.ex index 27e45f0..7493e65 100644 --- a/lib/ssh_subsystem_fwup.ex +++ b/lib/ssh_subsystem_fwup.ex @@ -50,22 +50,21 @@ defmodule SSHSubsystemFwup do * `:fwup_env` - a list of name,value tuples to be passed to the OS environment for fwup * `:fwup_extra_options` - additional options to pass to fwup like for setting public keys - * `:name` - the subsystem name. Defaults to `"fwup"`. Can be set to `"fwup:"` - to create a subsystem that runs a specific task when clients connect to it. * `:precheck_callback` - an MFArgs to call when there's a connection. If specified, the callback will be passed the username and the current set of options. If allowed, it should return `{:ok, new_options}`. Any other return value closes the connection. * `:success_callback` - an MFArgs to call when a firmware update completes successfully. Defaults to `{Nerves.Runtime, :reboot, []}`. - * `:task` - the task to run in the firmware update. Defaults to `"upgrade"` + * `:task` - the task to run in the firmware update. Defaults to `"upgrade"`. + This can be overridden by clients using SSH's `SendEnv` option to pass the + `FWUP_TASK` environment variable. """ @type options :: [ devpath: Path.t(), fwup_path: Path.t(), fwup_env: [{String.t(), String.t()}], fwup_extra_options: [String.t()], - name: String.t(), precheck_callback: mfargs() | nil, task: String.t(), success_callback: mfargs() @@ -78,76 +77,23 @@ defmodule SSHSubsystemFwup do @doc """ Helper for creating the SSH subsystem spec - This creates a single subsystem spec. By default, the subsystem is named - `"fwup"`. Use the `:name` option to set a custom name. A common pattern is - to create multiple subsystem specs for different tasks: + Clients can override the task by using SSH's `SendEnv` option to pass the + `FWUP_TASK` environment variable. For example: - ```elixir - :ssh.daemon([ - {:subsystems, [ - SSHSubsystemFwup.subsystem_spec(), - SSHSubsystemFwup.subsystem_spec(name: "fwup:complete", task: "complete"), - SSHSubsystemFwup.subsystem_spec(name: "fwup:ops", task: "ops") - ]} - ]) + ```shell + FWUP_TASK=complete cat firmware.fw | ssh -o SendEnv=FWUP_TASK -s device fwup ``` - Then clients can connect using `ssh -s device fwup:complete` to run a - specific task. - """ - @spec subsystem_spec(options()) :: :ssh.subsystem_spec() - def subsystem_spec(options \\ []) do - name = Keyword.get(options, :name, "fwup") - {to_charlist(name), {__MODULE__, options}} - end - - @doc """ - Helper for creating multiple SSH subsystem specs for different tasks - - This is a convenience function that creates subsystem specs for multiple - tasks. The first task in the list will be registered as both `fwup` (the - default subsystem name) and `fwup:`. Subsequent tasks will only - be registered as `fwup:`. - - Example: + Or using the provided upload tools: - ```elixir - :ssh.daemon([ - {:subsystems, SSHSubsystemFwup.subsystem_specs( - devpath: "/dev/mmcblk0", - tasks: ["upgrade", "complete", "ops"] - )} - ]) + ```shell + mix upload nerves.local --task complete + ./upload.sh --task complete nerves.local ``` - - This creates subsystem specs for: - - `fwup` and `fwup:upgrade` - both run the "upgrade" task - - `fwup:complete` - runs the "complete" task - - `fwup:ops` - runs the "ops" task - - Options are the same as `subsystem_spec/1`, except `:tasks` replaces `:task` - and `:name` is automatically set based on the task name. """ - @spec subsystem_specs(Keyword.t()) :: [:ssh.subsystem_spec()] - def subsystem_specs(options \\ []) do - {tasks, base_options} = Keyword.pop(options, :tasks, ["upgrade"]) - base_options = Keyword.delete(base_options, :name) - - tasks - |> Enum.with_index() - |> Enum.flat_map(fn {task, index} -> - task_options = Keyword.put(base_options, :task, task) - - if index == 0 do - # First task gets both the default "fwup" name and "fwup:" - [ - subsystem_spec(Keyword.put(task_options, :name, "fwup")), - subsystem_spec(Keyword.put(task_options, :name, "fwup:#{task}")) - ] - else - [subsystem_spec(Keyword.put(task_options, :name, "fwup:#{task}"))] - end - end) + @spec subsystem_spec(options()) :: :ssh.subsystem_spec() + def subsystem_spec(options \\ []) do + {~c"fwup", {__MODULE__, options}} end @impl :ssh_client_channel @@ -158,7 +104,7 @@ defmodule SSHSubsystemFwup do |> Keyword.merge(Application.get_all_env(:ssh_subsystem_fwup)) |> Keyword.merge(options) - {:ok, %{state: :running_fwup, id: nil, cm: nil, fwup: nil, options: combined_options}} + {:ok, %{state: :running_fwup, id: nil, cm: nil, fwup: nil, options: combined_options, env: %{}}} end defp default_options() do @@ -175,11 +121,18 @@ defmodule SSHSubsystemFwup do @impl :ssh_client_channel def handle_msg({:ssh_channel_up, channel_id, cm}, state) do - with {:ok, options} <- precheck(state.options[:precheck_callback], state.options), + # Check if FWUP_TASK was set via SendEnv and override the task option + options = + case Map.get(state.env, "FWUP_TASK") do + nil -> state.options + task -> Keyword.put(state.options, :task, task) + end + + with {:ok, options} <- precheck(options[:precheck_callback], options), :ok <- check_devpath(options[:devpath]) do - Logger.debug("ssh_subsystem_fwup: starting fwup") + Logger.debug("ssh_subsystem_fwup: starting fwup with task #{options[:task]}") fwup = FwupPort.open_port(options) - {:ok, %{state | id: channel_id, cm: cm, fwup: fwup}} + {:ok, %{state | id: channel_id, cm: cm, fwup: fwup, options: options}} else {:error, reason} -> _ = :ssh_connection.send(cm, channel_id, "Error: #{reason}") @@ -230,6 +183,20 @@ defmodule SSHSubsystemFwup do {:ok, state} end + def handle_ssh_msg({:ssh_cm, cm, {:env, channel_id, want_reply, var, value}}, state) do + # Store environment variable (convert charlists to strings if needed) + var_str = if is_list(var), do: to_string(var), else: var + value_str = if is_list(value), do: to_string(value), else: value + new_env = Map.put(state.env, var_str, value_str) + + # Reply if requested + if want_reply do + :ssh_connection.reply_request(cm, want_reply, :success, channel_id) + end + + {:ok, %{state | env: new_env}} + end + def handle_ssh_msg({:ssh_cm, _cm, {:eof, _channel_id}}, state) do {:ok, state} end diff --git a/test/ssh_subsystem_fwup_test.exs b/test/ssh_subsystem_fwup_test.exs index 7ac7159..2aaea26 100644 --- a/test/ssh_subsystem_fwup_test.exs +++ b/test/ssh_subsystem_fwup_test.exs @@ -42,31 +42,22 @@ defmodule SSHSubsystemFwupTest do end) end - def start_sshd_with_subsystems(subsystem_specs, devpath) do - {:ok, ref} = - :ssh.daemon(@port, [ - {:max_sessions, 1}, - {:user_passwords, [{~c"user", ~c"password"}]}, - {:system_dir, ~c"test/fixtures"}, - {:subsystems, subsystem_specs} - ]) - - on_exit(fn -> - :ssh.stop_daemon(ref) - devpath && File.rm!(devpath) - end) - end - def do_ssh(payload) do - do_ssh(payload, ~c"fwup") + do_ssh(payload, []) end - def do_ssh(payload, subsystem_name) do + def do_ssh(payload, env) when is_list(env) do connect_opts = [silently_accept_hosts: true, user: ~c"user", password: ~c"password"] {:ok, connection_ref} = :ssh.connect(:localhost, @port, connect_opts) {:ok, channel_id} = :ssh_connection.session_channel(connection_ref, 500) - :success = :ssh_connection.subsystem(connection_ref, channel_id, subsystem_name, 500) + + # Send environment variables before starting the subsystem + for {var, value} <- env do + :ssh_connection.setenv(connection_ref, channel_id, to_charlist(var), to_charlist(value), 500) + end + + :success = :ssh_connection.subsystem(connection_ref, channel_id, ~c"fwup", 500) # Sending data can fail if the remote side closes first. That's what happens # when the remote reports a fatal error and that's expected. @@ -310,26 +301,18 @@ defmodule SSHSubsystemFwupTest do assert match?(<<"Hello, world!", _::binary>>, File.read!(options[:devpath])) end - test "using named subsystem with fwup:task syntax", context do - devpath = Path.join(@tmpdir, "#{context.test}.img") - options = [ - success_callback: {Kernel, :send, [self(), :success]}, - devpath: devpath - ] - - File.touch!(devpath) + test "FWUP_TASK environment variable overrides default task", context do + options = default_options(context.test) + File.touch!(options[:devpath]) - # Register subsystems using the name: option - subsystems = [ - SSHSubsystemFwup.subsystem_spec(options ++ [name: "fwup"]), - SSHSubsystemFwup.subsystem_spec(options ++ [name: "fwup:myupgrade", task: "myupgrade"]) - ] + start_sshd(options) - start_sshd_with_subsystems(subsystems, devpath) - fw_contents = Fwup.create_firmware(task: "myupgrade") + # Create firmware with a custom task + fw_contents = Fwup.create_firmware(task: "custom_task") capture_log(fn -> - {output, exit_status} = do_ssh(fw_contents, ~c"fwup:myupgrade") + # Send FWUP_TASK environment variable to override the task + {output, exit_status} = do_ssh(fw_contents, [{"FWUP_TASK", "custom_task"}]) assert exit_status == 0 assert output =~ "Success!" @@ -339,70 +322,31 @@ defmodule SSHSubsystemFwupTest do assert_receive :success # Check that the update was applied - assert match?(<<"Hello, world!", _::binary>>, File.read!(devpath)) + assert match?(<<"Hello, world!", _::binary>>, File.read!(options[:devpath])) end - test "subsystem_specs generates multiple subsystem specs", context do - devpath = Path.join(@tmpdir, "#{context.test}.img") - - File.touch!(devpath) - - subsystems = SSHSubsystemFwup.subsystem_specs( - devpath: devpath, - success_callback: {Kernel, :send, [self(), :success]}, - tasks: ["upgrade", "complete"] - ) - - # Check that we get the expected number of subsystems - # First task generates 2 (fwup and fwup:upgrade), second task generates 1 (fwup:complete) - assert length(subsystems) == 3 - - # Check subsystem names - names = Enum.map(subsystems, fn {name, _} -> to_string(name) end) - assert "fwup" in names - assert "fwup:upgrade" in names - assert "fwup:complete" in names + test "FWUP_TASK environment variable overrides configured task", context do + # Configure with "upgrade" task but override via env to "override_task" + options = default_options(context.test) ++ [task: "upgrade"] + File.touch!(options[:devpath]) - start_sshd_with_subsystems(subsystems, devpath) + start_sshd(options) - # Test the default fwup subsystem (runs upgrade task) - fw_contents = Fwup.create_firmware(task: "upgrade") + # Create firmware with the override task + fw_contents = Fwup.create_firmware(task: "override_task") capture_log(fn -> - {output, exit_status} = do_ssh(fw_contents) + # Send FWUP_TASK environment variable to override the configured task + {output, exit_status} = do_ssh(fw_contents, [{"FWUP_TASK", "override_task"}]) assert exit_status == 0 assert output =~ "Success!" end) + # Check that the success function was called assert_receive :success - assert match?(<<"Hello, world!", _::binary>>, File.read!(devpath)) - end - - test "subsystem_specs allows using named task via fwup:taskname", context do - devpath = Path.join(@tmpdir, "#{context.test}.img") - - File.touch!(devpath) - - subsystems = SSHSubsystemFwup.subsystem_specs( - devpath: devpath, - success_callback: {Kernel, :send, [self(), :success]}, - tasks: ["upgrade", "mytask"] - ) - - start_sshd_with_subsystems(subsystems, devpath) - - # Test using fwup:mytask subsystem - fw_contents = Fwup.create_firmware(task: "mytask") - - capture_log(fn -> - {output, exit_status} = do_ssh(fw_contents, ~c"fwup:mytask") - - assert exit_status == 0 - assert output =~ "Success!" - end) - assert_receive :success - assert match?(<<"Hello, world!", _::binary>>, File.read!(devpath)) + # Check that the update was applied + assert match?(<<"Hello, world!", _::binary>>, File.read!(options[:devpath])) end end From 6546aebc4956b0c0d885a641002867722df07906 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 1 Dec 2025 20:20:49 +0000 Subject: [PATCH 5/5] Add validation for FWUP_TASK environment variable Co-authored-by: fhunleth <64669+fhunleth@users.noreply.github.com> --- lib/ssh_subsystem_fwup.ex | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/ssh_subsystem_fwup.ex b/lib/ssh_subsystem_fwup.ex index 7493e65..e9f2a04 100644 --- a/lib/ssh_subsystem_fwup.ex +++ b/lib/ssh_subsystem_fwup.ex @@ -125,7 +125,8 @@ defmodule SSHSubsystemFwup do options = case Map.get(state.env, "FWUP_TASK") do nil -> state.options - task -> Keyword.put(state.options, :task, task) + task when is_binary(task) -> Keyword.put(state.options, :task, validate_task(task)) + _ -> state.options end with {:ok, options} <- precheck(options[:precheck_callback], options), @@ -184,10 +185,18 @@ defmodule SSHSubsystemFwup do end def handle_ssh_msg({:ssh_cm, cm, {:env, channel_id, want_reply, var, value}}, state) do - # Store environment variable (convert charlists to strings if needed) + # Convert charlists to strings if needed var_str = if is_list(var), do: to_string(var), else: var value_str = if is_list(value), do: to_string(value), else: value - new_env = Map.put(state.env, var_str, value_str) + + # Only accept FWUP_TASK environment variable for security + new_env = + if var_str == "FWUP_TASK" do + Map.put(state.env, var_str, value_str) + else + Logger.debug("ssh_subsystem_fwup: ignoring environment variable #{var_str}") + state.env + end # Reply if requested if want_reply do @@ -249,6 +258,17 @@ defmodule SSHSubsystemFwup do {:ok, state} end + # Validate task name to only contain safe characters (alphanumeric, underscore, hyphen) + # to prevent potential command injection + defp validate_task(task) when is_binary(task) do + if Regex.match?(~r/^[a-zA-Z0-9_-]+$/, task) do + task + else + Logger.warning("ssh_subsystem_fwup: invalid task name #{inspect(task)}, using default") + "upgrade" + end + end + defp check_devpath(devpath) do if is_binary(devpath) and File.exists?(devpath) do :ok