Skip to content

Commit 33edc25

Browse files
authored
[gateway] Correct commit integrity elevation for personal repos on non-default refs (#4283)
`list_commits`/`get_commit` on non-default refs could label owner-authored commits in public personal repos as `none:*` because commit payloads often lack `author_association`, and collaborator-permission elevation was org-gated. This caused valid commits to be filtered when `min-integrity` was `approved`. - **Integrity elevation path** - Removed org-only short-circuit in `elevate_via_collaborator_permission`, so public personal repos can use collaborator-permission fallback the same as org repos. - Updated inline docs/comments to reflect the generalized behavior (missing/`NONE` association handling, not org-specific). - **Commit owner fast-path** - In `commit_integrity`, added a public-repo owner match shortcut: - when `author.login` matches the repo owner segment of `owner/repo`, integrity is raised to at least `writer`. - This covers the common `list_commits` shape where `author_association` is absent. - **Targeted tests** - Added coverage for owner-authored commits on public personal repos without `author_association`, asserting writer-level integrity. - Updated collaborator-permission fallback test semantics to match the new non-org behavior. ```rust if !repo_private { if let Some((owner, _)) = 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, ); } } integrity = elevate_via_collaborator_permission( author_login, repo_full_name, "commit", &format!("{}@{}", repo_full_name, short_sha), integrity, ctx, ); } ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build2733438357/b509/launcher.test /tmp/go-build2733438357/b509/launcher.test -test.testlogfile=/tmp/go-build2733438357/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rotocol/[email protected]/auth/auth.go rotocol/[email protected]/auth/authorization_code.go x_amd64/vet --gdwarf-5 ternal/wasm/bina-atomic -o x_amd64/vet 6163��` (dns block) > - Triggering command: `/tmp/go-build2946369755/b513/launcher.test /tmp/go-build2946369755/b513/launcher.test -test.testlogfile=/tmp/go-build2946369755/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o f07e34dca1.build--stateless-rpc lib/rustlib/x86_--helper-status lib/�� lib/rustlib/x86_--verbose lib/rustlib/x86_--no-progress lib/rustlib/x86_REDACTED lib/rustlib/x86_bash lib/rustlib/x86_/usr/bin/runc lib/rustlib/x86_--version lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libstd_detect-b16e5cb5eba3e0fd.rlib` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2733438357/b491/config.test /tmp/go-build2733438357/b491/config.test -test.testlogfile=/tmp/go-build2733438357/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true @v1.1.3/cpu/arm64/arm64.go 6163950/b151/ x_amd64/vet --gdwarf-5 pproxy -o x_amd64/vet 6163�� g_.a GQCceE2Bv x_amd64/vet --gdwarf-5` (dns block) > - Triggering command: `/tmp/go-build2946369755/b495/config.test /tmp/go-build2946369755/b495/config.test -test.testlogfile=/tmp/go-build2946369755/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o .o .o .o .o .o .o .o .o ndor/bin/as 2R/5XmsTr43ByGyUorigin` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build2733438357/b509/launcher.test /tmp/go-build2733438357/b509/launcher.test -test.testlogfile=/tmp/go-build2733438357/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rotocol/[email protected]/auth/auth.go rotocol/[email protected]/auth/authorization_code.go x_amd64/vet --gdwarf-5 ternal/wasm/bina-atomic -o x_amd64/vet 6163��` (dns block) > - Triggering command: `/tmp/go-build2946369755/b513/launcher.test /tmp/go-build2946369755/b513/launcher.test -test.testlogfile=/tmp/go-build2946369755/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o f07e34dca1.build--stateless-rpc lib/rustlib/x86_--helper-status lib/�� lib/rustlib/x86_--verbose lib/rustlib/x86_--no-progress lib/rustlib/x86_REDACTED lib/rustlib/x86_bash lib/rustlib/x86_/usr/bin/runc lib/rustlib/x86_--version lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libstd_detect-b16e5cb5eba3e0fd.rlib` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build2733438357/b509/launcher.test /tmp/go-build2733438357/b509/launcher.test -test.testlogfile=/tmp/go-build2733438357/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rotocol/[email protected]/auth/auth.go rotocol/[email protected]/auth/authorization_code.go x_amd64/vet --gdwarf-5 ternal/wasm/bina-atomic -o x_amd64/vet 6163��` (dns block) > - Triggering command: `/tmp/go-build2946369755/b513/launcher.test /tmp/go-build2946369755/b513/launcher.test -test.testlogfile=/tmp/go-build2946369755/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s .o .o .o .o .o .o f07e34dca1.build--stateless-rpc lib/rustlib/x86_--helper-status lib/�� lib/rustlib/x86_--verbose lib/rustlib/x86_--no-progress lib/rustlib/x86_REDACTED lib/rustlib/x86_bash lib/rustlib/x86_/usr/bin/runc lib/rustlib/x86_--version lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libstd_detect-b16e5cb5eba3e0fd.rlib` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2733438357/b518/mcp.test /tmp/go-build2733438357/b518/mcp.test -test.testlogfile=/tmp/go-build2733438357/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1n8gjiV1M -I x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -W .cfg olang.org/grpc@v-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2946369755/b522/mcp.test /tmp/go-build2946369755/b522/mcp.test -test.testlogfile=/tmp/go-build2946369755/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s lib/�� lib/rustlib/x86_64-REDACTED-linux-gnu/lib/librustc_std_workspace_alloc-76b5fe9328c1063f.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libminiz_oxide-2b6a8d2f6e1dc71b.rlib ache/go/1.25.9/x64/pkg/tool/linux_amd64/vet 64/src/runtime/cbash sql/driver/drive/usr/bin/runc cal/bin/as ache/go/1.25.9/x64/pkg/tool/linu/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de-d -ato��` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 6b4e51d + b20d6b8 commit 33edc25

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 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)