From 64223221668fa2138509cd9e4e80a14e787120e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 19:08:05 +0000 Subject: [PATCH 1/2] Reader role migration: rename the role in workload-identity rules too Migration 0013 renames an administrator's role named `reader` to `reader-` before it seeds the built-in `reader`. The rules of a workload-identity realm name their role, resolved by name on every CI token, so a rule that named the custom role would grant the built-in one, read on every repository, after the upgrade. The migration now rewrites those rules to the renamed role first; the realm's policy digest changes with it, so its exchanged keys end and its jobs exchange again. Fixes #749 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJb6V3DHD5wQrsYhKda --- crates/db/tests/migrations.rs | 87 +++++++++++++++++++++++++++++++ docs/changes/2.2.0/749.upgrade.md | 5 ++ migrations/0013_reader_role.sql | 30 ++++++++++- 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 docs/changes/2.2.0/749.upgrade.md diff --git a/crates/db/tests/migrations.rs b/crates/db/tests/migrations.rs index de07c427..6c41369e 100644 --- a/crates/db/tests/migrations.rs +++ b/crates/db/tests/migrations.rs @@ -1107,6 +1107,64 @@ async fn readers_before_the_upgrade_get_the_reader_role() { .await .expect("an administrator's role named reader"); assign(service, "reader").await; + // Workload-identity realms name their rules' roles: one names the + // administrator's `reader` (twice, between other rules), one does not. + let realm = |name: &'static str, issuer: &'static str, rules: serde_json::Value| { + let pool = pool.clone(); + async move { + sqlx::query_scalar::<_, i64>( + "INSERT INTO auth_realm (kind, name, priority, config, updated_at) \ + VALUES ('workload_identity', $1, \ + (SELECT coalesce(max(priority), 0) + 10 FROM auth_realm), $2, \ + '2026-01-01T00:00:00Z') \ + RETURNING id", + ) + .bind(name) + .bind(serde_json::json!({ + "issuer": issuer, + "allowed_audiences": ["forge"], + "rules": rules, + })) + .fetch_one(&pool) + .await + .expect("insert a workload-identity realm") + } + }; + let rule = |role: &str, project: &str| { + serde_json::json!({ + "role": role, + "conditions": [{ "claim": "project_path", "equals": project }], + }) + }; + let naming = realm( + "gitlab", + "https://gitlab.example.com", + serde_json::json!([ + rule("reader", "group/a"), + rule("ci-pub", "group/b"), + rule(" reader ", "group/c"), + ]), + ) + .await; + let other = realm( + "github", + "https://token.actions.githubusercontent.com", + serde_json::json!([rule("ci-pub", "org/repo")]), + ) + .await; + let config = |realm_id: i64| { + let pool = pool.clone(); + async move { + sqlx::query_as::<_, (serde_json::Value, chrono::DateTime)>( + "SELECT config, updated_at FROM auth_realm WHERE id = $1", + ) + .bind(realm_id) + .fetch_one(&pool) + .await + .expect("read a realm") + } + }; + let (other_before, other_updated) = config(other).await; forge_db::migrate(&pool).await.expect("migrate the rest"); @@ -1144,6 +1202,35 @@ async fn readers_before_the_upgrade_get_the_reader_role() { .await .expect("the built-in reader role"); assert!(builtin); + + // The rules that named the administrator's role name it still, under + // its new name, in their order; the other rules and realms are unchanged. + let (rewritten, updated) = config(naming).await; + let roles: Vec<&str> = rewritten["rules"] + .as_array() + .expect("the rules") + .iter() + .map(|rule| rule["role"].as_str().expect("a rule's role")) + .collect(); + assert_eq!(roles, [renamed.as_str(), "ci-pub", renamed.as_str()]); + assert_eq!( + rewritten["rules"][1], + rule("ci-pub", "group/b"), + "a rule naming another role is kept as it was" + ); + assert_eq!( + rewritten["rules"][0]["conditions"], + rule("reader", "group/a")["conditions"], + "a rewritten rule keeps its conditions" + ); + assert_eq!(rewritten["issuer"], "https://gitlab.example.com"); + let before = chrono::DateTime::parse_from_rfc3339("2026-01-01T00:00:00Z").unwrap(); + assert!(updated > before, "the rewritten realm records the change"); + assert_eq!( + config(other).await, + (other_before, other_updated), + "a realm whose rules name no `reader` is untouched" + ); } /// A Postgres server for one test: a throwaway Postgres 18 container matching diff --git a/docs/changes/2.2.0/749.upgrade.md b/docs/changes/2.2.0/749.upgrade.md new file mode 100644 index 00000000..aec6f746 --- /dev/null +++ b/docs/changes/2.2.0/749.upgrade.md @@ -0,0 +1,5 @@ +A workload-identity rule that named a role an administrator had named +`reader` names it under its new name, `reader-`, too, so the CI jobs it +matches keep that role's grants instead of the built-in `reader`'s (every +repository). The keys those realms' jobs exchanged before the upgrade stop +working with the change; the jobs exchange their ID tokens again. diff --git a/migrations/0013_reader_role.sql b/migrations/0013_reader_role.sql index ed0039de..2b07a38b 100644 --- a/migrations/0013_reader_role.sql +++ b/migrations/0013_reader_role.sql @@ -8,13 +8,41 @@ -- mode `off`, for an account without one. That level stays, as Roles only. -- -- A role an administrator named `reader` keeps its grants and members under --- the name `reader-`. +-- the name `reader-`. The rules of a workload-identity realm name their +-- role (`rules[].role`), resolved by name on every CI token: a rule that +-- named that role would grant the built-in `reader`, every repository, from +-- now on, so it names `reader-` too (#749). The realm's policy digest +-- changes with it, which ends the keys its jobs exchanged; they kept the +-- renamed role's id and exchange again. -- -- Every user that is neither an administrator nor a writer was a reader and -- gets the role, so they read every repository from now on. Service accounts -- do not: each stands for one pipeline or network, whose roles were chosen -- for it, and they become Roles only. -- --------------------------------------------------------------------------- +UPDATE auth_realm realm +SET config = jsonb_set( + realm.config, + '{rules}', + (SELECT jsonb_agg( + CASE WHEN btrim(rule ->> 'role') = 'reader' + THEN jsonb_set(rule, '{role}', to_jsonb('reader-' || custom.id)) + ELSE rule + END + ORDER BY ordinal) + FROM jsonb_array_elements(realm.config -> 'rules') + WITH ORDINALITY AS rules (rule, ordinal))), + updated_at = now() +FROM role custom +WHERE custom.name = 'reader' AND NOT custom.builtin + AND realm.kind = 'workload_identity' + AND jsonb_typeof(realm.config -> 'rules') = 'array' + AND EXISTS ( + SELECT 1 + FROM jsonb_array_elements(realm.config -> 'rules') AS named (rule) + WHERE btrim(named.rule ->> 'role') = 'reader' + ); + UPDATE role SET name = 'reader-' || id WHERE name = 'reader' AND NOT builtin; INSERT INTO role (name, description, builtin) VALUES From ebe897ec6ec6f98772d6b026bcba99e7533fad5a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 19:33:18 +0000 Subject: [PATCH 2/2] Document the reader case of a rule whose role does not exist A workload-identity rule that names `reader` while no role of that name exists (a deleted one) refused the jobs it matched; after the upgrade it resolves to the built-in `reader`. There is no role the migration could point it at, so the upgrade note and the migration's comment say so and ask administrators to check their rules before upgrading. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJb6V3DHD5wQrsYhKda --- docs/changes/2.2.0/749.upgrade.md | 7 ++++++- migrations/0013_reader_role.sql | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/changes/2.2.0/749.upgrade.md b/docs/changes/2.2.0/749.upgrade.md index aec6f746..016b8399 100644 --- a/docs/changes/2.2.0/749.upgrade.md +++ b/docs/changes/2.2.0/749.upgrade.md @@ -2,4 +2,9 @@ A workload-identity rule that named a role an administrator had named `reader` names it under its new name, `reader-`, too, so the CI jobs it matches keep that role's grants instead of the built-in `reader`'s (every repository). The keys those realms' jobs exchanged before the upgrade stop -working with the change; the jobs exchange their ID tokens again. +working with the change; the jobs exchange their ID tokens again. A rule +that names `reader` while no role of that name exists, for example because +the role was deleted, refused the jobs it matched until now; from the +upgrade on it grants the built-in `reader`, every repository. Before +upgrading, check the rules of each workload-identity realm under **Admin → +Authentication**, and point such a rule at a role that exists or remove it. diff --git a/migrations/0013_reader_role.sql b/migrations/0013_reader_role.sql index 2b07a38b..e570c9af 100644 --- a/migrations/0013_reader_role.sql +++ b/migrations/0013_reader_role.sql @@ -13,7 +13,10 @@ -- named that role would grant the built-in `reader`, every repository, from -- now on, so it names `reader-` too (#749). The realm's policy digest -- changes with it, which ends the keys its jobs exchanged; they kept the --- renamed role's id and exchange again. +-- renamed role's id and exchange again. A rule that names `reader` while no +-- role of that name exists (a deleted one) refused its jobs and grants the +-- built-in `reader` from now on: there is no role to point it at, so the +-- upgrade notes ask administrators to check their rules first. -- -- Every user that is neither an administrator nor a writer was a reader and -- gets the role, so they read every repository from now on. Service accounts