Skip to content

Commit 9e91827

Browse files
authored
feat: Maintainer reaction endorsement for integrity promotion/demotion (#3666)
Adds two new integrity mechanisms to the AllowOnly guard: maintainer 👍/❤️ reactions can promote an item to `approved` integrity, and maintainer 👎/😕 reactions can cap it at a configured level (default `none`). Disapproval always overrides endorsement. ## Config ```json { "allow-only": { "min-integrity": "approved", "endorsement-reactions": ["THUMBS_UP", "HEART"], "disapproval-reactions": ["THUMBS_DOWN", "CONFUSED"], "disapproval-integrity": "none", "endorser-min-integrity": "approved" } } ``` - **`endorsement-reactions`** / **`disapproval-reactions`**: `ReactionContent` enum values. Empty = disabled. - **`disapproval-integrity`**: integrity cap when disapproval detected. Default `none`. - **`endorser-min-integrity`**: minimum collaborator permission for a reactor to count. Default `approved` (write/maintain/admin). ## Precedence (new steps 3 & 4) `blocked_users` > author_association > trusted_users > merged > approval_labels > **endorsement** > **disapproval** ## Rust guard (`helpers.rs`, `lib.rs`) - 4 new `PolicyContext` fields - `has_maintainer_reaction_with_callback()`: extracts `reactions.nodes[]{user.login, content}` (GraphQL proxy shape), checks up to 20 reactions, resolves reactor integrity via `get_collaborator_permission` - `apply_endorsement_promotion()` / `apply_disapproval_demotion()` wired as steps 3/4 in `issue_integrity()` and `pr_integrity()` - `cap_integrity()`: min-of-two for demotion (inverse of `max_integrity`) - `AllowOnlyPolicy` deserialization extended for the 4 new fields ## Gateway-mode degradation When `reactions` field is present but has no `nodes` array (MCP server returns counts only, not per-user data), each reaction kind logs a warning **once per process lifetime** and skips evaluation entirely — no promotion or demotion occurs. ## Backend (`backend.rs`) - Per-request cache for `get_collaborator_permission` keyed `owner/repo:username` (case-insensitive) to bound API calls when multiple items share the same reactor ## Go config (`guard_policy.go`, `wasm.go`) - `AllowOnlyPolicy` + `NormalizedGuardPolicy` structs extended with 4 new fields - `NormalizeGuardPolicy`: uppercases reaction content values, deduplicates, validates integrity level strings - `buildStrictLabelAgentPayload`: allows and validates the 4 new `allow-only` keys > [!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-build2564705994/b514/launcher.test /tmp/go-build2564705994/b514/launcher.test -test.testlogfile=/tmp/go-build2564705994/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2564705994/b422/vet.cfg` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2193583002/b001/config.test /tmp/go-build2193583002/b001/config.test -test.testlogfile=/tmp/go-build2193583002/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s aw-m�� go aw-mcpg/guards/g--64 x_amd64/asm aw-mcpg/guards/g/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet aw-mcpg/guards/g-unsafeptr=false aw-mcpg/guards/g-unreachable=false x_amd64/asm aw-m�� g_.a Lj2wtMmGc x_amd64/compile k/gh-aw-mcpg/gh-/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2936037720/b001/config.test /tmp/go-build2936037720/b001/config.test -test.testlogfile=/tmp/go-build2936037720/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s --uid-owner 0 -j ACCEPT /home/REDACTED/wor/usr/lib/sysstat/sadc /home/REDACTED/wor-F /home/REDACTED/wor-L x_amd64/vet go_.�� 64/src/net ache/go/1.25.8/x1 64/pkg/tool/linu/var/log/sysstat -p internal/runtime-w -lang=go1.25 64/pkg/tool/linusecurity` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build2564705994/b514/launcher.test /tmp/go-build2564705994/b514/launcher.test -test.testlogfile=/tmp/go-build2564705994/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2564705994/b422/vet.cfg` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build2564705994/b514/launcher.test /tmp/go-build2564705994/b514/launcher.test -test.testlogfile=/tmp/go-build2564705994/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2564705994/b422/vet.cfg` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2564705994/b523/mcp.test /tmp/go-build2564705994/b523/mcp.test -test.testlogfile=/tmp/go-build2564705994/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build2564705994/b446/vet.cfg g_.a /home/REDACTED/go/pkg/mod/github.c-nolocalimports x_amd64/vet 64/src/runtime/c/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet json x_amd64/asm x_amd64/vet --no�� 1.80.0/grpclog/c-errorsas 1.80.0/grpclog/g-ifaceassert x_amd64/vet 64/src/runtime/c/opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet k/gh-aw-mcpg/gh--atomic x_amd64/compile x_amd64/vet` (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 df79a19 + a2d148c commit 9e91827

File tree

8 files changed

+1139
-26
lines changed

8 files changed

+1139
-26
lines changed

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,29 @@ fn repo_owner_type_cache() -> &'static Mutex<HashMap<String, bool>> {
2424
CACHE.get_or_init(|| Mutex::new(HashMap::new()))
2525
}
2626

27+
/// Cache for collaborator permission lookups keyed by "owner/repo:username" (all lowercase).
28+
/// This is a process-wide static cache that persists across requests. Because the WASM guard
29+
/// is instantiated per-request in the gateway, entries accumulate over the process lifetime.
30+
/// All key components (owner, repo, username) are lowercased since GitHub treats them as
31+
/// case-insensitive.
32+
fn collaborator_permission_cache() -> &'static Mutex<HashMap<String, Option<String>>> {
33+
static CACHE: OnceLock<Mutex<HashMap<String, Option<String>>>> = OnceLock::new();
34+
CACHE.get_or_init(|| Mutex::new(HashMap::new()))
35+
}
36+
37+
fn get_cached_collaborator_permission(key: &str) -> Option<Option<String>> {
38+
collaborator_permission_cache()
39+
.lock()
40+
.ok()
41+
.and_then(|cache| cache.get(key).cloned())
42+
}
43+
44+
fn set_cached_collaborator_permission(key: &str, permission: Option<String>) {
45+
if let Ok(mut cache) = collaborator_permission_cache().lock() {
46+
cache.insert(key.to_string(), permission);
47+
}
48+
}
49+
2750
fn get_cached_repo_visibility(repo_id: &str) -> Option<bool> {
2851
repo_visibility_cache()
2952
.lock()
@@ -449,6 +472,9 @@ pub fn get_issue_author_info(
449472
/// to GET /repos/{owner}/{repo}/collaborators/{username}/permission.
450473
/// Returns the user's effective permission (including inherited org permissions),
451474
/// which is more accurate than author_association for org admins.
475+
///
476+
/// Results are cached per `(owner, repo, username)` to avoid duplicate enrichment
477+
/// calls when the same reactor appears on multiple items in a response collection.
452478
pub fn get_collaborator_permission_with_callback(
453479
callback: GithubMcpCallback,
454480
owner: &str,
@@ -463,6 +489,28 @@ pub fn get_collaborator_permission_with_callback(
463489
return None;
464490
}
465491

492+
// Cache key lowercases owner, repo, and username since GitHub treats all three
493+
// as case-insensitive. This ensures "Org/Repo:Alice" and "org/repo:alice" share
494+
// the same cache entry.
495+
let cache_key = format!(
496+
"{}/{}:{}",
497+
owner.to_ascii_lowercase(),
498+
repo.to_ascii_lowercase(),
499+
username.to_ascii_lowercase()
500+
);
501+
502+
// Return cached permission if available.
503+
if let Some(cached) = get_cached_collaborator_permission(&cache_key) {
504+
crate::log_debug(&format!(
505+
"get_collaborator_permission: cache hit for {}/{} user {} → permission={:?}",
506+
owner, repo, username, cached
507+
));
508+
return cached.map(|permission| CollaboratorPermission {
509+
permission: Some(permission),
510+
login: Some(username.to_string()),
511+
});
512+
}
513+
466514
crate::log_debug(&format!(
467515
"get_collaborator_permission: fetching permission for {}/{} user {}",
468516
owner, repo, username
@@ -484,6 +532,7 @@ pub fn get_collaborator_permission_with_callback(
484532
"get_collaborator_permission: empty response for {}/{} user {}",
485533
owner, repo, username
486534
));
535+
set_cached_collaborator_permission(&cache_key, None);
487536
return None;
488537
}
489538
Err(code) => {
@@ -535,6 +584,8 @@ pub fn get_collaborator_permission_with_callback(
535584
owner, repo, username, permission, login
536585
));
537586

587+
set_cached_collaborator_permission(&cache_key, permission.clone());
588+
538589
Some(CollaboratorPermission { permission, login })
539590
}
540591

0 commit comments

Comments
 (0)