mirror of
https://github.com/storytold/craft-common-actions.git
synced 2026-11-13 14:02:52 +00:00
Harden contributor consent identities and preference preflight
This commit is contained in:
1 parent
4b2457cec2
commit
ed26e3cd27
4 files changed
+81
-5
No files matched your search
@@ -147,7 +147,10 @@ An unsuccessful refresh or missing artifact blocks the release. Existing callers
|
||||
|
||||
The shared generator uses local Git text/binary statistics and GitHub-linked author/co-author
|
||||
identities and merged PRs. Unlinked commits are counted as unattributed. Name preferences are
|
||||
verified against the commits that introduced their values; a maintainer override also requires
|
||||
verified against the commits that introduced their values. A Git email-linked author is trusted
|
||||
for consent only when GitHub reports a valid signature whose signer matches that author;
|
||||
otherwise verification falls back to the authenticated opener of an unambiguous merged PR.
|
||||
Ordinary commit and co-author credit attribution is unchanged. A maintainer override also requires
|
||||
a recorded `consent_source` and repository write access. Public profile names are fetched only
|
||||
for verified entries opting into synchronization. Recognized Anthropic Claude and OpenAI
|
||||
Codex/ChatGPT/GPT Co-Authored-By trailers supply model credits; other providers and unrecognized
|
||||
|
||||
@@ -39,7 +39,7 @@ def commit_authors(root, repo, head):
|
||||
query = """query($owner: String!, $name: String!, $head: String!, $cursor: String) {
|
||||
repository(owner: $owner, name: $name) { object(expression: $head) { ... on Commit {
|
||||
history(first: 100, after: $cursor) {
|
||||
nodes { oid author { user { login } }
|
||||
nodes { oid author { user { login } } signature { state signer { login } }
|
||||
authors(first: 100) { nodes { user { login } } pageInfo { hasNextPage } }
|
||||
}
|
||||
pageInfo { hasNextPage endCursor }
|
||||
@@ -59,7 +59,13 @@ def commit_authors(root, repo, head):
|
||||
if node["authors"]["pageInfo"]["hasNextPage"]:
|
||||
raise ValueError(f"More than 100 authors on {sha}; refusing incomplete credits")
|
||||
primary = node["author"]["user"]
|
||||
identities[sha] = primary["login"] if primary else None
|
||||
signature = node.get("signature") or {}
|
||||
signer = (signature.get("signer") or {}).get("login")
|
||||
signed_by_author = (signature.get("state") == "VALID" and primary is not None
|
||||
and signer is not None and signer.casefold() == primary["login"].casefold())
|
||||
# Email-linked authors still receive credit, but only a matching signer
|
||||
# authenticates a commit's author for opt-in name/consent checks.
|
||||
identities[sha] = primary["login"] if signed_by_author else None
|
||||
accounts = [actor["user"]["login"] for actor in node["authors"]["nodes"] if actor["user"]]
|
||||
if primary:
|
||||
accounts.append(primary["login"])
|
||||
@@ -211,6 +217,7 @@ def collect(root, repo):
|
||||
head = run(["git", "rev-parse", "HEAD"], root).strip()
|
||||
try:
|
||||
run(["git", "diff", "--quiet", head, "--", "contributors/people.toml"], root)
|
||||
run(["git", "cat-file", "-e", f"{head}:contributors/people.toml"], root)
|
||||
except RuntimeError as error:
|
||||
raise ValueError("Contributor preferences must match the release commit; "
|
||||
f"commit or restore contributors/people.toml ({error})") from error
|
||||
|
||||
@@ -106,6 +106,7 @@ class ContributorRefreshTests(unittest.TestCase):
|
||||
pages = []
|
||||
for sha, login, following in (("a" * 40, "alice", "next"), ("b" * 40, "bob", None)):
|
||||
node = dict(oid=sha, author=dict(user=dict(login=login)),
|
||||
signature=dict(state="VALID", signer=dict(login=login)),
|
||||
authors=dict(nodes=[dict(user=dict(login="alice")), dict(user=dict(login="bob"))],
|
||||
pageInfo=dict(hasNextPage=False)))
|
||||
pages.append(json.dumps(dict(data=dict(repository=dict(object=dict(history=dict(
|
||||
@@ -115,9 +116,49 @@ class ContributorRefreshTests(unittest.TestCase):
|
||||
self.assertEqual(primary, {"a" * 40: "alice", "b" * 40: "bob"})
|
||||
self.assertEqual(authors["b" * 40], {"alice": "alice", "bob": "bob"})
|
||||
bodies = [json.loads(call.kwargs["input_text"]) for call in mocked.call_args_list]
|
||||
self.assertIn("signature { state signer { login } }", bodies[0]["query"])
|
||||
self.assertIsNone(bodies[0]["variables"]["cursor"])
|
||||
self.assertEqual(bodies[1]["variables"]["cursor"], "next")
|
||||
|
||||
def test_consent_identity_requires_a_valid_matching_signer(self):
|
||||
"""Spoofable email attribution cannot authorize names; signed authors or PR openers can."""
|
||||
sha = self.git("rev-parse", "HEAD")
|
||||
signatures = [
|
||||
(None, False),
|
||||
(dict(state="INVALID", signer=dict(login="alice")), False),
|
||||
(dict(state="VALID", signer=None), False),
|
||||
(dict(state="VALID", signer=dict(login="mallory")), False),
|
||||
(dict(state="VALID", signer=dict(login="ALICE")), True),
|
||||
]
|
||||
real_run = credits.run
|
||||
for signature, expected in signatures:
|
||||
with self.subTest(signature=signature):
|
||||
node = dict(oid=sha, author=dict(user=dict(login="alice")), signature=signature,
|
||||
authors=dict(nodes=[dict(user=dict(login="alice"))],
|
||||
pageInfo=dict(hasNextPage=False)))
|
||||
page = json.dumps(dict(data=dict(repository=dict(object=dict(history=dict(
|
||||
nodes=[node], pageInfo=dict(hasNextPage=False, endCursor=None)))))))
|
||||
|
||||
def response(args, root, input_text=None):
|
||||
if args[:3] == ["gh", "api", "graphql"]:
|
||||
return page
|
||||
return real_run(args, root, input_text)
|
||||
|
||||
for opener in (None, "alice", "mallory"):
|
||||
via_pull = opener == "alice"
|
||||
self.pulls = [dict(number=1, sha=sha, login=opener)] if opener else []
|
||||
self.api_calls.clear()
|
||||
with patch.object(credits, "run", side_effect=response), \
|
||||
patch.object(credits, "api", side_effect=self.api):
|
||||
result = credits.collect(self.root, "owner/app")
|
||||
row = result["contributors"][0]
|
||||
self.assertEqual(row["login"], "alice")
|
||||
self.assertEqual(row["commits"], 1)
|
||||
self.assertEqual(row["people_entry"], expected or via_pull)
|
||||
self.assertEqual(row["github_name"], "Alice Public" if expected or via_pull else None)
|
||||
self.assertEqual(any(endpoint == "users/alice" for endpoint, _, _ in self.api_calls),
|
||||
expected or via_pull)
|
||||
|
||||
def test_invalid_people_field_is_an_actionable_error(self):
|
||||
"""Reject a malformed opt-in instead of guessing its intended value."""
|
||||
self.people.write_text('[people.alice]\nsync_github_name = "false"\n')
|
||||
@@ -263,6 +304,9 @@ class ContributorRefreshTests(unittest.TestCase):
|
||||
dict(number=1, sha=sha, login="alice"),
|
||||
dict(number=2, sha="f" * 40, login="outsider")]
|
||||
result = self.collect()
|
||||
pull_queries = [query for endpoint, query, _ in self.api_calls if "/pulls?" in endpoint]
|
||||
self.assertEqual(len(pull_queries), 1)
|
||||
self.assertIn("select(.merged_at != null)", pull_queries[0])
|
||||
self.assertEqual(len(result["contributors"]), 1)
|
||||
self.assertEqual(result["contributors"][0]["prs"], 1)
|
||||
|
||||
@@ -290,6 +334,24 @@ class ContributorRefreshTests(unittest.TestCase):
|
||||
self.assertEqual(self.api_calls, [])
|
||||
self.assertEqual(self.output.read_bytes(), original)
|
||||
|
||||
def test_missing_or_untracked_preferences_fail_before_api_access(self):
|
||||
"""An absent committed preference file must not trigger collection or leak local names."""
|
||||
self.git("rm", "contributors/people.toml")
|
||||
self.git("commit", "-m", "Remove preference file")
|
||||
original = self.output.read_bytes()
|
||||
for untracked in (False, True):
|
||||
with self.subTest(untracked=untracked):
|
||||
if untracked:
|
||||
self.people.write_text('[people.alice]\ndisplay_name = "Private untracked name"\n')
|
||||
with patch.object(credits, "commit_authors") as mocked_history, \
|
||||
patch.object(credits, "api") as mocked_api:
|
||||
with self.assertRaisesRegex(ValueError, "commit or restore") as error:
|
||||
credits.collect(self.root, "owner/app")
|
||||
self.assertNotIn("Private untracked name", str(error.exception))
|
||||
mocked_history.assert_not_called()
|
||||
mocked_api.assert_not_called()
|
||||
self.assertEqual(self.output.read_bytes(), original)
|
||||
|
||||
def test_atomic_write_retains_previous_snapshot_on_replace_failure(self):
|
||||
"""Preserve the last good file when disk replacement fails."""
|
||||
original = self.output.read_bytes()
|
||||
@@ -316,6 +378,7 @@ endpoint = sys.argv[2]
|
||||
if endpoint == "graphql":
|
||||
commits = json.loads(Path(os.environ["API_FIXTURE"]).read_text())["commits"]
|
||||
nodes = [dict(oid=row["sha"], author=dict(user=dict(login=row["login"])),
|
||||
signature=dict(state="VALID", signer=dict(login=row["login"])),
|
||||
authors=dict(nodes=[dict(user=dict(login=row["login"]))],
|
||||
pageInfo=dict(hasNextPage=False))) for row in commits]
|
||||
print(json.dumps(dict(data=dict(repository=dict(object=dict(history=dict(
|
||||
|
||||
@@ -119,21 +119,24 @@ class StalledMirrorTests(unittest.TestCase):
|
||||
('apt-get', FAKE_APT),
|
||||
('sudo', '#!/bin/bash\nexec "$@"\n'),
|
||||
('sleep', '#!/bin/bash\n'),
|
||||
('rm', '#!/bin/bash\n[[ "$*" == "-f /var/lib/man-db/auto-update" ]]\n'),
|
||||
('rm', '#!/bin/bash\nprintf "%s\\n" "$*" >> "$RM_LOG"\n[[ "$*" == "-f /var/lib/man-db/auto-update" ]]\n'),
|
||||
]:
|
||||
(bin_dir / name).write_text(body, newline='\n')
|
||||
(bin_dir / name).chmod(0o755)
|
||||
log = Path(tmpdir) / 'apt.log'
|
||||
log.touch()
|
||||
rm_log = Path(tmpdir) / 'rm.log'
|
||||
rm_log.touch()
|
||||
res = subprocess.run(
|
||||
['bash', '--noprofile', '--norc', '-e', '-o', 'pipefail', '-c', install_step['run']],
|
||||
env={**os.environ, 'PATH': f"{bin_dir}{os.pathsep}{os.environ['PATH']}", 'HOME': tmpdir,
|
||||
'PACKAGE_LIST': 'curl git ', 'APT_NETWORK_TIMEOUT': '1', 'APT_NETWORK_ATTEMPTS': '3',
|
||||
'APT_LOG': str(log), 'STALL_OP': stall_op, 'STALLS': str(stalls)},
|
||||
'APT_LOG': str(log), 'RM_LOG': str(rm_log), 'STALL_OP': stall_op, 'STALLS': str(stalls)},
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=120,
|
||||
)
|
||||
self.assertEqual(rm_log.read_text().splitlines(), ['-f /var/lib/man-db/auto-update'])
|
||||
return res, log.read_text().split()
|
||||
|
||||
def test_a_stalled_update_is_retried(self):
|
||||
|
||||
Reference in new issue
Block a user