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
35 changes: 33 additions & 2 deletions guards/github-guard/rust-guard/src/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4797,6 +4797,37 @@ mod tests {
}
}

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

for tool in &[
"update_pull_request_body",
"update_pull_request_draft_state",
"update_pull_request_state",
"update_pull_request_title",
] {
let (secrecy, integrity, _desc) = apply_tool_labels(
tool,
&tool_args,
repo_id,
vec![],
vec![],
String::new(),
&ctx,
);

assert_eq!(secrecy, vec![] as Vec<String>, "{} secrecy mismatch", tool);
assert_eq!(integrity, writer_integrity(repo_id, &ctx), "{} should have writer integrity", tool);
}
}

#[test]
fn test_apply_tool_labels_granular_pr_review_tools_writer_integrity() {
let ctx = default_ctx();
Expand Down Expand Up @@ -4826,8 +4857,8 @@ mod tests {
&ctx,
);

assert_eq!(secrecy, vec![] as Vec<String>, "{tool} secrecy mismatch");
assert_eq!(integrity, writer_integrity(repo_id, &ctx), "{tool} should have writer integrity");
assert_eq!(secrecy, vec![] as Vec<String>, "{} secrecy mismatch", tool);
assert_eq!(integrity, writer_integrity(repo_id, &ctx), "{} should have writer integrity", tool);
}
}

Expand Down
44 changes: 13 additions & 31 deletions guards/github-guard/rust-guard/src/labels/tool_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,49 +533,30 @@ pub fn apply_tool_labels(
integrity = writer_integrity(repo_id, ctx);
}

// === Granular issue update tools (repo-scoped writes) ===
// === Granular repo-scoped write operations ===
// Covers granular issue PATCH tools, sub-issue management, granular PR PATCH tools,
// and PR review tools. All follow: S = S(repo), I = writer.
"update_issue_assignees"
| "update_issue_body"
| "update_issue_labels"
| "update_issue_milestone"
| "update_issue_state"
| "update_issue_title"
| "update_issue_type" => {
// Granular PATCH tools that modify individual issue fields.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Sub-issue management tools (repo-scoped writes) ===
"add_sub_issue" | "remove_sub_issue" | "reprioritize_sub_issue" => {
// Sub-issue link creation, removal, and reordering.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Granular PR update tools (repo-scoped read-write) ===
"update_pull_request_body"
| "update_issue_type"
| "add_sub_issue"
| "remove_sub_issue"
| "reprioritize_sub_issue"
| "update_pull_request_body"
| "update_pull_request_draft_state"
| "update_pull_request_state"
| "update_pull_request_title" => {
// Granular PATCH tools that modify individual PR fields.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === PR review tools (repo-scoped writes) ===
"add_pull_request_review_comment"
| "update_pull_request_title"
| "add_pull_request_review_comment"
| "create_pull_request_review"
| "delete_pending_pull_request_review"
| "request_pull_request_reviewers"
| "resolve_review_thread"
| "submit_pending_pull_request_review"
| "unresolve_review_thread" => {
// PR review creation, commenting, submission, and thread resolution.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}
Expand Down Expand Up @@ -746,16 +727,17 @@ fn check_file_secrecy(
ctx: &PolicyContext,
) -> Vec<String> {
let path_lower = path.to_lowercase();
let segments: Vec<&str> = path_lower.split('/').collect();

// Check for sensitive file extensions/names
for pattern in SENSITIVE_FILE_PATTERNS {
if path_lower.ends_with(pattern) || path_lower.split('/').any(|seg| seg.starts_with(*pattern)) {
if path_lower.ends_with(pattern) || segments.iter().any(|seg| seg.starts_with(*pattern)) {
return policy_private_scope_label(owner, repo, repo_id, ctx);
}
}

// Get filename
let filename = path_lower.rsplit('/').next().unwrap_or(&path_lower);
let filename = segments.last().copied().unwrap_or(path_lower.as_str());

// Check for sensitive keywords in filename
for keyword in SENSITIVE_FILE_KEYWORDS {
Expand Down
Loading