Skip to content

Commit db46cd9

Browse files
committed
Merge pull request #249 from WODE25500/fix/webui-security
fix(webui): security hardening + launch theme crash; add CI
2 parents 8560800 + d549c86 commit db46cd9

5 files changed

Lines changed: 243 additions & 12 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
branches: [main]
6+
pull_request:
7+
branches: [main]
8+
9+
jobs:
10+
test:
11+
runs-on: ubuntu-latest
12+
strategy:
13+
fail-fast: false
14+
matrix:
15+
python-version: ["3.10", "3.11", "3.12"]
16+
steps:
17+
- uses: actions/checkout@v4
18+
- uses: actions/setup-python@v5
19+
with:
20+
python-version: ${{ matrix.python-version }}
21+
- name: Install
22+
run: pip install -e ".[dev]"
23+
- name: Pytest
24+
run: python -m pytest -q
25+
env:
26+
# Live backend/transcript tests are opt-in (gated on == "1"); keep
27+
# them off in CI so the run stays hermetic and fast.
28+
SKILLOPT_TEST_REAL_OPENCODE: "0"
29+
SKILLOPT_TEST_REAL_OPENCODE_SOURCE: "0"
30+
31+
webui:
32+
# The Gradio theme-compatibility path only exists because gradio changed
33+
# where `theme` lives across majors: <6 puts it on Blocks, >=6 on launch().
34+
# The main `test` job installs only `.[dev]`, so those tests importorskip
35+
# and never exercise this code — run them here across both supported majors.
36+
# 5.50.0 is the DECLARED minimum in the `webui` extra (gradio>=5.50.0,<7),
37+
# so the matrix tests the actual floor, not just a mid-version.
38+
runs-on: ubuntu-latest
39+
strategy:
40+
fail-fast: false
41+
matrix:
42+
gradio-version: ["5.50.0", "6.26.0"]
43+
steps:
44+
- uses: actions/checkout@v4
45+
- uses: actions/setup-python@v5
46+
with:
47+
python-version: "3.11"
48+
- name: Install (core + pinned gradio)
49+
run: pip install -e ".[dev]" "gradio==${{ matrix.gradio-version }}"
50+
- name: Pytest (gradio webui)
51+
run: python -m pytest tests/test_webui_build_gradio.py tests/test_webui_env_preflight.py -q
52+
53+
docs:
54+
runs-on: ubuntu-latest
55+
steps:
56+
- uses: actions/checkout@v4
57+
- uses: actions/setup-python@v5
58+
with:
59+
python-version: "3.11"
60+
- name: Install
61+
run: pip install -e ".[docs]"
62+
- name: Build docs (strict)
63+
run: mkdocs build --strict

‎pyproject.toml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,14 @@ searchqa = ["datasets>=2.18.0"]
4545
# Documentation site
4646
docs = ["mkdocs-material>=9.5.0", "mkdocstrings[python]>=0.24.0"]
4747
# WebUI dashboard
48-
webui = ["gradio>=4.0.0"]
48+
# Broad gradio support is tied to a single huggingface_hub bound: gradio 4/5
49+
# need huggingface-hub<1 (gradio 4.44 imports HfFolder), but gradio 6 needs
50+
# huggingface-hub>=1.16. A single bound cannot satisfy both, so narrow the range
51+
# to the two majors this project actually validates (5 exercises the Blocks
52+
# theme path, 6 the launch() theme path); the CI matrix tests both. The floor
53+
# is 5.50.0: earlier gradio 5.x (5.0-5.49) still import HfFolder or don't apply
54+
# the Blocks theme the same way, so they are not valid supported versions.
55+
webui = ["gradio>=5.50.0,<7"]
4956
# Development tools
5057
dev = ["ruff>=0.4.0", "pytest>=8.0.0"]
5158
# All optional dependencies (except docs/dev/webui)

‎skillopt_webui/app.py‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@
2424

2525
PROJECT_ROOT = Path(__file__).resolve().parent.parent
2626

27+
# Gradio moved where `theme` lives across versions: <=5 uses `Blocks(theme=...)`,
28+
# >=6 moved it to `launch()`. Detect the installed major so the WebUI works on
29+
# any supported version without an ignored-argument warning or a TypeError.
30+
_GRADIO_MAJOR = int((getattr(gr, "__version__", "4.0").split(".")[0]) or 4)
31+
2732

2833
# ─── Config helpers ──────────────────────────────────────────────────────────
2934

@@ -468,9 +473,10 @@ def render_pipeline_html(active_stage: str = "") -> str:
468473
def build_ui():
469474
configs = discover_configs()
470475

471-
with gr.Blocks(
472-
title="SkillOpt WebUI",
473-
) as app:
476+
_blocks_kwargs = {"title": "SkillOpt WebUI"}
477+
if _GRADIO_MAJOR < 6:
478+
_blocks_kwargs["theme"] = gr.themes.Soft(primary_hue="indigo")
479+
with gr.Blocks(**_blocks_kwargs) as app:
474480
gr.Markdown("# 🧠 SkillOpt Training Dashboard")
475481
gr.Markdown("*SKILLOPT: Executive Strategy for Self-Evolving Agent Skills — Configure, launch, and monitor training.*")
476482

@@ -598,6 +604,8 @@ def on_refresh():
598604

599605
def scan_outputs(out_dir):
600606
rows = []
607+
if not out_dir:
608+
return rows
601609
base = PROJECT_ROOT / out_dir
602610
if not base.exists():
603611
return rows
@@ -639,21 +647,41 @@ def scan_outputs(out_dir):
639647
return app
640648

641649

650+
def build_launch_kwargs(server_name: str, server_port: int, share: bool) -> dict:
651+
"""Build the launch kwargs exactly as production ``main()`` applies them.
652+
653+
Gradio 6 moved the theme to ``launch()``; applying it here (rather than in
654+
``build_ui``) keeps the theme on the right object for the installed major
655+
without an ignored-argument warning. Tests call this so they exercise the
656+
same path a real ``skillopt-webui`` run does instead of injecting their own.
657+
"""
658+
kwargs = dict(server_name=server_name, server_port=server_port, share=share)
659+
if _GRADIO_MAJOR >= 6:
660+
kwargs["theme"] = gr.themes.Soft(primary_hue="indigo")
661+
return kwargs
662+
663+
642664
def main():
643665
parser = argparse.ArgumentParser(description="SkillOpt WebUI")
644666
parser.add_argument("--port", type=int, default=7860)
645667
parser.add_argument("--share", action="store_true")
646-
parser.add_argument("--host", type=str, default="0.0.0.0",
647-
help="Server host. Use 0.0.0.0 for public access.")
668+
parser.add_argument("--host", type=str, default="127.0.0.1",
669+
help="Server host. Default is localhost; use 0.0.0.0 "
670+
"to expose publicly (no auth, use with care).")
648671
args = parser.parse_args()
649672

673+
if args.host and args.host not in ("127.0.0.1", "localhost", "::1"):
674+
print(
675+
f"⚠ warning: binding SkillOpt WebUI on {args.host} with no auth "
676+
"exposes the Output Explorer (reads any path you type) and the "
677+
"training controls to reachable clients. Prefer --host 127.0.0.1; "
678+
"use 0.0.0.0 only if you understand the risk.",
679+
file=sys.stderr,
680+
)
681+
650682
app = build_ui()
651-
app.launch(
652-
server_name=args.host,
653-
server_port=args.port,
654-
share=args.share,
655-
theme=gr.themes.Soft(primary_hue="indigo"),
656-
)
683+
launch_kwargs = build_launch_kwargs(args.host, args.port, args.share)
684+
app.launch(**launch_kwargs)
657685

658686

659687
if __name__ == "__main__":

‎tests/test_webui_build_gradio.py‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
"""Real Gradio build/launch smoke test (requires the `webui` extra).
2+
3+
Verifies the WebUI builds (and launches where the environment allows) on the
4+
installed Gradio without a TypeError or ignored-argument warning, and that the
5+
theme is placed on the right object for the installed major version — as a
6+
specific ``Soft`` schema, not merely "some non-None theme". Gradio 6 assigns a
7+
default theme when none is passed, so ``theme is not None`` would pass even if
8+
``Soft`` was never applied; these tests assert the exact theme.
9+
"""
10+
11+
from __future__ import annotations
12+
13+
import pytest
14+
15+
pytest.importorskip("gradio")
16+
17+
import gradio as gr # noqa: E402
18+
19+
import skillopt_webui.app as app # noqa: E402
20+
21+
22+
def _assert_soft_theme(theme) -> None:
23+
assert isinstance(theme, gr.themes.Soft), f"theme was not Soft: {theme!r}"
24+
25+
26+
def test_webui_builds_theme_on_blocks():
27+
"""build_ui() must not raise; for Gradio <6 the theme lives on Blocks."""
28+
ui = app.build_ui()
29+
assert ui is not None
30+
if app._GRADIO_MAJOR < 6:
31+
# Gradio 5 and below place the theme on Blocks. Assert it is the schema
32+
# we chose (Soft), not merely any non-None default.
33+
_assert_soft_theme(ui.theme)
34+
else:
35+
# Gradio 6 moved the theme to launch(); build_ui must NOT bake it into
36+
# the Blocks kwargs (that would be an ignored argument).
37+
assert ui.theme is None, "theme must live on launch kwargs for Gradio >=6"
38+
39+
40+
def test_launch_kwargs_follow_production_main_path():
41+
# The production path applies Soft; the test must not inject its own theme.
42+
kwargs = app.build_launch_kwargs(
43+
server_name="127.0.0.1", server_port=7860, share=False
44+
)
45+
if app._GRADIO_MAJOR >= 6:
46+
_assert_soft_theme(kwargs.get("theme"))
47+
assert kwargs["theme"].name == "soft"
48+
else:
49+
assert "theme" not in kwargs, "theme must live on Blocks for Gradio <6"
50+
51+
52+
def test_webui_builds_and_launches_theme():
53+
ui = app.build_ui()
54+
launch_kwargs = app.build_launch_kwargs(
55+
server_name="127.0.0.1", server_port=7860, share=False
56+
)
57+
launch_kwargs["prevent_thread_lock"] = True
58+
try:
59+
ui.launch(**launch_kwargs)
60+
except ValueError as exc:
61+
# Headless/sandboxed environments may not expose localhost; that is an
62+
# environment limitation, not a theme-compatibility bug.
63+
if "localhost is not accessible" in str(exc):
64+
pytest.skip("headless environment blocks localhost launch")
65+
raise
66+
try:
67+
# Assert Soft specifically: gradio >=6 would hand back a default theme
68+
# if Soft were not actually applied via the production kwargs.
69+
_assert_soft_theme(ui.theme)
70+
finally:
71+
ui.close()
72+
73+
74+
def test_gradio_major_detected():
75+
# The constant must reflect the installed Gradio major.
76+
assert app._GRADIO_MAJOR == int(gr.__version__.split(".")[0])

‎tests/test_webui_security.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
"""Tests for the SkillOpt WebUI security posture (bind default + public warning).
2+
3+
The WebUI is gradio-coupled, so we inject a minimal fake ``gradio`` module and
4+
mock ``build_ui``/``launch`` to exercise ``main()``'s argparse + host-check
5+
logic without the heavy ``webui`` extra.
6+
"""
7+
8+
from __future__ import annotations
9+
10+
import sys
11+
import types
12+
import unittest.mock as mock
13+
14+
import pytest
15+
16+
17+
@pytest.fixture
18+
def webui(monkeypatch):
19+
fake_gradio = types.ModuleType("gradio")
20+
fake_gradio.themes = types.SimpleNamespace(Soft=lambda **kw: mock.MagicMock())
21+
monkeypatch.setitem(sys.modules, "gradio", fake_gradio)
22+
import skillopt_webui.app as app
23+
24+
return app
25+
26+
27+
def test_main_defaults_host_to_localhost(webui, monkeypatch):
28+
"""The server must not be publicly bound by default."""
29+
webui_mod = webui
30+
launcher = mock.MagicMock()
31+
app_mock = mock.MagicMock()
32+
app_mock.launch = launcher
33+
monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock)
34+
monkeypatch.setattr(sys, "argv", ["app.py"])
35+
36+
webui_mod.main()
37+
38+
launcher.assert_called_once()
39+
_args, kwargs = launcher.call_args
40+
assert kwargs["server_name"] == "127.0.0.1"
41+
42+
43+
def test_main_warns_on_public_host(webui, monkeypatch, capsys):
44+
"""An explicit public bind must emit an unauthenticated-exposure warning."""
45+
webui_mod = webui
46+
launcher = mock.MagicMock()
47+
app_mock = mock.MagicMock()
48+
app_mock.launch = launcher
49+
monkeypatch.setattr(webui_mod, "build_ui", lambda: app_mock)
50+
monkeypatch.setattr(sys, "argv", ["app.py", "--host", "0.0.0.0"])
51+
52+
webui_mod.main()
53+
54+
captured = capsys.readouterr()
55+
assert "warning" in captured.err.lower()
56+
_args, kwargs = launcher.call_args
57+
assert kwargs["server_name"] == "0.0.0.0"

0 commit comments

Comments
 (0)