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

deno upgrade permission check should check cliOptionOutput #18411

Closed
loynoir opened this issue Mar 24, 2023 · 5 comments · Fixed by #18427 or #18994
Closed

deno upgrade permission check should check cliOptionOutput #18411

loynoir opened this issue Mar 24, 2023 · 5 comments · Fixed by #18427 or #18994
Labels
bug Something isn't working correctly

Comments

@loynoir
Copy link

loynoir commented Mar 24, 2023

Actual

$ deno upgrade --version 1.30.3 --output ./deno_1_30_3
error: You don't have write permission to /usr/bin/deno because it's owned by root.
Consider updating deno through your package manager if its installed from it.
Otherwise run `deno upgrade` as root.

Workaround

$ sudo deno upgrade --version 1.30.3 --output ./deno_1_30_3
Downloading https://github.com/denoland/deno/releases/download/v1.30.3/deno-x86_64-unknown-linux-gnu.zip
...

Expected

deno upgrade should permission check ./deno_1_30_3 instead of /usr/bin/deno

Related

#18405
#18406
#18438
#18440

@dsherret dsherret added bug Something isn't working correctly good first issue Good for newcomers labels Mar 24, 2023
@dsherret
Copy link
Member

Thanks! The incorrect code is here:

deno/cli/tools/upgrade.rs

Lines 267 to 286 in d740a9e

let current_exe_path = std::env::current_exe()?;
let metadata = fs::metadata(&current_exe_path)?;
let permissions = metadata.permissions();
if permissions.readonly() {
bail!(
"You do not have write permission to {}",
current_exe_path.display()
);
}
#[cfg(unix)]
if std::os::unix::fs::MetadataExt::uid(&metadata) == 0
&& !nix::unistd::Uid::effective().is_root()
{
bail!(concat!(
"You don't have write permission to {} because it's owned by root.\n",
"Consider updating deno through your package manager if its installed from it.\n",
"Otherwise run `deno upgrade` as root.",
), current_exe_path.display());
}

@dsherret
Copy link
Member

Reopining because the fix was reverted in #18467. We need to not error when the output path doesn't exists.

@dsherret dsherret reopened this Mar 31, 2023
@RyanClementsHax
Copy link
Contributor

Is this still a good first issue? I'm looking to help out somewhere

@itsajay1029
Copy link

Can I work on this @dsherret ? Pls assign it to me

@dsherret
Copy link
Member

Thanks, but I think there is already an open PR #18994

@dsherret dsherret removed the good first issue Good for newcomers label May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
4 participants