Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
3 changes: 2 additions & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ jobs:
go-version-file: "go.work"
- uses: taiki-e/install-action@cargo-llvm-cov
- uses: taiki-e/install-action@nextest
- run: cargo run --bin forest-dev --no-default-features --profile quick -- fetch-rpc-tests
- name: Generate code coverage
run: |
cargo llvm-cov --workspace --codecov --output-path lcov.info nextest
cargo llvm-cov --workspace --profile quick --codecov --output-path lcov.info test
env:
CC: "sccache clang"
CXX: "sccache clang++"
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ docker-run:
docker build -t forest:latest -f ./Dockerfile . && docker run forest

test:
# Skip RPC snapshot tests for debug build
export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1
cargo nextest run --workspace --no-fail-fast

Comment on lines 101 to 103
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 | 🟠 Major

export ... won’t affect the next recipe line (each line is a separate shell)

Line 103 exports FOREST_RPC_SNAPSHOT_TEST_OPT_OUT in a different shell than Line 104, so cargo nextest run ... likely won’t see it and the opt-out won’t apply.

Suggested fix (attach env var to the command(s) that need it):

 test:
 	# Skip RPC snapshot tests for debug build
-	export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1
-	cargo nextest run --workspace --no-fail-fast
+	FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo nextest run --workspace --no-fail-fast

 	# nextest doesn't run doctests https://github.com/nextest-rs/nextest/issues/16
 	# see also lib.rs::doctest_private
-	cargo test --doc --features doctest-private
+	FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo test --doc --features doctest-private

If you only want the opt-out for nextest (not doctests), drop the second env prefix.

🤖 Prompt for AI Agents
In Makefile lines 101-105 the `export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1` is in
a separate shell from the `cargo nextest run` line, so the env var won't be
visible; change the recipe to attach the env var to the command (e.g. prefix the
cargo invocation with `FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo nextest run
...`) or combine into one shell with `export ... && cargo ...`; if the opt-out
should only apply to nextest and not doctests, only prefix the `cargo nextest
run` command.

# nextest doesn't run doctests https://github.com/nextest-rs/nextest/issues/16
Expand Down
7 changes: 7 additions & 0 deletions src/bin/forest-dev.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

#[tokio::main(flavor = "multi_thread")]
async fn main() -> anyhow::Result<()> {
forest::forest_dev_main(std::env::args_os()).await
}
5 changes: 3 additions & 2 deletions src/bin/forest-tool.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

fn main() -> anyhow::Result<()> {
forest::forest_tool_main(std::env::args_os())
#[tokio::main(flavor = "multi_thread")]
async fn main() -> anyhow::Result<()> {
forest::forest_tool_main(std::env::args_os()).await
}
5 changes: 3 additions & 2 deletions src/bin/forest-wallet.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

fn main() -> anyhow::Result<()> {
forest::forest_wallet_main(std::env::args_os())
#[tokio::main(flavor = "multi_thread")]
async fn main() -> anyhow::Result<()> {
forest::forest_wallet_main(std::env::args_os()).await
}
18 changes: 18 additions & 0 deletions src/dev/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

use super::subcommands::Cli;
use crate::cli_shared::logger::setup_minimal_logger;
use clap::Parser as _;
use std::ffi::OsString;

pub async fn main<ArgT>(args: impl IntoIterator<Item = ArgT>) -> anyhow::Result<()>
where
ArgT: Into<OsString> + Clone,
{
// Capture Cli inputs
let Cli { cmd } = Cli::parse_from(args);
setup_minimal_logger();
let client = crate::rpc::Client::default_or_from_env(None)?;
cmd.run(client).await
}
5 changes: 5 additions & 0 deletions src/dev/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

pub mod main;
pub mod subcommands;
88 changes: 88 additions & 0 deletions src/dev/subcommands/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright 2019-2025 ChainSafe Systems
// SPDX-License-Identifier: Apache-2.0, MIT

use crate::cli_shared::cli::HELP_MESSAGE;
use crate::rpc::Client;
use crate::utils::net::{DownloadFileOption, download_file_with_cache};
use crate::utils::proofs_api::ensure_proof_params_downloaded;
use crate::utils::version::FOREST_VERSION_STRING;
use anyhow::Context as _;
use clap::Parser;
use directories::ProjectDirs;
use std::borrow::Cow;
use std::path::PathBuf;
use std::time::Duration;
use tokio::task::JoinSet;
use url::Url;

/// Command-line options for the `forest-dev` binary
#[derive(Parser)]
#[command(name = env!("CARGO_PKG_NAME"), bin_name = "forest-dev", author = env!("CARGO_PKG_AUTHORS"), version = FOREST_VERSION_STRING.as_str(), about = env!("CARGO_PKG_DESCRIPTION")
)]
#[command(help_template(HELP_MESSAGE))]
pub struct Cli {
#[command(subcommand)]
pub cmd: Subcommand,
}

/// forest-dev sub-commands
#[derive(clap::Subcommand)]
pub enum Subcommand {
/// Fetch RPC test snapshots to the local cache
FetchRpcTests,
}

impl Subcommand {
pub async fn run(self, _client: Client) -> anyhow::Result<()> {
match self {
Self::FetchRpcTests => fetch_rpc_tests().await,
}
}
}

async fn fetch_rpc_tests() -> anyhow::Result<()> {
crate::utils::proofs_api::maybe_set_proofs_parameter_cache_dir_env(
&crate::Config::default().client.data_dir,
);
ensure_proof_params_downloaded().await?;
let tests = include_str!("../../tool/subcommands/api_cmd/test_snapshots.txt")
.lines()
.map(|i| {
// Remove comment
i.split("#").next().unwrap().trim().to_string()
})
.filter(|l| !l.is_empty() && !l.starts_with('#'));
let mut joinset = JoinSet::new();
for test in tests {
joinset.spawn(fetch_rpc_test_snapshot(test.into()));
}
joinset.join_all().await;
Ok(())
}
Comment on lines +43 to +61
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 | 🟠 Major

Don’t ignore JoinSet results—fail fast and surface which snapshot(s) failed.
Right now joinset.join_all().await; discards all task outputs/errors, making CI behavior flaky and hard to debug when a fetch fails.

     let mut joinset = JoinSet::new();
     for test in tests {
         joinset.spawn(fetch_rpc_test_snapshot(test.into()));
     }
-    joinset.join_all().await;
+    while let Some(res) = joinset.join_next().await {
+        // JoinError (panic/cancel) OR fetch error should fail the command.
+        res??;
+    }
     Ok(())
 }
🤖 Prompt for AI Agents
In src/dev/subcommands/mod.rs around lines 43 to 61, the JoinSet results are
currently ignored which hides task failures; change the code to await each
spawned task result and propagate errors with context so CI fails fast and
reports which snapshot failed. Specifically, when spawning, associate the
snapshot name (clone or move the `test` string into the task) and after spawning
iterate over the joinset results (e.g., loop with joinset.join_next().await or
process the JoinSet returned futures) checking each task's JoinError and inner
Result; if a task panics or returns Err, return an anyhow::Error that includes
the snapshot identifier and the underlying error so the function fails
immediately and surfaces which snapshot fetch failed.


pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> {
let url: Url =
format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}")
.parse()
.with_context(|| format!("Failed to parse URL for test: {name}"))?;
let project_dir =
ProjectDirs::from("com", "ChainSafe", "Forest").context("failed to get project dir")?;
let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots");
let path = crate::utils::retry(
crate::utils::RetryArgs {
timeout: Some(Duration::from_secs(if crate::utils::is_ci() {
30
} else {
300
})),
max_retries: Some(5),
..Default::default()
},
|| async {
download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await
},
)
.await?
.path;
Ok(path)
}
Comment on lines +63 to +88
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 | 🟠 Major

Validate name to prevent cache path traversal via crafted URL paths.
Because fetch_rpc_test_snapshot is pub and download_file_with_cache() joins cache_dir with url.path(), a name containing ../ or path separators can escape the intended cache directory.

 pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> {
+    // Prevent path traversal / unexpected subpaths.
+    if name.contains("..") || name.contains('/') || name.contains('\\') {
+        anyhow::bail!("invalid snapshot name: {name}");
+    }
     let url: Url =
         format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}")
             .parse()
             .with_context(|| format!("Failed to parse URL for test: {name}"))?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> {
let url: Url =
format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}")
.parse()
.with_context(|| format!("Failed to parse URL for test: {name}"))?;
let project_dir =
ProjectDirs::from("com", "ChainSafe", "Forest").context("failed to get project dir")?;
let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots");
let path = crate::utils::retry(
crate::utils::RetryArgs {
timeout: Some(Duration::from_secs(if crate::utils::is_ci() {
30
} else {
300
})),
max_retries: Some(5),
..Default::default()
},
|| async {
download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await
},
)
.await?
.path;
Ok(path)
}
pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> {
// Prevent path traversal / unexpected subpaths.
if name.contains("..") || name.contains('/') || name.contains('\\') {
anyhow::bail!("invalid snapshot name: {name}");
}
let url: Url =
format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}")
.parse()
.with_context(|| format!("Failed to parse URL for test: {name}"))?;
let project_dir =
ProjectDirs::from("com", "ChainSafe", "Forest").context("failed to get project dir")?;
let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots");
let path = crate::utils::retry(
crate::utils::RetryArgs {
timeout: Some(Duration::from_secs(if crate::utils::is_ci() {
30
} else {
300
})),
max_retries: Some(5),
..Default::default()
},
|| async {
download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await
},
)
.await?
.path;
Ok(path)
}
🤖 Prompt for AI Agents
In src/dev/subcommands/mod.rs around lines 63 to 88, validate and sanitize the
incoming name to prevent cache path traversal: before constructing the URL,
reject or error on any name containing path separators or traversal patterns
(e.g. '/', '\\', "..") or percent-encoded equivalents ("%2F", "%5C", "%2E%2E"),
or constrain name to a safe whitelist (e.g. allow only alphanumeric, dots,
dashes and underscores); alternatively percent-encode the name so it cannot
produce extra path segments, and ensure any value joined into the cache path is
treated as a single filename (not a path) so download_file_with_cache cannot
escape cache_dir.

2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ mod cli;
mod cli_shared;
mod daemon;
mod db;
mod dev;
mod documentation;
mod eth;
mod f3;
Expand Down Expand Up @@ -124,6 +125,7 @@ pub use auth::{JWT_IDENTIFIER, verify_token};
pub use cli::main::main as forest_main;
pub use cli_shared::cli::{Client, Config};
pub use daemon::main::main as forestd_main;
pub use dev::main::main as forest_dev_main;
pub use key_management::{
ENCRYPTED_KEYSTORE_NAME, FOREST_KEYSTORE_PHRASE_ENV, KEYSTORE_NAME, KeyStore, KeyStoreConfig,
};
Expand Down
40 changes: 17 additions & 23 deletions src/tool/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use super::subcommands::Subcommand;
use crate::cli_shared::logger::setup_minimal_logger;
use clap::Parser as _;

pub fn main<ArgT>(args: impl IntoIterator<Item = ArgT>) -> anyhow::Result<()>
pub async fn main<ArgT>(args: impl IntoIterator<Item = ArgT>) -> anyhow::Result<()>
where
ArgT: Into<OsString> + Clone,
{
Expand All @@ -18,26 +18,20 @@ where

let client = crate::rpc::Client::default_or_from_env(None)?;

tokio::runtime::Builder::new_multi_thread()
.enable_all()
.build()?
.block_on(async {
// Run command
match cmd {
Subcommand::Backup(cmd) => cmd.run(),
Subcommand::Benchmark(cmd) => cmd.run().await,
Subcommand::StateMigration(cmd) => cmd.run().await,
Subcommand::Snapshot(cmd) => cmd.run().await,
Subcommand::Fetch(cmd) => cmd.run().await,
Subcommand::Archive(cmd) => cmd.run().await,
Subcommand::DB(cmd) => cmd.run().await,
Subcommand::Index(cmd) => cmd.run().await,
Subcommand::Car(cmd) => cmd.run().await,
Subcommand::Api(cmd) => cmd.run().await,
Subcommand::Net(cmd) => cmd.run().await,
Subcommand::Shed(cmd) => cmd.run(client).await,
Subcommand::State(cmd) => cmd.run().await,
Subcommand::Completion(cmd) => cmd.run(&mut std::io::stdout()),
}
})
match cmd {
Subcommand::Backup(cmd) => cmd.run(),
Subcommand::Benchmark(cmd) => cmd.run().await,
Subcommand::StateMigration(cmd) => cmd.run().await,
Subcommand::Snapshot(cmd) => cmd.run().await,
Subcommand::Fetch(cmd) => cmd.run().await,
Subcommand::Archive(cmd) => cmd.run().await,
Subcommand::DB(cmd) => cmd.run().await,
Subcommand::Index(cmd) => cmd.run().await,
Subcommand::Car(cmd) => cmd.run().await,
Subcommand::Api(cmd) => cmd.run().await,
Subcommand::Net(cmd) => cmd.run().await,
Subcommand::Shed(cmd) => cmd.run(client).await,
Subcommand::State(cmd) => cmd.run().await,
Subcommand::Completion(cmd) => cmd.run(&mut std::io::stdout()),
}
}
38 changes: 6 additions & 32 deletions src/tool/subcommands/api_cmd/test_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,22 +185,18 @@ async fn ctx(
mod tests {
use super::*;
use crate::Config;
use crate::utils::net::{DownloadFileOption, download_file_with_cache};
use crate::utils::misc::env::is_env_truthy;
use crate::utils::proofs_api::ensure_proof_params_downloaded;
use ahash::HashSet;
use anyhow::Context as _;
use directories::ProjectDirs;
use std::sync::LazyLock;
use std::time::{Duration, Instant};
use std::time::Instant;
use tokio::sync::Mutex;
use url::Url;

// To run a single test: cargo test --lib filecoin_multisig_statedecodeparams_1754230255631789 -- --nocapture
include!(concat!(env!("OUT_DIR"), "/__rpc_regression_tests_gen.rs"));

async fn rpc_regression_test_run(name: &str) {
// Skip for debug build on CI as the downloading is slow and flaky
if crate::utils::is_ci() && crate::utils::is_debug_build() {
if is_env_truthy("FOREST_RPC_SNAPSHOT_TEST_OPT_OUT") {
return;
}

Expand All @@ -213,31 +209,9 @@ mod tests {
);
ensure_proof_params_downloaded().await.unwrap();
}
let url: Url =
format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}")
.parse()
.with_context(|| format!("Failed to parse URL for test: {name}"))
.unwrap();
let project_dir = ProjectDirs::from("com", "ChainSafe", "Forest").unwrap();
let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots");
let path = crate::utils::retry(
crate::utils::RetryArgs {
timeout: Some(Duration::from_secs(if crate::utils::is_ci() {
20
} else {
120
})),
max_retries: Some(5),
..Default::default()
},
|| async {
download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await
},
)
.await
.unwrap()
.path;

let path = crate::dev::subcommands::fetch_rpc_test_snapshot(name.into())
.await
.unwrap();
// We need to set RNG seed so that tests are run with deterministic
// output. The snapshots should be generated with a node running with the same seed, if
// they are testing methods that are not deterministic, e.g.,
Expand Down
48 changes: 21 additions & 27 deletions src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,9 @@ pub mod stream;
pub mod version;

use anyhow::{Context as _, bail};
use futures::{
Future, FutureExt,
future::{FusedFuture, pending},
select,
};
use futures::Future;
use multiaddr::{Multiaddr, Protocol};
use std::{pin::Pin, str::FromStr, time::Duration};
use std::{str::FromStr, time::Duration};
use tokio::time::sleep;
use tracing::error;
use url::Url;
Expand Down Expand Up @@ -125,29 +121,26 @@ where
F: Future<Output = Result<T, E>>,
E: std::fmt::Debug,
{
let mut timeout: Pin<Box<dyn FusedFuture<Output = ()>>> = match args.timeout {
Some(duration) => Box::pin(sleep(duration).fuse()),
None => Box::pin(pending()),
};
let max_retries = args.max_retries.unwrap_or(usize::MAX);
let mut task = Box::pin(
async {
for _ in 0..max_retries {
match make_fut().await {
Ok(ok) => return Ok(ok),
Err(err) => error!("retrying operation after {err:?}"),
}
if let Some(delay) = args.delay {
sleep(delay).await;
}
let task = async {
for _ in 0..max_retries {
match make_fut().await {
Ok(ok) => return Ok(ok),
Err(err) => error!("retrying operation after {err:?}"),
}
if let Some(delay) = args.delay {
sleep(delay).await;
}
Err(RetryError::RetriesExceeded)
}
.fuse(),
);
select! {
_ = timeout => Err(RetryError::TimeoutExceeded),
res = task => res,
Err(RetryError::RetriesExceeded)
};

if let Some(timeout) = args.timeout {
tokio::time::timeout(timeout, task)
.await
.map_err(|_| RetryError::TimeoutExceeded)?
} else {
task.await
}
}

Expand All @@ -170,11 +163,11 @@ pub enum RetryError {
}

#[cfg(test)]
#[allow(dead_code)]
pub fn is_debug_build() -> bool {
cfg!(debug_assertions)
}

#[cfg(test)]
pub fn is_ci() -> bool {
// https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#default-environment-variables
misc::env::is_env_truthy("CI")
Expand All @@ -185,6 +178,7 @@ mod tests {
mod files;

use RetryError::{RetriesExceeded, TimeoutExceeded};
use futures::future::pending;
use std::{future::ready, sync::atomic::AtomicUsize};

use super::*;
Expand Down
Loading
Loading