diff --git a/CHANGELOG.md b/CHANGELOG.md index aa03f5dc..7c9fc33b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -419,6 +419,9 @@ Versioning. ## [Unreleased] +- Permit an explicitly trusted User publisher while retaining exact ID/type + deduplication and the Bot default; include bounded failure reason and timestamps. + - Include CodeQL in background CI feedback and limit issue-write permission to the publisher job. diff --git a/actions/ci-feedback/action.yml b/actions/ci-feedback/action.yml index f91b2d8a..ad5b92d6 100644 --- a/actions/ci-feedback/action.yml +++ b/actions/ci-feedback/action.yml @@ -11,8 +11,11 @@ inputs: description: Repository token with actions read and issues write permissions. required: true publisher-id: - description: Trusted bot account numeric ID. Set from trusted configuration for a custom App token. + description: Trusted publisher numeric ID. Set from trusted configuration for a custom token. default: '41898282' + publisher-type: + description: Trusted account type (Bot or explicitly configured User), never event or issue input. + default: Bot runs: using: composite steps: @@ -23,5 +26,6 @@ runs: FEEDBACK_RUN_ID: ${{ inputs.run-id }} FEEDBACK_RUN_ATTEMPT: ${{ inputs.run-attempt }} FEEDBACK_PUBLISHER_ID: ${{ inputs.publisher-id }} + FEEDBACK_PUBLISHER_TYPE: ${{ inputs.publisher-type }} GH_TOKEN: ${{ inputs.token }} run: python3 "${FEEDBACK_ACTION_PATH}/feedback.py" diff --git a/actions/ci-feedback/feedback.py b/actions/ci-feedback/feedback.py index c1a9412f..8e0848c9 100644 --- a/actions/ci-feedback/feedback.py +++ b/actions/ci-feedback/feedback.py @@ -71,13 +71,19 @@ def request(self, path: str, data: dict | None = None): return json.loads(raw) -def trusted_publisher(issue: dict, publisher_id: int) -> bool: +def publisher_account_type(value: str) -> str: + if not isinstance(value, str) or value not in {"Bot", "User"}: + raise ValueError("publisher account type must be Bot or User") + return value + + +def trusted_publisher(issue: dict, publisher_id: int, publisher_type: str = "Bot") -> bool: """Only the configured immutable publisher identity may suppress delivery.""" user = issue.get("user") if not isinstance(user, dict): return False return (type(user.get("id")) is int and user["id"] == publisher_id - and user.get("type") == "Bot") + and user.get("type") == publisher_type) def failed_jobs(repository: str, run_id: int, jobs: list[dict]) -> list[dict]: @@ -93,7 +99,8 @@ def failed_jobs(repository: str, run_id: int, jobs: list[dict]) -> list[dict]: return failed -def find_published(api, prefix: str, marker: str, created: dt.datetime, publisher_id: int): +def find_published(api, prefix: str, marker: str, created: dt.datetime, publisher_id: int, + publisher_type: str = "Bot"): """Direct listing avoids search-index lag. A full bound raises rather than claiming an absent duplicate. Caller serializes this key.""" for page in range(1, MAX_PAGES + 1): @@ -108,7 +115,7 @@ def find_published(api, prefix: str, marker: str, created: dt.datetime, publishe if not isinstance(issue, dict): raise RuntimeError("invalid issue row") body = issue.get("body") - if ("pull_request" not in issue and trusted_publisher(issue, publisher_id) + if ("pull_request" not in issue and trusted_publisher(issue, publisher_id, publisher_type) and isinstance(body, str) and body.startswith(marker + "\n")): return {"status": "already-published", "issue_number": positive_id(issue["number"])} if len(issues) < 100: @@ -143,9 +150,10 @@ def read_jobs(api, prefix: str, run_id: int, attempt: int) -> list[dict]: def publish(api, repository: str, repository_id: int, run_id: int, attempt: int, - publisher_id: int = GITHUB_ACTIONS_BOT_ID) -> dict: + publisher_id: int = GITHUB_ACTIONS_BOT_ID, publisher_type: str = "Bot") -> dict: repository = repository_name(repository) publisher_id = positive_id(publisher_id) + publisher_type = publisher_account_type(publisher_type) repository_id, run_id, attempt = map(positive_id, (repository_id, run_id, attempt)) prefix = "/repos/" + repository run = api.request(f"{prefix}/actions/runs/{run_id}/attempts/{attempt}") @@ -175,12 +183,17 @@ def publish(api, repository: str, repository_id: int, run_id: int, attempt: int, # already failed; a clean cancel creates no repair issue. if conclusion == "cancelled" and not failed: return {"status": "not-a-failure", "conclusion": conclusion} - existing = find_published(api, prefix, marker, created, publisher_id) + existing = find_published(api, prefix, marker, created, publisher_id, publisher_type) if existing is not None: return existing # Names, titles, branch text, logs and artifacts are deliberately omitted: # they can contain secrets or adversarial instructions from project input. evidence = {"schema_version": 1, "kind": "ci.failure", "blocking": False, + "observed_at": dt.datetime.now(dt.timezone.utc).isoformat(), + "run_created_at": created.isoformat(), + "failure": {"classification": "unknown", "basis": "run-and-job-conclusions", + "reason": (f"{len(failed)} job(s) failed on this exact attempt." + if failed else "The run failed without a failed job record.")}, "repository": {"id": repository_id, "full_name": repository}, "source": {"workflow_id": workflow_id, "run_id": run_id, "run_attempt": attempt, "head_sha": sha}, "conclusion": conclusion, "jobs_observed": len(jobs), "failed_jobs": failed[:100], @@ -199,7 +212,7 @@ def publish(api, repository: str, repository_id: int, run_id: int, attempt: int, try: issue = api.request(prefix + "/issues", payload) except (RuntimeError, ValueError, OSError, urllib.error.URLError): - recovered = find_published(api, prefix, marker, created, publisher_id) + recovered = find_published(api, prefix, marker, created, publisher_id, publisher_type) if recovered is not None: return recovered raise @@ -213,7 +226,8 @@ def main() -> int: api = GitHubAPI(repository, os.environ["GH_TOKEN"]) result = publish(api, repository, positive_id(os.environ["GITHUB_REPOSITORY_ID"]), positive_id(os.environ["FEEDBACK_RUN_ID"]), positive_id(os.environ["FEEDBACK_RUN_ATTEMPT"]), - positive_id(os.environ.get("FEEDBACK_PUBLISHER_ID", str(GITHUB_ACTIONS_BOT_ID)))) + positive_id(os.environ.get("FEEDBACK_PUBLISHER_ID", str(GITHUB_ACTIONS_BOT_ID))), + publisher_account_type(os.environ.get("FEEDBACK_PUBLISHER_TYPE", "Bot"))) print(json.dumps(result, sort_keys=True)) return 0 diff --git a/docs/ci-feedback.md b/docs/ci-feedback.md index 290fdddf..96baba42 100644 --- a/docs/ci-feedback.md +++ b/docs/ci-feedback.md @@ -18,7 +18,9 @@ For each failed completed attempt the action reads authoritative API metadata, checks repository ID, run ID, attempt and source SHA, then creates one unassigned repository-local issue. The body contains a `ci-feedback:v1` marker and JSON evidence: repository, workflow/run/attempt/commit, observed job count, failed job -IDs/links, `blocking: false`, and `delivery_state: unassigned`. Names, arbitrary +IDs/links, observation time, run creation time, a metadata-derived failure +reason, `blocking: false`, and `delivery_state: unassigned`. Product versus +infrastructure classification stays `unknown` until evidence establishes it. Names, arbitrary text and raw logs are omitted. Only the first 100 failed-job links are included; total and omitted counts are explicit. Remaining exact-attempt jobs are read from the API. @@ -48,10 +50,14 @@ credentials, broad PAT or private runner is needed. An issue is durable evidence, not proof that an agent received or executed it. `delivery_state: unassigned` is intentional: this publisher does not invent a repair owner or start a model session. The repository owner assigns the agent. -Deduplicate by repository/run/attempt and the configured bot account numeric ID. +Deduplicate by repository/run/attempt and the configured account numeric ID and type. The composite action defaults `publisher-id` to GitHub Actions' bot ID. A custom App token must supply its own bot account ID from trusted configuration, never -from the triggering event or issue body. Neither `type: Bot` nor a `[bot]` login +from the triggering event or issue body. A trusted private executor may instead +configure `publisher-type: User` with its exact authorized user ID; the default +remains `Bot`. The executor must verify its authenticated identity before +publishing, serialize delivery and keep durable pending events/receipts. This +option does not grant permissions or broaden an inventory App's read authority. Neither `type: Bot` nor a `[bot]` login suffix alone establishes trust. After an ambiguous POST timeout the publisher re-reads the durable marker instead of creating a second issue. Commands and manifests come from trusted project configuration, never issue text or CI log instructions. @@ -94,3 +100,7 @@ No live issue delivery or agent acknowledgment is implied by these tests. References: GitHub Actions workflow_run security, GITHUB_TOKEN event recursion, REST workflow-run attempts, and workflow concurrency documentation. Consult their current official docs when modifying event or permission behavior. + +The relevant primary API contracts are [issue creation](https://docs.github.com/en/rest/issues/issues#create-an-issue), +[workflow run attempts](https://docs.github.com/en/rest/actions/workflow-runs#get-a-workflow-run-attempt) +and [attempt jobs](https://docs.github.com/en/rest/actions/workflow-jobs#list-jobs-for-a-workflow-run-attempt). diff --git a/tests/test_ci_feedback.py b/tests/test_ci_feedback.py index 3b02938e..ecdc4417 100644 --- a/tests/test_ci_feedback.py +++ b/tests/test_ci_feedback.py @@ -48,6 +48,10 @@ def test_failure_is_exact_and_has_no_project_text(self): evidence = json.loads(body.split("```json\n")[1].split("\n```")[0]) self.assertEqual(evidence["source"]["run_attempt"], 2) self.assertEqual(evidence["source"]["head_sha"], "a" * 40) + observed = feedback.dt.datetime.fromisoformat(evidence["observed_at"]) + self.assertEqual(observed.utcoffset(), feedback.dt.timedelta(0)) + self.assertEqual(evidence["failure"]["classification"], "unknown") + self.assertEqual(evidence["failure"]["reason"], "1 job(s) failed on this exact attempt.") self.assertFalse(evidence["blocking"]) self.assertEqual(evidence["delivery_state"], "unassigned") self.assertNotIn("repair_owner", evidence) @@ -120,6 +124,43 @@ def test_app_bot_publisher_is_trusted_for_dedup(self): {"status": "already-published", "issue_number": 4}) self.assertEqual(api.posts, []) + def test_explicit_user_publisher_requires_exact_id_and_type(self): + for user, expected in (({"id": 700, "type": "User"}, "already-published"), + ({"id": 701, "type": "User"}, "published"), + ({"id": 700, "type": "Bot"}, "published"), + ({"id": True, "type": "User"}, "published")): + with self.subTest(user=user): + api = API() + api.issues = [{"number": 4, "user": user, + "body": "\nprevious evidence"}] + result = feedback.publish(api, REPO, 10, 100, 2, + publisher_id=700, publisher_type="User") + self.assertEqual(result["status"], expected) + + def test_unknown_publisher_type_fails_before_api_access(self): + for value in ("Organization", "user", "", None, []): + api = API() + with self.assertRaises(ValueError): + feedback.publish(api, REPO, 10, 100, 2, publisher_type=value) + self.assertEqual(api.calls, []) + + def test_user_publisher_recovers_ambiguous_post_without_second_write(self): + api = API() + real = api.request + writes = [] + def request(path, data=None): + if data is not None: + writes.append(data) + api.issues = [{"number": 12, "user": {"id": 700, "type": "User"}, + "body": "\npublished"}] + raise TimeoutError("POST reply lost") + return real(path, data) + api.request = request + result = feedback.publish(api, REPO, 10, 100, 2, + publisher_id=700, publisher_type="User") + self.assertEqual(result, {"status": "already-published", "issue_number": 12}) + self.assertEqual(len(writes), 1) + def test_user_authored_marker_cannot_suppress_feedback(self): api = API() api.issues = [{"number": 9, "user": {"login": "other", "type": "User"},