From 119c114daa02f1d89734be1c0d9019a528c40245 Mon Sep 17 00:00:00 2001 From: Jason Morcos Date: Sun, 2 Aug 2026 10:02:52 -0700 Subject: [PATCH] ci: add pull request validation --- .github/workflows/ci.yml | 130 +++++++++++++++++ tests/test_public_api_contract.py | 82 +++++++++++ tests/test_share_safety.py | 96 +++++++++++++ tests/test_worker_cleanup.py | 63 ++++++++ tools/check_distribution.py | 126 ++++++++++++++++ tools/check_share_safety.py | 229 ++++++++++++++++++++++++++++++ 6 files changed, 726 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100644 tests/test_public_api_contract.py create mode 100644 tests/test_share_safety.py create mode 100644 tests/test_worker_cleanup.py create mode 100755 tools/check_distribution.py create mode 100755 tools/check_share_safety.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..5d8e6a4 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,130 @@ +name: Validate + +on: + pull_request: + branches: + - main + push: + branches: + - main + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: validate-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + tests: + name: Python ${{ matrix.python-version }} + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + python-version: + - "3.11" + - "3.12" + - "3.13" + - "3.14" + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v7 + with: + python-version: ${{ matrix.python-version }} + - run: python -m pip install --upgrade pip + - run: python -m pip install -e ".[dev]" + - run: python -m pytest -q + + dependency-bounds: + name: Dependencies (${{ matrix.mode }}) + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + include: + - mode: floor + python-version: "3.11" + - mode: latest + python-version: "3.14" + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v7 + with: + python-version: ${{ matrix.python-version }} + - run: python -m pip install --upgrade pip + - if: matrix.mode == 'floor' + run: >- + python -m pip install + "cbor2==5.6.0" + "pyOpenSSL==23.0.0" + "pytest==8.0.0" + - if: matrix.mode == 'floor' + run: python -m pip install --no-deps -e . + - if: matrix.mode == 'latest' + run: python -m pip install -e ".[dev]" + - run: python -m pytest -q + + package: + name: Package artifacts + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + - uses: actions/setup-python@v7 + with: + python-version: "3.14" + - run: python -m pip install --upgrade pip + - run: python -m pip install build hatchling hatch-vcs + - run: python -m build + - run: python tools/check_distribution.py dist + - name: Install and import wheel + run: | + python -m venv "$RUNNER_TEMP/wheel-smoke" + "$RUNNER_TEMP/wheel-smoke/bin/python" -m pip install \ + dist/*.whl + cd "$RUNNER_TEMP" + "$RUNNER_TEMP/wheel-smoke/bin/python" -I -c \ + "from smartthings_local.protocol.dtls_session import DtlsCoapSession" + - name: Install and import sdist + run: | + python -m venv "$RUNNER_TEMP/sdist-smoke" + "$RUNNER_TEMP/sdist-smoke/bin/python" -m pip install \ + dist/*.tar.gz + cd "$RUNNER_TEMP" + "$RUNNER_TEMP/sdist-smoke/bin/python" -I -c \ + "from smartthings_local.ocf.state_cache import StateCache" + + share-safety: + name: Share safety + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + with: + fetch-depth: 0 + - uses: actions/setup-python@v7 + with: + python-version: "3.14" + - name: Select comparison base + id: comparison + env: + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PUSH_BEFORE_SHA: ${{ github.event.before }} + run: | + if [ -n "$PR_BASE_SHA" ]; then + echo "sha=$PR_BASE_SHA" >> "$GITHUB_OUTPUT" + elif [ -n "$PUSH_BEFORE_SHA" ] && \ + [ "$PUSH_BEFORE_SHA" != "0000000000000000000000000000000000000000" ]; then + echo "sha=$PUSH_BEFORE_SHA" >> "$GITHUB_OUTPUT" + else + echo "sha=$(git rev-parse HEAD^)" >> "$GITHUB_OUTPUT" + fi + - run: >- + python tools/check_share_safety.py + --changed-since "${{ steps.comparison.outputs.sha }}" diff --git a/tests/test_public_api_contract.py b/tests/test_public_api_contract.py new file mode 100644 index 0000000..0a33e78 --- /dev/null +++ b/tests/test_public_api_contract.py @@ -0,0 +1,82 @@ +"""Compatibility baseline for the published API and LocalThings consumer.""" + +from __future__ import annotations + +import inspect + +from smartthings_local.ocf.observe_refresh import ObserveRefreshTask +from smartthings_local.ocf.state_cache import StateCache +from smartthings_local.protocol.dtls_session import DtlsCoapSession + + +def _parameter_names(callable_object) -> list[str]: + return list(inspect.signature(callable_object).parameters) + + +def test_dtls_session_constructor_keeps_file_memory_and_local_port_inputs(): + assert _parameter_names(DtlsCoapSession) == [ + "host", + "port", + "cert_path", + "key_path", + "cert_pem", + "key_pem", + "on_notification", + "mtu", + "rate_limit_rps", + "local_port", + ] + + +def test_dtls_session_keeps_current_consumer_methods(): + expected = { + "close", + "connect", + "get", + "join", + "pace", + "ping", + "post", + "refresh_observes", + "start_reader", + "subscribe", + } + assert expected <= set(dir(DtlsCoapSession)) + assert _parameter_names(DtlsCoapSession.get) == [ + "self", + "path_segs", + "query", + "timeout", + ] + assert _parameter_names(DtlsCoapSession.post) == [ + "self", + "path_segs", + "body_cbor", + "timeout", + ] + assert _parameter_names(DtlsCoapSession.subscribe) == ["self", "path_segs"] + + +def test_state_cache_keeps_current_consumer_surface(): + assert _parameter_names(StateCache) == ["descriptor"] + expected = { + "apply_optimistic", + "apply_rep", + "freshness_s", + "get", + "index_device_tree", + "set_on_change", + "snapshot", + "stalest", + } + assert expected <= set(dir(StateCache)) + + +def test_observe_refresh_task_keeps_current_consumer_surface(): + assert _parameter_names(ObserveRefreshTask) == [ + "session", + "paths", + "interval_s", + "logger", + ] + assert _parameter_names(ObserveRefreshTask.run_forever) == ["self", "stop"] diff --git a/tests/test_share_safety.py b/tests/test_share_safety.py new file mode 100644 index 0000000..0b971fc --- /dev/null +++ b/tests/test_share_safety.py @@ -0,0 +1,96 @@ +from __future__ import annotations + +import subprocess + +from tools import check_share_safety + + +def test_documentation_addresses_and_synthetic_uuid_are_safe(): + text = ( + "192.0.2.10 198.51.100.20 203.0.113.30 " + "2001:db8::10 11111111-2222-3333-4444-555555555555" + ) + assert check_share_safety.scan_text("fixture.txt", text) == [] + + +def test_findings_never_echo_matched_content(): + cases = { + "PEM_PRIVATE_KEY": "-----BEGIN " + "PRIVATE KEY-----", + "EMAIL_ADDRESS": "person" + "@example.net", + "MAC_ADDRESS": "aa:bb:cc:" + "dd:ee:ff", + "NON_DOCUMENTATION_IPV4": "10." + "24.8.9", + "NON_DOCUMENTATION_IPV6": "fd00" + 2 * chr(58) + "1234", + "PRIVATE_DNS": "appliance" + chr(46) + "house" + chr(46) + "local", + "HOME_PATH": "/" + "Users/person/private.txt", + "CREDENTIAL_URL": "https://user:" + "pass" + chr(64) + "example.net/data", + "SECRET_ASSIGNMENT": ( + "access_token " + chr(61) + " " + chr(34) + "never-print-this" + chr(34) + ), + "SERIAL_ASSIGNMENT": ( + "serialNumber " + chr(61) + " " + chr(34) + "device-123456" + chr(34) + ), + "REAL_TIMESTAMP": "2026-08-02" + "T12:34:56Z", + "QR_PAYLOAD": "qr_" + "payload = value", + "UUID": "12345678-1234-4234-9234-" + "123456789abc", + } + for rule_id, value in cases.items(): + findings = check_share_safety.scan_text("candidate.txt", value) + rendered = "\n".join(finding.render() for finding in findings) + assert f"candidate.txt:1:{rule_id}" in rendered + assert value not in rendered + + +def test_binary_and_archive_inputs_are_rejected(tmp_path): + binary = tmp_path / "fixture.bin" + binary.write_bytes(b"before\x00after") + capture = tmp_path / "fixture.pcap" + capture.write_text("text-looking content") + + assert check_share_safety.scan_file(binary, "fixture.bin") == [ + check_share_safety.Finding("fixture.bin", 0, "BINARY_CONTENT") + ] + assert check_share_safety.scan_file(capture, "fixture.pcap") == [ + check_share_safety.Finding("fixture.pcap", 0, "FORBIDDEN_FILE_TYPE") + ] + + +def test_changed_paths_include_staged_unstaged_and_untracked_files( + tmp_path, monkeypatch +): + def git(*args): + return subprocess.run( + [ + "git", + "-c", + "commit.gpgsign=false", + "-c", + "user.name=Test", + "-c", + "user.email=" + "test" + chr(64) + "example.invalid", + *args, + ], + cwd=tmp_path, + capture_output=True, + check=True, + text=True, + ) + + git("init", "--quiet") + baseline = tmp_path / "baseline.txt" + baseline.write_text("before\n") + git("add", "baseline.txt") + git("commit", "--quiet", "-m", "baseline") + base = git("rev-parse", "HEAD").stdout.strip() + + staged = tmp_path / "staged.txt" + staged.write_text("staged\n") + git("add", "staged.txt") + baseline.write_text("after\n") + (tmp_path / "untracked.txt").write_text("untracked\n") + + monkeypatch.chdir(tmp_path) + assert check_share_safety._changed_paths(base) == [ + "baseline.txt", + "staged.txt", + "untracked.txt", + ] diff --git a/tests/test_worker_cleanup.py b/tests/test_worker_cleanup.py new file mode 100644 index 0000000..080c5e4 --- /dev/null +++ b/tests/test_worker_cleanup.py @@ -0,0 +1,63 @@ +"""Deterministic baseline checks for the current OCF worker stop contract.""" + +from __future__ import annotations + +import threading + +from smartthings_local.ocf.keepalive import KeepaliveTask +from smartthings_local.ocf.observe_refresh import ObserveRefreshTask +from smartthings_local.ocf.poll_scheduler import PollScheduler, PollTier +from smartthings_local.ocf.state_cache import StateCache + + +class _Session: + def ping(self): + return None + + def refresh_observes(self, paths): + return None + + +class _Descriptor: + def on_observation(self, state, href, rep): + return None + + +def _assert_worker_stops(target, name: str): + stop = threading.Event() + entered = threading.Event() + errors: list[str] = [] + + def run(): + entered.set() + try: + target(stop) + except Exception as error: # noqa: BLE001 # pragma: no cover + errors.append(type(error).__name__) + + worker = threading.Thread(target=run, name=name) + worker.start() + assert entered.wait(0.5), f"{name} did not enter its worker" + stop.set() + worker.join(0.5) + assert not worker.is_alive(), f"{name} did not stop" + assert errors == [], f"{name} raised {errors[0]}" + + +def test_keepalive_worker_stops_without_waiting_for_interval(): + task = KeepaliveTask(_Session(), interval_s=3600.0) + _assert_worker_stops(task.run_forever, "test-keepalive") + + +def test_observe_refresh_worker_stops_without_waiting_for_interval(): + task = ObserveRefreshTask(_Session(), [], interval_s=3600.0) + _assert_worker_stops(task.run_forever, "test-observe-refresh") + + +def test_poll_scheduler_worker_stops_without_leaking_thread(): + scheduler = PollScheduler( + _Session(), + StateCache(_Descriptor()), + [PollTier("idle", interval_s=3600.0, paths=())], + ) + _assert_worker_stops(scheduler.run_forever, "test-poll-scheduler") diff --git a/tools/check_distribution.py b/tools/check_distribution.py new file mode 100755 index 0000000..c246b92 --- /dev/null +++ b/tools/check_distribution.py @@ -0,0 +1,126 @@ +#!/usr/bin/env python3 +"""Verify that SmartThings-Local wheel and sdist contents are intentional.""" + +from __future__ import annotations + +import argparse +import subprocess +import tarfile +import zipfile +from pathlib import Path, PurePosixPath + + +class DistributionError(RuntimeError): + """An artifact contains a missing, unexpected, or unsafe member.""" + + +def _tracked_files() -> set[str]: + proc = subprocess.run( + ["git", "ls-files", "-z", "--", "smartthings_local", "tests"], + capture_output=True, + check=True, + ) + return {value.decode("utf-8") for value in proc.stdout.split(b"\0") if value} + + +def _safe_member(name: str) -> bool: + path = PurePosixPath(name) + return bool(name) and not path.is_absolute() and ".." not in path.parts + + +def _expected_package_files() -> set[str]: + tracked = { + path for path in _tracked_files() if path.startswith("smartthings_local/") + } + tracked.add("smartthings_local/_version.py") + return tracked + + +def check_wheel(path: Path) -> None: + with zipfile.ZipFile(path) as archive: + names = set(archive.namelist()) + if not names or any(not _safe_member(name) for name in names): + raise DistributionError("wheel has an unsafe member") + + package_files = {name for name in names if name.startswith("smartthings_local/")} + if package_files != _expected_package_files(): + raise DistributionError( + "wheel package contents differ from the tracked package" + ) + + metadata = names - package_files + roots = {name.split("/", 1)[0] for name in metadata} + if len(roots) != 1: + raise DistributionError("wheel must contain one dist-info directory") + dist_info = roots.pop() + if not dist_info.endswith(".dist-info"): + raise DistributionError("wheel metadata directory is invalid") + expected_metadata = { + f"{dist_info}/METADATA", + f"{dist_info}/WHEEL", + f"{dist_info}/licenses/LICENSE", + f"{dist_info}/RECORD", + } + if metadata != expected_metadata: + raise DistributionError("wheel metadata contents are unexpected") + + +def check_sdist(path: Path) -> None: + with tarfile.open(path, mode="r:gz") as archive: + members = archive.getmembers() + if not members or any( + not member.isfile() or member.issym() or member.islnk() for member in members + ): + raise DistributionError("sdist must contain regular files only") + names = {member.name for member in members} + if any(not _safe_member(name) for name in names): + raise DistributionError("sdist has an unsafe member") + + roots = {name.split("/", 1)[0] for name in names} + if len(roots) != 1: + raise DistributionError("sdist must contain one top-level directory") + root = roots.pop() + relative = {name[len(root) + 1 :] for name in names if name.startswith(f"{root}/")} + expected = _tracked_files() | { + ".gitignore", + ".hgignore", + "LICENSE", + "PKG-INFO", + "README.md", + "pyproject.toml", + "smartthings_local/_version.py", + } + if relative != expected: + raise DistributionError("sdist contents differ from the intended source set") + + +def check_directory(directory: Path) -> None: + wheels = sorted(directory.glob("*.whl")) + sdists = sorted(directory.glob("*.tar.gz")) + if len(wheels) != 1 or len(sdists) != 1: + raise DistributionError("expected exactly one wheel and one sdist") + check_wheel(wheels[0]) + check_sdist(sdists[0]) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("directory", type=Path) + args = parser.parse_args() + try: + check_directory(args.directory) + except ( + DistributionError, + OSError, + subprocess.SubprocessError, + tarfile.TarError, + zipfile.BadZipFile, + ): + print("distribution check failed") + return 1 + print("distribution contents verified") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tools/check_share_safety.py b/tools/check_share_safety.py new file mode 100755 index 0000000..b35907d --- /dev/null +++ b/tools/check_share_safety.py @@ -0,0 +1,229 @@ +#!/usr/bin/env python3 +"""Check changed public files for common private-data and secret shapes. + +Findings contain only path, line, and rule ID. Matched content is never +printed because it may itself be sensitive. +""" + +from __future__ import annotations + +import argparse +import ipaddress +import re +import subprocess +from collections.abc import Iterable +from dataclasses import dataclass +from pathlib import Path + +MAX_TEXT_BYTES = 2 * 1024 * 1024 +DOCUMENTATION_IPV4 = tuple( + ipaddress.ip_network(value) + for value in ("192.0.2.0/24", "198.51.100.0/24", "203.0.113.0/24") +) +DOCUMENTATION_IPV6 = ipaddress.ip_network("2001:db8::/32") +SAFE_UUIDS = { + "00000000-0000-0000-0000-000000000000", + "11111111-2222-3333-4444-555555555555", +} +FORBIDDEN_SUFFIXES = { + ".7z", + ".apk", + ".cap", + ".db", + ".der", + ".gz", + ".jks", + ".key", + ".p12", + ".pcap", + ".pcapng", + ".pfx", + ".sqlite", + ".sqlite3", + ".tar", + ".tgz", + ".zip", +} + +PATTERNS = ( + ( + "PEM_PRIVATE_KEY", + re.compile(r"-----BEGIN (?:RSA |EC |OPENSSH |DSA )?PRIVATE KEY-----"), + ), + ( + "EMAIL_ADDRESS", + re.compile(r"[A-Za-z0-9._%+-]+@(?:[A-Za-z0-9-]+\.)+[A-Za-z]{2,}"), + ), + ( + "MAC_ADDRESS", + re.compile(r"(?i)(? str: + return f"{self.path}:{self.line}:{self.rule_id}" + + +def _safe_ipv4(value: str) -> bool: + address = ipaddress.ip_address(value) + return ( + address.is_loopback + or address.is_unspecified + or any(address in network for network in DOCUMENTATION_IPV4) + ) + + +def _safe_ipv6(value: str) -> bool: + address = ipaddress.ip_address(value.strip("[]").split("%", 1)[0]) + return ( + address.is_loopback or address.is_unspecified or address in DOCUMENTATION_IPV6 + ) + + +def scan_text(path: str, text: str) -> list[Finding]: + findings: set[Finding] = set() + for line_number, line in enumerate(text.splitlines(), start=1): + for rule_id, pattern in PATTERNS: + if pattern.search(line): + findings.add(Finding(path, line_number, rule_id)) + for match in UUID_RE.finditer(line): + if match.group(0).lower() not in SAFE_UUIDS: + findings.add(Finding(path, line_number, "UUID")) + for match in IPV4_RE.finditer(line): + if not _safe_ipv4(match.group(0)): + findings.add(Finding(path, line_number, "NON_DOCUMENTATION_IPV4")) + for match in IPV6_RE.finditer(line): + try: + safe = _safe_ipv6(match.group(0)) + except ValueError: + continue + if not safe: + findings.add(Finding(path, line_number, "NON_DOCUMENTATION_IPV6")) + return sorted(findings) + + +def scan_file(path: Path, display_path: str) -> list[Finding]: + if path.is_symlink(): + return [Finding(display_path, 0, "SYMLINK")] + if path.suffix.casefold() in FORBIDDEN_SUFFIXES: + return [Finding(display_path, 0, "FORBIDDEN_FILE_TYPE")] + data = path.read_bytes() + if len(data) > MAX_TEXT_BYTES: + return [Finding(display_path, 0, "FILE_TOO_LARGE")] + if b"\x00" in data: + return [Finding(display_path, 0, "BINARY_CONTENT")] + try: + text = data.decode("utf-8", errors="strict") + except UnicodeDecodeError: + return [Finding(display_path, 0, "NON_UTF8_CONTENT")] + return scan_text(display_path, text) + + +def _changed_paths(base: str) -> list[str]: + commands = ( + [ + "git", + "diff", + "--name-only", + "--diff-filter=ACMR", + "-z", + f"{base}..HEAD", + "--", + ], + ["git", "diff", "--cached", "--name-only", "--diff-filter=ACMR", "-z", "--"], + ["git", "diff", "--name-only", "--diff-filter=ACMR", "-z", "--"], + ) + changed = [ + subprocess.run(command, capture_output=True, check=True) for command in commands + ] + untracked = subprocess.run( + ["git", "ls-files", "--others", "--exclude-standard", "-z"], + capture_output=True, + check=True, + ) + return sorted( + { + value.decode("utf-8") + for output in (*(result.stdout for result in changed), untracked.stdout) + for value in output.split(b"\0") + if value + } + ) + + +def check_paths(paths: Iterable[str]) -> list[Finding]: + findings: list[Finding] = [] + for value in paths: + findings.extend(scan_file(Path(value), value)) + return sorted(set(findings)) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("paths", nargs="*") + parser.add_argument("--changed-since") + args = parser.parse_args() + try: + paths = _changed_paths(args.changed_since) if args.changed_since else args.paths + if not paths: + raise ValueError("no paths selected") + findings = check_paths(paths) + except (OSError, UnicodeDecodeError, subprocess.SubprocessError, ValueError): + print("share-safety scan failed closed:SCAN_ERROR") + return 2 + for finding in findings: + print(finding.render()) + return 1 if findings else 0 + + +if __name__ == "__main__": + raise SystemExit(main())