feat(cargo-php)!: escalate privilege and to copy extension and edit ini file#482
feat(cargo-php)!: escalate privilege and to copy extension and edit ini file#482dilawar wants to merge 4 commits intoextphprs:masterfrom
Conversation
Pull Request Test Coverage Report for Build 16323306954Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Xenira
left a comment
Xenira
left a comment
There was a problem hiding this comment.
That looks a lot like what I was thinking about. Thanks!
Maybe we can check if we are running with root without needing the dependency, but haven't looked into that.
| // write to a temp file | ||
| let tempf = std::env::temp_dir().join("__tmp_cargo_php"); | ||
| std::fs::write(&tempf, content)?; | ||
|
|
||
| // Now move. `rename` will overwrite existing file. | ||
| if std::fs::rename(&tempf, filepath).is_err() { | ||
| #[cfg(unix)] | ||
| { | ||
| // if not successful, try with sudo on unix. | ||
| let _ = std::process::Command::new("sudo") | ||
| .arg("mv") | ||
| .arg(&tempf) | ||
| .arg(filepath) | ||
| .status()?; | ||
| } | ||
|
|
||
| #[cfg(not(unix))] | ||
| anyhow::bail!("failed to write to {filepath:?}"); | ||
| } |
There was a problem hiding this comment.
Technically this could override changes made between the time the tmp file is created and its copied.
But I think that is fine for now.
There was a problem hiding this comment.
I tried adding a write lock, but then I didn't know how to do it properly on Windows without pulling-in windows-rs crate.
There is https://crates.io/crates/tempfile that may be a cross-platform solution.
I'll borrow the function from |
Xenira
changed the title
Xenira
changed the title
|
@dilawar Thank you! Could you update the guide documentation to include the new behaviour? The git commit history could benefit from a cleanup as well. See the CONTRIBUTING.md. |
|
I'll definitely give it a shot, but I am terrible at writing! I'll rebase/cleanup git history. |
I can do the documentation :) |
|
nix::unistd looks like a better fit. |
And escalate sudo by default in both commands.
since libc is already in cargo.lock Update crates/cli/src/lib.rs Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com> Update crates/cli/src/lib.rs comment -> doc Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com> Update crates/cli/src/lib.rs Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com> Update crates/cli/src/lib.rs Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com> Update crates/cli/src/lib.rs comment -> doc Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com> Update crates/cli/src/lib.rs remove commented out line Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com> Update crates/cli/src/lib.rs Co-authored-by: Xenira <1288524+Xenira@users.noreply.github.com>
4 participants
Fixes: #481
BREAKING CHANGE: installing extensions as root user will now fail unless
--bypass-root-checkis provided