From d78a221607fe8f10d922832bfe499affc015c1fc Mon Sep 17 00:00:00 2001 From: Mathew Beane Date: Sun, 4 Oct 2026 16:50:10 -0400 Subject: [PATCH] feat(provision): read the WiFi password from a file or a prompt --password puts the WiFi password in argv, where ps and shell history can see it (#1754). This adds two ways to give it without that: - --password-file PATH reads it from a file, dropping one trailing newline (\n or \r\n). The file must exist and not be empty. If group or others can read it, the script refuses and suggests chmod 600. --allow-insecure-password-file accepts such a file with a warning, for read-only mounts such as 0444 secrets. - When --ssid is given with no password flag and stdin is a terminal, the script asks with getpass. It reuses a saved password only if it was saved for the same SSID. Without a terminal (scripts, CI) nothing is asked and the existing missing-credentials check applies. --password still works, but now prints a one-line warning on stderr that points at --password-file. The two flags are mutually exclusive. The firmware README examples now use --password-file. Tests: 10 new tests in tests/test_provision_secrets.py, with placeholder credentials and getpass and the flasher stubbed. 8 fail on the previous commit; the other 2 check that nothing is asked without a terminal or when a saved password matches the SSID. The suite goes from 25 to 35 passing. Refs #1754 Co-Authored-By: claude-flow --- firmware/esp32-csi-node/README.md | 15 ++- firmware/esp32-csi-node/provision.py | 70 +++++++++- .../tests/test_provision_secrets.py | 127 ++++++++++++++++++ 3 files changed, 209 insertions(+), 3 deletions(-) diff --git a/firmware/esp32-csi-node/README.md b/firmware/esp32-csi-node/README.md index 7448eedae..f8ac34e09 100644 --- a/firmware/esp32-csi-node/README.md +++ b/firmware/esp32-csi-node/README.md @@ -161,10 +161,16 @@ board default and emits no antenna-path claim. ### 3. Provision WiFi credentials (no reflash needed) ```bash +# Keep the WiFi password in an owner-only file, not on the command line. +printf '%s\n' 'YourPass' > ~/.wifi-pass && chmod 600 ~/.wifi-pass python firmware/esp32-csi-node/provision.py --port COM7 \ - --ssid "YourSSID" --password "YourPass" --target-ip 192.168.1.20 + --ssid "YourSSID" --password-file ~/.wifi-pass --target-ip 192.168.1.20 ``` +Leave out both `--password` and `--password-file` and the script asks for the +password when run from a terminal. `--password` still works, but it shows up in +`ps` and shell history. + ### 4. Start the sensing server ```bash @@ -440,10 +446,15 @@ The easiest way to write NVS settings: ```bash python firmware/esp32-csi-node/provision.py --port COM7 \ --ssid "MyWiFi" \ - --password "MyPassword" \ + --password-file ~/.wifi-pass \ --target-ip 192.168.1.20 ``` +The password file must be owner-only (`chmod 600`); one trailing newline is +dropped. Without a password flag the script prompts on a terminal; in scripts +and CI it doesn't prompt and fails if no password was given now or saved +earlier for that port. + ### NVS Key Reference #### Network Settings diff --git a/firmware/esp32-csi-node/provision.py b/firmware/esp32-csi-node/provision.py index 1c37c1135..87b45587d 100644 --- a/firmware/esp32-csi-node/provision.py +++ b/firmware/esp32-csi-node/provision.py @@ -41,6 +41,7 @@ ADDITIVE-BY-DEFAULT (issue #391, #574 phase 1): import argparse import csv +import getpass import io import json import os @@ -208,6 +209,43 @@ def redact_secrets(state: dict) -> dict: return shown +def read_password_file(path: str, allow_insecure: bool = False) -> str: + """Read the WiFi password from `path`, dropping one trailing newline. + + Keeps the password out of argv and shell history (#1754). Refuses a file + that group or others can read unless `allow_insecure` is set. Raises + ValueError with a message meant for the user. + """ + try: + with open(path, "rb") as f: + mode = stat.S_IMODE(os.fstat(f.fileno()).st_mode) + data = f.read() + except FileNotFoundError: + raise ValueError(f"--password-file {path} does not exist") from None + except OSError as exc: + raise ValueError(f"could not read --password-file {path}: {exc}") from None + if sys.platform != "win32" and mode & 0o044: + if not allow_insecure: + raise ValueError( + f"--password-file {path} is readable by group or others " + f"(mode {oct(mode)}). Run 'chmod 600 {path}', or pass " + f"--allow-insecure-password-file." + ) + print(f"WARNING: --password-file {path} is readable by group or others " + f"(mode {oct(mode)}).", file=sys.stderr) + try: + text = data.decode("utf-8") + except UnicodeDecodeError: + raise ValueError(f"--password-file {path} is not UTF-8 text") from None + if text.endswith("\r\n"): + text = text[:-2] + elif text.endswith("\n"): + text = text[:-1] + if not text: + raise ValueError(f"--password-file {path} is empty") + return text + + def load_state(port: str, state_dir: str) -> dict: """Return the merged-state dict for `port`, or `{}` if absent / unreadable.""" path = _state_path_for(port, state_dir) @@ -425,7 +463,16 @@ def main(): ) parser.add_argument("--baud", type=int, default=460800, help="Flash baud rate (default: 460800)") parser.add_argument("--ssid", help="WiFi SSID") - parser.add_argument("--password", help="WiFi password") + password_source = parser.add_mutually_exclusive_group() + password_source.add_argument("--password", + help="WiFi password. Visible in ps and shell history; " + "prefer --password-file.") + password_source.add_argument("--password-file", metavar="PATH", + help="Read the WiFi password from PATH (one trailing newline " + "is dropped). The file must not be readable by group or others.") + parser.add_argument("--allow-insecure-password-file", action="store_true", + help="Accept a --password-file that group or others can read, with a " + "warning (e.g. a read-only 0444 secrets mount).") parser.add_argument("--target-ip", help="Aggregator host IP (e.g. 192.168.1.20)") parser.add_argument("--target-port", type=int, help="Aggregator UDP port (default: 5005)") parser.add_argument("--node-id", type=int, help="Node ID 0-255 (default: 1)") @@ -479,6 +526,17 @@ def main(): # or early exit (#1754). harden_state_dir(args.state_dir) + if args.password_file is not None: + try: + args.password = read_password_file( + args.password_file, args.allow_insecure_password_file) + except ValueError as exc: + parser.error(str(exc)) + elif args.password is not None: + print("WARNING: --password is visible in ps and shell history; " + "use --password-file instead.", file=sys.stderr) + cli_ssid, cli_password = args.ssid, args.password + # --- Per-port state load + merge (additive-by-default, #391 / #574) --- if args.reset: path = _state_path_for(args.port, args.state_dir) @@ -490,6 +548,16 @@ def main(): prior = load_state(args.port, args.state_dir) merged = merge_state_into_args(args, prior) + # A new --ssid with no password given: ask for it on a terminal instead of + # requiring it on the command line. The saved password is reused only when + # it belongs to the same SSID. Without a terminal (scripts, CI) nothing is + # asked and the WiFi-credential check below applies as before. + if (not args.state and cli_ssid is not None and cli_password is None + and (prior.get("password") is None or prior.get("ssid") != cli_ssid) + and sys.stdin.isatty()): + args.password = getpass.getpass(f"WiFi password for {cli_ssid}: ") + merged["password"] = args.password + if args.state: shown = merged if args.show_secrets else redact_secrets(merged) print(json.dumps(shown, indent=2, sort_keys=True)) diff --git a/firmware/esp32-csi-node/tests/test_provision_secrets.py b/firmware/esp32-csi-node/tests/test_provision_secrets.py index 65c0e39e0..847fe4a1a 100644 --- a/firmware/esp32-csi-node/tests/test_provision_secrets.py +++ b/firmware/esp32-csi-node/tests/test_provision_secrets.py @@ -214,5 +214,132 @@ class TestIntermediateArtifacts(_TempDirCase): self.assertEqual(seen["csv_mode"], 0o600) +class TestPasswordInput(_TempDirCase): + """Ways to give the password without putting it on the command line.""" + + BASE = ("--port", "COM7", "--target-ip", "192.0.2.10") + + def setUp(self): + super().setUp() + self.flashed_csv = [] + + def password_file(self, content, mode=0o600): + path = os.path.join(self.root, "wifi-pass.txt") + with open(path, "w", encoding="utf-8", newline="") as f: + f.write(content) + os.chmod(path, mode) + return path + + def provision(self, *argv, tty=False, typed=None): + """Run main() with flashing stubbed; return (code, stderr, getpass mock).""" + def fake_generate(csv_content, _size): + self.flashed_csv.append(csv_content) + return b"nvs" + + stdin = mock.Mock() + stdin.isatty.return_value = tty + # read_chip_mac only exists once state is keyed by board MAC (#1755); + # stub it so these tests never reach esptool or a serial port. + with mock.patch.object(provision, "generate_nvs_binary", side_effect=fake_generate), \ + mock.patch.object(provision, "flash_nvs"), \ + mock.patch.object(provision, "read_chip_mac", return_value=None, + create=True), \ + mock.patch.object(sys, "stdin", stdin), \ + mock.patch("getpass.getpass", return_value=typed) as prompt: + code, _, err = self.run_main(*self.BASE, *argv, "--state-dir", self.state_dir) + return code, err, prompt + + def flashed_password(self): + rows = provision.csv.DictReader(io.StringIO(self.flashed_csv[-1])) + return {row["key"]: row["value"] for row in rows}.get("password") + + def test_password_file_is_read_and_one_newline_dropped(self): + path = self.password_file(FAKE_PASSWORD + " \n\n") + + code, err, _ = self.provision("--ssid", "test-ssid", "--password-file", path) + + self.assertEqual(code, 0) + # Only one newline goes; trailing spaces and the second newline stay. + self.assertEqual(self.flashed_password(), FAKE_PASSWORD + " \n") + self.assertNotIn("visible in ps", err) + + def test_password_file_crlf_counts_as_one_newline(self): + path = self.password_file(FAKE_PASSWORD + "\r\n") + self.provision("--ssid", "test-ssid", "--password-file", path) + self.assertEqual(self.flashed_password(), FAKE_PASSWORD) + + @POSIX_ONLY + def test_group_or_world_readable_password_file_is_refused(self): + for mode in (0o644, 0o640, 0o604): + with self.subTest(mode=oct(mode)): + path = self.password_file(FAKE_PASSWORD + "\n", mode) + code, err, _ = self.provision("--ssid", "test-ssid", "--password-file", path) + self.assertEqual(code, 2) + self.assertIn("chmod 600", err) + + @POSIX_ONLY + def test_insecure_password_file_allowed_with_flag_and_warning(self): + path = self.password_file(FAKE_PASSWORD + "\n", 0o644) + + code, err, _ = self.provision("--ssid", "test-ssid", "--password-file", path, + "--allow-insecure-password-file") + + self.assertEqual(code, 0) + self.assertIn("readable by group or others", err) + self.assertEqual(self.flashed_password(), FAKE_PASSWORD) + + def test_missing_or_empty_password_file_is_an_error(self): + missing = os.path.join(self.root, "nope.txt") + code, err, _ = self.provision("--ssid", "test-ssid", "--password-file", missing) + self.assertEqual(code, 2) + self.assertIn("does not exist", err) + + empty = self.password_file("\n") + code, err, _ = self.provision("--ssid", "test-ssid", "--password-file", empty) + self.assertEqual(code, 2) + self.assertIn("is empty", err) + + def test_password_and_password_file_are_mutually_exclusive(self): + path = self.password_file(FAKE_PASSWORD) + + code, err, _ = self.provision("--ssid", "test-ssid", "--password", FAKE_PASSWORD, + "--password-file", path) + + self.assertEqual(code, 2) + self.assertIn("not allowed with argument", err) + + def test_password_on_command_line_warns(self): + code, err, _ = self.provision("--ssid", "test-ssid", "--password", FAKE_PASSWORD) + + self.assertEqual(code, 0) + self.assertIn("--password is visible in ps and shell history", err) + self.assertIn("--password-file", err) + + def test_terminal_prompts_for_password_of_a_new_ssid(self): + code, _, prompt = self.provision("--ssid", "test-ssid", tty=True, typed=FAKE_PASSWORD) + + self.assertEqual(code, 0) + prompt.assert_called_once() + self.assertEqual(self.flashed_password(), FAKE_PASSWORD) + self.assertEqual(provision.load_state("COM7", self.state_dir)["password"], FAKE_PASSWORD) + + def test_no_prompt_when_saved_password_matches_the_ssid(self): + provision.save_state("COM7", self.state_dir, + {"ssid": "test-ssid", "password": FAKE_PASSWORD}) + + code, _, prompt = self.provision("--ssid", "test-ssid", tty=True, typed="unused") + + self.assertEqual(code, 0) + prompt.assert_not_called() + self.assertEqual(self.flashed_password(), FAKE_PASSWORD) + + def test_no_prompt_without_a_terminal(self): + code, err, prompt = self.provision("--ssid", "test-ssid", tty=False, typed="unused") + + prompt.assert_not_called() + self.assertEqual(code, 2) + self.assertIn("--password", err) + + if __name__ == "__main__": unittest.main()