Skip to content

Rust guard: remove list_commits secrecy clone and add project-item path labeling regressions#6941

Merged
lpcox merged 2 commits into
mainfrom
copilot/rust-guard-improve-list-commits
Jun 3, 2026
Merged

Rust guard: remove list_commits secrecy clone and add project-item path labeling regressions#6941
lpcox merged 2 commits into
mainfrom
copilot/rust-guard-improve-list-commits

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 3, 2026

This tightens two remaining weak spots in response_paths.rs: list_commits still deep-cloned secrecy labels on every call, and the list_project_items / projects_list path-labeling branch lacked direct regression coverage for its security-relevant edge cases.

  • list_commits: eliminate the last Vec<String> deep clone

    • Convert repo secrecy to SharedLabels immediately.
    • Reuse it via cheap Arc clones for per-item labels.
    • Move the same shared value into default_labels on its final use.
    let default_secrecy: crate::SharedLabels =
        repo_visibility_secrecy(&arg_owner, &arg_repo, &default_repo, ctx).into();
    
    // per-item
    secrecy: default_secrecy.clone(),
    
    // default labels
    secrecy: default_secrecy,
  • Project-item response-path regressions

    • Add coverage for the fail-secure branch when an ISSUE item has no repo context.
    • Assert DRAFT_ISSUE stays owner-writer integrity with empty secrecy.
    • Assert projects_list remains behaviorally identical to list_project_items.
  • items_path contract

    • Add an explicit assertion that { "items": [...] } responses propagate items_path == Some("/items") and produce /items/{n} entry paths.

These changes keep the response-labeling behavior unchanged where already correct, while removing unnecessary allocation/copying in commit labeling and documenting the intended contract around heterogeneous project items.

Copilot AI changed the title [WIP] Improve performance by eliminating redundant Vec clone in list_commits Rust guard: remove list_commits secrecy clone and add project-item path labeling regressions Jun 3, 2026
Copilot finished work on behalf of lpcox June 3, 2026 13:59
Copilot AI requested a review from lpcox June 3, 2026 13:59
@lpcox lpcox marked this pull request as ready for review June 3, 2026 14:49
Copilot AI review requested due to automatic review settings June 3, 2026 14:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the Rust guard’s response path labeling in response_paths.rs by removing a remaining per-call deep clone in list_commits secrecy labels and by adding targeted regression tests for security-sensitive project-item path labeling edge cases.

Changes:

  • Convert list_commits default secrecy labels to SharedLabels immediately and reuse via cheap Arc clones (eliminating the last Vec<String> deep clone in that path).
  • Add regression tests for list_project_items/projects_list to ensure fail-secure behavior when repo context is missing, correct handling of DRAFT_ISSUE, and alias behavioral equivalence.
  • Add an explicit test assertion that { "items": [...] } responses forward items_path == Some("/items") and produce /items/{n} entry paths.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/labels/response_paths.rs Removes an unnecessary secrecy-label deep clone for list_commits and adds regression tests covering project-item labeling and items_path propagation.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 1aeb8fb into main Jun 3, 2026
29 checks passed
@lpcox lpcox deleted the copilot/rust-guard-improve-list-commits branch June 3, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rust-guard] Rust Guard: list_commits Vec clone + project-item test coverage

3 participants