Skip to content

Commit 05da8b6

Browse files
Copilotlpcox
andauthored
fix: elevate personal repo owner commits on non-default branches
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/7ab311b0-8ace-4ecc-bb17-83fa8b9b00d8 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent 8b73dd4 commit 05da8b6

2 files changed

Lines changed: 36 additions & 14 deletions

File tree

guards/github-guard/rust-guard/src/labels/helpers.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,14 +1310,13 @@ pub fn collaborator_permission_floor(
13101310
/// Rank threshold for writer-level integrity (none=1, reader=2, writer=3, merged=4).
13111311
const WRITER_RANK: u8 = 3;
13121312

1313-
/// Attempt to elevate integrity for an author in an org-owned repository
1313+
/// Attempt to elevate integrity for an author in a public repository
13141314
/// by checking their effective collaborator permission.
13151315
///
13161316
/// When `author_association` gives insufficient integrity (below writer level),
13171317
/// this function checks the user's effective permission via the GitHub
1318-
/// collaborator permission API. This correctly handles org owners/admins whose
1319-
/// `author_association` is reported as "NONE" because their access is inherited
1320-
/// through org ownership rather than direct collaboration.
1318+
/// collaborator permission API. This correctly handles owners/admins whose
1319+
/// `author_association` is absent or reported as "NONE".
13211320
///
13221321
/// Backend calls are cached per-user, so repeated lookups for the same author
13231322
/// across list/search items are inexpensive.
@@ -1346,10 +1345,6 @@ pub fn elevate_via_collaborator_permission(
13461345
Some((o, r)) if !o.is_empty() && !r.is_empty() => (o, r),
13471346
_ => return integrity,
13481347
};
1349-
let is_org = super::backend::is_repo_org_owned(owner, repo).unwrap_or(false);
1350-
if !is_org {
1351-
return integrity;
1352-
}
13531348
crate::log_debug(&format!(
13541349
"[integrity] {}:{}: author_association floor below writer (rank={}), checking collaborator permission for {}",
13551350
resource_label, resource_id, integrity_rank(repo_full_name, &integrity, ctx), author_login
@@ -1676,8 +1671,23 @@ pub fn commit_integrity(
16761671

16771672
let mut integrity = author_association_floor(item, repo_full_name, ctx);
16781673

1679-
// Collaborator permission fallback for org repos (handles org owners/admins
1680-
// whose author_association is "NONE" due to inherited org access).
1674+
// For public personal repositories, commit payloads often omit
1675+
// `author_association`. Ensure owner-authored commits still get writer floor.
1676+
if !repo_private {
1677+
if let Some((owner, _repo)) = repo_full_name.split_once('/') {
1678+
if !owner.is_empty() && author_login.eq_ignore_ascii_case(owner) {
1679+
integrity = max_integrity(
1680+
repo_full_name,
1681+
integrity,
1682+
writer_integrity(repo_full_name, ctx),
1683+
ctx,
1684+
);
1685+
}
1686+
}
1687+
}
1688+
1689+
// Collaborator permission fallback for public repos (handles owners/admins
1690+
// whose author_association is missing or "NONE").
16811691
if !repo_private {
16821692
let sha = item.get("sha").and_then(|v| v.as_str()).unwrap_or("unknown");
16831693
let short_sha = if sha.len() > 8 { &sha[..8] } else { sha };

guards/github-guard/rust-guard/src/labels/mod.rs

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,20 @@ mod tests {
826826
);
827827
}
828828

829+
#[test]
830+
fn test_commit_integrity_owner_on_public_personal_repo_without_association() {
831+
let ctx = default_ctx();
832+
let repo = "ahmadabdalla/example";
833+
let owner_commit = json!({
834+
"author": {"login": "ahmadabdalla"}
835+
});
836+
837+
assert_eq!(
838+
commit_integrity(&owner_commit, repo, false, false, &ctx),
839+
writer_integrity(repo, &ctx)
840+
);
841+
}
842+
829843
#[test]
830844
fn test_trusted_first_party_bot_detection() {
831845
use super::helpers::is_trusted_first_party_bot;
@@ -5187,17 +5201,15 @@ mod tests {
51875201
}
51885202

51895203
#[test]
5190-
fn test_elevate_via_collab_permission_no_elevation_non_org_repo() {
5191-
// In test mode, is_repo_org_owned returns None (no cache) → unwrap_or(false)
5192-
// so the function should return integrity unchanged
5204+
fn test_elevate_via_collab_permission_lookup_failure_keeps_integrity() {
51935205
let ctx = default_ctx();
51945206
let repo = "github/copilot";
51955207
let none = none_integrity(repo, &ctx);
51965208
let result = helpers::elevate_via_collaborator_permission(
51975209
"dsyme", repo, "issue", "github/copilot#42",
51985210
none.clone(), &ctx,
51995211
);
5200-
assert_eq!(result, none, "should return unchanged when repo is not org-owned (cache miss → false)");
5212+
assert_eq!(result, none, "should return unchanged when collaborator lookup yields no result");
52015213
}
52025214

52035215
#[test]

0 commit comments

Comments
 (0)