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
100 changes: 100 additions & 0 deletions guards/github-guard/rust-guard/src/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4141,4 +4141,104 @@ mod tests {
writer_integrity(repo, &ctx)
);
}

// =========================================================================
// pin_issue / unpin_issue tests
// =========================================================================

#[test]
fn test_apply_tool_labels_pin_issue_writer_integrity() {
let ctx = default_ctx();
let repo_id = "github/copilot";
let tool_args = json!({
"owner": "github",
"repo": "copilot",
"issue_number": 42
});

let (_secrecy, integrity, desc) = apply_tool_labels(
"pin_issue",
&tool_args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);

assert_eq!(
integrity,
writer_integrity(repo_id, &ctx),
"pin_issue should require writer-level integrity"
);
assert_eq!(desc, "issue:github/copilot#42");
}

#[test]
fn test_apply_tool_labels_unpin_issue_writer_integrity() {
let ctx = default_ctx();
let repo_id = "github/copilot";
let tool_args = json!({
"owner": "github",
"repo": "copilot",
"issue_number": 7
});

let (_secrecy, integrity, desc) = apply_tool_labels(
"unpin_issue",
&tool_args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);

assert_eq!(
integrity,
writer_integrity(repo_id, &ctx),
"unpin_issue should require writer-level integrity"
);
assert_eq!(desc, "issue:github/copilot#7");
}

// =========================================================================
// transfer_repository tests
// =========================================================================

#[test]
fn test_apply_tool_labels_transfer_repository_secrecy_inherits_repo_visibility() {
// apply_tool_labels sets the correct secrecy for transfer_repository.
// The blocked_integrity enforcement happens in label_resource (lib.rs) via
// is_blocked_tool(), not in apply_tool_labels, because ensure_integrity_baseline
// would otherwise raise blocked-level tags to none-level.
let ctx = default_ctx();
let repo_id = "github/copilot";
let tool_args = json!({
"owner": "github",
"repo": "copilot",
"new_owner": "other-org"
});

let (secrecy, _integrity, _desc) = apply_tool_labels(
"transfer_repository",
&tool_args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);

// In the test environment the backend host is unavailable, so
// apply_repo_visibility_secrecy returns current_secrecy unchanged
// (the cfg(test) fallback path). An empty initial secrecy therefore
// remains empty. What matters is that the call completes without
// panicking and returns no unexpected secrecy labels.
assert!(
secrecy.is_empty(),
"transfer_repository secrecy should be empty in test env (no backend); got: {:?}",
secrecy
);
}
}
25 changes: 25 additions & 0 deletions guards/github-guard/rust-guard/src/labels/tool_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,31 @@ pub fn apply_tool_labels(
}
}

// === Issue Pin/Unpin (repo-scoped write) ===
"pin_issue" | "unpin_issue" => {
// Pinning/unpinning an issue is a repo-level cosmetic write operation.
// S = S(repo) — inherits from repository visibility
// I = writer (requires repo write access to change issue pin state)
if !owner.is_empty() && !repo.is_empty() {
if let Some(issue_num) =
extract_number_as_string(tool_args, field_names::ISSUE_NUMBER)
{
desc = format!("issue:{}/{}#{}", owner, repo, issue_num);
}
}
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the new pin_issue / unpin_issue arm, integrity = writer_integrity(repo_id, ctx) is applied unconditionally. If repo_id is empty (e.g., malformed/missing owner+repo in tool_args), writer_integrity("") produces unscoped base labels including "approved" (see existing test_empty_scope_integrity in labels/mod.rs), which is broader than intended for a repo-scoped operation. Please gate this so writer integrity is only assigned when repo_id is non-empty; otherwise keep/return a conservative integrity (e.g., leave as-is/empty so the baseline becomes none). Adding a regression test for the empty-scope case would help prevent accidental global approval.

Suggested change
integrity = writer_integrity(repo_id, ctx);
if !repo_id.is_empty() {
integrity = writer_integrity(repo_id, ctx);
} else {
// Malformed or missing repo scope: avoid assigning unscoped writer integrity.
// Use an empty integrity baseline so no global approval is implied.
integrity.clear();
}

Copilot uses AI. Check for mistakes.
}

// === Repository Transfer (blocked: irreversible ownership change) ===
"transfer_repository" => {
// Repository transfers are irreversible and cannot be allowed by agents.
// Blocking is enforced in label_resource via is_blocked_tool(); this arm
// applies repo-visibility secrecy so the resource is at least correctly
// classified before the integrity override happens in label_resource.
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
}

// Search issues: extract repo scope from query or tool_args when available
"search_issues" => {
let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
Expand Down
61 changes: 59 additions & 2 deletions guards/github-guard/rust-guard/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ mod tools;

use labels::constants::policy_integrity;
use labels::{
extract_repo_info, extract_repo_info_from_search_query, MinIntegrity, PolicyContext,
PolicyScopeEntry, ScopeKind,
blocked_integrity, extract_repo_info, extract_repo_info_from_search_query, MinIntegrity,
PolicyContext, PolicyScopeEntry, ScopeKind,
};
use serde::{Deserialize, Serialize};
use serde_json::Value;
Expand Down Expand Up @@ -670,6 +670,20 @@ pub extern "C" fn label_resource(
let baseline_scope = infer_scope_for_baseline(&input.tool_name, &input.tool_args, &repo_id);
let final_integrity = labels::ensure_integrity_baseline(&baseline_scope, final_integrity, &ctx);

// Unconditionally blocked tools: override integrity to blocked_integrity so the
// DIFC evaluator always denies them. This must happen after ensure_integrity_baseline
// because that helper would otherwise raise blocked-level tags to none-level.
let final_integrity = if tools::is_blocked_tool(&input.tool_name) {
log_info(&format!(
" tool '{}' is unconditionally blocked — overriding integrity to blocked",
input.tool_name
));
let scope = if repo_id.is_empty() { "global" } else { &repo_id };
blocked_integrity(scope, &ctx)
} else {
final_integrity
};

// Log computed labels
log_info(&format!(" desc={}", final_desc));
if final_secrecy.is_empty() {
Expand Down Expand Up @@ -1131,4 +1145,47 @@ mod tests {
let inferred = infer_scope_for_baseline("search_pull_requests", &tool_args, "");
assert_eq!(inferred, "github/gh-aw-mcpg");
}

#[test]
fn transfer_repository_integrity_is_blocked_after_ensure_baseline() {
// Verify that the is_blocked_tool + blocked_integrity override logic produces
// a "blocked:" tag, proving that ensure_integrity_baseline cannot raise it
// back to "none:".
let ctx = PolicyContext::default();
let repo_id = "github/copilot";

let tool_args = json!({
"owner": "github",
"repo": "copilot",
"new_owner": "other-org"
});

let (_, integrity, _) = labels::apply_tool_labels(
"transfer_repository",
&tool_args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);
let baseline_scope =
infer_scope_for_baseline("transfer_repository", &tool_args, repo_id);
let after_baseline = labels::ensure_integrity_baseline(&baseline_scope, integrity, &ctx);

// Simulate the is_blocked_tool override performed in label_resource
let final_integrity = if tools::is_blocked_tool("transfer_repository") {
let scope = if repo_id.is_empty() { "global" } else { repo_id };
blocked_integrity(scope, &ctx)
} else {
after_baseline
};

assert!(
final_integrity.iter().any(|t| t.contains("blocked")),
"transfer_repository must have blocked integrity after label_resource override; \
got: {:?}",
final_integrity
);
}
}
61 changes: 61 additions & 0 deletions guards/github-guard/rust-guard/src/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ pub const WRITE_OPERATIONS: &[&str] = &[
// Pre-emptive entries for anticipated future MCP tools (no equivalent tool today)
"archive_repository", // gh repo archive
"transfer_issue", // gh issue transfer
"transfer_repository", // gh repo transfer — blocked: repo ownership transfer is irreversible
"pin_issue", // gh issue pin
"unpin_issue", // gh issue unpin
"enable_workflow", // gh workflow enable
"disable_workflow", // gh workflow disable
"set_secret", // gh secret set
Expand Down Expand Up @@ -103,3 +106,61 @@ pub fn is_lock_operation(tool_name: &str) -> bool {
pub fn is_unlock_operation(tool_name: &str) -> bool {
tool_name.starts_with("unlock_")
}

/// Check if a tool is unconditionally blocked (always denied regardless of agent integrity).
///
/// Blocked tools are listed here when the operation is considered too dangerous
/// to ever permit via an agent, even if the agent would otherwise satisfy the
/// integrity requirements for a normal write operation.
///
/// Current entries:
/// - `transfer_repository`: repository ownership transfer is irreversible and
/// must never be performed by an automated agent.
pub fn is_blocked_tool(tool_name: &str) -> bool {
matches!(tool_name, "transfer_repository")
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_is_blocked_tool_transfer_repository() {
assert!(
is_blocked_tool("transfer_repository"),
"transfer_repository must be unconditionally blocked"
);
}

#[test]
fn test_is_blocked_tool_other_write_ops_not_blocked() {
// Regular write operations should not be blocked
for op in &["create_issue", "add_issue_comment", "pin_issue", "unpin_issue"] {
assert!(
!is_blocked_tool(op),
"{} should not be blocked",
op
);
}
}

#[test]
fn test_transfer_repository_is_write_operation() {
assert!(
is_write_operation("transfer_repository"),
"transfer_repository must be classified as a write operation"
);
}

#[test]
fn test_pin_unpin_issue_are_write_operations() {
assert!(
is_write_operation("pin_issue"),
"pin_issue must be classified as a write operation"
);
assert!(
is_write_operation("unpin_issue"),
"unpin_issue must be classified as a write operation"
);
}
}
Loading