Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
### Added

- [#6012](https://github.com/ChainSafe/forest/issues/6012): Stricter validation of address arguments in `forest-wallet` subcommands.
- [#6008](https://github.com/ChainSafe/forest/issues/6008): `FOREST_PATH` environment variable to override the data directory for `forest`, `forest-cli`, `forest-tool` and `forest-wallet`. Takes precedence over `client.data_dir` in the config file. The daemon also logs the resolved data directory on startup.

### Changed

Expand Down
1 change: 1 addition & 0 deletions docs/docs/users/reference/env_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ process.
| `FOREST_MAX_FILTER_HEIGHT_RANGE` | positive integer | 2880 | 2880 | The maximum filter height range allowed, a conservative limit of one day |
| `FOREST_STATE_MIGRATION_THREADS` | integer | Depends on the machine. | 3 | The number of threads for state migration thread-pool. Advanced users only. |
| `FOREST_CONFIG_PATH` | string | /$FOREST_HOME/com.ChainSafe.Forest/config.toml | `/path/to/config.toml` | Forest configuration path. Alternatively supplied via `--config` cli parameter. |
| `FOREST_PATH` | directory path | platform-specific (`directories::ProjectDirs`) | `/var/lib/forest` | Override the Forest data directory. Honored by `forest`, `forest-cli`, `forest-tool` and `forest-wallet`. Takes precedence over the `client.data_dir` setting in the config file. |
| `FOREST_TEST_RNG_FIXED_SEED` | non-negative integer | empty | 0 | Override RNG with a reproducible one seeded by the value. This should never be used out of test context for security. |
| `RUST_LOG` | string | empty | `debug,forest_libp2p::service=info` | Allows for log level customization. |
| `FOREST_IGNORE_DRAND` | 1 or true | empty | 1 | Ignore Drand validation. |
Expand Down
30 changes: 30 additions & 0 deletions src/cli_shared/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ use crate::networks::NetworkChain;
use crate::utils::io::read_toml;
use std::path::PathBuf;

/// Environment variable that overrides the Forest data directory.
pub const FOREST_PATH_ENV: &str = "FOREST_PATH";

/// Returns the value of [`FOREST_PATH_ENV`] when set to a non-empty string.
pub fn forest_path_from_env() -> Option<PathBuf> {
std::env::var(FOREST_PATH_ENV)
.ok()
.filter(|s| !s.is_empty())
.map(PathBuf::from)
}

cfg_if::cfg_if! {
if #[cfg(feature = "rustalloc")] {
} else if #[cfg(feature = "jemalloc")] {
Expand Down Expand Up @@ -37,6 +48,9 @@ pub fn read_config(
if let Some(chain) = chain_opt {
config.chain = chain;
}
if let Some(data_dir) = forest_path_from_env() {
config.client.data_dir = data_dir;
}
Ok((path, config))
}

Expand Down Expand Up @@ -68,6 +82,22 @@ mod tests {
assert_eq!(config.chain(), &NetworkChain::Butterflynet);
}

#[test]
#[serial_test::serial]
fn read_config_forest_path_env_override() {
let temp_dir = tempfile::tempdir().expect("couldn't create temp dir");
// SAFETY: tests touching the process environment are gated by `#[serial]`.
unsafe {
std::env::set_var(FOREST_PATH_ENV, temp_dir.path());
}
let (_, config) = read_config(None, None).unwrap();
// SAFETY: tests touching the process environment are gated by `#[serial]`.
unsafe {
std::env::remove_var(FOREST_PATH_ENV);
}
assert_eq!(config.client.data_dir, temp_dir.path());
}
Comment on lines +85 to +99
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Restore the previous FOREST_PATH value in the test cleanup

Line 96 always removes the variable. If FOREST_PATH was already set before this test, global process state is changed for later tests. Save the previous value and restore it via a drop guard so cleanup still runs on panic.

Proposed fix
 #[test]
 #[serial_test::serial]
 fn read_config_forest_path_env_override() {
     let temp_dir = tempfile::tempdir().expect("couldn't create temp dir");
+    let previous_forest_path = std::env::var_os(FOREST_PATH_ENV);
+    struct ForestPathEnvGuard(Option<std::ffi::OsString>);
+    impl Drop for ForestPathEnvGuard {
+        fn drop(&mut self) {
+            // SAFETY: tests touching the process environment are gated by `#[serial]`.
+            unsafe {
+                if let Some(value) = &self.0 {
+                    std::env::set_var(FOREST_PATH_ENV, value);
+                } else {
+                    std::env::remove_var(FOREST_PATH_ENV);
+                }
+            }
+        }
+    }
+    let _guard = ForestPathEnvGuard(previous_forest_path);
     // SAFETY: tests touching the process environment are gated by `#[serial]`.
     unsafe {
         std::env::set_var(FOREST_PATH_ENV, temp_dir.path());
     }
     let (_, config) = read_config(None, None).unwrap();
-    // SAFETY: tests touching the process environment are gated by `#[serial]`.
-    unsafe {
-        std::env::remove_var(FOREST_PATH_ENV);
-    }
     assert_eq!(config.client.data_dir, temp_dir.path());
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli_shared/mod.rs` around lines 85 - 99, The test
read_config_forest_path_env_override mutates the global FOREST_PATH_ENV without
restoring any prior value, so capture the previous value (Option<String>) before
calling std::env::set_var and install a drop guard that will restore the
previous state (std::env::set_var(previous) or std::env::remove_var if None)
when it goes out of scope; update the test (around uses of FOREST_PATH_ENV and
read_config) to use this guard so cleanup runs even on panic and the global
process state is preserved for other tests.


#[test]
fn read_config_with_path() {
let default_config = Config::default();
Expand Down
1 change: 1 addition & 0 deletions src/daemon/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ fn startup_init(config: &Config) -> anyhow::Result<()> {
"Starting Forest daemon, version {}",
FOREST_VERSION_STRING.as_str()
);
info!("Data directory: {}", config.client.data_dir.display());
Ok(())
}

Expand Down
11 changes: 7 additions & 4 deletions src/wallet/subcommands/wallet_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,15 @@ impl WalletBackend {
}

fn new_local(client: rpc::Client, want_encryption: bool) -> anyhow::Result<Self> {
let Some(dir) = ProjectDirs::from("com", "ChainSafe", "Forest-Wallet") else {
bail!("Failed to find wallet directory");
let wallet_dir = if let Some(forest_path) = crate::cli_shared::forest_path_from_env() {
forest_path
} else {
let Some(dir) = ProjectDirs::from("com", "ChainSafe", "Forest-Wallet") else {
bail!("Failed to find wallet directory");
};
dir.data_dir().to_path_buf()
};

let wallet_dir = dir.data_dir().to_path_buf();

let is_encrypted = wallet_dir.join(ENCRYPTED_KEYSTORE_NAME).exists();

// Always use the encrypted keystore if it exists. It it does not exist,
Expand Down
Loading