Skip to content

Commit 5c065ca

Browse files
authored
rust-guard/tools.rs: add BLOCKED_TOOLS const and predicate test coverage (#4943)
`is_blocked_tool()` inlined its list in a `matches!()` expression, inconsistent with the discoverable `WRITE_OPERATIONS`/`READ_WRITE_OPERATIONS` const arrays and invisible to any caller wanting to enumerate blocked tools. Four predicate functions (`is_merge_operation`, `is_delete_operation`, `is_lock_operation`, `is_unlock_operation`) had zero test coverage despite being security-critical paths in `labels/tool_rules.rs`. ## Changes - **`BLOCKED_TOOLS` const array** — extracted from `is_blocked_tool()`'s `matches!()` body into a `pub const &[&str]` parallel to `WRITE_OPERATIONS`/`READ_WRITE_OPERATIONS`, with per-entry comments: ```rust pub const BLOCKED_TOOLS: &[&str] = &[ "transfer_repository", // irreversible ownership transfer "archive_repository", // repo settings change; unsupported "unarchive_repository", // symmetric to archive_repository "rename_repository", // breaks clone URLs and integrations "create_agent_task", // unsupported agent-task creation ]; pub fn is_blocked_tool(tool_name: &str) -> bool { BLOCKED_TOOLS.contains(&tool_name) } ``` - **Predicate test coverage** — added `test_is_merge_operation`, `test_is_delete_operation`, `test_is_lock_operation`, `test_is_unlock_operation` (each with positive, negative, and empty-string cases), plus `test_lock_and_unlock_contribute_to_write_operations` to verify `is_write_operation`'s delegation to the lock/unlock predicates. > [!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-build2801713912/b513/launcher.test /tmp/go-build2801713912/b513/launcher.test -test.testlogfile=/tmp/go-build2801713912/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s ortc�� .cfg 64/src/html/enti-ifaceassert x_amd64/vet . ions =0 x_amd64/vet .cfg�� 0430085/b366/_pkg_.a 0430085/b314/ x_amd64/vet --gdwarf-5 rse p=/opt/hostedtoo--format x_amd64/vet` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2801713912/b495/config.test /tmp/go-build2801713912/b495/config.test -test.testlogfile=/tmp/go-build2801713912/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s 0430�� 1.80.0/encoding/gzip/gzip.go aw-mcpg/internal/difc/capabilities.go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -I g_.a -I x_amd64/vet --gdwarf-5 telabs/wazero/ap-atomic -o x_amd64/vet` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build2801713912/b513/launcher.test /tmp/go-build2801713912/b513/launcher.test -test.testlogfile=/tmp/go-build2801713912/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s ortc�� .cfg 64/src/html/enti-ifaceassert x_amd64/vet . ions =0 x_amd64/vet .cfg�� 0430085/b366/_pkg_.a 0430085/b314/ x_amd64/vet --gdwarf-5 rse p=/opt/hostedtoo--format x_amd64/vet` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build2801713912/b513/launcher.test /tmp/go-build2801713912/b513/launcher.test -test.testlogfile=/tmp/go-build2801713912/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s ortc�� .cfg 64/src/html/enti-ifaceassert x_amd64/vet . ions =0 x_amd64/vet .cfg�� 0430085/b366/_pkg_.a 0430085/b314/ x_amd64/vet --gdwarf-5 rse p=/opt/hostedtoo--format x_amd64/vet` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build2801713912/b522/mcp.test /tmp/go-build2801713912/b522/mcp.test -test.testlogfile=/tmp/go-build2801713912/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s -W .cfg k/gh-aw-mcpg/gh--w x_amd64/vet . --gdwarf2 --64 x_amd64/vet .cfg�� 0430085/b400/_pkg_.a pkg/mod/go.opentelemetry.io/otel/sdk@v1.43.0/internal/x/x.go x_amd64/vet --gdwarf-5 g/grpc/internal//usr/bin/runc -o 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 ef3ed83 + 4666211 commit 5c065ca

1 file changed

Lines changed: 67 additions & 8 deletions

File tree

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

guards/github-guard/rust-guard/src/tools.rs

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,19 @@ pub fn is_unlock_operation(tool_name: &str) -> bool {
156156
tool_name.starts_with("unlock_")
157157
}
158158

159+
/// Tools that are unconditionally blocked regardless of agent integrity.
160+
///
161+
/// These operations are too dangerous or unsupported to ever permit via an agent.
162+
/// Entries here should also appear in `WRITE_OPERATIONS` or `READ_WRITE_OPERATIONS`
163+
/// so the tool is still subject to all normal write-path checks before being denied.
164+
pub const BLOCKED_TOOLS: &[&str] = &[
165+
"transfer_repository", // irreversible ownership transfer
166+
"archive_repository", // repo settings change; unsupported
167+
"unarchive_repository", // symmetric to archive_repository
168+
"rename_repository", // breaks clone URLs and integrations
169+
"create_agent_task", // unsupported agent-task creation
170+
];
171+
159172
/// Check if a tool is unconditionally blocked (always denied regardless of agent integrity).
160173
///
161174
/// Blocked tools are listed here when the operation is considered too dangerous
@@ -174,20 +187,23 @@ pub fn is_unlock_operation(tool_name: &str) -> bool {
174187
/// - `create_agent_task`: creates a Copilot coding-agent job that opens a branch and PR;
175188
/// unsupported as a directly invocable agent operation.
176189
pub fn is_blocked_tool(tool_name: &str) -> bool {
177-
matches!(
178-
tool_name,
179-
"transfer_repository"
180-
| "archive_repository"
181-
| "unarchive_repository"
182-
| "rename_repository"
183-
| "create_agent_task"
184-
)
190+
BLOCKED_TOOLS.contains(&tool_name)
185191
}
186192

187193
#[cfg(test)]
188194
mod tests {
189195
use super::*;
190196

197+
#[test]
198+
fn blocked_tools_are_classified_as_write_or_read_write() {
199+
for &tool in BLOCKED_TOOLS {
200+
assert!(
201+
WRITE_OPERATIONS.contains(&tool) || READ_WRITE_OPERATIONS.contains(&tool),
202+
"blocked tool `{tool}` must also be classified in WRITE_OPERATIONS or READ_WRITE_OPERATIONS"
203+
);
204+
}
205+
}
206+
191207
#[test]
192208
fn test_is_blocked_tool_transfer_repository() {
193209
assert!(
@@ -420,6 +436,49 @@ mod tests {
420436
}
421437
}
422438

439+
#[test]
440+
fn test_is_merge_operation() {
441+
assert!(is_merge_operation("merge_pull_request"));
442+
assert!(is_merge_operation("merge_upstream"));
443+
assert!(!is_merge_operation("create_pull_request"));
444+
assert!(!is_merge_operation("update_pull_request"));
445+
assert!(!is_merge_operation(""));
446+
}
447+
448+
#[test]
449+
fn test_is_delete_operation() {
450+
assert!(is_delete_operation("delete_file"));
451+
assert!(is_delete_operation("delete_branch"));
452+
assert!(is_delete_operation("delete_release"));
453+
assert!(!is_delete_operation("create_repository"));
454+
assert!(!is_delete_operation(""));
455+
}
456+
457+
#[test]
458+
fn test_is_lock_operation() {
459+
assert!(is_lock_operation("lock_issue"));
460+
assert!(is_lock_operation("lock_pull_request"));
461+
assert!(!is_lock_operation("unlock_issue"));
462+
assert!(!is_lock_operation("create_issue"));
463+
assert!(!is_lock_operation(""));
464+
}
465+
466+
#[test]
467+
fn test_is_unlock_operation() {
468+
assert!(is_unlock_operation("unlock_issue"));
469+
assert!(is_unlock_operation("unlock_pull_request"));
470+
assert!(!is_unlock_operation("lock_issue"));
471+
assert!(!is_unlock_operation("create_issue"));
472+
assert!(!is_unlock_operation(""));
473+
}
474+
475+
#[test]
476+
fn test_lock_and_unlock_contribute_to_write_operations() {
477+
// is_write_operation delegates to is_lock_operation and is_unlock_operation
478+
assert!(is_write_operation("lock_issue"));
479+
assert!(is_write_operation("unlock_issue"));
480+
}
481+
423482
#[test]
424483
fn test_granular_pr_update_tools_are_read_write_operations() {
425484
for op in &[

0 commit comments

Comments
 (0)