diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62c1c66..32c43bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,20 +93,3 @@ jobs: - name: Verify distribution metadata (publish dry run) run: twine check dist/* - # NEW -- regression guard for the internal-reference leak this repo already - # had once (see scripts/check_public_repo_hygiene.py for what it looks for - # and why). Public repo, so this stays a permanent gate, not a one-time fix. - public-repo-hygiene: - name: public-repo-hygiene - runs-on: ubuntu-latest - steps: - - name: Check out repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - name: Set up Python - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: '3.12' - - - name: Scan for internal-only references - run: python3 scripts/check_public_repo_hygiene.py diff --git a/.github/workflows/public-repo-hygiene.yml b/.github/workflows/public-repo-hygiene.yml new file mode 100644 index 0000000..906e41e --- /dev/null +++ b/.github/workflows/public-repo-hygiene.yml @@ -0,0 +1,29 @@ +# Description: Thin caller for the shared public-repo-hygiene reusable workflow +# in Comfy-Org/github-workflows, which owns the checker AND its known-public +# allowlist. This replaces the in-tree copy this repo used to carry. +# +# Why the checker is not in this repo any more: the old job ran +# `scripts/check_public_repo_hygiene.py` straight out of the PR's own checkout, +# so a PR could widen the allowlist — or disable the scan — and go green. The +# reusable loads the checker from the pinned `workflows_ref` commit instead, so +# a PR here cannot reach it through this workflow's inputs. The allowlist is +# deliberately NOT an input, for the same reason. +# +# The pin is kept fresh by bump-public-repo-hygiene-callers.yml once this repo is +# enrolled in the PUBLIC_REPO_HYGIENE_CALLERS roster. +name: Public Repo Hygiene + +on: + pull_request: + push: + branches: [main] + +jobs: + hygiene: + permissions: + contents: read + uses: Comfy-Org/github-workflows/.github/workflows/public-repo-hygiene.yml@aad06972e20732d6a9167552c5b3247c4f5bac58 # github-workflows main (aad0697) + with: + # REQUIRED, and must equal the `uses:` SHA above — the reusable asserts + # they match, which is what pins the checker to the reviewed commit. + workflows_ref: aad06972e20732d6a9167552c5b3247c4f5bac58 diff --git a/AGENTS.md b/AGENTS.md index 20e8e7f..35bf1df 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,12 +100,40 @@ for `py310`. Anything newer than 3.10 syntax breaks a quarter of the matrix. ### `public-repo-hygiene` -This repo is public. [`scripts/check_public_repo_hygiene.py`](scripts/check_public_repo_hygiene.py) -scans every git-tracked file (except its own source and -`src/comfy_low/models/`) and fails on three categories: +This repo is public. [`.github/workflows/public-repo-hygiene.yml`](.github/workflows/public-repo-hygiene.yml) +is a thin caller into the shared reusable workflow in `Comfy-Org/github-workflows`, +which owns both the checker and its known-public allowlist — this repo used to +carry its own copy of the checker (`scripts/check_public_repo_hygiene.py`), but +that let a PR widen the allowlist or disable the scan and still go green. The +reusable loads the checker from a pinned `workflows_ref` commit instead, so a +PR here cannot reach it through this workflow's inputs. + +**What the pin does and does not buy.** `workflows_ref` binds the checker to the +commit the caller's own `uses:` line selected — the reusable asserts the two +match, and that is all it asserts. A `pull_request` run executes the workflow +file from the PR head, so a PR that rewrites *both* the `uses:` SHA and +`workflows_ref` (or replaces the caller job outright) still satisfies that +equality check while running a checker of its own choosing. That is true of +every reusable workflow on GitHub, not a quirk of this one. + +The control for it is out of band, and it is a branch-protection setting rather +than a file in this repo: `main` requires an approving **code-owner** review and +dismisses stale approvals on every new push, so a change under +`.github/workflows/` cannot land on its author's say-so. +[`.github/CODEOWNERS`](.github/CODEOWNERS) owns `*`, which is what puts the +workflow directory under that requirement — but CODEOWNERS *alone* enforces +nothing, it only names reviewers. The rule is not absolute either: it is not +enforced for administrators. Relax the branch-protection setting and the pin's +guarantee relaxes with it, silently. + +The job runs from its own workflow file, not `ci.yml`, so the status-check +context it reports is `hygiene / public-repo-hygiene`. + +The checker scans every git-tracked file (except `src/comfy_low/models/`) and +fails on three categories: 1. **Ticket-shaped identifiers** — anything matching `[A-Z]{2,6}-\d{2,6}`. - Common tech acronyms are handled by an explicit allowlist in the script. + Common tech acronyms are handled by an explicit allowlist. 2. **Internal collaboration-tool links/markers** — Notion, Slack archive and client links, Google Docs/Drive, Datadog, PostHog project links, Linear, and `incident-` strings. @@ -116,14 +144,8 @@ scans every git-tracked file (except its own source and Practical consequence: **do not paste internal context into code comments, docstrings, commit-adjacent docs, or test fixtures.** Describe *why* in plain prose instead of linking to where the discussion happened. If a flag is a -genuine false positive, extend the allowlist in the script with a comment -explaining why — do not loosen the regex. - -Run it locally with plain Python; it needs no dependencies: - -```bash -python3 scripts/check_public_repo_hygiene.py -``` +genuine false positive, the allowlist lives in `Comfy-Org/github-workflows`, +not this repo — file the fix there. ### `build-check` diff --git a/CHANGELOG.md b/CHANGELOG.md index 4662bda..397c9a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -178,10 +178,10 @@ Maintenance release. No API changes — existing code needs no updates. empty package metadata. - Stop sweeping local dev droppings into the sdist — it now contains only what is needed to build and run the tests. -- The repository moved from `Comfy-Org/ComfyPythonSDK` to - `Comfy-Org/comfy-python-sdk`. GitHub redirects the old URLs and the PyPI - package name is unchanged (`comfy-sdk`). This is the first release to carry - the corrected repository/issues URLs in its published metadata. +- The repository moved from `ComfyPythonSDK` to `Comfy-Org/comfy-python-sdk`. + GitHub redirects the old URLs and the PyPI package name is unchanged + (`comfy-sdk`). This is the first release to carry the corrected + repository/issues URLs in its published metadata. - Docstrings for the public methods that had none; README aligned with the TypeScript and Swift SDK READMEs. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa2b132..a2aa703 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,9 +56,11 @@ CI runs two more jobs beyond the four above: - **`build-check`** — builds the sdist and wheel and runs `twine check`, so a broken distribution is caught in PR CI instead of at release time. -- **`public-repo-hygiene`** — `python3 scripts/check_public_repo_hygiene.py` - scans for internal-only references. This is a public repo; the check is a - permanent gate, not a one-time cleanup. +- **`public-repo-hygiene`** — a thin caller into the shared reusable workflow + in `Comfy-Org/github-workflows`, which scans for internal-only references. + This is a public repo; the check is a permanent gate, not a one-time + cleanup. It lives in its own workflow file rather than `ci.yml`, so it + reports as `hygiene / public-repo-hygiene`. See AGENTS.md for details. ## The codegen trap: `src/comfy_low/models/_generated.py` diff --git a/scripts/check_public_repo_hygiene.py b/scripts/check_public_repo_hygiene.py deleted file mode 100644 index cd2c798..0000000 --- a/scripts/check_public_repo_hygiene.py +++ /dev/null @@ -1,189 +0,0 @@ -#!/usr/bin/env python3 -"""Fail CI if the tree contains markers that only make sense in an internal -(private) context — this repo is public. - -This is a lightweight regression guard, not a secrets scanner: it looks for -categories of internal-only references (ticket-style IDs, internal -collaboration-tool links, and repo names outside the known-public set), not -credentials. It intentionally uses small, explicit allow/deny lists instead -of a single clever regex, so a false positive is a one-line list edit instead -of a mystery. - -Run: python3 scripts/check_public_repo_hygiene.py -""" - -from __future__ import annotations - -import re -import subprocess -import sys -from pathlib import Path - -ROOT = Path(__file__).resolve().parent.parent - -# Files this script itself doesn't need to scan (its own source, lockfiles, -# and generated/vendored output that isn't hand-authored). -EXCLUDE_PATHS = { - Path("scripts/check_public_repo_hygiene.py"), -} -EXCLUDE_DIR_PREFIXES = ( - "src/comfy_low/models/", # generated from the vendored spec -) - -# --- Category 1: ticket-shaped identifiers (TEAM-1234) ------------------- -# Generic shape rather than a guessed list of real internal team keys, so we -# don't need to encode (and thus disclose) an internal naming scheme here. -# Catches false positives on common tech acronyms via an explicit allowlist -# below -- extend that list, not the regex, when a legitimate term trips it. -TICKET_RE = re.compile(r"\b[A-Z]{2,6}-\d{2,6}\b") -TICKET_ALLOWLIST = { - "UTF-8", - "ISO-8601", - "SHA-256", - "SHA-384", - "SHA-512", - "AES-128", - "AES-256", - "RFC-2119", - "RFC-7231", - "RFC-3339", - "OAUTH-2", - "IPV-4", - "IPV-6", - "X-25519", - "WIN-32", - "WIN-64", -} - -# --- Category 2: internal collaboration-tool links/markers ---------------- -INTERNAL_MARKER_RES = [ - re.compile(r"notion\.(so|site)/", re.IGNORECASE), - re.compile(r"slack\.com/(archives|client)/", re.IGNORECASE), - re.compile(r"\bapp\.slack\.com\b", re.IGNORECASE), - re.compile(r"docs\.google\.com/", re.IGNORECASE), - re.compile(r"drive\.google\.com/", re.IGNORECASE), - re.compile(r"app\.datadoghq\.com/", re.IGNORECASE), - re.compile(r"\bposthog\.com/project/", re.IGNORECASE), - re.compile(r"\blinear\.app/", re.IGNORECASE), - re.compile(r"\bincident-\d+\b", re.IGNORECASE), -] - -# --- Category 3: references to Comfy-Org repos outside the known-public set -# Default-deny: only these are known to be public. Anything else under -# `Comfy-Org/` gets flagged so a maintainer can either scrub it or add -# it here once confirmed public. (No private repo names are listed here on -# purpose -- the point of default-deny is that we never need to.) -PUBLIC_COMFY_ORG_REPOS = { - "comfy-api-proxy", - "comfy-cla", - "comfy-cli", - "comfy-cloud-mcp-server", - "Comfy-Desktop", - "comfy-python-sdk", - "comfy-swift-sdk", - "comfy-typescript-sdk", - # This repo's pre-rename name (v0.1.5 moved it to comfy-python-sdk). - # Public, and GitHub still redirects it, so historical references -- the - # CHANGELOG's rename note, old release-notes compare links -- stay valid. - "ComfyPythonSDK", - "ComfyUI_frontend", - "ComfyUI", - # Reusable GitHub Actions workflows (cursor-review and friends). Verified - # public: `gh repo view Comfy-Org/github-workflows` reports - # visibility=PUBLIC. That check is the ONLY basis for an entry here -- a - # resolving `uses:` reference is not evidence, because a private repo can - # share its reusable workflows org-internally (Settings > Actions > access), - # so a passing run says nothing about visibility. - "github-workflows", -} -# CODEOWNERS team handles (`@Comfy-Org/`) are inherently public on a -# public repo -- GitHub renders the CODEOWNERS owners to anyone who can see the -# repo, so listing them here is not a leak. These mirror the sibling repos' -# CODEOWNERS (e.g. comfy-api-proxy). An `@Comfy-Org/` handle NOT in this -# set is still flagged, so a genuinely-internal team reference surfaces. -PUBLIC_COMFY_ORG_TEAMS = { - "comfy-cloud-team", - "core-engine-team", -} -REPO_REF_RE = re.compile(r"Comfy-Org/([A-Za-z0-9_.-]+)") - - -def _tracked_files() -> list[Path]: - out = subprocess.run( - ["git", "ls-files"], cwd=ROOT, check=True, capture_output=True, text=True - ).stdout - return [Path(p) for p in out.splitlines() if p] - - -def _is_excluded(rel: Path) -> bool: - if rel in EXCLUDE_PATHS: - return True - return any(str(rel).startswith(prefix) for prefix in EXCLUDE_DIR_PREFIXES) - - -def _check_file(rel: Path) -> list[str]: - findings: list[str] = [] - abs_path = ROOT / rel - try: - text = abs_path.read_text(encoding="utf-8") - except (UnicodeDecodeError, OSError): - return findings # binary or unreadable; not in scope - - for lineno, line in enumerate(text.splitlines(), start=1): - for match in TICKET_RE.finditer(line): - if match.group(0).upper() not in TICKET_ALLOWLIST: - findings.append(f"{rel}:{lineno}: possible internal ticket ID: {match.group(0)!r}") - - for pattern in INTERNAL_MARKER_RES: - if pattern.search(line): - findings.append( - f"{rel}:{lineno}: internal collaboration-tool marker: {line.strip()!r}" - ) - - for match in REPO_REF_RE.finditer(line): - name = match.group(1) - # A leading `@` makes this a CODEOWNERS team handle, not a repo ref. - if match.start() > 0 and line[match.start() - 1] == "@": - if name not in PUBLIC_COMFY_ORG_TEAMS: - findings.append( - f"{rel}:{lineno}: reference to @Comfy-Org/{name}, a team not in the " - "known-public allowlist (scripts/check_public_repo_hygiene.py) -- " - "confirm it's public and add it, or remove the reference" - ) - continue - if name not in PUBLIC_COMFY_ORG_REPOS: - findings.append( - f"{rel}:{lineno}: reference to Comfy-Org/{name}, which is not in the " - "known-public allowlist (scripts/check_public_repo_hygiene.py) -- " - "confirm it's public and add it, or remove the reference" - ) - - return findings - - -def main() -> int: - all_findings: list[str] = [] - for rel in _tracked_files(): - if _is_excluded(rel): - continue - all_findings.extend(_check_file(rel)) - - if all_findings: - print( - "ERROR: possible internal-only references found in this public repo:\n", file=sys.stderr - ) - for finding in all_findings: - print(f" {finding}", file=sys.stderr) - print( - "\nIf this is a genuine false positive, extend the allowlist in " - "scripts/check_public_repo_hygiene.py with a comment explaining why.", - file=sys.stderr, - ) - return 1 - - print("OK: no internal-only references found") - return 0 - - -if __name__ == "__main__": - raise SystemExit(main())