From 75d087ba1283df40dfae663e5bd8c6167138a147 Mon Sep 17 00:00:00 2001 From: Changyong Gong Date: Sat, 10 Oct 2026 16:37:15 +0800 Subject: [PATCH] Clarify team-memory public source token contract Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/actions/issuelens/action.yml | 6 +- tests/test_team_memory_coordinator.py | 117 ++++++++++++++++++++++--- tests/test_team_memory_workflow.py | 17 ++++ tests/test_workflow_dispatch_action.py | 12 +++ 4 files changed, 137 insertions(+), 15 deletions(-) diff --git a/.github/actions/issuelens/action.yml b/.github/actions/issuelens/action.yml index 073785e..7fa0ae5 100644 --- a/.github/actions/issuelens/action.yml +++ b/.github/actions/issuelens/action.yml @@ -30,7 +30,11 @@ inputs: required: false default: '{}' source-github-token: - description: Independent source read token for coordinated requests (Contents, Actions and Pull requests read); cross-repository callers supply source-scoped access. + description: >- + Source read token for coordinated requests (Contents, Actions and Pull requests read). + Defaults to the built-in GITHUB_TOKEN for same-repository and supported public + cross-repository reads. Explicit overrides remain supported; an empty token fails. + Dispatch writes still require target Actions write access. required: false default: ${{ github.token }} source-run-id: diff --git a/tests/test_team_memory_coordinator.py b/tests/test_team_memory_coordinator.py index 5584eff..dddccb8 100644 --- a/tests/test_team_memory_coordinator.py +++ b/tests/test_team_memory_coordinator.py @@ -3,6 +3,7 @@ import os import pathlib import unittest +import urllib.error from unittest.mock import patch import test_team_memory_workflow as memory_tests @@ -138,13 +139,50 @@ def test_cross_repository_source_develop_is_independent_of_coordinator_main(self else: self.assertIn("/repos/example/gradle/", request.full_url + "/") + def test_public_cross_repository_reads_accept_caller_token_or_explicit_override(self): + self.select_cross_repository() + self.pull["base"] = {"ref": "develop", "repo": self.source_project} + for name in ("c0", "c1"): + self.associations["data"]["repository"][name]["associatedPullRequests"]["nodes"][0]["mergeCommit"] = None + for source_token in (self.environment["GH_TOKEN"], "fake-source-token"): + with self.subTest(source_token=source_token): + self.environment["SOURCE_GH_TOKEN"] = source_token + self.execute("preflight", self.preflight_responses() + self.responses(self.pull)) + envelope = self.prepared_envelope() + self.assertEqual(self.action_outputs()["eligible"], "true") + self.assertEqual(envelope["metadata"]["repository"], self.source_project["full_name"]) + self.assertEqual(envelope["metadata"]["source_tip_sha"], self.tip) + self.assertEqual(envelope["metadata"]["pull_requests"][0]["merge_commit_sha"], self.merge_sha) + requests = [call.args[0] for call in self.opener.open.call_args_list] + self.assertEqual([request.full_url for request in requests], [ + f"https://api.github.com/repos/{self.repository}", + "https://api.github.com/repos/example/gradle", + "https://api.github.com/repos/example/gradle/actions/runs/123456/attempts/2", + "https://api.github.com/repos/example/gradle/branches/develop", + f"https://api.github.com/repos/example/gradle/compare/{self.after}...{self.tip}?per_page=1&page=2", + f"https://api.github.com/repos/example/gradle/compare/{self.before}...{self.after}?per_page=100&page=1", + "https://api.github.com/graphql", + "https://api.github.com/repos/example/gradle/pulls/27", + ]) + self.assertEqual(requests[0].get_header("Authorization"), "Bearer " + self.environment["GH_TOKEN"]) + self.assertTrue(all(request.get_header("Authorization") == "Bearer " + source_token + for request in requests[1:])) + self.assertEqual(json.loads(requests[-2].data)["variables"], {"owner": "example", "name": "gradle"}) + for credential in (self.environment["GH_TOKEN"], source_token): + self.assertNotIn(credential, json.dumps(envelope)) + self.assertNotIn(credential, self.output.getvalue()) + self.token.assert_not_called() + def test_same_repository_is_default_and_unchanged_tip_needs_no_ancestry_read(self): self.environment["SOURCE_REPOSITORY"] = "" + self.environment["SOURCE_GH_TOKEN"] = self.environment["GH_TOKEN"] self.branch["commit"]["sha"] = self.after self.associations["data"]["repository"]["defaultBranchRef"]["target"]["oid"] = self.after self.execute("preflight", self.responses(self.project, self.source_run, self.branch, self.comparison, self.associations)) self.assertEqual(self.prepared_envelope()["metadata"]["source_tip_sha"], self.after) self.assertEqual(self.opener.open.call_count, 5) + self.assertTrue(all(call.args[0].get_header("Authorization") == "Bearer " + self.environment["GH_TOKEN"] + for call in self.opener.open.call_args_list)) def test_rebase_rest_identity_lookup_keeps_the_source_token(self): for name in ("c0", "c1"): @@ -164,9 +202,7 @@ def test_invalid_or_mixed_source_inputs_fail_before_network(self): {"SOURCE_RUN_ID": ""}, {"SOURCE_RUN_ATTEMPT": ""}, {"PUSH_BEFORE": ""}, {"PUSH_AFTER": ""}, {"SOURCE_RUN_ID": "1; echo unsafe"}, {"SOURCE_RUN_ATTEMPT": "0"}, {"PUSH_BEFORE": "0" * 40}, {"PUSH_AFTER": "short"}, {"PUSH_AFTER": self.before}, {"DISPATCH_PR": "27"}, - {"SOURCE_REPOSITORY": "example/private"}, {"SOURCE_GH_TOKEN": " "}, - {"SOURCE_GH_TOKEN": "fake-source-token\nPRIVATE"}, {"SOURCE_GH_TOKEN": "token\x01private"}, - {"SOURCE_GH_TOKEN": "t" * 4097}, + {"SOURCE_REPOSITORY": "example/private"}, {"SOURCE_REPOSITORY": "example/.."}, {"SOURCE_REPOSITORY": "example/../secret"}, {"SOURCE_REPOSITORY": "example/\nsecret"}, {"SOURCE_REPOSITORY": "-example/project"}, {"SOURCE_REPOSITORIES": " " * 4097}, @@ -184,6 +220,41 @@ def test_invalid_or_mixed_source_inputs_fail_before_network(self): self.assertFalse((self.directory / "output.txt").exists()) self.token.assert_not_called() + def test_missing_or_invalid_source_token_does_not_fall_back_to_caller_token(self): + self.select_cross_repository() + for source_token in (None, "", " ", "fake-source-token\nPRIVATE", "token\x01private", "t" * 4097): + with self.subTest(source_token=source_token): + if source_token is None: + self.environment.pop("SOURCE_GH_TOKEN", None) + else: + self.environment["SOURCE_GH_TOKEN"] = source_token + with self.assertRaisesRegex(SystemExit, "source-github-token must be non-empty printable ASCII"): + self.execute("preflight", []) + self.opener.open.assert_not_called() + self.token.assert_not_called() + self.assertFalse((self.directory / "output.txt").exists()) + self.assertEqual(list(self.directory.glob("issuelens-request-*.json")), []) + + def test_public_source_auth_failure_never_falls_back_or_retries(self): + self.select_cross_repository() + for source_token in (self.environment["GH_TOKEN"], "fake-source-token"): + for status in (401, 403): + for offset in (1, 2, 5, 6): + with self.subTest(source_token=source_token, status=status, offset=offset): + self.environment["SOURCE_GH_TOKEN"] = source_token + failure = urllib.error.HTTPError( + "https://api.github.com", status, source_token + " PRIVATE API DETAIL", {}, None) + with self.assertRaises(SystemExit) as raised: + self.execute("preflight", self.preflight_responses()[:offset] + [failure]) + self.assertEqual(self.opener.open.call_count, offset + 1) + self.assertTrue(all(call.args[0].get_header("Authorization") == "Bearer " + source_token + for call in self.opener.open.call_args_list[1:])) + self.assertNotIn(source_token, str(raised.exception)) + self.assertNotIn("PRIVATE", str(raised.exception)) + self.assertFalse((self.directory / "output.txt").exists()) + self.assertEqual(list(self.directory.glob("issuelens-request-*.json")), []) + self.token.assert_not_called() + def test_coordinator_inputs_are_not_available_to_other_adapters_or_workflows(self): original = self.environment.copy() for changes in ( @@ -199,24 +270,28 @@ def test_coordinator_inputs_are_not_available_to_other_adapters_or_workflows(sel self.token.assert_not_called() def test_forged_source_runs_fail_before_discovery_or_login(self): + self.select_cross_repository() + self.environment["SOURCE_GH_TOKEN"] = self.environment["GH_TOKEN"] original = copy.deepcopy(self.source_run) cases = [ {"id": 123457}, {"run_attempt": 1}, {"event": "pull_request"}, {"path": ".github/workflows/untrusted.yml"}, {"head_branch": "untrusted"}, - {"head_sha": self.tip}, {"repository": {**self.project, "id": 101}}, - {"head_repository": {**self.project, "id": 101}}, {"actor": {"login": "unsafe\nactor"}}, + {"head_sha": self.tip}, {"repository": {**self.source_project, "id": 201}}, + {"repository": {**self.source_project, "full_name": "example/renamed"}}, + {"head_repository": {**self.source_project, "id": 201}}, {"actor": {"login": "unsafe\nactor"}}, ] for changes in cases: with self.subTest(changes=changes): self.source_run = {**original, **changes} with self.assertRaises(SystemExit): self.execute("preflight", self.source_responses()) - self.assertEqual(self.opener.open.call_count, 2) + self.assertEqual(self.opener.open.call_count, 3) self.assertFalse((self.directory / "output.txt").exists()) self.token.assert_not_called() def test_cross_repository_policy_identity_and_visibility_fail_closed(self): self.select_cross_repository() + self.environment["SOURCE_GH_TOKEN"] = self.environment["GH_TOKEN"] original = self.source_project.copy() for changes in ({"id": 201}, {"full_name": "example/renamed"}, {"visibility": "private"}, {"visibility": "internal"}): with self.subTest(changes=changes): @@ -227,11 +302,17 @@ def test_cross_repository_policy_identity_and_visibility_fail_closed(self): self.assertEqual(self.opener.open.call_count, 2) self.token.assert_not_called() self.source_project.update(original) - self.project["visibility"] = "private" - with self.assertRaisesRegex(SystemExit, "public"): - self.execute("preflight", self.preflight_responses()) + for visibility in ("private", "internal"): + with self.subTest(coordinator_visibility=visibility): + self.project["visibility"] = visibility + with self.assertRaisesRegex(SystemExit, "public"): + self.execute("preflight", self.preflight_responses()) + self.assertEqual(self.opener.open.call_count, 2) + self.token.assert_not_called() def test_branch_ancestry_and_ref_races_fail_before_agent_login(self): + self.select_cross_repository() + self.environment["SOURCE_GH_TOKEN"] = self.environment["GH_TOKEN"] originals = copy.deepcopy((self.branch, self.ancestry, self.associations)) for mutation in ("branch", "sha", "diverged", "merge_base", "ref_race"): with self.subTest(mutation=mutation): @@ -275,11 +356,19 @@ def test_complete_comparison_pagination_covers_1000_and_requires_every_page(self pages = [{**self.comparison, "ahead_by": 1000, "total_commits": 1000, "commits": [{"sha": sha} for sha in shas[start:start + 100]]} for start in range(0, 1000, 100)] - with patch.dict(os.environ, self.environment, clear=True), patch.object(action, "github_read", side_effect=pages) as read: - result = action.read_reconciliation_inventory(self.repository, self.before, self.after, float("inf")) - self.assertEqual(result, shas) - self.assertEqual(read.call_count, 10) - self.assertTrue(read.call_args.args[0].endswith("?per_page=100&page=10")) + for source_token in (self.environment["GH_TOKEN"], "fake-source-token"): + with self.subTest(source_token=source_token): + self.environment["SOURCE_GH_TOKEN"] = source_token + with patch.dict(os.environ, self.environment, clear=True), \ + patch.object(action, "github_read", side_effect=pages) as read: + result = action.read_reconciliation_inventory(self.repository, self.before, self.after, float("inf")) + self.assertEqual(result, shas) + self.assertEqual(read.call_count, 10) + self.assertEqual([call.args[0] for call in read.call_args_list], [ + f"/repos/{self.repository}/compare/{self.before}...{self.after}?per_page=100&page={page}" + for page in range(1, 11) + ]) + self.assertTrue(all(call.kwargs["token"] == source_token for call in read.call_args_list)) for mutation in ("missing", "duplicate", "count", "base"): with self.subTest(mutation=mutation): changed = copy.deepcopy(pages) diff --git a/tests/test_team_memory_workflow.py b/tests/test_team_memory_workflow.py index 2904608..997a19a 100644 --- a/tests/test_team_memory_workflow.py +++ b/tests/test_team_memory_workflow.py @@ -101,6 +101,23 @@ def test_preflight_precedes_pinned_login_and_submission(self): self.assertEqual(self.action_metadata["inputs"]["output-mode"]["default"], "hybrid") self.assertEqual(self.action_metadata["inputs"]["summary-mode"]["default"], "full") + def test_token_defaults_keep_public_reads_and_dispatch_writes_separate(self): + for name in ("github-token", "source-github-token"): + with self.subTest(input=name): + self.assertEqual(self.action_metadata["inputs"][name]["default"], "${{ github.token }}") + self.assertEqual(self.action_metadata["inputs"][name]["required"], "false") + self.assertNotIn(name, self.steps[-1]["with"]) + description = self.action_metadata["inputs"]["source-github-token"]["description"] + for requirement in ("GITHUB_TOKEN", "public cross-repository reads", "Explicit overrides", + "empty token fails", "target Actions write access"): + self.assertIn(requirement, description) + self.assertNotIn("Independent", description) + self.assertNotIn("source-scoped", description) + dispatch, = self.dispatch_job["steps"] + self.assertNotIn("dispatch-token", dispatch["with"]) + self.assertEqual(self.queue_metadata["inputs"]["dispatch-token"]["default"], "${{ github.token }}") + self.assertIn("Contents read and Actions write", self.queue_metadata["inputs"]["dispatch-token"]["description"]) + def test_wiki_identity_output_descriptions_do_not_imply_success(self): for name in ("wiki-repository", "wiki-sha"): with self.subTest(output=name): diff --git a/tests/test_workflow_dispatch_action.py b/tests/test_workflow_dispatch_action.py index 8250c72..79eef9d 100644 --- a/tests/test_workflow_dispatch_action.py +++ b/tests/test_workflow_dispatch_action.py @@ -196,6 +196,18 @@ def test_unknown_or_unacknowledged_post_is_not_retried_or_reported_as_job_succes for call in self.opener.open.call_args_list), 1) self.assertNotIn("accepted", self.output.getvalue()) + def test_public_target_reads_do_not_authorize_dispatch_writes(self): + failure = urllib.error.HTTPError("https://api.github.com", 403, "PRIVATE ACTIONS WRITE DETAIL", {}, None) + with self.assertRaises(SystemExit) as raised: + self.execute(self.responses({**self.project, "visibility": "public"}, self.workflow, self.branch) + [failure]) + requests = [call.args[0] for call in self.opener.open.call_args_list] + self.assertEqual([request.get_method() for request in requests], ["GET"] * 3 + ["POST"]) + self.assertTrue(all(request.get_header("Authorization") == "Bearer " + self.environment["DISPATCH_TOKEN"] + for request in requests)) + self.assertNotIn("PRIVATE", str(raised.exception)) + self.assertNotIn("accepted", self.output.getvalue()) + self.assertNotIn(self.environment["DISPATCH_TOKEN"], str(raised.exception)) + def test_output_failure_does_not_deny_an_acknowledged_dispatch(self): with patch("builtins.print", side_effect=OSError("PRIVATE OUTPUT DETAIL")): with self.assertRaisesRegex(SystemExit, "Dispatch acknowledged but output unavailable") as raised: