Skip to content

Add link to cargo fetch_with_cli and clarify intent - #39

Merged
leighmcculloch merged 1 commit into
mainfrom
fetch-with-cli-comment
Jul 16, 2026
Merged

Add link to cargo fetch_with_cli and clarify intent#39
leighmcculloch merged 1 commit into
mainfrom
fetch-with-cli-comment

Conversation

@leighmcculloch

Copy link
Copy Markdown
Member

What

Add a link to the cargo fetch_with_cli source alongside the comment on the git() helper, and add a note clarifying that stripping the path-redirecting GIT_* env vars is not a security measure against malicious injection but simply avoids ambient env vars that redirect git's path access to a different repository.

Why

The comment referenced cargo's fetch_with_cli sanitization by name but gave no pointer to the actual code. A direct link makes the rationale easier to verify. The added note prevents a reader from misreading the env var stripping as a security boundary when it is only a correctness measure.

@leighmcculloch
leighmcculloch marked this pull request as ready for review July 16, 2026 02:36
Copilot AI review requested due to automatic review settings July 16, 2026 02:36
@leighmcculloch
leighmcculloch enabled auto-merge (squash) July 16, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a pinned Cargo source link and clarifies that Git environment sanitization is a correctness measure, not a security boundary.

Changes:

  • Links to Cargo’s fetch_with_cli implementation.
  • Clarifies the intent of removing path-redirecting environment variables.

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

@leighmcculloch
leighmcculloch merged commit c2a0cd0 into main Jul 16, 2026
13 checks passed
@leighmcculloch
leighmcculloch deleted the fetch-with-cli-comment branch July 16, 2026 07:08
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.

3 participants