Skip to content

Commit 4c3be22

Browse files
RajivTSmeta-codesync[bot]
authored andcommitted
Lowercase ACL name in create_repos to match Hipster
Summary: Hipster auto-lowercases ACL names on write but Mononoke's per-permission push-time check is case-sensitive against the ACL name stored on the repo. Without normalization, a repo whose name contains uppercase characters ends up configured with an ACL name no live Hipster entry matches, breaking every push that requires a permission grant. Concrete failure caught this in production: XF-APAC GitHub-to-Mononoke mirror sync, 2026-06-24 16:37–16:44 UTC (4 retries, ~62 min wall-clock on the longest one). The repos were freshly created via `scsc create-repos XF-APAC/dreamwright-v2 XF-APAC/storykit-mono`, the Mononoke repo configs got `custom_acl_name="repos/git/XF-APAC"`, but the live Hipster entry was `repos/git/xf-apac` (Hipster always lowercases on `acl.create`). `gitimport --bypass-all-hooks` failed with invalid request: In order to use BYPASS_ALL_HOOKS pushvar one needs to be member of the scm group OR have access to write_no_hooks action on repo ACL even though `SANDCASTLE_TAG:scm_github_mirror_sync` was correctly granted `write_no_hooks` on the (lowercase) ACL. par-msl (the first mirror-sync tenant) was unaffected only because `par-msl` is already entirely lowercase, so the bytes matched by coincidence. Any tenant with mixed-case org slug would have hit this on day one, but XF-APAC is the first such tenant we onboarded. This diff lowercases the slug in both `make_full_acl_name_from_repo_name` and `make_top_level_acl_name_from_repo_name` so Mononoke stores the same bytes Hipster does, regardless of how the org slug is cased on github.com. The fix is forward-only — existing repos with uppercase characters in `custom_acl_name` need their stored config updated separately (XF-APAC's two repos will be patched manually in configerator before this lands). Reviewed By: YousefSalama Differential Revision: D109585065 fbshipit-source-id: a13e51a2aa2e899f28b4f2f2894bf82c7b5dd2cc
1 parent 7213fa7 commit 4c3be22

1 file changed

Lines changed: 75 additions & 2 deletions

File tree

eden/mononoke/servers/scs/scs_methods/src/methods/create_repos.rs

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,28 @@ async fn try_fetching_repo_acl(
321321
}
322322
}
323323

324+
/// Build a Hipster ACL name for a repo, lowercased.
325+
///
326+
/// Hipster auto-lowercases ACL names on write (visible in `consumer.id_data`
327+
/// from `hipstercli getrawacl`). Mononoke's per-permission push-time check
328+
/// (e.g. the `BYPASS_ALL_HOOKS` pushvar check on `write_no_hooks`) is
329+
/// case-sensitive against the ACL name stored on the repo, so without
330+
/// normalization a repo whose name has uppercase characters ends up
331+
/// configured with an ACL name that no Hipster entry matches.
332+
///
333+
/// Concrete failure mode: XF-APAC/dreamwright-v2 mirror sync, 2026-06-24
334+
/// 16:44 UTC. The Mononoke repo was configured with
335+
/// `custom_acl_name="repos/git/XF-APAC"` but the actual Hipster entry was
336+
/// `repos/git/xf-apac`. Every `gitimport --bypass-all-hooks` push to it
337+
/// failed with "needs … write_no_hooks action on repo ACL" because the
338+
/// case-sensitive grant lookup missed. par-msl was unaffected only because
339+
/// its org slug was already lowercase.
340+
///
341+
/// Lowercasing here keeps Mononoke's stored ACL name byte-equal to what
342+
/// Hipster will return for the same logical ACL, for every tenant
343+
/// regardless of how their org slug is cased on github.com.
324344
fn make_full_acl_name_from_repo_name(repo_name: &str) -> String {
325-
format!("repos/git/{repo_name}")
345+
format!("repos/git/{}", repo_name.to_lowercase())
326346
}
327347

328348
fn make_top_level_acl_name_from_repo_name(repo_name: &str) -> String {
@@ -332,8 +352,11 @@ fn make_top_level_acl_name_from_repo_name(repo_name: &str) -> String {
332352
// _IDENTITY_SUBDIR mapping in configerator/source/scm/mononoke/repos/generate_repo_index.py.
333353
// NOTE for future implementer: any logging added inside add_repo() must use debug! not info!
334354
// — info! in add_repo() breaks .t integration tests (project memory).
355+
//
356+
// Case normalization: see `make_full_acl_name_from_repo_name` docstring
357+
// for the rationale (XF-APAC mirror sync SEV, 2026-06-24).
335358
let (top_level, _rest) = repo_name.split_once('/').unwrap_or((repo_name, ""));
336-
format!("repos/git/{top_level}")
359+
format!("repos/git/{}", top_level.to_lowercase())
337360
}
338361

339362
#[cfg(fbcode_build)]
@@ -1354,6 +1377,56 @@ mod tests {
13541377
);
13551378
}
13561379

1380+
#[mononoke::test]
1381+
fn test_make_top_level_acl_name_lowercases_uppercase_org() {
1382+
// Regression for the XF-APAC mirror sync failure on 2026-06-24:
1383+
// before this fix, the uppercase repo name flowed through verbatim
1384+
// to the ACL name on the Mononoke repo config, producing a
1385+
// case-mismatch with the lowercased Hipster ACL.
1386+
assert_eq!(
1387+
make_top_level_acl_name_from_repo_name("XF-APAC/dreamwright-v2"),
1388+
"repos/git/xf-apac",
1389+
);
1390+
}
1391+
1392+
#[mononoke::test]
1393+
fn test_make_top_level_acl_name_preserves_already_lowercase_org() {
1394+
// Existing tenants (par-msl) must keep producing the byte-equal
1395+
// ACL name they had before this fix — otherwise their repo
1396+
// configs would point at a different name than the live Hipster
1397+
// entries on the next config rewrite.
1398+
assert_eq!(
1399+
make_top_level_acl_name_from_repo_name("par-msl/risk-test"),
1400+
"repos/git/par-msl",
1401+
);
1402+
}
1403+
1404+
#[mononoke::test]
1405+
fn test_make_top_level_acl_name_no_slash() {
1406+
// Defensive: repo name without a slash falls back to using the
1407+
// whole name as the top-level (matches the pre-fix behavior
1408+
// shape), still lowercased.
1409+
assert_eq!(
1410+
make_top_level_acl_name_from_repo_name("Single-Segment"),
1411+
"repos/git/single-segment",
1412+
);
1413+
}
1414+
1415+
#[mononoke::test]
1416+
fn test_make_full_acl_name_lowercases() {
1417+
// Custom-ACL path (callers with `custom_acl.is_some()`) also goes
1418+
// through Hipster's lowercasing, so the full ACL name must be
1419+
// lowercased end-to-end.
1420+
assert_eq!(
1421+
make_full_acl_name_from_repo_name("XF-APAC/Dreamwright-V2"),
1422+
"repos/git/xf-apac/dreamwright-v2",
1423+
);
1424+
assert_eq!(
1425+
make_full_acl_name_from_repo_name("par-msl/risk-test"),
1426+
"repos/git/par-msl/risk-test",
1427+
);
1428+
}
1429+
13571430
#[mononoke::test]
13581431
fn test_to_repo_spec_tshirt_size_mapping() {
13591432
assert_eq!(

0 commit comments

Comments
 (0)