Skip to content

Commit

Permalink
fix(cli): canonicalize keystore path (#19312)
Browse files Browse the repository at this point in the history
## Description

Fix a bug where a relative keystore path was written out to the config
upon creation when the config itself was specified at a relative path.

The fix is to canonicalize the keystore path before using it, but we
need to handle multiple edge cases:

- A wallet config created in the current working directory as a relative
path (and relative paths in general).
- A wallet config created at the very root of the filesystem (although
this should never happen, the correct behaviour is not to default to the
default config directory here).

## Test plan

```
sui$ cargo build --bin sui
sui$ export SUI=~/sui/target/debug/sui
sui$ mv ~/.sui/sui_config/client.yaml{,~}
sui$ cd ~

# Create the config at the default location using a relative path
sui$ $SUI client --client.config .sui/sui_config/client.yaml gas
sui$ cd -
sui$ $SUI client gas

# Create the config at some other location, using a relative path
sui$ mkdir -p /tmp/a
sui$ cd /tmp/a
a$ $SUI client --client.config ./client.yaml gas
a$ cd -
sui$ $SUI client --client.config /tmp/a/client.yaml gas

# Create the config at some other location, using a relative path
# without a root
sui$ mkdir -p /tmp/b
sui$ cd /tmp/b
b$ $SUI client --client.config client.yaml gas
b$ cd -
sui$ $SUI client --client.config /tmp/b/client.yaml gas
```

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [x] CLI: Fixes a bug where the CLI would write out a config with a
relative path for the keystore that would only work if the CLI was
subsequently called from the same directory that the config was first
created in.
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
amnn committed Sep 11, 2024
1 parent 3012fa4 commit 9cd655a
Showing 1 changed file with 21 additions and 5 deletions.
26 changes: 21 additions & 5 deletions crates/sui/src/sui_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::fire_drill::{run_fire_drill, FireDrill};
use crate::genesis_ceremony::{run, Ceremony};
use crate::keytool::KeyToolCommand;
use crate::validator_commands::SuiValidatorCommand;
use anyhow::{anyhow, bail, ensure};
use anyhow::{anyhow, bail, ensure, Context};
use clap::*;
use fastcrypto::traits::KeyPair;
use move_analyzer::analyzer;
Expand Down Expand Up @@ -1138,10 +1138,26 @@ async fn prompt_if_no_config(
};

if let Some(env) = env {
let keystore_path = wallet_conf_path
.parent()
.unwrap_or(&sui_config_dir()?)
.join(SUI_KEYSTORE_FILENAME);
let keystore_path = match wallet_conf_path.parent() {
// Wallet config was created in the current directory as a relative path.
Some(parent) if parent.as_os_str().is_empty() => {
std::env::current_dir().context("Couldn't find current directory")?
}

// Wallet config was given a path with some parent (could be relative or absolute).
Some(parent) => parent
.canonicalize()
.context("Could not find sui config directory")?,

// No parent component and the wallet config was the empty string, use the default
// config.
None if wallet_conf_path.as_os_str().is_empty() => sui_config_dir()?,

// Wallet config was requested at the root of the file system ...for some reason.
None => wallet_conf_path.to_owned(),
}
.join(SUI_KEYSTORE_FILENAME);

let mut keystore = Keystore::from(FileBasedKeystore::new(&keystore_path)?);
let key_scheme = if accept_defaults {
SignatureScheme::ED25519
Expand Down

0 comments on commit 9cd655a

Please sign in to comment.