Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 20 additions & 10 deletions guards/github-guard/rust-guard/src/labels/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1310,14 +1310,13 @@ pub fn collaborator_permission_floor(
/// Rank threshold for writer-level integrity (none=1, reader=2, writer=3, merged=4).
const WRITER_RANK: u8 = 3;

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

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

// Collaborator permission fallback for org repos (handles org owners/admins
// whose author_association is "NONE" due to inherited org access).
// For public personal repositories, commit payloads often omit
// `author_association`. Ensure owner-authored commits still get writer floor.
if !repo_private {
if let Some((owner, _repo)) = repo_full_name.split_once('/') {
if author_login.eq_ignore_ascii_case(owner) {
integrity = max_integrity(
repo_full_name,
integrity,
writer_integrity(repo_full_name, ctx),
ctx,
);
}
}
}

// Collaborator permission fallback for public repos (handles owners/admins
// whose author_association is missing or "NONE").
if !repo_private {
let sha = item.get("sha").and_then(|v| v.as_str()).unwrap_or("unknown");
let short_sha = if sha.len() > 8 { &sha[..8] } else { sha };
Expand Down
20 changes: 16 additions & 4 deletions guards/github-guard/rust-guard/src/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,20 @@ mod tests {
);
}

#[test]
fn test_commit_integrity_owner_on_public_personal_repo_without_association() {
let ctx = default_ctx();
let repo = "ahmadabdalla/example";
let owner_commit = json!({
"author": {"login": "ahmadabdalla"}
});

assert_eq!(
commit_integrity(&owner_commit, repo, false, false, &ctx),
writer_integrity(repo, &ctx)
);
}

#[test]
fn test_trusted_first_party_bot_detection() {
use super::helpers::is_trusted_first_party_bot;
Expand Down Expand Up @@ -5187,17 +5201,15 @@ mod tests {
}

#[test]
fn test_elevate_via_collab_permission_no_elevation_non_org_repo() {
// In test mode, is_repo_org_owned returns None (no cache) → unwrap_or(false)
// so the function should return integrity unchanged
fn test_elevate_via_collab_permission_lookup_failure_keeps_integrity() {
let ctx = default_ctx();
let repo = "github/copilot";
let none = none_integrity(repo, &ctx);
let result = helpers::elevate_via_collaborator_permission(
"dsyme", repo, "issue", "github/copilot#42",
none.clone(), &ctx,
);
assert_eq!(result, none, "should return unchanged when repo is not org-owned (cache miss → false)");
assert_eq!(result, none, "should return unchanged when collaborator lookup yields no result");
}

#[test]
Expand Down
Loading