mirror of
https://github.com/ruvnet/RuView.git
synced 2026-10-22 06:32:35 -05:00
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 <ruv@ruv.net>
This commit is contained in:
1 parent
c00deadaf7
commit
d78a221607
3 files changed
+209
-3
No files matched your search
@@ -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
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user