Skip to content

Make --network take precedence over rpc-url/passphrase env vars #2579

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.

Sign up for GitHub

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

Open
fnando wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Make --network take precedence over rpc-url/passphrase env vars #2579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be773aa
Make --network take precedence over rpc-url env vars.
fnando May 14, 2026
e66925e
Warn when --network overrides rpc-url/passphrase and document precede…
fnando May 14, 2026
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
12 changes: 12 additions & 0 deletions FULL_HELP_DOCS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ Anything after the `--` double dash (the "slop") is parsed as arguments to the c

stellar contract invoke --id CCR6QKTWZQYW6YUJ7UP7XXZRLWQPFRV6SWBLQS4ZQOSAF4BOUD77OTE2 --source alice --network testnet -- hello --to world

CONFIGURATION

Most commands read their network and source account from three sources, applied in this order of precedence (highest first):

1. CLI flags e.g. --network, --rpc-url, --source
2. Environment vars shell env, including values loaded from a `.env` file in the current directory (e.g. STELLAR_NETWORK, STELLAR_RPC_URL, STELLAR_ACCOUNT)
3. `use` defaults set via `stellar network use NAME` and `stellar keys use NAME`, loaded as env vars at startup

Run `stellar env` to see every active setting and where it came from.

Network resolution has one extra rule: if a network name is set from any source, the named configuration is loaded from disk and any rpc-url/network-passphrase from any source is ignored. A warning is printed when this happens — pass `--quiet` to suppress it.

**Usage:** `stellar [OPTIONS] <COMMAND>`

###### **Subcommands:**
Expand Down
59 changes: 59 additions & 0 deletions cmd/crates/soroban-test/tests/it/integration/network.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,62 @@ async fn network_info_includes_id_in_json_output() {
"baefd734b8d3e48472cff83912375fedbc7573701912fe308af730180f97d74a"
);
}

// TestEnv pre-sets STELLAR_RPC_URL and STELLAR_NETWORK_PASSPHRASE on every
// command, so any invocation that adds `--network local` already exercises the
// cross-source precedence path. The local network is bundled (no `network add`
// required) and points at the running sandbox, so the warning fires before any
// RPC call needs to succeed.
#[tokio::test]
async fn network_flag_warns_when_env_rpc_url_present() {
let sandbox = &TestEnv::new();

sandbox
.new_assert_cmd("network")
.args(["info", "--network", "local"])
.assert()
.stderr(predicate::str::contains(
"--network=local takes precedence; ignoring --rpc-url / STELLAR_RPC_URL and --network-passphrase / STELLAR_NETWORK_PASSPHRASE",
));
}

#[tokio::test]
async fn network_flag_warning_lists_only_set_overrides() {
let sandbox = &TestEnv::new();

sandbox
.new_assert_cmd("network")
.args(["info", "--network", "local"])
.env_remove("STELLAR_NETWORK_PASSPHRASE")
.assert()
.stderr(
predicate::str::contains("ignoring --rpc-url / STELLAR_RPC_URL").and(
predicate::str::contains("--network-passphrase / STELLAR_NETWORK_PASSPHRASE").not(),
),
);
}

#[tokio::test]
async fn network_flag_warning_fires_for_env_only_network() {
let sandbox = &TestEnv::new();

sandbox
.new_assert_cmd("network")
.arg("info")
.env("STELLAR_NETWORK", "local")
.assert()
.stderr(predicate::str::contains(
"--network=local takes precedence; ignoring",
));
}

#[tokio::test]
async fn network_flag_warning_suppressed_by_quiet() {
let sandbox = &TestEnv::new();

sandbox
.new_assert_cmd("network")
.args(["--quiet", "info", "--network", "local"])
.assert()
.stderr(predicate::str::contains("takes precedence").not());
}
4 changes: 3 additions & 1 deletion cmd/soroban-cli/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::commands::contract::invoke::Error::ArgParsing;
use crate::commands::contract::Error::{Deploy, Invoke};
use crate::commands::Error::Contract;
use crate::config::{locator::cli_config_file, Config};
use crate::print::Print;
use crate::print::{self, Print};
use crate::upgrade_check::upgrade_check;
use crate::{commands, env_vars, Root};
use std::error::Error;
Expand Down Expand Up @@ -44,6 +44,8 @@ pub async fn main() {
}
});

print::set_quiet(root.global_args.quiet);

// Now use root to setup the logger
if let Some(level) = root.global_args.log_level() {
let mut e_filter = EnvFilter::from_default_env()
Expand Down
12 changes: 12 additions & 0 deletions cmd/soroban-cli/src/commands/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,18 @@ Use contracts like a CLI:
Anything after the `--` double dash (the \"slop\") is parsed as arguments to the contract-specific CLI, generated on-the-fly from the contract schema. For the hello world example, with a function called `hello` that takes one string argument `to`, here's how you invoke it:

stellar contract invoke --id CCR6QKTWZQYW6YUJ7UP7XXZRLWQPFRV6SWBLQS4ZQOSAF4BOUD77OTE2 --source alice --network testnet -- hello --to world

CONFIGURATION

Most commands read their network and source account from three sources, applied in this order of precedence (highest first):

1. CLI flags e.g. --network, --rpc-url, --source
2. Environment vars shell env, including values loaded from a `.env` file in the current directory (e.g. STELLAR_NETWORK, STELLAR_RPC_URL, STELLAR_ACCOUNT)
3. `use` defaults set via `stellar network use NAME` and `stellar keys use NAME`, loaded as env vars at startup

Run `stellar env` to see every active setting and where it came from.

Network resolution has one extra rule: if a network name is set from any source, the named configuration is loaded from disk and any rpc-url/network-passphrase from any source is ignored. A warning is printed when this happens — pass `--quiet` to suppress it.
";

#[derive(Parser, Debug)]
Expand Down
60 changes: 54 additions & 6 deletions cmd/soroban-cli/src/config/network.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::collections::HashMap;
use std::str::FromStr;
use std::sync::Once;
use stellar_strkey::ed25519::PublicKey;
use url::Url;

use super::locator;
use crate::print::{self, Print};
use crate::utils::http;
use crate::{
commands::HEADING_RPC,
Expand Down Expand Up @@ -38,8 +40,6 @@ STELLAR_NETWORK, STELLAR_RPC_URL and STELLAR_NETWORK_PASSPHRASE"#
"network passphrase is used but rpc-url is missing, use `--rpc-url` or `STELLAR_RPC_URL`"
)]
MissingRpcUrl,
#[error("cannot use both `--rpc-url` and `--network`")]
CannotUseBothRpcAndNetwork,
#[error(transparent)]
Rpc(#[from] rpc::Error),
#[error(transparent)]
Expand Down Expand Up @@ -96,21 +96,49 @@ pub struct Args {
pub network: Option<String>,
}

// Emits the network-precedence warning at most once per process so commands
// that call `Args::get` multiple times (e.g. sign + sign_fee_bump) don't spam
// the user. Honors the global `--quiet` flag recorded in `cli::main`.
fn warn_if_overridden(network: &str, rpc_url: Option<&str>, network_passphrase: Option<&str>) {
static WARN_ONCE: Once = Once::new();
let ignored = match (rpc_url.is_some(), network_passphrase.is_some()) {
(true, true) => {
"--rpc-url / STELLAR_RPC_URL and --network-passphrase / STELLAR_NETWORK_PASSPHRASE"
}
(true, false) => "--rpc-url / STELLAR_RPC_URL",
(false, true) => "--network-passphrase / STELLAR_NETWORK_PASSPHRASE",
(false, false) => return,
};
WARN_ONCE.call_once(|| {
Print::new(print::is_quiet()).warnln(format!(
"--network={network} takes precedence; ignoring {ignored}"
));
});
}
Comment on lines +99 to +117
Copy link
Copy Markdown

@claude claude Bot May 14, 2026

Choose a reason for hiding this comment

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

A few thoughts on this helper:

1. The --network={network} wording is misleading when the source is env / use.
The third integration test (network_flag_warning_fires_for_env_only_network) intentionally exercises a setup where the user typed no --network flag — only STELLAR_NETWORK=local. The user then sees --network=local takes precedence and may legitimately wonder which --network flag the warning refers to. Consider phrasing the warning in a source-agnostic way, e.g. network "local" takes precedence (or surfacing the actual source: network "local" (from STELLAR_NETWORK) takes precedence). This is also where STELLAR_NETWORK_SOURCE, already set by set_env_value_from_config, could be consulted to disambiguate env vs use.

2. --rpc-url / STELLAR_RPC_URL reads as one combined thing.
Slash-joined names can scan as "the rpc-url/stellar-rpc-url option". Minor, but or is unambiguous: ignoring --rpc-url or STELLAR_RPC_URL.

3. The Once is process-global.
That's fine for the typical CLI invocation (one process, one warning). It's worth knowing that in a long-lived test/library context, a second Args::get with overrides would be silenced even if the first call had none — Once doesn't reset. Not a bug given current usage, just a caveat for future use.

4. The Print::new(...) allocation only matters when not quiet.
You can short-circuit before formatting the message: if print::is_quiet() { return; } at the top of the call_once. Saves a format! allocation when --quiet is set. Trivial.


impl Args {
pub fn get(&self, locator: &locator::Args) -> Result<Network, Error> {
// A named network always resolves via the on-disk config, ignoring any
// rpc-url / passphrase that may have been picked up from env vars. This
// is what lets `--network foo` win over a stray STELLAR_RPC_URL in
// `.env`. Conflicts typed entirely on the CLI fall through here too;
// the explicit name takes precedence.
match (
self.network.as_deref(),
self.rpc_url.clone(),
self.network_passphrase.clone(),
) {
(Some(network), rpc_url, network_passphrase) => {
warn_if_overridden(network, rpc_url.as_deref(), network_passphrase.as_deref());
Ok(locator.read_network(network)?)
}
Comment on lines +131 to +134
Copy link
Copy Markdown

@claude claude Bot May 14, 2026

Choose a reason for hiding this comment

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

Precedence violation when --rpc-url is an explicit CLI flag and --network comes from a use default.

set_env_from_config in cli.rs populates STELLAR_NETWORK from stellar network use NAME. Clap then assigns it to self.network via env = "STELLAR_NETWORK". By the time we reach this arm, self.network = Some("…") is indistinguishable from a CLI-typed --network flag.

So a user who runs:

stellar network use testnet           # sets the on-disk default
stellar tx ... --rpc-url=http://custom.example.com --network-passphrase=foo

will have their explicit --rpc-url CLI flag silently overridden by the use default — a (use, CLI, CLI) source-tuple. That contradicts the precedence ladder you just added to the help text (CLI > env > use). The new "extra rule" documents the carve-out, but the more typical user expectation is that a flag they typed wins over a config default they may have forgotten about.

If you want to honor the documented precedence rigorously, clap exposes ArgMatches::value_source() which lets you check whether each value came from CommandLine, EnvVariable, or DefaultValue/programmatic. The rule could then be: ignore rpc-url/passphrase only when they have a strictly lower source than the network name. That's more code but it matches the documented precedence and avoids surprising the network use user.

At minimum, this trade-off (network always wins, regardless of source) deserves to be called out explicitly in CHANGELOG / release notes since it's a behavior change.

(None, None, None) => {
// Fall back to testnet as the default network if no config default is set
Ok(DEFAULTS.get(DEFAULT_NETWORK_KEY).unwrap().into())
}
(_, Some(_), None) => Err(Error::MissingNetworkPassphrase),
(_, None, Some(_)) => Err(Error::MissingRpcUrl),
(Some(network), None, None) => Ok(locator.read_network(network)?),
(_, Some(rpc_url), Some(network_passphrase)) => {
(None, Some(_), None) => Err(Error::MissingNetworkPassphrase),
(None, None, Some(_)) => Err(Error::MissingRpcUrl),
(None, Some(rpc_url), Some(network_passphrase)) => {
let rpc_headers = self
.rpc_headers
.iter()
Expand Down Expand Up @@ -725,4 +753,24 @@ mod tests {
let bad = "not a url";
assert_eq!(redact_rpc_url(bad), bad);
}

#[tokio::test]
Comment on lines +756 to +757
async fn network_name_wins_over_rpc_tuple() {
use crate::test_utils::with_env_set;

let tmp = tempfile::tempdir().unwrap();
with_env_set("STELLAR_CONFIG_HOME", tmp.path(), || {
let args = Args {
network: Some("testnet".to_string()),
rpc_url: Some("http://other.example.com:59999".to_string()),
network_passphrase: Some("Some Other Passphrase".to_string()),
rpc_headers: Vec::new(),
};

let result = args.get(&locator::Args::default()).unwrap();

assert_eq!(result.rpc_url, "https://soroban-testnet.stellar.org");
assert_eq!(result.network_passphrase, passphrase::TESTNET);
});
Comment on lines +756 to +774
Copy link
Copy Markdown

@claude claude Bot May 14, 2026

Choose a reason for hiding this comment

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

This test is a good smoke test of the precedence flip, but note that "testnet" is hard-coded in DEFAULTS (see locator.rs:390), so read_network returns the built-in default without ever touching the tmpdir you set up via STELLAR_CONFIG_HOME. In effect, this only proves that a built-in default beats the rpc tuple — not that a user-saved (stellar network add) on-disk network does.

To exercise the actual on-disk path (the case from the PR description: ".env file sets STELLAR_RPC_URL + STELLAR_NETWORK_PASSPHRASE, user has saved a custom network"), you'd need to either:

  • write a network JSON under tmp/network/foo.json and use network: Some("foo"), or
  • assert on a named network that overrides the built-in (e.g. testnet with a custom on-disk rpc).

Also worth a unit-level assertion that warn_if_overridden did/didn't fire, even if just by capturing stderr — currently that's only covered by the integration tests, which means the warning is silently untested in the fast unit-test loop.

}
}
16 changes: 16 additions & 0 deletions cmd/soroban-cli/src/print.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::io::{self, Write};
use std::sync::OnceLock;
use std::{env, fmt::Display};

use crate::xdr::{Error as XdrError, Transaction};
Expand All @@ -7,6 +8,21 @@ use crate::{
config::network::Network, utils::explorer_url_for_transaction, utils::transaction_hash,
};

static QUIET: OnceLock<bool> = OnceLock::new();

/// Record whether `--quiet` was passed on the command line. Called once from
/// `cli::main` after parsing so resolvers running deep in the call stack can
/// honor the global flag without it being threaded through every signature.
pub fn set_quiet(quiet: bool) {
let _ = QUIET.set(quiet);
}

/// Read the recorded `--quiet` flag, defaulting to `false` if [`set_quiet`]
/// hasn't run yet (e.g. in unit tests that don't go through `cli::main`).
pub fn is_quiet() -> bool {
*QUIET.get().unwrap_or(&false)
}
Comment on lines +11 to +24
Copy link
Copy Markdown

@claude claude Bot May 14, 2026

Choose a reason for hiding this comment

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

A process-global OnceLock<bool> is a pragmatic fix for not threading quiet through 25 call sites, but it's worth flagging a couple of properties:

  • Cross-test pollution risk. If any future test calls set_quiet(true) directly (or via a code path that does), every subsequent test in the same cargo test process sees is_quiet() == true. The current PR is safe because all integration tests use subprocesses (new_assert_cmd), but a future in-process unit test that touches this could silently mute warnings in unrelated tests. A short doc-comment warning against calling set_quiet from tests would be helpful.

  • let _ = QUIET.set(quiet); silently swallows the second-set error. That's fine because cli::main only calls it once, but it would be more robust to either debug_assert!(QUIET.set(quiet).is_ok()) or rename it to init_quiet to signal "call exactly once".

  • Mild alternative if you wanted to avoid global state entirely: thread an Arc<Print> (or just the bool) through Args::get via the locator: &locator::Args parameter — the locator already carries config-dir state through the call graph. Not necessarily worth the refactor, but it would localize the side effect.


#[derive(Clone)]
pub struct Print {
pub quiet: bool,
Expand Down
Loading