Skip to content

[rust-guard] Rust Guard: Deduplicate unsafe write-back pattern and merge search_code/search_commits arms #7620

@github-actions

Description

@github-actions

🦀 Rust Guard Improvement Report

Improvement 1: Extract write_bytes_to_output unsafe helper in lib.rs

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

Problem

The same 4-line unsafe write-back block appears identically 4 times across the three FFI entry-point functions:

  • label_agent (line ~727)
  • label_resource (line ~875)
  • label_response path-based path (line ~989)
  • label_response legacy path (line ~1054)

Duplicated unsafe code is particularly problematic: any future fix (e.g. bounds check, alignment check) must be applied in all four places, and any missed copy is a latent bug.

Before

// Repeated 4 times, word-for-word:
let output_bytes = output_json.as_bytes();
unsafe {
    let dest = slice::from_raw_parts_mut(output_ptr as *mut u8, output_bytes.len());
    dest.copy_from_slice(output_bytes);
}

After

Add a private helper near the top of lib.rs, below the logging helpers:

/// Copy `bytes` into the WASM linear-memory output buffer.
///
/// # Safety
/// `output_ptr` must point to at least `bytes.len()` writable bytes in WASM
/// linear memory. The caller must have already verified `bytes.len() <= output_size`.
unsafe fn write_bytes_to_output(output_ptr: u32, bytes: &[u8]) {
    let dest = slice::from_raw_parts_mut(output_ptr as *mut u8, bytes.len());
    dest.copy_from_slice(bytes);
}

Then each call site becomes a one-liner:

unsafe { write_bytes_to_output(output_ptr, output_json.as_bytes()); }

Why This Matters

  • Concentrates unsafe surface: all raw-pointer manipulation lives in one named, documented function
  • Eliminates 12 lines of duplication (3 lines × 4 sites)
  • Future-proof: a bounds-check improvement or alignment note only needs one edit
  • The helper name makes the intent clear at call sites — no comment needed

Improvement 2: Merge search_code and search_commits match arms in tool_rules.rs

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

Problem

The search_code and search_commits arms in apply_tool_labels are nearly identical — 15 lines of code repeated with only two trivial differences:

  1. The desc format string prefix ("search_code:" vs "search_commits:")
  2. The search_commits else-branch has a redundant baseline_scope = Cow::Borrowed(repo_id) reassignment — baseline_scope is already initialised to Cow::Borrowed(repo_id) at the top of the function, so this assignment is a no-op.

Before

// === Code Search ===
"search_code" => {
    let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
    if !s_repo_id.is_empty() {
        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);
        baseline_scope = Cow::Owned(s_repo_id);
    } else {
        secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
        integrity = writer_integrity(repo_id, ctx);
    }
}

// === Commit Search ===
"search_commits" => {
    // ... comment block ...
    let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
    if !s_repo_id.is_empty() {
        desc = format!("search_commits:{}", s_repo_id);
        secrecy = apply_repo_visibility_secrecy(&s_owner, &s_repo, &s_repo_id, secrecy, ctx);
        integrity = writer_integrity(&s_repo_id, ctx);
        baseline_scope = Cow::Owned(s_repo_id);
    } else {
        secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
        integrity = writer_integrity(repo_id, ctx);
        if !repo_id.is_empty() {
            baseline_scope = Cow::Borrowed(repo_id);  // ← redundant: already the initial value
        }
    }
}

After

// === Code / Commit Search — repo-scoped read with query-based scope resolution ===
// S = S(repo); I = approved — both tools resolve scope from the query's repo: qualifier
// first, then fall back to tool_args owner/repo.
"search_code" | "search_commits" => {
    let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo);
    if !s_repo_id.is_empty() {
        desc = format!("{}:{}", tool_name, s_repo_id);
        secrecy = apply_repo_visibility_secrecy(&s_owner, &s_repo, &s_repo_id, secrecy, ctx);
        integrity = writer_integrity(&s_repo_id, ctx);
        baseline_scope = Cow::Owned(s_repo_id);
    } else {
        secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
        integrity = writer_integrity(repo_id, ctx);
    }
}

Why This Matters

  • Removes ~15 lines of exact duplication — any future change to search scope labeling (e.g. adding per-item response labels) only needs one edit
  • Removes the misleading redundant baseline_scope reassignment in search_commits else-branch that suggests a meaningful difference where none exists
  • All existing tests (test_apply_tool_labels_search_code, test_apply_tool_labels_search_commits) continue to pass unmodified — the format!("{}:{}", tool_name, s_repo_id) expansion produces the same strings as before

Codebase Health Summary

  • Total Rust files: 9
  • Total lines: 15,721
  • Areas analyzed: lib.rs, labels/helpers.rs, labels/tool_rules.rs, labels/response_paths.rs, labels/response_items.rs, labels/mod.rs, labels/backend.rs, labels/constants.rs, tools.rs
  • Areas with no further improvements: labels/constants.rs, labels/response_items.rs (test coverage added June 12), labels/response_paths.rs (then_some + SharedLabels hoisting applied), labels/helpers.rs (item_number, short_sha, from_policy_str, item_has_config_label, get_string_field all improved)

Generated by Rust Guard Improver • Run: §27611936233

Generated by Rust Guard Improver · 1.3K AIC · ⊞ 34K ·

  • expires on Jun 23, 2026, 10:59 AM UTC

Metadata

Metadata

Assignees

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