[rust-guard] Rust Guard: Consolidate duplicate granular-write match arms and fix redundant path splitting

[github-actions]

🦀 Rust Guard Improvement Report

Improvement 1: Consolidate 4 Identical "Granular Write" Match Arms

Category: Duplication
File(s): guards/github-guard/rust-guard/src/labels/tool_rules.rs
Effort: Small (< 15 min)
Risk: Low

Problem

Lines 535–583 of tool_rules.rs contain four consecutive match arms that cover 21 different tools across four semantic groups:

• Granular issue update tools (7): update_issue_assignees, update_issue_body, update_issue_labels, update_issue_milestone, update_issue_state, update_issue_title, update_issue_type
• Sub-issue management tools (3): add_sub_issue, remove_sub_issue, reprioritize_sub_issue
• Granular PR update tools (4): update_pull_request_body, update_pull_request_draft_state, update_pull_request_state, update_pull_request_title
• PR review tools (7): 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

Every one of these arms has identical two-line bodies and the same // S = S(repo); I = writer semantics, creating ~50 lines of repeated boilerplate (4 section headers + 4 comment blocks + 4 copies of the same two assignment lines).

Before

// === Granular issue update tools (repo-scoped writes) ===
"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_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"
| "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);
}

After

// === Granular repo-scoped write operations ===
// Covers granular issue PATCH tools, sub-issue management, granular PR PATCH tools,
// and PR review tools. All follow the same labeling policy: 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"
| "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"
| "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" => {
    secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
    integrity = writer_integrity(repo_id, ctx);
}

Why This Matters

Removes ~30 lines of pure duplication. These four groups belong to the same operational tier (granular writes on repo resources) and have provably identical semantics — merging them makes that uniformity explicit and prevents future drift where someone copies the block for a new tool but accidentally changes only some of the copies.

---

Improvement 2: Eliminate Redundant Path Splitting in check_file_secrecy

Category: Performance (WASM-specific)
File(s): guards/github-guard/rust-guard/src/labels/tool_rules.rs
Effort: Small (< 15 min)
Risk: Low

Problem

check_file_secrecy (lines 740–773) traverses the lowercased file path 10 times:

1. path_lower.split('/') is called 9 times — once per entry in SENSITIVE_FILE_PATTERNS — inside the first for loop (line 752).
2. path_lower.rsplit('/').next() traverses the string a 10th time to extract the filename (line 758).

Each call to split('/') produces a fresh iterator that walks the entire string. For a typical path like .github/workflows/ci.yml (3 segments), this is 9 full string walks for the first loop alone, all producing identical segment sequences.

for pattern in SENSITIVE_FILE_PATTERNS {   // runs 9 times
    if path_lower.ends_with(pattern)
        || path_lower.split('/').any(|seg| seg.starts_with(*pattern))  // ← new iterator each time
    {
        return policy_private_scope_label(owner, repo, repo_id, ctx);
    }
}

let filename = path_lower.rsplit('/').next().unwrap_or(&path_lower);  // ← 10th traversal

Suggested Change

Collect path segments into a Vec<&str> once, then use it for both the pattern loop and the filename extraction. This reduces 10 string traversals to 1.

Before

fn check_file_secrecy(
    path: &str,
    default_secrecy: Vec<String>,
    owner: &str,
    repo: &str,
    repo_id: &str,
    ctx: &PolicyContext,
) -> Vec<String> {
    let path_lower = path.to_lowercase();

    // 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)) {
            return policy_private_scope_label(owner, repo, repo_id, ctx);
        }
    }

    // Get filename
    let filename = path_lower.rsplit('/').next().unwrap_or(&path_lower);

    // Check for sensitive keywords in filename
    for keyword in SENSITIVE_FILE_KEYWORDS {
        if filename.contains(keyword) {
            return policy_private_scope_label(owner, repo, repo_id, ctx);
        }
    }

    // Workflow files may contain secrets
    if path_lower.starts_with(".github/workflows/") {
        return policy_private_scope_label(owner, repo, repo_id, ctx);
    }

    default_secrecy
}

After

fn check_file_secrecy(
    path: &str,
    default_secrecy: Vec<String>,
    owner: &str,
    repo: &str,
    repo_id: &str,
    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) || segments.iter().any(|seg| seg.starts_with(*pattern)) {
            return policy_private_scope_label(owner, repo, repo_id, ctx);
        }
    }

    // Get filename (last segment, falling back to full path if no slashes)
    let filename = segments.last().copied().unwrap_or(path_lower.as_str());

    // Check for sensitive keywords in filename
    for keyword in SENSITIVE_FILE_KEYWORDS {
        if filename.contains(keyword) {
            return policy_private_scope_label(owner, repo, repo_id, ctx);
        }
    }

    // Workflow files may contain secrets
    if path_lower.starts_with(".github/workflows/") {
        return policy_private_scope_label(owner, repo, repo_id, ctx);
    }

    default_secrecy
}

Why This Matters

In WASM, iterating a Vec<&str> is faster than re-creating and walking a string iterator 9 times. check_file_secrecy is called for every get_file_contents tool call, so it's a moderately hot path. The fix also makes the code intent clearer: "split once, use the segments in multiple places" is more readable than the implicit repeated splitting.

The segments.last().copied().unwrap_or(path_lower.as_str()) is semantically identical to rsplit('/').next().unwrap_or(&path_lower) — both return the last /-delimited segment, or the whole string if there are none.

---

Codebase Health Summary

• Total Rust files: 10
• Total lines: ~12,426
• Areas analyzed: lib.rs, tools.rs, labels/tool_rules.rs, labels/constants.rs, labels/backend.rs, labels/helpers.rs, labels/response_items.rs, labels/response_paths.rs
• Areas with no further improvements identified: tools.rs (well-tested, clean), labels/constants.rs (minimal, no redundancy)

---

Generated by Rust Guard Improver • Run: §24503179194

Generated by Rust Guard Improver · ● 1.3M · ◷

• expires on Apr 23, 2026, 9:45 AM UTC