diff --git a/.github/time/housekeeping.py b/.github/time/housekeeping.py index 670e439..ee019fc 100644 --- a/.github/time/housekeeping.py +++ b/.github/time/housekeeping.py @@ -68,6 +68,12 @@ def age_days(ts: str | None, now: dt.datetime) -> float: return (now - d).total_seconds() / 86400 if d else 0.0 +def labelled(name: str, query: str = "state=open") -> list[dict]: + """Issues (and PRs) that carry `name`. Each row is checked too: Gitea ignores a filter on a label the repo does not + have and returns everything, which on the first live run listed every open issue as a parked card.""" + return [i for i in paged(f"{repo()}/issues?{query}&labels={urllib.parse.quote(name)}") if tm.has_label(i, name)] + + def same_repo(pr: dict) -> bool: return ((pr.get("head") or {}).get("repo") or {}).get("full_name", tm.REPO) == tm.REPO # a fork's branch is not ours @@ -121,7 +127,7 @@ def person(user: dict | None) -> str | None: def signup_people() -> dict[str, set[str]]: """venture slug -> distinct people who opened an early-access issue for it.""" out: dict[str, set[str]] = {} - for i in paged(f"{repo()}/issues?state=all&labels=signup&type=issues"): + for i in labelled("signup", "state=all&type=issues"): m = re.search(r"^###\s*Venture\s*$\n+(.*?)(?=^###\s|\Z)", i.get("body") or "", re.M | re.S) who = person(i.get("user")) if m and who and m.group(1).strip(): @@ -131,7 +137,7 @@ def signup_people() -> dict[str, set[str]]: def step_signals(live: bool, now: dt.datetime) -> int: days = int(float(os.environ.get("TIME_VENTURE_MEASURE_DAYS") or 7)) - due = [i for i in paged(f"{repo()}/issues?state=open&labels=signal&type=issues") + due = [i for i in labelled("signal", "state=open&type=issues") if not i.get("pull_request") and age_days(i.get("created_at"), now) >= days] if not due: return 0 @@ -183,7 +189,7 @@ def stale(now: dt.datetime) -> dict[str, list[tuple[int, float, str]]]: def add(cat: str, number: int, age: float, title: str) -> None: out.setdefault(cat, []).append((number, age, title)) - for i in paged(f"{repo()}/issues?state=open&labels=needs-human"): # issues and PRs alike + for i in labelled("needs-human"): # issues and PRs alike age = age_days(i.get("updated_at") or i.get("created_at"), now) if age > NEEDS_HUMAN_DAYS: add(f"`needs-human`, untouched for over {NEEDS_HUMAN_DAYS} days", i["number"], age, i.get("title", "")) @@ -191,9 +197,9 @@ def stale(now: dt.datetime) -> dict[str, list[tuple[int, float, str]]]: age = age_days(p.get("updated_at") or p.get("created_at"), now) if age >= PR_IDLE_DAYS: add(f"open PRs idle {PR_IDLE_DAYS} days or more", p["number"], age, p.get("title", "")) - for i in paged(f"{repo()}/issues?state=open&labels=parked&type=issues"): + for i in labelled("parked", "state=open&type=issues"): add("parked cards", i["number"], age_days(i.get("updated_at") or i.get("created_at"), now), i.get("title", "")) - for card in paged(f"{repo()}/issues?state=open&labels=ready&type=issues"): # last beat: error for a while, or parked + for card in labelled("ready", "state=open&type=issues"): # last beat: error for a while, or parked beats = [b for b in tm.beats_of(paged(f"{repo()}/issues/{card['number']}/comments")) if b["beat"] not in ("blocked", "capped")] last = beats[-1] if beats else None if not last: @@ -216,7 +222,7 @@ def stale_text(found: dict[str, list[tuple[int, float, str]]]) -> str: def log_issue() -> int: - for i in paged(f"{repo()}/issues?state=open&labels=astra&type=issues"): + for i in labelled("astra", "state=open&type=issues"): if i.get("title") == LOG_TITLE: return i["number"] return LOG_FALLBACK diff --git a/.github/time/test_housekeeping.py b/.github/time/test_housekeeping.py index 5c458a9..d27930c 100644 --- a/.github/time/test_housekeeping.py +++ b/.github/time/test_housekeeping.py @@ -23,13 +23,15 @@ LOG = 57 class Forge: - def __init__(self, branches=(), prs=(), issues=(), milestones=(), reactions=None, comments=None): + def __init__(self, branches=(), prs=(), issues=(), milestones=(), reactions=None, comments=None, repo_labels=None): self.branches = [{"name": b} if isinstance(b, str) else b for b in branches] self.prs = list(prs) self.issues = {i["number"]: {"state": "open", "labels": [], "created_at": iso(0), "updated_at": iso(0), "body": "", **i} for i in issues} self.milestones = [dict(m) for m in milestones] self.reactions = reactions or {} self.cmts = comments or {} + # the repo's labels; by default the ones its issues carry. Gitea ignores a filter on any other label. + self.repo_labels = set(repo_labels) if repo_labels is not None else {l for i in self.issues.values() for l in i["labels"]} self.writes = [] # every non-GET call self.posted = [] # (issue, body) @@ -72,7 +74,7 @@ class Forge: if route == "/repos/o/r/issues" and method == "GET": rows = [i for i in self.issues.values() if (q.get("state", ["open"])[0] in ("all", i["state"])) - and all(l in [x for x in i["labels"]] for l in q.get("labels", [""])[0].split(",") if l)] + and all(l in i["labels"] for l in q.get("labels", [""])[0].split(",") if l and l in self.repo_labels)] return self.page([{**i, "labels": [{"name": l} for l in i["labels"]]} for i in rows], path) if route.startswith("/repos/o/r/issues/") and method == "PATCH": self.issues[int(route.split("/")[-1])].update(body) @@ -161,12 +163,26 @@ def test_stale(): t = texts[0] assert "#30" in t and "#40" in t and "#32" in t, t assert "#31" not in t and "#41" not in t and "#33" not in t, t - assert any("3 stale" in b for _, b in f.posted) or any("4 stale" in b for _, b in f.posted), f.posted + assert any("3 stale" in b for _, b in f.posted), f.posted # #30, #32, #40: exactly three # an empty list posts no stale comment, only the one summary line f = Forge(); f.run() assert [b for _, b in f.posted if "stale list" in b] == [] and len(f.posted) == 1 and "0 stale" in f.posted[0][1] +def test_unknown_label(): + """No `parked` label in the repo: Gitea answers `labels=parked` with every open issue. None of them is stale, and + a `signal` filter answered with everything must not close an old issue that only looks like a signal.""" + f = Forge(issues=[{"number": 57, "title": "Astra log", "labels": ["astra"], "updated_at": iso(9)}, + {"number": 72, "title": "a parent", "labels": ["planned"], "updated_at": iso(9)}, + {"number": 96, "title": "queued card", "labels": ["queued"], "updated_at": iso(9)}, + {"number": 99, "title": "old note", "labels": [], "created_at": iso(30), "body": "see ?venture=spent"}], + repo_labels={"astra", "planned", "queued"}) + f.run() + assert [b for _, b in f.posted if "stale list" in b] == [], f.posted + assert any("0 stale" in b for _, b in f.posted) and f.issues[99]["state"] == "open", f.posted + assert [n for n, _ in f.posted] == [LOG], "the summary goes to the Astra log, found by its own label" + + def test_dry_run(): f = Forge(branches=["main", "time/card-53"], prs=[pr(1, "time/card-53", "closed", True)], issues=[sig(10, "spent", 8), {"number": 30, "labels": ["needs-human"], "updated_at": iso(9)}], @@ -178,7 +194,7 @@ def test_dry_run(): if __name__ == "__main__": - tests = [test_branches, test_signals, test_milestones, test_stale, test_dry_run] + tests = [test_branches, test_signals, test_milestones, test_stale, test_unknown_label, test_dry_run] for t in tests: t() print("ok", t.__name__) diff --git a/.github/time/test_time.py b/.github/time/test_time.py index 04a8b94..9b91bef 100644 --- a/.github/time/test_time.py +++ b/.github/time/test_time.py @@ -810,6 +810,20 @@ if _gt is None: os.environ.pop("GITHUB_TOKEN", None) else: os.environ["GITHUB_TOKEN"] = _gt print("ALL OK 54") +# 54b Gitea ignores a filter on a label the repo does not have and returns every issue: issues() keeps only the rows that +# carry every label the query names (a row with no labels field, as in the fakes here, is taken as filtered) +_h54 = tm.http +tm.http = lambda *a, **k: [{"number": 1, "labels": [{"name": "spec"}]}, {"number": 2, "labels": []}, + {"number": 3, "labels": [{"name": "queued"}]}, {"number": 4}, + {"number": 5, "pull_request": {"merged": False}, "labels": [{"name": "spec"}]}, + {"number": 6, "labels": [{"name": "spec"}, {"name": "ready"}]}] +assert [i["number"] for i in tm.issues("state=open&labels=spec")] == [1, 4, 6], "only spec cards, never the rest" +assert [i["number"] for i in tm.issues("state=open&labels=spec,ready")] == [4, 6], "every named label" +assert [i["number"] for i in tm.issues("state=open")] == [1, 2, 3, 4, 6], "no label named: no filter" +assert tm.has_label({"labels": ["ready"]}, "ready") and not tm.has_label({}, "ready") +tm.http = _h54 +print("ALL OK 54b") + # the worker itself (worker.js), against a fake D1, Workers AI and Pages import shutil as _sh, subprocess as _sp # noqa: E402 if _sh.which("node"): diff --git a/.github/time/time.py b/.github/time/time.py index bce9fce..57c3bc1 100644 --- a/.github/time/time.py +++ b/.github/time/time.py @@ -77,9 +77,19 @@ def gh(method: str, path: str, body=None, *, as_time: bool | None = None): time.sleep(0.5 * (attempt + 1)) +def has_label(item: dict, name: str) -> bool: + return any((l.get("name") if isinstance(l, dict) else l) == name for l in item.get("labels") or []) + + def issues(query: str) -> list[dict]: - """Open/closed issues (never PRs) of this repo; `query` is the Gitea query string without type/limit.""" - return [i for i in gh("GET", f"/repos/{REPO}/issues?type=issues&limit=50&{query}") if not i.get("pull_request")] + """Open/closed issues (never PRs) of this repo; `query` is the Gitea query string without type/limit. + + Each row is checked against the labels the query names: Gitea ignores a filter on a label the repo does not have + and returns every issue, so a deleted `spec` label would make every open issue a spec to split. A row with no + `labels` field at all (older payloads, test fakes) is taken as the server filtered it.""" + want = [l for l in (urllib.parse.parse_qs(query).get("labels") or [""])[0].split(",") if l] + rows = [i for i in gh("GET", f"/repos/{REPO}/issues?type=issues&limit=50&{query}") if not i.get("pull_request")] + return [i for i in rows if "labels" not in i or all(has_label(i, l) for l in want)] _label_ids: dict[str, int] | None = None diff --git a/CHANGELOG.md b/CHANGELOG.md index 38dadd6..041a6c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## [Unreleased] ### Security +- A label filter is checked on every row. Gitea ignores `labels=` for a label the repo does not have and returns every issue: housekeeping's first live run listed all 59 open issues as parked cards (there is no `parked` label), and the same answer to `labels=spec` would have had Time split every open issue as a spec. `issues()` in time.py and the sweep's queries now keep only rows that carry the labels they asked for. - A commit status from one of Time's own workflows (`heartbeat /`, `mention /`, `venture /`, `astra /`, `doctor /`, `update /`, `steward /`) is not a check: it neither blocks nor earns a merge. A seat's review is a `pull_request_review` event that wakes `heartbeat.yml` during a heartbeat; the concurrency group cancels the run and Gitea records it as a failure on the PR head, which had stopped approved, CI-green PRs from landing. - The worker's own commit status (`time/worker-run`) can fail or delay a merge but never earns one: with no other check on the PR head, Time dispatches the project's CI workflow on the PR branch (`TIME_CI_WORKFLOW`, default `ci.yml`, which must accept `workflow_dispatch`; GitHub skips `pull_request` runs for pushes made with the Actions token) and merges on that. `off` keeps the old "no checks" stop. - `.github/` is protected from auto-merge whatever `TIME_PROTECTED` says.