diff --git a/plugins/openclaw/slash_sleep.py b/plugins/openclaw/slash_sleep.py index c8576661..f0f84cd7 100755 --- a/plugins/openclaw/slash_sleep.py +++ b/plugins/openclaw/slash_sleep.py @@ -19,6 +19,7 @@ import json import os import shutil +import subprocess import sys from pathlib import Path from datetime import datetime @@ -104,8 +105,8 @@ def run_category(category: str, *, dry_run: bool = False) -> int: print(f"=== /sleep run {category}{' (dry-run)' if dry_run else ''} ===") print(f" cmd: {' '.join(cmd)}") - rc = os.system(" ".join(f'"{c}"' for c in cmd)) - return rc + result = subprocess.run(cmd) + return result.returncode def run_all(*, dry_run: bool = False) -> int: diff --git a/requirements.txt b/requirements.txt index 5db9e702..2c5f13ba 100644 --- a/requirements.txt +++ b/requirements.txt @@ -24,7 +24,7 @@ httpx>=0.27.0 # json_repair>=0.61.0 # ── Optional: WebUI dashboard ──────────────────── -# gradio>=4.0.0 +# gradio>=5.50.0 # ── Optional: Documentation site ───────────────── # mkdocs-material>=9.5.0 diff --git a/skillopt/envs/spreadsheetbench/rollout.py b/skillopt/envs/spreadsheetbench/rollout.py index aff78938..edf8b96c 100644 --- a/skillopt/envs/spreadsheetbench/rollout.py +++ b/skillopt/envs/spreadsheetbench/rollout.py @@ -314,7 +314,8 @@ def process_one( # ── Stage 1: run ReAct agent on test case 1 ───────────────────── result["phase"] = "agent" - work_dir = tempfile.mkdtemp(prefix=f"react_{task_id}_") + safe_task_id = "".join(c if c.isalnum() or c in "-_" else "_" for c in str(task_id)) + work_dir = tempfile.mkdtemp(prefix=f"react_{safe_task_id}_") try: # Copy input so agent works in an isolated directory work_input = os.path.join(work_dir, os.path.basename(ip1)) diff --git a/skillopt_webui/app.py b/skillopt_webui/app.py index 75e0ef1d..9e7e0a5a 100644 --- a/skillopt_webui/app.py +++ b/skillopt_webui/app.py @@ -24,6 +24,38 @@ PROJECT_ROOT = Path(__file__).resolve().parent.parent + +def _ensure_under_project(path: Path) -> Path: + """Resolve *path* and fail closed unless it stays under PROJECT_ROOT.""" + resolved = path.resolve() + try: + resolved.relative_to(PROJECT_ROOT.resolve()) + except ValueError: + raise ValueError(f"Path escapes project root: {path}") + return resolved + + +def _resolve_project_path(user_path: str, *, subdir: str | None = None) -> Path: + """Resolve a UI-supplied path, optionally constrained to a project subdir. + + UI callbacks must never trust Gradio component values (e.g. dropdown text): + traversal, absolute paths, and symlinks are all rejected here, at the point + the path is about to be consumed. + """ + if not user_path: + raise ValueError("Path is empty") + candidate = Path(user_path) + if not candidate.is_absolute(): + candidate = PROJECT_ROOT / candidate + resolved = _ensure_under_project(candidate) + if subdir is not None: + allowed = (PROJECT_ROOT / subdir).resolve() + try: + resolved.relative_to(allowed) + except ValueError: + raise ValueError(f"Path must be under {subdir!r}: {user_path}") + return resolved + # Gradio moved where `theme` lives across versions: <=5 uses `Blocks(theme=...)`, # >=6 moved it to `launch()`. Detect the installed major so the WebUI works on # any supported version without an ignored-argument warning or a TypeError. @@ -42,15 +74,86 @@ def discover_configs() -> list[str]: def load_config(path: str) -> dict: """Load a YAML config file.""" - with open(PROJECT_ROOT / path) as f: + config_file = _resolve_project_path(path, subdir="configs") + with open(config_file) as f: return yaml.safe_load(f) +def scan_outputs(out_dir: str) -> list: + """Digest experiment results strictly under PROJECT_ROOT. + + The Output Explorer callback. Any path that escapes PROJECT_ROOT is + rejected (empty result) at the point data is read, so a traversal arg + can never read files outside the project. + """ + rows = [] + if not out_dir: + return rows + try: + base = _resolve_project_path(out_dir) + except ValueError: + return rows + if not base.exists() or not base.is_dir(): + return rows + for bench_dir in sorted(base.iterdir()): + try: + bench_dir = _ensure_under_project(bench_dir) + except ValueError: + continue + if not bench_dir.is_dir(): + continue + for run_dir in sorted(bench_dir.iterdir()): + try: + run_dir = _ensure_under_project(run_dir) + except ValueError: + continue + if not run_dir.is_dir(): + continue + cfg_file = run_dir / "config.yaml" + score = "—" + steps = "—" + if cfg_file.exists(): + try: + cfg_file = _ensure_under_project(cfg_file) + c = yaml.safe_load(cfg_file.read_text()) + steps = str(c.get("train", {}).get("num_steps", "—")) + except Exception: + pass + # Try to find best score from logs + for log_f in run_dir.glob("**/*.jsonl"): + try: + log_f = _ensure_under_project(log_f) + with open(log_f) as f: + for line in f: + d = json.loads(line) + if "score" in d: + score = f"{d['score']:.4f}" + except Exception: + pass + rows.append([ + run_dir.name, + bench_dir.name, + score, + steps, + ]) + return rows + + def config_to_display(cfg: dict) -> str: """Pretty-print config for display.""" return yaml.dump(cfg, default_flow_style=False, sort_keys=False) +def config_preview(path: str) -> str: + """Registered config-preview callback: YAML text for an in-tree config.""" + if not path: + return "" + try: + return config_to_display(load_config(path)) + except Exception as exc: + return f"Error: {exc}" + + def _can_connect_to_url(url: str, timeout: float = 0.5) -> bool: parsed = urlparse(url) host = parsed.hostname @@ -113,7 +216,8 @@ def validate_training_config( if value is not None and value != "" ] try: - cfg = flatten_config(load_merged_config(str(PROJECT_ROOT / config_path), cfg_options)) + config_file = _resolve_project_path(config_path, subdir="configs") + cfg = flatten_config(load_merged_config(str(config_file), cfg_options)) except Exception as exc: return f"❌ Invalid config: {exc}" @@ -490,7 +594,7 @@ def build_ui(): label="Config File", value=configs[0] if configs else None, ) - config_preview = gr.Code( + config_preview_box = gr.Code( label="Config Preview", language="yaml", interactive=False, @@ -525,15 +629,7 @@ def build_ui(): status_text = gr.Textbox(label="Status", interactive=False) - def on_config_change(path): - if path: - try: - return config_to_display(load_config(path)) - except Exception as e: - return f"Error: {e}" - return "" - - config_dropdown.change(on_config_change, config_dropdown, config_preview) + config_dropdown.change(config_preview, config_dropdown, config_preview_box) def on_launch(cfg_path, lr_val, sched, epochs, batch, workers, slow_update, meta_skill, gate): @@ -602,46 +698,6 @@ def on_refresh(): label="Experiments", ) - def scan_outputs(out_dir): - rows = [] - if not out_dir: - return rows - base = PROJECT_ROOT / out_dir - if not base.exists(): - return rows - for bench_dir in sorted(base.iterdir()): - if not bench_dir.is_dir(): - continue - for run_dir in sorted(bench_dir.iterdir()): - if not run_dir.is_dir(): - continue - cfg_file = run_dir / "config.yaml" - score = "—" - steps = "—" - if cfg_file.exists(): - try: - c = yaml.safe_load(cfg_file.read_text()) - steps = str(c.get("train", {}).get("num_steps", "—")) - except Exception: - pass - # Try to find best score from logs - for log_f in run_dir.glob("**/*.jsonl"): - try: - with open(log_f) as f: - for line in f: - d = json.loads(line) - if "score" in d: - score = f"{d['score']:.4f}" - except Exception: - pass - rows.append([ - run_dir.name, - bench_dir.name, - score, - steps, - ]) - return rows - scan_btn.click(scan_outputs, output_dir, results_table) return app @@ -661,6 +717,44 @@ def build_launch_kwargs(server_name: str, server_port: int, share: bool) -> dict return kwargs +def resolve_auth(auth_user, auth_pass, env=None): + """Resolve basic-auth credentials from the CLI flags and the environment. + + Returns ``(user, password)`` when authentication is configured, or ``None`` + for a deliberately unconfigured local run. Raises ``ValueError`` when a + configuration is present but invalid. + + Precedence is per field: a flag supplied on the command line wins over its + environment variable. "Not supplied" and "supplied but blank" are different + states, because treating them as the same one fails open - ``--auth-user + ""`` must not quietly fall back to ``SKILLOPT_WEBUI_USER``, and a pair of + blank variables (an empty secret, a template that was never filled in) must + not read as "no authentication requested" when the operator meant to + require login. + """ + env = os.environ if env is None else env + + def pick(cli_value, env_name, flag): + if cli_value is not None: + return cli_value, flag + if env_name in env: + return env[env_name], env_name + return None, None + + user, user_src = pick(auth_user, "SKILLOPT_WEBUI_USER", "--auth-user") + password, password_src = pick(auth_pass, "SKILLOPT_WEBUI_PASS", "--auth-pass") + + if user_src is None and password_src is None: + return None + if user_src is None or password_src is None: + missing = "--auth-pass" if user_src is not None else "--auth-user" + raise ValueError(f"{user_src or password_src} is set but {missing} is not") + for value, src in ((user, user_src), (password, password_src)): + if not value.strip(): + raise ValueError(f"{src} is blank") + return (user, password) + + def main(): parser = argparse.ArgumentParser(description="SkillOpt WebUI") parser.add_argument("--port", type=int, default=7860) @@ -668,6 +762,10 @@ def main(): parser.add_argument("--host", type=str, default="127.0.0.1", help="Server host. Default is localhost; use 0.0.0.0 " "to expose publicly (no auth, use with care).") + parser.add_argument("--auth-user", type=str, default=None, + help="Username for basic auth (or set SKILLOPT_WEBUI_USER).") + parser.add_argument("--auth-pass", type=str, default=None, + help="Password for basic auth (or set SKILLOPT_WEBUI_PASS).") args = parser.parse_args() if args.host and args.host not in ("127.0.0.1", "localhost", "::1"): @@ -679,8 +777,36 @@ def main(): file=sys.stderr, ) + if args.share: + print( + "⚠ warning: --share creates a public tunnel (gradio.live) with no " + "authentication by default. Anyone with the URL can start/stop " + "training and browse the filesystem via Output Explorer. " + "Use --auth-user / --auth-pass (or SKILLOPT_WEBUI_USER / " + "SKILLOPT_WEBUI_PASS) to require login.", + file=sys.stderr, + ) + + # Fail-closed: authentication requires BOTH credentials, from any source. + # Supplying only one of them - or supplying blank ones - must not silently + # launch the UI unauthenticated (a deployment could expose the training + # controls without login). + try: + auth = resolve_auth(args.auth_user, args.auth_pass) + except ValueError as exc: + print( + f"SKILLOPT_WEBUI authentication is misconfigured: {exc}. Provide both " + "--auth-user and --auth-pass (or both SKILLOPT_WEBUI_USER and " + "SKILLOPT_WEBUI_PASS) to require login, or neither to run without " + "authentication. Refusing to start.", + file=sys.stderr, + ) + sys.exit(1) + app = build_ui() launch_kwargs = build_launch_kwargs(args.host, args.port, args.share) + if auth: + launch_kwargs["auth"] = auth app.launch(**launch_kwargs) diff --git a/tests/test_webui_env_preflight.py b/tests/test_webui_env_preflight.py index 5b84d862..6917304f 100644 --- a/tests/test_webui_env_preflight.py +++ b/tests/test_webui_env_preflight.py @@ -7,7 +7,9 @@ def _write_config(tmp_path, model): - config_path = tmp_path / "config.yaml" + config_dir = tmp_path / "configs" + config_dir.mkdir(exist_ok=True) + config_path = config_dir / "demo.yaml" config_path.write_text( yaml.safe_dump({ "model": model, @@ -15,7 +17,7 @@ def _write_config(tmp_path, model): }), encoding="utf-8", ) - return str(config_path) + return "configs/demo.yaml" def test_build_training_env_loads_project_dotenv(tmp_path, monkeypatch): @@ -37,6 +39,7 @@ def test_build_training_env_loads_project_dotenv(tmp_path, monkeypatch): def test_preflight_reports_missing_openai_chat_endpoint(tmp_path, monkeypatch): + monkeypatch.setattr(webui_app, "PROJECT_ROOT", tmp_path) monkeypatch.delenv("AZURE_OPENAI_ENDPOINT", raising=False) monkeypatch.delenv("OPTIMIZER_AZURE_OPENAI_ENDPOINT", raising=False) monkeypatch.delenv("TARGET_AZURE_OPENAI_ENDPOINT", raising=False) @@ -56,6 +59,7 @@ def test_preflight_reports_missing_openai_chat_endpoint(tmp_path, monkeypatch): def test_preflight_reports_unreachable_qwen_endpoint(tmp_path, monkeypatch): + monkeypatch.setattr(webui_app, "PROJECT_ROOT", tmp_path) monkeypatch.setattr(webui_app, "_can_connect_to_url", lambda _url: False) config_path = _write_config( tmp_path, @@ -74,6 +78,7 @@ def test_preflight_reports_unreachable_qwen_endpoint(tmp_path, monkeypatch): def test_preflight_accepts_reachable_qwen_endpoint(tmp_path, monkeypatch): + monkeypatch.setattr(webui_app, "PROJECT_ROOT", tmp_path) seen_urls = [] monkeypatch.setattr(webui_app, "_can_connect_to_url", lambda url: seen_urls.append(url) or True) config_path = _write_config( diff --git a/tests/test_webui_security.py b/tests/test_webui_security.py index 5886da65..cd021e95 100644 --- a/tests/test_webui_security.py +++ b/tests/test_webui_security.py @@ -7,6 +7,7 @@ from __future__ import annotations +import itertools import sys import types import unittest.mock as mock @@ -55,3 +56,323 @@ def test_main_warns_on_public_host(webui, monkeypatch, capsys): assert "warning" in captured.err.lower() _args, kwargs = launcher.call_args assert kwargs["server_name"] == "0.0.0.0" + + +def test_main_warns_on_share(webui, monkeypatch, capsys): + """--share must emit a public-tunnel warning.""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py", "--share"]) + + webui_mod.main() + + captured = capsys.readouterr() + assert "share" in captured.err.lower() + assert "public" in captured.err.lower() or "tunnel" in captured.err.lower() + + +def test_main_auth_via_cli_args(webui, monkeypatch): + """--auth-user and --auth-pass must enable Gradio basic auth.""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py", "--auth-user", "admin", "--auth-pass", "s3cret"]) + + webui_mod.main() + + _args, kwargs = launcher.call_args + assert kwargs.get("auth") == ("admin", "s3cret") + + +def test_main_auth_via_env(webui, monkeypatch): + """SKILLOPT_WEBUI_USER / SKILLOPT_WEBUI_PASS must enable auth without CLI args.""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py"]) + monkeypatch.setenv("SKILLOPT_WEBUI_USER", "envuser") + monkeypatch.setenv("SKILLOPT_WEBUI_PASS", "envpass") + + webui_mod.main() + + _args, kwargs = launcher.call_args + assert kwargs.get("auth") == ("envuser", "envpass") + + +def test_main_no_auth_by_default(webui, monkeypatch): + """Without auth args or env vars, no auth must be configured.""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py"]) + monkeypatch.delenv("SKILLOPT_WEBUI_USER", raising=False) + monkeypatch.delenv("SKILLOPT_WEBUI_PASS", raising=False) + + webui_mod.main() + + _args, kwargs = launcher.call_args + assert "auth" not in kwargs or kwargs["auth"] is None + + +def test_scan_outputs_rejects_path_traversal(webui, tmp_path, monkeypatch): + """The scan_outputs callback must not enumerate directories outside PROJECT_ROOT.""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + (tmp_path / "outputs").mkdir() + # Every traversal / escape form is denied at consumption: no rows, no reads. + for bad in ("/../../etc/passwd", "../outside", "outputs/../../../etc", "C:\\Windows"): + assert webui.scan_outputs(bad) == [], f"traversal {bad!r} must be denied" + + +def test_scan_outputs_allows_valid_subdir(webui, tmp_path, monkeypatch): + """scan_outputs must accept directories within PROJECT_ROOT.""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + (tmp_path / "outputs" / "bench1" / "run1").mkdir(parents=True) + (tmp_path / "outputs" / "bench1" / "run1" / "config.yaml").write_text("a: 1\n", encoding="utf-8") + rows = webui.scan_outputs("outputs") + assert rows, "valid in-tree output area must be digested" + + +def test_scan_outputs_callback_consumes_within_project(webui, tmp_path, monkeypatch): + """The registered scan_outputs callback must digest data only inside PROJECT_ROOT + at the point data is actually read (traversal denied, in-tree consumed).""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + # A traversal arg must be denied at consumption: no rows, no data read. + assert webui.scan_outputs("/../../etc/passwd") == [] + assert webui.scan_outputs("../outside") == [] + # A valid in-tree output area is digested (config.yaml read per run dir). + (tmp_path / "outputs/bench1/run1").mkdir(parents=True) + (tmp_path / "outputs/bench1/run1/config.yaml").write_text("alpha: 1\n", encoding="utf-8") + rows = webui.scan_outputs("outputs") + assert rows, f"expected rows from a valid in-tree output area, got {rows!r}" + + +def test_config_preview_rejects_relative_traversal(webui, tmp_path, monkeypatch): + """The config-preview callback must not read YAML outside PROJECT_ROOT.""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + outside_dir = tmp_path.parent / (tmp_path.name + "_outside") + outside_dir.mkdir() + secret = outside_dir / "secret.yaml" + secret.write_text("password: dummy-secret-value\n", encoding="utf-8") + + result = webui.config_preview(f"../{outside_dir.name}/secret.yaml") + + assert "dummy-secret-value" not in result + + +def test_config_preview_rejects_absolute_outside_path(webui, tmp_path, monkeypatch): + """An absolute path escaping PROJECT_ROOT must be denied at consumption.""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + outside_dir = tmp_path.parent / (tmp_path.name + "_outside_abs") + outside_dir.mkdir() + secret = outside_dir / "secret.yaml" + secret.write_text("password: dummy-secret-value\n", encoding="utf-8") + + result = webui.config_preview(str(secret)) + + assert "dummy-secret-value" not in result + + +def test_config_preview_allows_configs_under_project(webui, tmp_path, monkeypatch): + """An in-tree config under configs/ must still preview normally.""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + (tmp_path / "configs").mkdir() + (tmp_path / "configs" / "demo.yaml").write_text("name: demo\n", encoding="utf-8") + + result = webui.config_preview("configs/demo.yaml") + + assert "name: demo" in result + + +def test_validate_training_config_rejects_outside_path(webui, tmp_path, monkeypatch): + """Launch preflight must reject a config path that escapes PROJECT_ROOT.""" + monkeypatch.setattr(webui, "PROJECT_ROOT", tmp_path) + outside_dir = tmp_path.parent / (tmp_path.name + "_outside_train") + outside_dir.mkdir() + (outside_dir / "train.yaml").write_text("name: demo\n", encoding="utf-8") + + result = webui.validate_training_config( + f"../{outside_dir.name}/train.yaml", + {}, + ) + + assert result is not None, "path escaping PROJECT_ROOT must fail closed" + + +def test_main_rejects_incomplete_cli_auth_user_only(webui, monkeypatch): + """--auth-user without --auth-pass must fail closed (never launch).""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py", "--host", "0.0.0.0", "--auth-user", "admin"]) + with pytest.raises(SystemExit): + webui_mod.main() + launcher.assert_not_called() + + +def test_main_rejects_incomplete_cli_auth_pass_only(webui, monkeypatch): + """--auth-pass without --auth-user must fail closed (never launch).""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py", "--host", "0.0.0.0", "--auth-pass", "s3cret"]) + with pytest.raises(SystemExit): + webui_mod.main() + launcher.assert_not_called() + + +def test_main_rejects_incomplete_env_auth_user_only(webui, monkeypatch): + """Only SKILLOPT_WEBUI_USER set must fail closed (never launch).""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py", "--host", "0.0.0.0"]) + monkeypatch.setenv("SKILLOPT_WEBUI_USER", "envuser") + monkeypatch.delenv("SKILLOPT_WEBUI_PASS", raising=False) + with pytest.raises(SystemExit): + webui_mod.main() + launcher.assert_not_called() + + +def test_main_rejects_incomplete_env_auth_pass_only(webui, monkeypatch): + """Only SKILLOPT_WEBUI_PASS set must fail closed (never launch).""" + webui_mod = webui + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock) + monkeypatch.setattr(sys, "argv", ["app.py", "--host", "0.0.0.0"]) + monkeypatch.setenv("SKILLOPT_WEBUI_PASS", "envpass") + monkeypatch.delenv("SKILLOPT_WEBUI_USER", raising=False) + with pytest.raises(SystemExit): + webui_mod.main() + launcher.assert_not_called() + + +@pytest.fixture +def auth_probe(webui, monkeypatch): + """Run the real ``main()`` with a mocked UI and report what it launched. + + Returns ``(launcher, exit_code, builder)``, where ``exit_code`` is ``None`` + when ``main()`` returned normally and the ``SystemExit`` code otherwise, so + a test can tell "refused to start" from "started without auth". ``builder`` + is the mocked ``build_ui``, so a rejection test can also assert the UI was + never constructed at all rather than only that it was not launched. + """ + def _run(argv=(), env=None): + launcher = mock.MagicMock() + app_mock = mock.MagicMock() + app_mock.launch = launcher + builder = mock.MagicMock(return_value=app_mock) + monkeypatch.setattr(webui, "build_ui", builder) + monkeypatch.setattr(sys, "argv", ["app.py", *argv]) + for name in ("SKILLOPT_WEBUI_USER", "SKILLOPT_WEBUI_PASS"): + monkeypatch.delenv(name, raising=False) + for name, value in (env or {}).items(): + monkeypatch.setenv(name, value) + code = None + try: + webui.main() + except SystemExit as exc: + code = exc.code + return launcher, code, builder + + return _run + + +def test_main_auth_matrix_launches_with_complete_configuration(auth_probe): + """CLI-only, env-only and field-mixed sources must each enable auth.""" + for argv, env, expected in ( + (["--auth-user", "admin", "--auth-pass", "s3cret"], {}, ("admin", "s3cret")), + ( + [], + {"SKILLOPT_WEBUI_USER": "envuser", "SKILLOPT_WEBUI_PASS": "envpass"}, + ("envuser", "envpass"), + ), + (["--auth-user", "admin"], {"SKILLOPT_WEBUI_PASS": "envpass"}, ("admin", "envpass")), + ): + launcher, code, _builder = auth_probe(argv, env) + + assert code is None, f"complete configuration must launch: {argv} {env}" + assert launcher.call_args.kwargs.get("auth") == expected + + +def test_main_cli_credentials_take_precedence_over_env(auth_probe): + """Precedence is per field: an explicit flag wins over its variable.""" + launcher, code, _builder = auth_probe( + ["--auth-user", "cli", "--auth-pass", "clipass"], + {"SKILLOPT_WEBUI_USER": "envuser", "SKILLOPT_WEBUI_PASS": "envpass"}, + ) + + assert code is None + assert launcher.call_args.kwargs["auth"] == ("cli", "clipass") + + +def _expected_auth_outcome(cli_user, cli_pass, env_user, env_pass): + """The decision table, restated independently of the implementation. + + ``None`` means the source was not supplied at all, which is different from + an empty string: a supplied-but-blank credential is a misconfiguration. + """ + has_user = cli_user is not None or env_user is not None + has_pass = cli_pass is not None or env_pass is not None + if not has_user and not has_pass: + return "no-auth" + if not has_user or not has_pass: + return "refuse" + user = cli_user if cli_user is not None else env_user + password = cli_pass if cli_pass is not None else env_pass + if not user.strip() or not password.strip(): + return "refuse" + return (user, password) + + +@pytest.mark.parametrize( + "cli_user, cli_pass, env_user, env_pass", + list(itertools.product( + (None, "", "u"), # absent, supplied-but-blank, supplied + (None, "", "p"), + (None, "", "u"), + (None, "", "p"), + )), +) +def test_main_auth_decision_table(auth_probe, cli_user, cli_pass, env_user, env_pass): + """Every absent/blank/supplied combination across both sources.""" + argv = [] + if cli_user is not None: + argv += ["--auth-user", cli_user] + if cli_pass is not None: + argv += ["--auth-pass", cli_pass] + env = {} + if env_user is not None: + env["SKILLOPT_WEBUI_USER"] = env_user + if env_pass is not None: + env["SKILLOPT_WEBUI_PASS"] = env_pass + + launcher, code, builder = auth_probe(argv, env) + expected = _expected_auth_outcome(cli_user, cli_pass, env_user, env_pass) + + if expected == "refuse": + assert code == 1, "invalid auth must stop before the UI" + assert builder.call_count == 0 + launcher.assert_not_called() + elif expected == "no-auth": + assert code is None + assert "auth" not in launcher.call_args.kwargs + else: + assert code is None + assert launcher.call_args.kwargs["auth"] == expected