diff --git a/api/tests/test_ci_hygiene.py b/api/tests/test_ci_hygiene.py new file mode 100644 index 0000000..8113221 --- /dev/null +++ b/api/tests/test_ci_hygiene.py @@ -0,0 +1,118 @@ +"""CI hygiene guard (house rule 6): lockfile-only installs, no host-port services.""" + +from __future__ import annotations + +import importlib.util +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(ROOT / "scripts")) +_spec = importlib.util.spec_from_file_location("ci_hygiene", ROOT / "scripts" / "ci_hygiene.py") +hy = importlib.util.module_from_spec(_spec) +sys.modules["ci_hygiene"] = hy +_spec.loader.exec_module(hy) + +WF = ".github/workflows/ci.yml" + + +@pytest.mark.parametrize("path, text", [ + (WF, " .venv/bin/pip install -e \".[dev]\""), # windy-git's own, before e64a1b5 + ("Dockerfile", "RUN pip install --no-cache-dir -e ."), # windy-git image, before e64a1b5 + (WF, " - run: uv pip install -e \".[dev]\""), # WindyCloud #109's CI + (WF, " run: pip install fastapi uvicorn"), + (WF, " - run: uv sync --all-extras"), # windy-mind style, not locked + (WF, " - run: npm install"), # windy-drops / windytalk + (WF, " - run: npm install --no-save --no-audit --no-fund jsdom"), # windy-pro reality-check + (WF, " - run: yarn install"), + (WF, " - run: cd web && pnpm install"), + ("docker/api.Dockerfile", "RUN apt-get update && pip install requests"), +]) +def test_floating_installs_are_flagged(path, text): + assert [k for k, _ in hy.scan_line(path, text)] == ["floating install"] + + +@pytest.mark.parametrize("path, text", [ + (WF, " python3 -m pip install -q uv==0.12.5"), # exact tool pin + (WF, " uv sync --locked --extra dev"), + (WF, " - run: uv sync --frozen"), + (WF, " - run: npm ci"), + (WF, " - run: npm install --no-save jsdom@24.1.0"), + (WF, " - run: pip install -r requirements.lock --require-hashes"), + (WF, " - run: pip install -r requirements.txt"), + ("Dockerfile", " && pip install --no-cache-dir --require-hashes -r /tmp/requirements.txt \\\\"), + ("Dockerfile", "RUN pip install --no-cache-dir --no-deps -e ."), # project only, deps from the lock + (WF, " .venv/bin/pip install -q --upgrade pip"), + (WF, " - run: yarn install --frozen-lockfile"), + (WF, " # - run: npm install (commented out)"), + (WF, " - run: echo 'pip is great'"), +]) +def test_locked_or_pinned_installs_pass(path, text): + assert hy.scan_line(path, text) == [] + + +@pytest.mark.parametrize("text, port", [ + (" - 5432:5432", "5432"), # windy-mind / eternitas (collided 09-23) + (" - '15432:5432'", "15432"), # WindyCloud + (' - "6379:6379"', "6379"), +]) +def test_services_publishing_a_host_port_are_flagged(text, port): + [(kind, match)] = hy.scan_line(WF, text) + assert kind == "host port" and port in match + + +def test_host_port_rule_is_for_workflows_only(): + assert hy.scan_line("docker-compose.yml", " - 5432:5432") == [] + + +@pytest.mark.parametrize("path, ok", [ + (".github/workflows/ci.yml", True), (".gitea/workflows/check.yaml", True), + ("Dockerfile", True), ("api/Dockerfile.prod", True), ("docker/web.Dockerfile", True), + ("scripts/setup.sh", False), ("README.md", False), ("node_modules/x/Dockerfile", False), + (".github/lint/x.yml", False), +]) +def test_scope_is_ci_workflows_and_dockerfiles(path, ok): + assert hy.path_ok(path) is ok + + +def test_warn_mode_never_turns_red(monkeypatch): + monkeypatch.setattr(hy, "MODE", "warn") + state, desc, f = hy.status_for([hy.cg.Finding(WF, 12, "floating install", "npm install (use npm ci)")], True) + assert state == "success" and desc.startswith("⚠ WARN (not blocking): 1 CI hygiene issue in CI/Dockerfiles") + + +def test_allow_file_loads_and_is_empty_today(): + assert hy.cg.load_allow(hy.ALLOW_FILE) == [] + + +@pytest.mark.parametrize("path, text, want", [ + ("Dockerfile", "COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv", "ghcr.io/astral-sh/uv:latest"), # Mail #147 + ("Dockerfile", "FROM python:latest", "python:latest"), + ("Dockerfile", "FROM --platform=linux/amd64 node:latest AS web", "node:latest"), + (WF, " image: postgres:latest", "postgres:latest"), + (WF, " - uses: docker://ghcr.io/foo/bar:latest", "ghcr.io/foo/bar:latest"), +]) +def test_latest_images_are_flagged(path, text, want): + hits = hy.scan_line(path, text) + assert ("floating image", want) in hits + + +@pytest.mark.parametrize("text", [ + "COPY pyproject.toml uv.lock* ./", # Windy Mail #147 + "COPY package.json package-lock.json* ./", +]) +def test_optional_lock_globs_are_flagged(text): + assert [k for k, _ in hy.scan_line("Dockerfile", text)] == ["optional lock"] + + +@pytest.mark.parametrize("path, text", [ + ("Dockerfile", "COPY --from=ghcr.io/astral-sh/uv:0.12.5 /uv /usr/local/bin/uv"), + ("Dockerfile", "FROM python:3.12-slim"), + ("Dockerfile", "COPY pyproject.toml uv.lock ./"), + ("Dockerfile", "COPY src/*.py ./src/"), + ("Dockerfile", "RUN echo latest release notes"), +]) +def test_pinned_images_and_real_locks_pass(path, text): + assert hy.scan_line(path, text) == [] diff --git a/api/tests/test_pr_status_bridge.py b/api/tests/test_pr_status_bridge.py index 2f8240a..c690c14 100644 --- a/api/tests/test_pr_status_bridge.py +++ b/api/tests/test_pr_status_bridge.py @@ -382,3 +382,11 @@ def test_guard_that_cannot_run_posts_nothing(fake, monkeypatch): monkeypatch.setitem(sys.modules, "compute_guard", _Guard(None)) bridge.post_compute_guard("windy-chat", SHA, "main", True) assert f.posted == [] + + +def test_ci_hygiene_posts_under_its_own_context(fake, monkeypatch): + f = fake(statuses=[{"context": "windy-git/compute-guard", "state": "success", "description": "WARN 1"}]) + monkeypatch.setitem(sys.modules, "ci_hygiene", _Guard([_F()])) + bridge.post_ci_hygiene("windy-chat", SHA, "main", True) + # the compute-guard status with the same description must not suppress it + assert [(p["context"], p["description"]) for p in f.posted] == [("windy-git/ci-hygiene", "WARN 1")] diff --git a/ci/ci-hygiene-allow.yml b/ci/ci-hygiene-allow.yml new file mode 100644 index 0000000..c33b7da --- /dev/null +++ b/ci/ci-hygiene-allow.yml @@ -0,0 +1,5 @@ +# CI hygiene allow-list: installs that may float, or services that may publish +# a host port. House rule 6 (09-23): installs come from a lockfile. Every entry +# is an exception and MUST say why. Paths are fnmatch globs from the repo root. +# Owner: Windy Git lane (13); changes go through the orchestrator. +allow: [] diff --git a/scripts/ci_hygiene.py b/scripts/ci_hygiene.py new file mode 100644 index 0000000..5572e31 --- /dev/null +++ b/scripts/ci_hygiene.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +"""CI hygiene guard: installs come from a lockfile, never "latest" (house rule 6). + +A floating install lets CI test different versions than prod ships, and a +rebuild silently changes prod. Windy Cloud's OpenAPI test failed on exactly +that (fastapi 0.141.1 in CI vs 0.136.0 on the dev box) and all three Cloud +cells floated in prod. Also flags services that publish a HOST port: every +CI job shares one dind daemon, so two jobs publishing 5432 collide ("port is +already allocated", Windy Mind runs 147/176). + +WARN-ONLY (`windy-git/ci-hygiene`, green + "⚠ WARN"); CI_HYGIENE_MODE=block +turns it red once the lanes report clean. Scans CI workflow files and +Dockerfiles only. PR heads: lines the PR adds. Default branch: every line. + +OK (not flagged): + pip / uv pip install -r FILE (with or without --require-hashes), --no-deps, + exact pins (tool==1.2.3), pip/setuptools/wheel upgrades + uv sync --locked | --frozen npm ci + npm install pkg@1.2.3 (every package exact-pinned) + yarn install --frozen-lockfile / --immutable pnpm install --frozen-lockfile +Also flagged: `:latest` images (FROM / COPY --from / image: / docker://) and +`COPY uv.lock* ...`-style globs that build without the lock (Windy Mail #147). +Exceptions: ci/ci-hygiene-allow.yml, one reason per entry. + + python3 scripts/ci_hygiene.py report [repo ...] +""" + +from __future__ import annotations + +import hashlib +import os +import re +import shlex +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import compute_guard as cg # noqa: E402 (shared walker, cache and allow-list loader) + +ROOT = Path(__file__).resolve().parents[1] +ALLOW_FILE = Path(os.environ.get("CI_HYGIENE_ALLOW", ROOT / "ci" / "ci-hygiene-allow.yml")) +MODE = os.environ.get("CI_HYGIENE_MODE", "warn") + +# CI workflow files and Dockerfiles; never vendored copies. +INCLUDE = re.compile(r"(^|/)\.(github|gitea)/workflows/[^/]+\.ya?ml$|(^|/)(Dockerfile[^/]*|[^/]+\.Dockerfile)$") +NEVER = re.compile(r"(^|/)(node_modules|vendor|third_party)/") +PREFILTER = (r"pip3? install|pip install|uv sync|npm (install|i )|yarn install|pnpm install" + r"|^\s*-\s*['\"]?[0-9]+:[0-9]+|:latest|lock[^ ]*\*") + +TOOLING = {"pip", "setuptools", "wheel"} +DOCKER_FILE = re.compile(r"(^|/)(Dockerfile[^/]*|[^/]+\.Dockerfile)$") +LATEST = re.compile(r"(?:^\s*FROM\s+(?:--platform=\S+\s+)?|--from=|image:\s*['\"]?|docker://)([\w./-]+):latest\b", re.I) +LOCKNAME = re.compile(r"(uv\.lock|poetry\.lock|package-lock\.json|pnpm-lock\.yaml|yarn\.lock|requirements[^ ]*\.(txt|lock))", re.I) +EXACT_PY = re.compile(r"^[A-Za-z0-9._-]+(\[[^\]]*\])?==[A-Za-z0-9.+!-]+$") +EXACT_NPM = re.compile(r"^(@[^/@]+/)?[^/@]+@\d+\.\d+\.\d+([-+][0-9A-Za-z.-]+)?$") +HOST_PORT = re.compile(r"^\s*-\s*['\"]?(\d{2,5}):(\d{2,5})['\"]?\s*(#.*)?$") +PIP_VALUE_FLAGS = {"-c", "--constraint", "-i", "--index-url", "--extra-index-url", "-f", + "--find-links", "--target", "-t", "--python", "--prefix", "--root", "--platform", + "--python-version", "--implementation", "--abi", "--only-binary", "--no-binary"} + + +def path_ok(path: str) -> bool: + return bool(INCLUDE.search(path)) and not NEVER.search(path) + + +def _commands(text: str) -> list[list[str]]: + """Split a shell line into simple commands (&&, ||, ;, |), tokenized.""" + out = [] + for part in re.split(r"&&|\|\||;|\|", text): + try: + toks = shlex.split(part, comments=True) + except ValueError: + toks = part.split() + # Dockerfile RUN prefix / sudo / env-prefixed assignments + while toks and (toks[0] in ("RUN", "sudo", "exec", "-", "run:", "command:") + or re.match(r"^[A-Z_][A-Z0-9_]*=", toks[0])): + toks = toks[1:] + if toks: + out.append(toks) + return out + + +def _pip_problem(args: list[str]) -> str | None: + if "-r" in args or "--requirement" in args or any(a.startswith("--requirement=") for a in args): + return None + if "--no-deps" in args: + return None + pkgs, skip = [], False + for a in args: + if skip: + skip = False + continue + if a in PIP_VALUE_FLAGS: + skip = True + continue + if a.startswith("-") and a not in ("-e", "--editable"): + continue + if a in ("-e", "--editable"): + continue + pkgs.append(a) + loose = [p for p in pkgs if not EXACT_PY.match(p) and p.split("[")[0].lower() not in TOOLING] + if loose: + return f"floating pip install: {' '.join(loose)[:40]}" + return None + + +def scan_line(path: str, text: str) -> list[tuple[str, str]]: + if cg.COMMENT.match(text): + return [] + hits = [] + if "/workflows/" in path and HOST_PORT.match(text): + hits.append(("host port", f"service publishes host port {HOST_PORT.match(text).group(1)} (shared dind)")) + return hits + # Windy Mail #147: a `:latest` build/tool image floats exactly like an + # unpinned package, and `COPY uv.lock* ./` builds WITHOUT the lock when it + # is missing instead of failing. + m = LATEST.search(text) + if m: + hits.append(("floating image", f"{m.group(1)}:latest")) + if DOCKER_FILE.search(path) and re.match(r"^\s*COPY\b", text, re.I): + globbed = [t for t in text.split() if "*" in t and LOCKNAME.search(t)] + if globbed: + hits.append(("optional lock", f"COPY {globbed[0]} (must fail if the lock is missing)")) + for toks in _commands(text): + low = [t.lower() for t in toks] + # pip install / python -m pip install / uv pip install + for i in range(len(low) - 1): + if os.path.basename(low[i]) in ("pip", "pip3") and low[i + 1] == "install": + prob = _pip_problem(toks[i + 2:]) + if prob: + hits.append(("floating install", prob)) + break + if low[:2] == ["uv", "sync"] and not ({"--locked", "--frozen"} & set(low)): + hits.append(("floating install", "uv sync without --locked/--frozen")) + if low[:1] == ["npm"] and len(low) > 1 and low[1] in ("install", "i", "add"): + pkgs = [t for t in toks[2:] if not t.startswith("-")] + if not pkgs or not all(EXACT_NPM.match(p) for p in pkgs): + hits.append(("floating install", f"npm {low[1]} {' '.join(pkgs)[:30]}".strip() + " (use npm ci)")) + if low[:2] == ["yarn", "install"] and not ({"--frozen-lockfile", "--immutable"} & set(low)): + hits.append(("floating install", "yarn install without --frozen-lockfile")) + if low[:2] == ["pnpm", "install"] and "--frozen-lockfile" not in low: + hits.append(("floating install", "pnpm install without --frozen-lockfile")) + return hits + + +def check(repo: str, sha: str, default_branch: str, is_default_head: bool): + bare = cg.WORK / f"{repo}.git" + if not bare.is_dir(): + return None + allow = cg.load_allow(ALLOW_FILE) + rules = hashlib.sha256((PREFILTER + INCLUDE.pattern + EXACT_PY.pattern + EXACT_NPM.pattern).encode()).hexdigest()[:8] + fp = cg._fingerprint(allow) + ":" + rules # hashlib, not hash(): hash() is per-process random + kw = dict(line_fn=scan_line, path_ok=path_ok) + if is_default_head: + return cg.cached_scan(f"hyg-tree:{repo}:{sha}:{fp}", + lambda: cg.scan_tree(repo, bare, sha, allow, prefilter=PREFILTER, **kw)) + return cg.cached_scan(f"hyg-pr:{repo}:{sha}:{fp}", + lambda: cg.scan_added(repo, bare, f"refs/heads/{default_branch}", sha, allow, **kw)) + + +def status_for(findings, whole_tree: bool): + scope = "in CI/Dockerfiles" if whole_tree else "added" + if not findings: + return "success", f"OK: no floating install or host-port service {scope}", None + f = findings[0] + n = len(findings) + state = "failure" if MODE == "block" else "success" + lead = "BLOCKED" if MODE == "block" else "⚠ WARN (not blocking)" + return state, f"{lead}: {n} CI hygiene issue{'s' if n > 1 else ''} {scope}, e.g. {f.path}:{f.line} {f.match}"[:140], f + + +def report(repos: list[str]) -> int: + allow = cg.load_allow(ALLOW_FILE) + total = 0 + for repo in repos: + bare = cg.WORK / f"{repo}.git" + if not bare.is_dir(): + print(f"## {repo}: no sync clone, skipped") + continue + head = cg._git(bare, "symbolic-ref", "--short", "HEAD").strip() + sha = cg._git(bare, "rev-parse", head).strip() + fs = cg.scan_tree(repo, bare, sha, allow, line_fn=scan_line, path_ok=path_ok, prefilter=PREFILTER) + total += len(fs) + print(f"## {repo} ({head} {sha[:7]}): {len(fs)} issue(s)") + for f in fs: + print(f" {f.path}:{f.line} [{f.kind}] {f.match}") + print(f"TOTAL {total}") + return 0 + + +if __name__ == "__main__": + if len(sys.argv) >= 2 and sys.argv[1] == "report": + default = os.environ.get("BRIDGE_REPOS", "").split() or sorted( + p.name.removesuffix(".git") for p in cg.WORK.glob("*.git")) + sys.exit(report(sys.argv[2:] or default)) + sys.exit(__doc__) diff --git a/scripts/compute_guard.py b/scripts/compute_guard.py index 87a6061..0ebe03d 100644 --- a/scripts/compute_guard.py +++ b/scripts/compute_guard.py @@ -130,11 +130,20 @@ def _git(bare: Path, *args: str) -> str: ).stdout -def scan_tree(repo: str, bare: Path, sha: str, allow: list[dict]) -> list[Finding]: +def _default_path_ok(path: str) -> bool: + return not SKIP.search(path) + + +def scan_tree(repo: str, bare: Path, sha: str, allow: list[dict], *, line_fn=None, + path_ok=None, prefilter: str | None = None) -> list[Finding]: """Every line in the tree at `sha` (default branch: the baseline).""" # A cheap prefilter by git, then the real rules in Python. - pre = "|".join([re.escape(h) for h in HOSTS] + KEYS + ["anthropic", "openai", "groq", "mistral", - "generativeai", "genai", "cohere", "together", "cerebras", "litellm"]) + # Other guards (ci_hygiene) reuse this walker with their own line rules. + line_fn = line_fn or scan_line + path_ok = path_ok or _default_path_ok + pre = prefilter or "|".join([re.escape(h) for h in HOSTS] + KEYS + [ + "anthropic", "openai", "groq", "mistral", "generativeai", "genai", "cohere", + "together", "cerebras", "litellm"]) try: out = _git(bare, "grep", "-nIE", "-e", pre, sha, "--", ".") except subprocess.CalledProcessError as e: @@ -148,24 +157,26 @@ def scan_tree(repo: str, bare: Path, sha: str, allow: list[dict]) -> list[Findin _, path, line, text = raw.split(":", 3) except ValueError: continue - if SKIP.search(path) or allowed(repo, path, allow): + if not path_ok(path) or allowed(repo, path, allow): continue - for kind, match in scan_line(path, text): + for kind, match in line_fn(path, text): found.append(Finding(path, int(line), kind, match)) return found -def scan_added(repo: str, bare: Path, base_ref: str, sha: str, allow: list[dict]) -> list[Finding]: +def scan_added(repo: str, bare: Path, base_ref: str, sha: str, allow: list[dict], **kw) -> list[Finding]: """Only the lines a PR adds, vs its merge-base with the default branch.""" mb = _git(bare, "merge-base", base_ref, sha).strip() diff = _git(bare, "diff", "-U0", "--no-color", "--no-ext-diff", mb, sha) - return parse_added(repo, diff, allow) + return parse_added(repo, diff, allow, **kw) HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@") -def parse_added(repo: str, diff: str, allow: list[dict]) -> list[Finding]: +def parse_added(repo: str, diff: str, allow: list[dict], *, line_fn=None, path_ok=None) -> list[Finding]: + line_fn = line_fn or scan_line + path_ok = path_ok or _default_path_ok found, path, line = [], None, 0 for raw in diff.splitlines(): if raw.startswith("+++ "): @@ -179,8 +190,8 @@ def parse_added(repo: str, diff: str, allow: list[dict]) -> list[Finding]: if path is None or raw.startswith("--- "): continue if raw.startswith("+"): - if not (SKIP.search(path) or allowed(repo, path, allow)): - for kind, match in scan_line(path, raw[1:]): + if path_ok(path) and not allowed(repo, path, allow): + for kind, match in line_fn(path, raw[1:]): found.append(Finding(path, line, kind, match)) line += 1 return found diff --git a/scripts/pr_status_bridge.py b/scripts/pr_status_bridge.py index 148878f..11b709f 100755 --- a/scripts/pr_status_bridge.py +++ b/scripts/pr_status_bridge.py @@ -344,34 +344,45 @@ def post_statuses(repo: str, sha: str) -> None: GUARD_CTX = "windy-git/compute-guard" +HYGIENE_CTX = "windy-git/ci-hygiene" def post_compute_guard(repo: str, sha: str, default_branch: str, is_default_head: bool) -> None: - """Windy Mind is the only door to AI compute: flag direct provider use (warn-only). + """Windy Mind is the only door to AI compute: flag direct provider use (warn-only).""" + _post_guard("compute_guard", GUARD_CTX, repo, sha, default_branch, is_default_head) - Non-fatal and never a fake OK: if the guard can't run, nothing is posted. - """ + +def post_ci_hygiene(repo: str, sha: str, default_branch: str, is_default_head: bool) -> None: + """House rule 6: lockfile-only installs, pinned images, no host-port services (warn-only).""" + _post_guard("ci_hygiene", HYGIENE_CTX, repo, sha, default_branch, is_default_head) + + +def _post_guard(modname: str, ctx: str, repo: str, sha: str, default_branch: str, + is_default_head: bool) -> None: + """One code path for every repo-scanning guard. Non-fatal and never a fake OK: + if the guard can't run, nothing is posted.""" try: - import compute_guard as cg # same directory; loaded lazily so the bridge never depends on it + import importlib - findings = cg.check(repo, sha, default_branch, is_default_head) - except Exception as e: # noqa: BLE001 — the guard must never break CI signals - print(f" {repo}@{sha[:7]} compute-guard skipped ({type(e).__name__}: {str(e)[:80]})") + g = importlib.import_module(modname) # same directory; lazy so the bridge never depends on it + findings = g.check(repo, sha, default_branch, is_default_head) + except Exception as e: # noqa: BLE001 — a guard must never break CI signals + print(f" {repo}@{sha[:7]} {ctx} skipped ({type(e).__name__}: {str(e)[:80]})") return if findings is None: return - state, desc, first = cg.status_for(findings, whole_tree=is_default_head) + state, desc, first = g.status_for(findings, whole_tree=is_default_head) st, existing = github("GET", f"/repos/{GH_OWNER}/{repo}/commits/{sha}/statuses?per_page=100") for s in existing or []: # newest first: compare the latest guard status only - if s["context"] == GUARD_CTX: + if s["context"] == ctx: if (s["state"], s.get("description")) == (state, desc): return break url = (f"{PUBLIC}/{WG_OWNER}/{repo}/src/commit/{sha}/{first.path}#L{first.line}" if first else f"{PUBLIC}/{WG_OWNER}/{repo}/src/commit/{sha}") st, _ = github("POST", f"/repos/{GH_OWNER}/{repo}/statuses/{sha}", - {"state": state, "context": GUARD_CTX, "description": desc, "target_url": url}) - print(f" {repo}@{sha[:7]} {GUARD_CTX} = {state} ({len(findings)} finding(s)) -> {st}") + {"state": state, "context": ctx, "description": desc, "target_url": url}) + print(f" {repo}@{sha[:7]} {ctx} = {state} ({len(findings)} finding(s)) -> {st}") def main() -> int: @@ -392,6 +403,7 @@ def main() -> int: for sha in dict.fromkeys(shas): post_statuses(repo, sha) post_compute_guard(repo, sha, default_branch, sha == default_head) + post_ci_hygiene(repo, sha, default_branch, sha == default_head) except Exception as e: # one repo's failure must not hide the others' print(f" FAILED {repo}: {e}") failed = 1