Skip to content

Commit baa83f9

Browse files
authored
Guard: ignore stale maintainer reactions when content is edited after endorsement (#4228)
Maintainer reactions were treated as evergreen endorsements, even if an issue/PR/comment was edited after the reaction was added. This allowed post-endorsement content mutation to retain `approved` integrity. - **Stale endorsement detection in reaction evaluation** - Updated `has_maintainer_reaction_with_callback()` to compare item and reaction timestamps. - If `item.updatedAt`/`updated_at` is newer than reaction `createdAt`/`created_at`, that reaction is ignored for integrity promotion/demotion. - Timestamp extraction supports both camelCase and snake_case payload variants already seen in guard inputs. - **Observability for skipped reactions** - Added debug logging when a reaction is skipped as stale, including reactor, reaction type, and compared timestamps. - **Coverage for timestamp edge cases** - Added focused unit tests for: - unmodified item + endorsement (counts) - item edited after endorsement (ignored) - endorsement after latest edit (counts) - mixed stale and fresh reactions (fresh still counts) - missing timestamps (preserves prior behavior) ```rust if let (Some(item_updated), Some(reaction_created)) = (item_updated_at, reaction_created_at) { if item_updated > reaction_created { // stale endorsement/disapproval: content changed after reaction continue; } } ``` > [!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-build4228467833/b509/launcher.test /tmp/go-build4228467833/b509/launcher.test -test.testlogfile=/tmp/go-build4228467833/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1.0.9/bool.go 1.0.9/bool_func.go x_amd64/vet --gdwarf-5 ut-2993393641.c -o x_amd64/vet 1043�� g_.a -trimpath x_amd64/vet -p go-sdk/internal/-atomic -lang=go1.24 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build3552295474/b513/launcher.test /tmp/go-build3552295474/b513/launcher.test -test.testlogfile=/tmp/go-build3552295474/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build4228467833/b491/config.test /tmp/go-build4228467833/b491/config.test -test.testlogfile=/tmp/go-build4228467833/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true @v1.1.3/cpu/arm/arm.go 1043343/b166/ x_amd64/vet --gdwarf-5 backoff -o x_amd64/vet 1043�� g_.a 1043343/b166/ x_amd64/vet -p 64 -lang=go1.24 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build3552295474/b495/config.test /tmp/go-build3552295474/b495/config.test -test.testlogfile=/tmp/go-build3552295474/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s 1043�� /tmp/go-build212/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/degit -goversion bin/rustc -c=4 -nolocalimports -importcfg bin/rustc /tmp�� /home/REDACTED/go//home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de/opt/hostedtoolcache/go/1.25.9/x64/pkg/tool/linux_amd64/vet /home/REDACTED/go//home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust-guard/target/de/tmp/go-build4250267068/b498/vet.cfg .cfg /endpointshardingit 777.build_scriptpush 777.dq1kj865068v-v -guard/target/deorigin` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build4228467833/b509/launcher.test /tmp/go-build4228467833/b509/launcher.test -test.testlogfile=/tmp/go-build4228467833/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1.0.9/bool.go 1.0.9/bool_func.go x_amd64/vet --gdwarf-5 ut-2993393641.c -o x_amd64/vet 1043�� g_.a -trimpath x_amd64/vet -p go-sdk/internal/-atomic -lang=go1.24 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build3552295474/b513/launcher.test /tmp/go-build3552295474/b513/launcher.test -test.testlogfile=/tmp/go-build3552295474/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build4228467833/b509/launcher.test /tmp/go-build4228467833/b509/launcher.test -test.testlogfile=/tmp/go-build4228467833/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 1.0.9/bool.go 1.0.9/bool_func.go x_amd64/vet --gdwarf-5 ut-2993393641.c -o x_amd64/vet 1043�� g_.a -trimpath x_amd64/vet -p go-sdk/internal/-atomic -lang=go1.24 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build3552295474/b513/launcher.test /tmp/go-build3552295474/b513/launcher.test -test.testlogfile=/tmp/go-build3552295474/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build4228467833/b518/mcp.test /tmp/go-build4228467833/b518/mcp.test -test.testlogfile=/tmp/go-build4228467833/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true om/tetratelabs/w-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet . --gdwarf2 --64 x_amd64/vet .cfg�� 1043343/b261/_pkg_.a /tmp/go-build2121043343/b298/ x_amd64/vet . telabs/wazero/in/usr/bin/runc --64 x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build3552295474/b522/mcp.test /tmp/go-build3552295474/b522/mcp.test -test.testlogfile=/tmp/go-build3552295474/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s lib/�� lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libobject-926daa94a00ee327.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libmemchr-48d5b0db80402653.rlib lib/rustlib/x86_64-REDACTED-linux-gnu/lib/libaddr2line-3367f26bd486b29d.rlib lib/rustlib/x86_bash lib/rustlib/x86_/usr/bin/runc lib/rustlib/x86_--version 05ed-cgu.00.rcgu.o 05ed�� 05ed-cgu.02.rcgu.o 05ed-cgu.03.rcgu.o 05ed-cgu.04.rcgu.o 05ed-cgu.05.rcgubash 05ed-cgu.06.rcgu/usr/bin/runc 05ed-cgu.07.rcgu--version 05ed-cgu.08.rcgu.o` (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 971fb5a + 2db6685 commit baa83f9

1 file changed

Lines changed: 123 additions & 0 deletions

File tree

  • guards/github-guard/rust-guard/src/labels

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

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,14 @@ pub fn has_maintainer_reaction_with_callback(
467467
};
468468

469469
let endorser_min_rank = integrity_level_rank(endorser_min);
470+
let item_updated_at = item
471+
.get("lastEditedAt")
472+
.or_else(|| item.get("editedAt"))
473+
.or_else(|| item.get("last_edited_at"))
474+
.or_else(|| item.get("edited_at"))
475+
.or_else(|| item.get("updatedAt"))
476+
.or_else(|| item.get("updated_at"))
477+
.and_then(|v| v.as_str());
470478

471479
for node in nodes.iter().take(MAX_REACTIONS_TO_CHECK) {
472480
let content = match node.get("content").and_then(|v| v.as_str()) {
@@ -490,6 +498,26 @@ pub fn has_maintainer_reaction_with_callback(
490498
None => continue,
491499
};
492500

501+
let reaction_created_at = node
502+
.get("createdAt")
503+
.or_else(|| node.get("created_at"))
504+
.and_then(|v| v.as_str());
505+
if let (Some(item_updated), Some(reaction_created)) = (item_updated_at, reaction_created_at) {
506+
if item_updated > reaction_created {
507+
crate::log_debug(&format!(
508+
"[integrity] {}: skipping stale {} reaction {} from @{} \
509+
(item updatedAt={} > reaction createdAt={})",
510+
repo_full_name,
511+
reaction_kind,
512+
content,
513+
login,
514+
item_updated,
515+
reaction_created
516+
));
517+
continue;
518+
}
519+
}
520+
493521
// Fetch reactor's collaborator permission to determine their integrity level.
494522
let perm = super::backend::get_collaborator_permission_with_callback(
495523
callback, owner, repo, login,
@@ -2008,6 +2036,101 @@ mod tests {
20082036
));
20092037
}
20102038

2039+
#[test]
2040+
fn test_has_maintainer_reaction_honors_unmodified_item_endorsement() {
2041+
let ctx = ctx_with_endorsement_reactions(vec!["THUMBS_UP"]);
2042+
let item = serde_json::json!({
2043+
"number": 42,
2044+
"updatedAt": "2026-04-20T00:00:00Z",
2045+
"reactions": {"nodes": [{
2046+
"user": {"login": "alice"},
2047+
"content": "THUMBS_UP",
2048+
"createdAt": "2026-04-20T00:00:00Z"
2049+
}]}
2050+
});
2051+
assert!(has_maintainer_reaction_with_callback(
2052+
&item, "owner/repo", &ctx.endorsement_reactions, "approved", &ctx,
2053+
admin_permission_callback, "endorsement"
2054+
));
2055+
}
2056+
2057+
#[test]
2058+
fn test_has_maintainer_reaction_skips_stale_endorsement_after_edit() {
2059+
let ctx = ctx_with_endorsement_reactions(vec!["THUMBS_UP"]);
2060+
let item = serde_json::json!({
2061+
"number": 42,
2062+
"updatedAt": "2026-04-21T00:00:00Z",
2063+
"reactions": {"nodes": [{
2064+
"user": {"login": "alice"},
2065+
"content": "THUMBS_UP",
2066+
"createdAt": "2026-04-20T00:00:00Z"
2067+
}]}
2068+
});
2069+
assert!(!has_maintainer_reaction_with_callback(
2070+
&item, "owner/repo", &ctx.endorsement_reactions, "approved", &ctx,
2071+
admin_permission_callback, "endorsement"
2072+
));
2073+
}
2074+
2075+
#[test]
2076+
fn test_has_maintainer_reaction_honors_endorsement_added_after_edit() {
2077+
let ctx = ctx_with_endorsement_reactions(vec!["THUMBS_UP"]);
2078+
let item = serde_json::json!({
2079+
"number": 42,
2080+
"updated_at": "2026-04-20T00:00:00Z",
2081+
"reactions": {"nodes": [{
2082+
"user": {"login": "alice"},
2083+
"content": "THUMBS_UP",
2084+
"createdAt": "2026-04-21T00:00:00Z"
2085+
}]}
2086+
});
2087+
assert!(has_maintainer_reaction_with_callback(
2088+
&item, "owner/repo", &ctx.endorsement_reactions, "approved", &ctx,
2089+
admin_permission_callback, "endorsement"
2090+
));
2091+
}
2092+
2093+
#[test]
2094+
fn test_has_maintainer_reaction_counts_fresh_when_stale_and_fresh_mixed() {
2095+
let ctx = ctx_with_endorsement_reactions(vec!["THUMBS_UP"]);
2096+
let item = serde_json::json!({
2097+
"number": 42,
2098+
"updatedAt": "2026-04-21T00:00:00Z",
2099+
"reactions": {"nodes": [
2100+
{
2101+
"user": {"login": "alice"},
2102+
"content": "THUMBS_UP",
2103+
"createdAt": "2026-04-20T00:00:00Z"
2104+
},
2105+
{
2106+
"user": {"login": "bob"},
2107+
"content": "THUMBS_UP",
2108+
"createdAt": "2026-04-22T00:00:00Z"
2109+
}
2110+
]}
2111+
});
2112+
assert!(has_maintainer_reaction_with_callback(
2113+
&item, "owner/repo", &ctx.endorsement_reactions, "approved", &ctx,
2114+
admin_permission_callback, "endorsement"
2115+
));
2116+
}
2117+
2118+
#[test]
2119+
fn test_has_maintainer_reaction_missing_timestamps_keeps_existing_behavior() {
2120+
let ctx = ctx_with_endorsement_reactions(vec!["THUMBS_UP"]);
2121+
let item = serde_json::json!({
2122+
"number": 42,
2123+
"reactions": {"nodes": [{
2124+
"user": {"login": "alice"},
2125+
"content": "THUMBS_UP"
2126+
}]}
2127+
});
2128+
assert!(has_maintainer_reaction_with_callback(
2129+
&item, "owner/repo", &ctx.endorsement_reactions, "approved", &ctx,
2130+
admin_permission_callback, "endorsement"
2131+
));
2132+
}
2133+
20112134
#[test]
20122135
fn test_cap_integrity_at_none() {
20132136
let ctx = test_ctx();

0 commit comments

Comments
 (0)