Skip to content

[rust-guard] Rust Guard: Deduplicate search scope resolution and trusted-author integrity elevation in tool_rules.rs #2986

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: search_code Should Use resolve_search_scope Like Its Siblings

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

Problem

resolve_search_scope (line ~60) was explicitly introduced to deduplicate scope-extraction logic across search tools. The search_issues (line ~218) and search_pull_requests (line ~356) arms call it correctly — but search_code (line ~484) bypasses it and hand-rolls the same logic with three mutable scoped_* variables:

// search_issues / search_pull_requests — using the helper ✅
let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
if !s_repo_id.is_empty() {
    desc = format!("search_issues:{}", s_repo_id);
    secrecy = apply_repo_visibility_secrecy(&s_owner, &s_repo, &s_repo_id, secrecy, ctx);
    ...
}
// search_code — reinventing the wheel ❌ (~16 extra lines)
let mut scoped_owner = owner.clone();
let mut scoped_repo = repo.clone();
let mut scoped_repo_id = repo_id.to_string();

if (scoped_owner.is_empty() || scoped_repo.is_empty() || scoped_repo_id.is_empty())
    && tool_args.get("query").and_then(|v| v.as_str()).is_some()
{
    let query = tool_args.get("query").and_then(|v| v.as_str()).unwrap_or("");
    let (q_owner, q_repo, q_repo_id) = extract_repo_info_from_search_query(query);
    if !q_repo_id.is_empty() {
        scoped_owner = q_owner;
        scoped_repo = q_repo;
        scoped_repo_id = q_repo_id;
        baseline_scope = scoped_repo_id.clone();
        desc = format!("search_code:{}", scoped_repo_id);
    }
}
secrecy = apply_repo_visibility_secrecy(&scoped_owner, &scoped_repo, &scoped_repo_id, secrecy, ctx);
integrity = writer_integrity(&scoped_repo_id, ctx);

Suggested Change

Replace the search_code arm body with a call to resolve_search_scope, matching the pattern used by the other search arms. The only extra step is updating baseline_scope when a repo is resolved.

Before

"search_code" => {
    // Code search can expose private code
    // S(code) = inherits from repo secrecy
    // I(code) = approved - code from repository
    let mut scoped_owner = owner.clone();
    let mut scoped_repo = repo.clone();
    let mut scoped_repo_id = repo_id.to_string();

    if (scoped_owner.is_empty() || scoped_repo.is_empty() || scoped_repo_id.is_empty())
        && tool_args.get("query").and_then(|v| v.as_str()).is_some()
    {
        let query = tool_args
            .get("query")
            .and_then(|v| v.as_str())
            .unwrap_or("");
        let (q_owner, q_repo, q_repo_id) = extract_repo_info_from_search_query(query);
        if !q_repo_id.is_empty() {
            scoped_owner = q_owner;
            scoped_repo = q_repo;
            scoped_repo_id = q_repo_id;
            baseline_scope = scoped_repo_id.clone();
            desc = format!("search_code:{}", scoped_repo_id);
        }
    }

    secrecy = apply_repo_visibility_secrecy(
        &scoped_owner,
        &scoped_repo,
        &scoped_repo_id,
        secrecy,
        ctx,
    );
    integrity = writer_integrity(&scoped_repo_id, ctx);
}

After

"search_code" => {
    // Code search can expose private code
    // S(code) = inherits from repo secrecy
    // I(code) = approved - code from repository
    let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
    if !s_repo_id.is_empty() {
        baseline_scope = s_repo_id.clone();
        desc = format!("search_code:{}", s_repo_id);
        secrecy = apply_repo_visibility_secrecy(&s_owner, &s_repo, &s_repo_id, secrecy, ctx);
        integrity = writer_integrity(&s_repo_id, ctx);
    } else {
        secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
        integrity = writer_integrity(repo_id, ctx);
    }
}

Note: The original code applied apply_repo_visibility_secrecy even when scoped_repo_id was still equal to repo_id (i.e., no query scope was found). The else branch preserves that existing behaviour.

Why This Matters

resolve_search_scope was created precisely to avoid this duplication. Having search_code skip it creates a subtle maintenance hazard: if the resolution logic is ever changed in resolve_search_scope, search_code will silently diverge. The refactor is ~10 lines smaller and consistent with the existing pattern.


Improvement 2: Extract Trusted-Author + Collaborator-Permission Integrity Elevation Into a Shared Helper

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

Problem

The get_issue/issue_read arm and the get_pull_request/pull_request_read arm each contain an essentially identical ~45-line block that:

  1. Computes an integrity floor from author_association
  2. Elevates trusted bots/users to writer level
  3. Falls back to collaborator permission for org repos when the floor is insufficient
  4. Logs each step with near-identical messages

The only differences between the two copies are the variable name (floor vs integrity) and the resource label used in log messages ("issue_read" vs "pull_request_read").

// Issue arm (lines ~124–176) and PR arm (lines ~251–308) are nearly identical:
let mut floor = author_association_floor_from_str(repo_id, info.author_association.as_deref(), ctx);
if let Some(ref login) = info.author_login {
    if is_trusted_first_party_bot(login)
        || is_configured_trusted_bot(login, ctx)
        || is_trusted_user(login, ctx)
    {
        floor = max_integrity(repo_id, floor, writer_integrity(repo_id, ctx), ctx);
    }
}
if floor.len() < 3 {
    let is_org = super::backend::is_repo_org_owned(&owner, &repo).unwrap_or(false);
    if is_org {
        if let Some(ref login) = info.author_login {
            crate::log_info(&format!(
                "issue_read {}/{}#{}: author_association floor insufficient ...",
                ...
            ));
            if let Some(collab) = super::backend::get_collaborator_permission(&owner, &repo, login) {
                let perm_floor = collaborator_permission_floor(repo_id, collab.permission.as_deref(), ctx);
                let merged = max_integrity(repo_id, floor, perm_floor, ctx);
                crate::log_info(...);
                floor = merged;
            } else {
                crate::log_info(...);
            }
        }
    }
}

Suggested Change

Extract a private function resolve_author_integrity at the top of tool_rules.rs:

Before

Two copies of ~45 lines in the issue and PR match arms.

After

/// Compute integrity for a user-authored resource (issue or PR), applying:
///   1. `author_association` floor
///   2. Trusted bot/user elevation to writer level
///   3. Collaborator-permission fallback for org repos
fn resolve_author_integrity(
    owner: &str,
    repo: &str,
    repo_id: &str,
    author_login: Option<&str>,
    author_association: Option<&str>,
    resource_label: &str, // e.g. "issue_read" or "pull_request_read"
    resource_num: &str,
    base_integrity: Vec<String>,
    ctx: &PolicyContext,
) -> Vec<String> {
    let mut floor = author_association_floor_from_str(repo_id, author_association, ctx);

    if let Some(login) = author_login {
        if is_trusted_first_party_bot(login)
            || is_configured_trusted_bot(login, ctx)
            || is_trusted_user(login, ctx)
        {
            floor = max_integrity(repo_id, floor, writer_integrity(repo_id, ctx), ctx);
        }
        if floor.len() < 3 {
            let is_org = super::backend::is_repo_org_owned(owner, repo).unwrap_or(false);
            if is_org {
                crate::log_info(&format!(
                    "{} {}/{}#{}: author_association floor insufficient (len={}), checking collaborator permission for {}",
                    resource_label, owner, repo, resource_num, floor.len(), login
                ));
                if let Some(collab) = super::backend::get_collaborator_permission(owner, repo, login) {
                    let perm_floor = collaborator_permission_floor(
                        repo_id,
                        collab.permission.as_deref(),
                        ctx,
                    );
                    let merged = max_integrity(repo_id, floor, perm_floor, ctx);
                    crate::log_info(&format!(
                        "{} {}/{}#{}: collaborator permission={:?} → merged floor len={}",
                        resource_label, owner, repo, resource_num, collab.permission, merged.len()
                    ));
                    floor = merged;
                } else {
                    crate::log_info(&format!(
                        "{} {}/{}#{}: collaborator permission lookup returned None for {}, keeping author_association floor",
                        resource_label, owner, repo, resource_num, login
                    ));
                }
            }
        }
    }

    max_integrity(repo_id, base_integrity, floor, ctx)
}

Call sites become:

// Issue arm
integrity = resolve_author_integrity(
    &owner, &repo, repo_id,
    info.author_login.as_deref(),
    info.author_association.as_deref(),
    "issue_read", &issue_num,
    integrity, ctx,
);

// PR arm
integrity = resolve_author_integrity(
    &owner, &repo, repo_id,
    facts.author_login.as_deref(),
    facts.author_association.as_deref(),
    "pull_request_read", &number,
    integrity, ctx,
);

Why This Matters

~90 lines collapse to ~40 (the helper + 2 call sites). Any future change to the elevation policy — e.g., adding a new trusted-bot check or changing the collaborator-permission threshold — only needs to be made in one place. The current duplication is a silent bug risk: the two copies have already drifted slightly in log wording ("floor insufficient" vs "integrity insufficient"), showing the maintenance pressure already present.


Codebase Health Summary

  • Total Rust files: 9
  • Total lines: 10,898
  • Areas analyzed: labels/tool_rules.rs, labels/response_items.rs, labels/response_paths.rs, labels/constants.rs, labels/helpers.rs, labels/backend.rs, labels/mod.rs, lib.rs, tools.rs
  • Areas with no further improvements: (none yet — all areas still have opportunities)

Generated by Rust Guard Improver • Run: §23841970279

Generated by Rust Guard Improver ·

  • expires on Apr 8, 2026, 9:38 AM UTC

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions