mirror of
https://github.com/horizon-rust/horizon.git
synced 2026-11-07 11:06:11 -10:00
Mod registry: decide project ownership by numeric GitHub ID
Owners were GitHub logins, and a login can be claimed by someone else once its owner renames or deletes the account. For a project without a key, a reclaimed login could publish an owner update that merges on its own. - Entry gains `owner_ids` (#[serde(default)], omitted when empty). The form records the lister's ID for new projects (file and repository listings). The registry review notes any change to it. - ownership(): when an entry has owner_ids, only an ID in them is an owner, so a matching login on another account is refused. Without owner_ids (older entries) the login matches, but the update always waits for a reviewer and the pull request says so. - The automatic owner update requires an ID match. - update_trailer is v2: `owner-update-v2` message with the submitter's numeric ID, and `format: v2` / `submitter_id:` lines in the trailer. - tools/registry_backfill_owner_ids.py: one-off, dry run by default, resolves owners' logins to IDs, cross-checks them against the IDs the form recorded in the entry's commits, and leaves alone (and prints) any entry it can't resolve completely. - docs/api/owner-update-v2.md: what the registry repository's owner-update workflow must check, and the deployment order. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SJ3stH5q6PgMAVUSmJqKDe
This commit is contained in:
1 parent
a4a254cd05
commit
526bfcbbe1
8 files changed
+418
-11
No files matched your search
@@ -464,7 +464,7 @@ async fn rate(
|
||||
format!("{id} isn't published in the registry"),
|
||||
));
|
||||
};
|
||||
if entry.owners.iter().any(|o| o.eq_ignore_ascii_case(&login)) {
|
||||
if ownership(&entry, &login, account) != Ownership::None {
|
||||
return Err(Refusal::new(
|
||||
StatusCode::FORBIDDEN,
|
||||
"you publish this project: its players rate it",
|
||||
@@ -773,10 +773,41 @@ fn submission_tag_valid(config: &Config, branch: &str, login: &str, tag: &str) -
|
||||
mac.verify_slice(&tag).is_ok()
|
||||
}
|
||||
|
||||
/// How a signed-in account relates to a project's owners.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum Ownership {
|
||||
/// Its numeric ID is one of `owner_ids`: the account that listed it (or was added), whatever
|
||||
/// its login is now.
|
||||
ById,
|
||||
/// The entry records no IDs (from before they were recorded) and its login is listed: an owner
|
||||
/// as far as anyone can tell, but its updates always wait for a reviewer.
|
||||
ByLoginOnly,
|
||||
None,
|
||||
}
|
||||
|
||||
/// Who `login` (GitHub account `user_id`) is to `entry`. When the entry has IDs, only they count: a
|
||||
/// login listed in `owners` but held by another account is someone who claimed it after a rename.
|
||||
pub fn ownership(entry: &Entry, login: &str, user_id: u64) -> Ownership {
|
||||
if !entry.owner_ids.is_empty() {
|
||||
return if entry.owner_ids.contains(&user_id) {
|
||||
Ownership::ById
|
||||
} else {
|
||||
Ownership::None
|
||||
};
|
||||
}
|
||||
if entry.owners.iter().any(|o| o.eq_ignore_ascii_case(login)) {
|
||||
Ownership::ByLoginOnly
|
||||
} else {
|
||||
Ownership::None
|
||||
}
|
||||
}
|
||||
|
||||
/// The trailer of an owner update's pull request: a comment holding what the
|
||||
/// update is and an HMAC-SHA256 (hex, with `key`) over it, which the registry
|
||||
/// repository's `owner-update` workflow verifies with the same secret
|
||||
/// (`scripts/owner_update.py` there builds the same message).
|
||||
/// (`scripts/owner_update.py` there builds the same message). v2 adds the
|
||||
/// submitter's numeric GitHub ID, which the workflow checks against the
|
||||
/// entry's `owner_ids` on main.
|
||||
pub fn update_trailer(
|
||||
key: &[u8],
|
||||
id: &str,
|
||||
@@ -784,8 +815,10 @@ pub fn update_trailer(
|
||||
sha256: &str,
|
||||
file: &str,
|
||||
submitter: &str,
|
||||
submitter_id: u64,
|
||||
) -> String {
|
||||
let message = format!("owner-update-v1\n{id}\n{version}\n{sha256}\n{file}\n{submitter}");
|
||||
let message =
|
||||
format!("owner-update-v2\n{id}\n{version}\n{sha256}\n{file}\n{submitter}\n{submitter_id}");
|
||||
let mut mac = Hmac::<Sha256>::new_from_slice(key).expect("HMAC takes any key length");
|
||||
mac.update(message.as_bytes());
|
||||
let hmac: String = mac
|
||||
@@ -795,7 +828,7 @@ pub fn update_trailer(
|
||||
.map(|b| format!("{b:02x}"))
|
||||
.collect();
|
||||
format!(
|
||||
"<!-- owner-update\nid: {id}\nversion: {version}\nsha256: {sha256}\nfile: {file}\nsubmitter: {submitter}\nhmac: {hmac}\n-->"
|
||||
"<!-- owner-update\nformat: v2\nid: {id}\nversion: {version}\nsha256: {sha256}\nfile: {file}\nsubmitter: {submitter}\nsubmitter_id: {submitter_id}\nhmac: {hmac}\n-->"
|
||||
)
|
||||
}
|
||||
|
||||
@@ -860,6 +893,7 @@ pub fn publish(
|
||||
// An owner update: an owner's new version and nothing else of the entry
|
||||
// changes (key and owners included).
|
||||
let mut owner_update = false;
|
||||
let mut matched = Ownership::ById;
|
||||
let (entry, blob) = match existing {
|
||||
None => (
|
||||
Entry {
|
||||
@@ -871,6 +905,7 @@ pub fn publish(
|
||||
description: form.description.clone(),
|
||||
authors: form.authors.clone(),
|
||||
owners: vec![login.to_owned()],
|
||||
owner_ids: vec![user_id],
|
||||
license: form.license.trim().to_owned(),
|
||||
source: form.source.trim().to_owned(),
|
||||
icon: form.icon.clone(),
|
||||
@@ -880,7 +915,8 @@ pub fn publish(
|
||||
None,
|
||||
),
|
||||
Some((mut entry, blob)) => {
|
||||
if !entry.owners.iter().any(|o| o.eq_ignore_ascii_case(login)) {
|
||||
matched = ownership(&entry, login, user_id);
|
||||
if matched == Ownership::None {
|
||||
return Err(Refusal::new(
|
||||
StatusCode::FORBIDDEN,
|
||||
format!(
|
||||
@@ -942,7 +978,10 @@ pub fn publish(
|
||||
a.versions.clear();
|
||||
b.versions.clear();
|
||||
// A mod turning native is reviewed by hand, never an owner update.
|
||||
owner_update = a == b
|
||||
// Only an owner known by ID publishes on their own: a login match alone
|
||||
// could be someone who claimed the login after a rename.
|
||||
owner_update = matched == Ownership::ById
|
||||
&& a == b
|
||||
&& !turns_native
|
||||
&& (entry.key.is_none() || version.signature.is_some())
|
||||
&& version
|
||||
@@ -1073,6 +1112,15 @@ pub fn publish(
|
||||
source = entry.source,
|
||||
lists = [capabilities, requires].concat().join("\n"),
|
||||
);
|
||||
let body = if matched == Ownership::ByLoginOnly {
|
||||
format!(
|
||||
"{body}\n\n⚠️ This entry records no owner IDs, so @{login} (GitHub ID {user_id}) was matched by login only. \
|
||||
Check it is the same account that listed the project (a renamed owner's login can be claimed by anyone), \
|
||||
then add `owner_ids = [{user_id}]` to the entry."
|
||||
)
|
||||
} else {
|
||||
body
|
||||
};
|
||||
let trailer = config
|
||||
.update_key
|
||||
.as_deref()
|
||||
@@ -1085,6 +1133,7 @@ pub fn publish(
|
||||
&sha256,
|
||||
&v.file.name,
|
||||
login,
|
||||
user_id,
|
||||
)
|
||||
});
|
||||
let body = match &trailer {
|
||||
@@ -1130,6 +1179,7 @@ pub fn list(config: &Config, login: &str, user_id: u64, form: &Form) -> Result<S
|
||||
description: form.description.clone(),
|
||||
authors: form.authors.clone(),
|
||||
owners: vec![login.to_owned()],
|
||||
owner_ids: vec![user_id],
|
||||
license: form.license.trim().to_owned(),
|
||||
source: String::new(),
|
||||
icon: form.icon.clone(),
|
||||
|
||||
@@ -434,12 +434,31 @@ async fn a_submission_becomes_a_pull_request() {
|
||||
assert_eq!(mock.lock().unwrap().assets.len(), 1, "uploaded once");
|
||||
}
|
||||
|
||||
/// The mock GitHub's account IDs: `alice` is 7, anyone else 8.
|
||||
const ALICE_ID: u64 = 7;
|
||||
|
||||
/// `mods/coords_hud.toml` on main as `alice` owns it: what the form's page
|
||||
/// says, a dummy first version.
|
||||
/// says, a dummy first version. Its `owner_ids` is the account the mock
|
||||
/// signs in as `alice`.
|
||||
fn published(owner: &str, key: bool) -> String {
|
||||
published_with_ids(owner, key, &[ALICE_ID])
|
||||
}
|
||||
|
||||
/// [`published`] with the owner IDs given (none: an entry from before they were recorded).
|
||||
fn published_with_ids(owner: &str, key: bool, ids: &[u64]) -> String {
|
||||
let f = form();
|
||||
let ids = match ids {
|
||||
[] => String::new(),
|
||||
ids => format!(
|
||||
"owner_ids = [{}]\n",
|
||||
ids.iter()
|
||||
.map(u64::to_string)
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
),
|
||||
};
|
||||
let text = format!(
|
||||
"id = \"coords_hud\"\nkind = \"mod\"\nname = \"Coordinates HUD\"\nsummary = \"Your coordinates in a corner.\"\nauthors = [\"Alice\"]\nowners = [\"{owner}\"]\nlicense = \"MIT\"\nsource = \"https://github.com/alice/coords\"\n{}\n[[versions]]\nversion = \"0.0.1\"\npublished = \"2026-01-01\"\ngame = \"26.3\"\nfile = \"coords_hud-0.0.1.hmod\"\nsha256 = \"{}\"\n",
|
||||
"id = \"coords_hud\"\nkind = \"mod\"\nname = \"Coordinates HUD\"\nsummary = \"Your coordinates in a corner.\"\nauthors = [\"Alice\"]\nowners = [\"{owner}\"]\n{ids}license = \"MIT\"\nsource = \"https://github.com/alice/coords\"\n{}\n[[versions]]\nversion = \"0.0.1\"\npublished = \"2026-01-01\"\ngame = \"26.3\"\nfile = \"coords_hud-0.0.1.hmod\"\nsha256 = \"{}\"\n",
|
||||
if key {
|
||||
format!("key = {}\n", f["key"])
|
||||
} else {
|
||||
@@ -482,7 +501,7 @@ async fn an_owners_update_is_signed_for_the_repository() {
|
||||
let trailer = body.split("<!-- owner-update\n").nth(1).expect(&body);
|
||||
let mut mac = Hmac::<sha2::Sha256>::new_from_slice(UPDATE_KEY).unwrap();
|
||||
mac.update(
|
||||
format!("owner-update-v1\ncoords_hud\n0.1.0\n{HMOD_SHA}\ncoords_hud-0.1.0.hmod\nalice")
|
||||
format!("owner-update-v2\ncoords_hud\n0.1.0\n{HMOD_SHA}\ncoords_hud-0.1.0.hmod\nalice\n{ALICE_ID}")
|
||||
.as_bytes(),
|
||||
);
|
||||
let hmac: String = mac
|
||||
@@ -494,7 +513,7 @@ async fn an_owners_update_is_signed_for_the_repository() {
|
||||
assert_eq!(
|
||||
trailer,
|
||||
format!(
|
||||
"id: coords_hud\nversion: 0.1.0\nsha256: {HMOD_SHA}\nfile: coords_hud-0.1.0.hmod\nsubmitter: alice\nhmac: {hmac}\n-->"
|
||||
"format: v2\nid: coords_hud\nversion: 0.1.0\nsha256: {HMOD_SHA}\nfile: coords_hud-0.1.0.hmod\nsubmitter: alice\nsubmitter_id: {ALICE_ID}\nhmac: {hmac}\n-->"
|
||||
),
|
||||
);
|
||||
assert_eq!(labels, [(1, json!(["owner-update"]))]);
|
||||
@@ -543,6 +562,59 @@ async fn everything_else_waits_for_a_reviewer() {
|
||||
assert!(body.contains("<!-- owner-update"));
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn a_login_match_alone_waits_for_a_reviewer() {
|
||||
// An entry from before owner IDs were recorded: alice is matched by login, so the
|
||||
// pull request opens, but without the owner-update trailer, and says why.
|
||||
let (body, labels) = update(
|
||||
published_with_ids("alice", true, &[]),
|
||||
|_| {},
|
||||
Some(UPDATE_KEY),
|
||||
)
|
||||
.await;
|
||||
assert!(!body.contains("<!-- owner-update"), "{body}");
|
||||
assert!(labels.is_empty());
|
||||
assert!(body.contains("matched by login only"), "{body}");
|
||||
assert!(
|
||||
body.contains(&format!("owner_ids = [{ALICE_ID}]")),
|
||||
"{body}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn a_reclaimed_login_is_not_the_owner() {
|
||||
// The entry's alice was account 99; the account signing in as alice now is 7: someone who
|
||||
// claimed the login after a rename. Refused, and nothing reaches GitHub.
|
||||
let (mut client, mock) = setup_keyed(|_| {}, Some(UPDATE_KEY)).await;
|
||||
mock.lock().unwrap().main.insert(
|
||||
"mods/coords_hud.toml".into(),
|
||||
published_with_ids("alice", true, &[99]),
|
||||
);
|
||||
let file = std::fs::read(fixtures().join("coords_hud-0.1.0.hmod")).unwrap();
|
||||
let csrf = client.sign_in().await;
|
||||
let (status, json) = client.submit(&csrf, &form(), &file).await;
|
||||
assert_eq!(status, StatusCode::FORBIDDEN, "{json}");
|
||||
assert!(
|
||||
json["error"].as_str().unwrap().contains("only its owners"),
|
||||
"{json}"
|
||||
);
|
||||
let m = mock.lock().unwrap();
|
||||
assert!(m.pulls.is_empty() && m.assets.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn a_renamed_owner_keeps_publishing_by_id() {
|
||||
// The entry still lists the old login; the account (ID 7) signs in under its new one.
|
||||
let (body, labels) = update(
|
||||
published_with_ids("alice-old-name", true, &[ALICE_ID]),
|
||||
|_| {},
|
||||
Some(UPDATE_KEY),
|
||||
)
|
||||
.await;
|
||||
assert!(body.contains("<!-- owner-update"), "{body}");
|
||||
assert_eq!(labels, [(1, json!(["owner-update"]))]);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn what_the_form_refuses() {
|
||||
let (mut client, mock) = setup().await;
|
||||
|
||||
@@ -43,6 +43,12 @@ pub struct Entry {
|
||||
pub authors: Vec<String>,
|
||||
/// GitHub logins allowed to publish updates.
|
||||
pub owners: Vec<String>,
|
||||
/// The owners' numeric GitHub IDs, which decide who they are: GitHub hands a renamed or deleted
|
||||
/// account's login to whoever claims it next, while an ID is never reused. The form records the
|
||||
/// lister's; the registry's backfill fills in older entries. When it is empty, an update is matched
|
||||
/// by login and always waits for a reviewer.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub owner_ids: Vec<u64>,
|
||||
#[serde(default, skip_serializing_if = "String::is_empty")]
|
||||
pub license: String,
|
||||
/// Defaults to the repository's address.
|
||||
@@ -657,6 +663,22 @@ pub fn review(base: &[Entry], head: &[Entry], author: &str) -> Review {
|
||||
new.owners.join(", ")
|
||||
));
|
||||
}
|
||||
if new.owner_ids != old.owner_ids {
|
||||
let ids = |ids: &[u64]| match ids {
|
||||
[] => "(none)".to_owned(),
|
||||
ids => ids
|
||||
.iter()
|
||||
.map(u64::to_string)
|
||||
.collect::<Vec<_>>()
|
||||
.join(", "),
|
||||
};
|
||||
r.notes.push(format!(
|
||||
"{}: owner IDs {} -> {}: check each is the account its login names",
|
||||
old.id,
|
||||
ids(&old.owner_ids),
|
||||
ids(&new.owner_ids)
|
||||
));
|
||||
}
|
||||
if new.key != old.key {
|
||||
r.notes.push(format!(
|
||||
"{}: its signing key changes; updates signed with the old one stop working",
|
||||
|
||||
@@ -210,6 +210,33 @@ fn an_entry_written_reads_back() {
|
||||
assert!(text.contains("[[versions]]"), "{text}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn owner_ids_are_optional_and_kept() {
|
||||
// Entries from before IDs were recorded still read, and write no `owner_ids`.
|
||||
let old: Entry = toml::from_str(&hud_entry()).unwrap();
|
||||
assert!(old.owner_ids.is_empty());
|
||||
assert!(!old.to_toml().unwrap().contains("owner_ids"));
|
||||
let text = hud_entry().replacen("owners = ", "owner_ids = [583231]\nowners = ", 1);
|
||||
let new: Entry = toml::from_str(&text).unwrap();
|
||||
assert_eq!(new.owner_ids, [583231]);
|
||||
assert_eq!(
|
||||
toml::from_str::<Entry>(&new.to_toml().unwrap()).unwrap(),
|
||||
new
|
||||
);
|
||||
// A change of IDs is pointed out to reviewers.
|
||||
let r = review(
|
||||
std::slice::from_ref(&old),
|
||||
std::slice::from_ref(&new),
|
||||
"ludovic111",
|
||||
);
|
||||
assert!(
|
||||
r.notes
|
||||
.iter()
|
||||
.any(|n| n.contains("owner IDs (none) -> 583231")),
|
||||
"{r:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_review_protects_published_files() {
|
||||
let base: Entry = toml::from_str(&hud_entry()).unwrap();
|
||||
|
||||
@@ -108,6 +108,7 @@ id = "coords_hud" # the file's name, and the manifest's id
|
||||
kind = "mod" # mod | native-mod | modpack | resourcepack | shader
|
||||
repo = "horizon-rust/coords-hud" # GitHub owner/name: its releases are its versions
|
||||
owners = ["ludovic111"] # GitHub logins responsible for the project
|
||||
owner_ids = [12345678] # their numeric GitHub IDs: these decide who an owner is (a login can change hands)
|
||||
key = "RWT..." # optional: the minisign public key every version is signed with
|
||||
icon = "icons/coords_hud.png" # optional: a PNG of at most 256 KiB in the repository, or https://
|
||||
# The page. Each field may instead come from horizon-registry.toml in the repo (below); these win.
|
||||
@@ -170,7 +171,9 @@ The submission form's service (`crates/horizon-mod-registry`) lists a new projec
|
||||
reads the repository's address and the page, and opens the pull request. It no longer takes updates for a project listed
|
||||
with a `repo`: "publish a GitHub release in <repo>". Projects without a `repo` can still be updated through the form
|
||||
(it uploads the file to the repository's `staging` release as `<sha256>.<extension>` and opens a pull request; an
|
||||
owner's update with nothing else changed is merged by `owner-update` once the checks pass). An owner may turn a `mod` into a `native-mod` once by sending an `.hnmod`
|
||||
owner's update with nothing else changed is merged by `owner-update` once the checks pass; only an owner whose numeric
|
||||
GitHub ID is in `owner_ids` gets that, and an entry without `owner_ids` matches its owners by login and always waits for
|
||||
a reviewer: [owner-update-v2.md](owner-update-v2.md)). An owner may turn a `mod` into a `native-mod` once by sending an `.hnmod`
|
||||
(reviewed by hand, never an owner update); no other kind change goes through. A submission that GitHub broke off after
|
||||
making its `submit/<id>-<version>` branch can be sent again: the same account's retry starts that branch again from
|
||||
`main` while it has no open pull request; anyone else's is refused.
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
# What the registry's `owner-update` workflow must check (trailer v2)
|
||||
|
||||
The registry repository (horizon-rust/horizon-mods) merges an owner's update on its own when the pull request carries
|
||||
an `owner-update` trailer that the submission service (`crates/horizon-mod-registry`, `update_trailer`) signed with
|
||||
`REGISTRY_UPDATE_KEY`. Its workflow and `scripts/owner_update.py` live in that repository, not here. Since owner IDs,
|
||||
the trailer is v2, and the workflow must check all of the following before it merges. Any check that fails leaves the
|
||||
pull request to a reviewer (remove the label, comment why); it never merges partly.
|
||||
|
||||
## The trailer
|
||||
|
||||
The last HTML comment of the pull request body:
|
||||
|
||||
```text
|
||||
<!-- owner-update
|
||||
format: v2
|
||||
id: <project id>
|
||||
version: <version>
|
||||
sha256: <hex>
|
||||
file: <id>-<version>.<extension>
|
||||
submitter: <login>
|
||||
submitter_id: <numeric GitHub ID>
|
||||
hmac: <hex>
|
||||
-->
|
||||
```
|
||||
|
||||
`hmac` is HMAC-SHA256 with `REGISTRY_UPDATE_KEY`, lowercase hex, over exactly these bytes (`\n` between fields, no
|
||||
trailing newline):
|
||||
|
||||
```text
|
||||
owner-update-v2\n<id>\n<version>\n<sha256>\n<file>\n<submitter>\n<submitter_id>
|
||||
```
|
||||
|
||||
## The checks
|
||||
|
||||
1. **Format.** `format: v2` is present and every field above is there once. Refuse `v1` trailers (no `format` line)
|
||||
once the service is deployed: a v1 trailer names the submitter by login only.
|
||||
2. **HMAC.** Recompute it over the v2 message and compare in constant time (`hmac.compare_digest`).
|
||||
3. **The submitter is the pull request's.** `submitter_id` equals the numeric ID in the pull request head commit's
|
||||
`Co-authored-by: <login> <<id>+<login>@users.noreply.github.com>` line, which the service writes. Compare the IDs,
|
||||
not the logins.
|
||||
4. **The submitter owns the project, by ID.** Read `mods/<id>.toml` **on main** (not the pull request's copy):
|
||||
`submitter_id` must be in its `owner_ids`. An entry without `owner_ids` never merges on its own (the service
|
||||
doesn't sign those, so a trailer on one is forged or stale).
|
||||
5. **Nothing else changes.** The pull request touches only `mods/<id>.toml`. Its new entry equals main's except
|
||||
for one added `[[versions]]` entry, first, whose `version`, `sha256` and `file` are the trailer's. `owners`,
|
||||
`owner_ids`, `key`, `kind`, `repo` and the page fields are identical (the service already only signs such updates;
|
||||
check again, since the pull request branch can be pushed to after it opened).
|
||||
6. **The file.** `staging` holds `<sha256>.<extension>` and its SHA-256 is `sha256`; the usual CI checks
|
||||
(`cargo horizon-mod registry check`, the trial run, a signature when the entry has a `key`) pass at the head
|
||||
that is merged.
|
||||
7. **The head merged is the head checked.** Merge with the head SHA the checks ran on (`gh pr merge --match-head-commit
|
||||
<sha>`), so a push between check and merge can't slip in.
|
||||
|
||||
## Order of deployment
|
||||
|
||||
1. In the registry repository: update `scripts/owner_update.py` and the workflow to the checks above, accepting v2
|
||||
only; deploy the CI's `cargo horizon-mod` from a Horizon commit that knows `owner_ids` (older ones refuse the field).
|
||||
2. Run `tools/registry_backfill_owner_ids.py <checkout>` (dry run, then `--write`) and merge its pull request by hand.
|
||||
3. Deploy the submission service with this change. Until step 1 is live, its v2 trailers fail verification and the
|
||||
updates simply wait for a reviewer.
|
||||
@@ -0,0 +1,44 @@
|
||||
import unittest
|
||||
|
||||
from tools import registry_backfill_owner_ids as bf
|
||||
|
||||
ENTRY = 'id = "coords_hud"\nkind = "mod"\nname = "Coordinates HUD"\nowners = ["alice", "Bob"]\nlicense = "MIT"\n'
|
||||
USERS = {"alice": {"id": 7, "login": "alice", "type": "User"}, "bob": {"id": 8, "login": "Bob", "type": "User"}}
|
||||
|
||||
|
||||
def resolve(login):
|
||||
return USERS.get(login.lower())
|
||||
|
||||
|
||||
class BackfillTests(unittest.TestCase):
|
||||
def test_every_owner_resolved_is_written_after_owners(self):
|
||||
ids, problems = bf.plan(ENTRY, "", resolve)
|
||||
self.assertEqual((ids, problems), ([7, 8], []))
|
||||
self.assertIn('owners = ["alice", "Bob"]\nowner_ids = [7, 8]\nlicense', bf.with_owner_ids(ENTRY, ids))
|
||||
|
||||
def test_an_entry_with_ids_is_left_alone(self):
|
||||
self.assertEqual(bf.plan(bf.with_owner_ids(ENTRY, [7, 8]), "", resolve), (None, []))
|
||||
|
||||
def test_an_unknown_login_leaves_the_whole_entry_alone(self):
|
||||
ids, problems = bf.plan(ENTRY.replace('"Bob"', '"gone"'), "", resolve)
|
||||
self.assertIsNone(ids)
|
||||
self.assertIn("no such user", problems[0])
|
||||
|
||||
def test_a_login_that_changed_hands_is_reported_not_written(self):
|
||||
log = ("Update coords_hud 0.2.0\n\nSubmitted by @alice through the form.\n\n"
|
||||
"Co-authored-by: alice <[email protected]>\n")
|
||||
ids, problems = bf.plan(ENTRY, log, resolve)
|
||||
self.assertIsNone(ids)
|
||||
self.assertIn("form recorded [99]", problems[0])
|
||||
# The same account the form recorded: written.
|
||||
ok = log.replace("99+alice", "7+alice")
|
||||
self.assertEqual(bf.plan(ENTRY, ok, resolve), ([7, 8], []))
|
||||
|
||||
def test_owners_not_on_one_line_are_left_for_a_person(self):
|
||||
ids, problems = bf.plan(ENTRY.replace('owners = ["alice", "Bob"]', 'owners = [\n "alice",\n]'), "", resolve)
|
||||
self.assertIsNone(ids)
|
||||
self.assertIn("by hand", problems[0])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,129 @@
|
||||
#!/usr/bin/env python3
|
||||
"""One-off: fills in `owner_ids` for the registry's entries that predate them (crates/horizon-mods, `Entry`).
|
||||
|
||||
registry_backfill_owner_ids.py <registry checkout> shows what it would write (changes nothing)
|
||||
registry_backfill_owner_ids.py <registry checkout> --write writes `owner_ids = [...]` into mods/<id>.toml
|
||||
|
||||
Run it in a checkout of horizon-rust/horizon-mods, with `gh` signed in, after the registry's CI runs a
|
||||
cargo-horizon-mod that knows the field (older ones refuse unknown fields). Commit the result as one pull request.
|
||||
|
||||
For each entry without `owner_ids`, every login in `owners` is looked up (`gh api users/<login>`). An entry is
|
||||
written only when every owner resolves, because once an entry has IDs, only they count. A login GitHub no longer
|
||||
knows, or whose current account differs from the one the form recorded when it submitted for that login (the
|
||||
`Co-authored-by: <login> <<id>+<login>@users.noreply.github.com>` lines of the entry's commits), is printed and the
|
||||
entry is left alone: that is either a rename (look at who it is) or a login someone else now holds. Nothing is
|
||||
guessed. The exit status is 1 when anything was left alone.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import re
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
LOGIN = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})$")
|
||||
OWNERS = re.compile(r"^owners[ \t]*=[ \t]*\[(?P<list>[^\]\n]*)\][ \t]*$", re.M)
|
||||
OWNER_IDS = re.compile(r"^owner_ids\s*=", re.M)
|
||||
CO_AUTHOR = re.compile(r"^Co-authored-by: (?P<login>[^<\s]+) <(?P<id>\d+)\+(?P=login)@users\.noreply\.github\.com>\s*$",
|
||||
re.M | re.I)
|
||||
|
||||
|
||||
def owners_of(text: str) -> list[str] | None:
|
||||
"""The `owners` array, when it is on one line as the registry writes it; None otherwise."""
|
||||
m = OWNERS.search(text)
|
||||
if not m:
|
||||
return None
|
||||
return [o.strip().strip('"').strip("'") for o in m.group("list").split(",") if o.strip()]
|
||||
|
||||
|
||||
def with_owner_ids(text: str, ids: list[int]) -> str:
|
||||
"""`text` with `owner_ids = [...]` on the line after `owners`."""
|
||||
m = OWNERS.search(text)
|
||||
assert m, "no owners line"
|
||||
line = f"owner_ids = [{', '.join(str(i) for i in ids)}]"
|
||||
return text[: m.end()] + "\n" + line + text[m.end():]
|
||||
|
||||
|
||||
def recorded_ids(log: str) -> dict[str, set[int]]:
|
||||
"""login (lowercase) -> the account IDs the form recorded for it in these commit messages."""
|
||||
found: dict[str, set[int]] = {}
|
||||
for m in CO_AUTHOR.finditer(log):
|
||||
found.setdefault(m.group("login").lower(), set()).add(int(m.group("id")))
|
||||
return found
|
||||
|
||||
|
||||
def lookup(login: str) -> dict | None:
|
||||
r = subprocess.run(["gh", "api", f"users/{login}", "--jq", '{"id": .id, "login": .login, "type": .type}'],
|
||||
capture_output=True, text=True)
|
||||
return json.loads(r.stdout) if r.returncode == 0 else None
|
||||
|
||||
|
||||
def history(repo: Path, path: Path) -> str:
|
||||
r = subprocess.run(["git", "-C", str(repo), "log", "--follow", "--format=%B", "--", str(path.relative_to(repo))],
|
||||
capture_output=True, text=True)
|
||||
return r.stdout if r.returncode == 0 else ""
|
||||
|
||||
|
||||
def plan(text: str, log: str, resolve) -> tuple[list[int] | None, list[str]]:
|
||||
"""The IDs to write for one entry (None: leave it alone) and the problems found. `resolve(login)` is
|
||||
GitHub's user for a login, or None."""
|
||||
if OWNER_IDS.search(text):
|
||||
return None, []
|
||||
owners = owners_of(text)
|
||||
if not owners:
|
||||
return None, ["no one-line `owners = [...]`: fill it in by hand"]
|
||||
recorded = recorded_ids(log)
|
||||
ids, problems = [], []
|
||||
for login in owners:
|
||||
if not LOGIN.match(login):
|
||||
problems.append(f"{login!r} is not a GitHub login")
|
||||
continue
|
||||
user = resolve(login)
|
||||
if not user or user.get("type") != "User":
|
||||
problems.append(f"@{login}: GitHub has no such user now (deleted or renamed)")
|
||||
continue
|
||||
seen = recorded.get(login.lower(), set())
|
||||
if seen and user["id"] not in seen:
|
||||
problems.append(f"@{login} is account {user['id']} now, but the form recorded {sorted(seen)} for that "
|
||||
"login: a rename or a reclaimed login; decide by hand")
|
||||
continue
|
||||
ids.append(int(user["id"]))
|
||||
return (None if problems else ids), problems
|
||||
|
||||
|
||||
def main(argv: list[str]) -> int:
|
||||
if not argv or argv[0].startswith("-") or any(a not in ("--write",) for a in argv[1:]):
|
||||
sys.exit(__doc__)
|
||||
repo, write = Path(argv[0]).resolve(), "--write" in argv[1:]
|
||||
mods = sorted((repo / "mods").glob("*.toml"))
|
||||
if not mods:
|
||||
sys.exit(f"{repo}/mods has no entries: is this a checkout of the registry?")
|
||||
cache: dict[str, dict | None] = {}
|
||||
|
||||
def resolve(login: str) -> dict | None:
|
||||
key = login.lower()
|
||||
if key not in cache:
|
||||
cache[key] = lookup(login)
|
||||
return cache[key]
|
||||
|
||||
left, done = 0, 0
|
||||
for path in mods:
|
||||
text = path.read_text()
|
||||
ids, problems = plan(text, history(repo, path), resolve)
|
||||
if problems:
|
||||
left += 1
|
||||
for p in problems:
|
||||
print(f"{path.name}: {p}", file=sys.stderr)
|
||||
if ids is None:
|
||||
continue
|
||||
done += 1
|
||||
print(f"{path.name}: owner_ids = {ids}")
|
||||
if write:
|
||||
path.write_text(with_owner_ids(text, ids))
|
||||
print(f"{done} entries {'written' if write else 'to write (dry run: --write writes them)'}, {left} left alone")
|
||||
return 1 if left else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main(sys.argv[1:]))
|
||||
Reference in new issue
Block a user