Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate VCS command paths with "--" #10483

Merged
merged 1 commit into from
Mar 17, 2022
Merged

Separate VCS command paths with "--" #10483

merged 1 commit into from
Mar 17, 2022

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 17, 2022

When building a VCS command, there may be ambiguity if a relative path
looks like an option, like "-path" or "--path". All of the VCS commands
that we use support a bare "--" separator for non-option arguments,
which is good practice to apply here.

This does not affect the cargo CLI, as it already makes sure to use
absolute paths for these calls via value_of_path().

When building a VCS command, there may be ambiguity if a relative path
looks like an option, like "-path" or "--path". All of the VCS commands
that we use support a bare "--" separator for non-option arguments,
which is good practice to apply here.

This does not affect the cargo CLI, as it already makes sure to use
absolute paths for these calls via `value_of_path()`.
@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2022
@cuviper
Copy link
Member Author

cuviper commented Mar 17, 2022

This was raised with the Security Response WG as a vector for command injection, particularly with hg --config=alias.init=!arbitrary-cmd. We agreed to make this issue public, because any untrusted use of such arguments should already be responsible for path sanitization before making this call.

@Eh2406
Copy link
Contributor

Eh2406 commented Mar 17, 2022

This looks Ok to me, we can revert if I missed something.
@bors r+

@bors
Copy link
Contributor

bors commented Mar 17, 2022

📌 Commit 58508e2 has been approved by Eh2406

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 17, 2022
@bors
Copy link
Contributor

bors commented Mar 17, 2022

⌛ Testing commit 58508e2 with merge 1023fa5...

@bors
Copy link
Contributor

bors commented Mar 17, 2022

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing 1023fa5 to master...

@bors bors merged commit 1023fa5 into rust-lang:master Mar 17, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 19, 2022
Update cargo

9 commits in 65c82664263feddc5fe2d424be0993c28d46377a..109bfbd055325ef87a6e7f63d67da7e838f8300b
2022-03-09 02:32:56 +0000 to 2022-03-17 21:43:09 +0000
- Refactor RegistryData::load to handle management of the index cache (rust-lang/cargo#10482)
- Separate VCS command paths with "--" (rust-lang/cargo#10483)
- Fix panic when artifact target is used for `[target.'cfg(<target>)'.dependencies` (rust-lang/cargo#10433)
- Bump [email protected] and [email protected] (rust-lang/cargo#10479)
- vendor: Don't allow multiple values for --sync (rust-lang/cargo#10448)
- Use types to make clere (credential process || token) (rust-lang/cargo#10471)
- Warning on conflicting keys (rust-lang/cargo#10316)
- Registry functions return Poll to enable parallel fetching of index data (rust-lang/cargo#10064)
- Refine the contributor guide (rust-lang/cargo#10468)
@ehuss ehuss added this to the 1.61.0 milestone Apr 7, 2022
@cuviper cuviper deleted the vcs-- branch July 18, 2023 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants