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
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions clash-bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ ring = ["clash-lib/ring"]
[dependencies]
clap = { version = "4", features = ["derive"] }
anyhow = "1"
base64 = "0.22"
time = { version = "0.3", features = ["formatting", "local-offset", "macros"] }

clash-lib = { path = "../clash-lib", default-features = false }

Expand All @@ -49,3 +51,6 @@ human-panic = "2.0"


aws-lc-rs = { version = "1.16", optional = true, default-features = false }

[dev-dependencies]
tempfile = "3"
275 changes: 236 additions & 39 deletions clash-bin/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ static GLOBAL: Jemalloc = Jemalloc;

extern crate clash_lib as clash;

use anyhow::{Context, anyhow};
use base64::{Engine, engine::general_purpose::STANDARD};
use clap::Parser;
use clash::TokioRuntime;
use std::{
io::Write,
io::{Read, Write},
path::{Path, PathBuf},
process::exit,
};
use time::{OffsetDateTime, macros::format_description};

const DEFAULT_CONFIG_FILE: &str = "config.yaml";
const DEFAULT_CONFIG_CONTENT: &str = "mixed-port: 7890";

#[derive(Parser)]
#[clap(author, about, long_about = None)]
Expand All @@ -35,10 +41,11 @@ struct Cli {
visible_short_aliases = ['f'], // -f is used by clash, it is a compatibility option
value_parser,
value_name = "FILE",
default_value = "config.yaml",
help = "Specify configuration file"
)]
config: PathBuf,
config: Option<PathBuf>,
#[clap(long = "config-string", hide = true, value_name = "BASE64")]
config_string: Option<String>,
#[clap(
short = 't',
long,
Expand Down Expand Up @@ -93,13 +100,7 @@ fn main() -> anyhow::Result<()> {

// Those arguments are for compatibility with `mihomo`
// Technically, I do not think `mihomo` is a modern/standard POSIX Cli program
let args: Vec<String> = std::env::args()
.map(|arg| match arg.as_str() {
"-ext-ctl-unix" => "--ext-ctl-unix".to_string(),
"-ext-ctl-pipe" => "--ext-ctl-pipe".to_string(),
_ => arg,
})
.collect();
let args = preprocess_args(std::env::args());
let cli = Cli::parse_from(args);

if cli.version {
Expand All @@ -111,43 +112,28 @@ fn main() -> anyhow::Result<()> {
exit(0)
}

let file = cli
.directory
.as_ref()
.unwrap_or(&std::env::current_dir().unwrap())
.join(cli.config)
.to_string_lossy()
.to_string();

if !Path::new(&file).exists() {
let default_config = "port: 7890";
let mut config_file = match std::fs::File::create(&file) {
Ok(config_file) => config_file,
_ => {
eprintln!("default profile cannot be created: {file}");
exit(1);
}
};

if config_file.write_all(default_config.as_bytes()).is_err() {
eprintln!("default profile cannot be written: {file}");
let config_input = match ConfigInput::resolve(&cli) {
Ok(config_input) => config_input,
Err(err) => {
print_cli_log("fatal", &err.to_string());
exit(1);
};
}
};

println!(
"the configuration file cannot be found, the template has been created \
and used: {file}"
);
if let Err(err) = config_input.ensure_file() {
print_cli_log("fatal", &err.to_string());
exit(1);
}

if cli.test_config {
match clash::Config::File(file.clone()).try_parse() {
match config_input.try_parse() {
Ok(_) => {
println!("configuration file {file} test is successful");
print_test_success(&config_input);
exit(0);
}
Err(e) => {
eprintln!("configuration file {file} test failed: {e}");
print_cli_log("error", &e.to_string());
print_test_failure(&config_input);
exit(1);
}
}
Expand All @@ -174,7 +160,7 @@ fn main() -> anyhow::Result<()> {
)));
}

let mut config = clash::Config::File(file).try_parse()?;
let mut config = config_input.try_parse()?;

config.general.controller.external_controller_ipc = cli.controller_ipc;

Expand Down Expand Up @@ -221,3 +207,214 @@ fn main() -> anyhow::Result<()> {
.inspect_err(|err| eprintln!("Failed to start clash: {err}"))?;
Ok(())
}

enum ConfigInput {
File { path: PathBuf, display_path: String },
Bytes { content: String },
}

impl ConfigInput {
fn resolve(cli: &Cli) -> anyhow::Result<Self> {
if let Some(config) = cli.config_string.as_deref() {
return decode_config(config);
}
if let Ok(config) = std::env::var("CLASH_CONFIG_STRING") {
return decode_config(&config);
}

let current_dir =
std::env::current_dir().context("get current directory")?;
let home_dir = cli
.directory
.clone()
.or_else(|| std::env::var_os("CLASH_HOME_DIR").map(PathBuf::from));
let directory = match home_dir {
Some(directory) if directory.is_absolute() => directory,
Some(directory) => current_dir.join(directory),
None => current_dir,
};

if let Some(config) = cli.config.as_ref() {
return resolve_config_path(config, &directory);
}

if let Some(file) = std::env::var_os("CLASH_CONFIG_FILE") {
return resolve_config_file(&PathBuf::from(file));
}
Comment on lines +217 to +243
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

Preserve CLI-over-env precedence for config selection.

resolve() checks CLASH_CONFIG_STRING before cli.config, so an explicit -f/--config is ignored whenever that env var is set. That makes the file flag impossible to use in processes that export CLASH_CONFIG_STRING.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-bin/src/main.rs` around lines 217 - 243, The resolve() logic currently
checks CLASH_CONFIG_STRING before honoring the CLI file flag, so update the
precedence to prefer CLI flags: keep checking cli.config_string first
(decode_config), then if cli.config is Some call resolve_config_path, then only
afterwards check the environment CLASH_CONFIG_STRING (decode_config) and
CLASH_CONFIG_FILE (resolve_config_file); ensure you reference and preserve calls
to decode_config, resolve_config_path, and resolve_config_file and maintain the
directory resolution logic used for resolve_config_path.

Comment on lines +241 to +243
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

Relative env-based config paths are resolved from the wrong base.

resolve_config_file() has no directory context, so CLASH_CONFIG_FILE=foo.yaml and the empty-stdin fallback from -f - both resolve from the process cwd instead of --directory / CLASH_HOME_DIR. That breaks the same path semantics the cli.config branch uses.

Also applies to: 343-355

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-bin/src/main.rs` around lines 241 - 243, The CLASH_CONFIG_FILE branch
calls resolve_config_file(&PathBuf::from(file)) which resolves relative paths
against the process CWD instead of the chosen --directory / CLASH_HOME_DIR
context used by the cli.config branch; change the logic so relative env paths
are resolved against the same directory context: either (A) update
resolve_config_file to accept a base directory parameter and pass cli.directory
/ CLASH_HOME_DIR into it, or (B) before calling resolve_config_file, if
PathBuf::from(file) is relative, join it with the directory variable used
elsewhere (the cli.directory / home dir used in the cli.config branch) and pass
that absolute path; apply the same fix to the other env-based branches mentioned
(around lines 343-355) to ensure consistent resolution.


Ok(Self::from_file_path(directory.join(DEFAULT_CONFIG_FILE)))
}

fn from_file_path(path: PathBuf) -> Self {
let path =
absolutize(path).unwrap_or_else(|_| PathBuf::from(DEFAULT_CONFIG_FILE));
let display_path = path.to_string_lossy().to_string();
Self::File { path, display_path }
}

fn ensure_file(&self) -> anyhow::Result<()> {
let Self::File { path, .. } = self else {
return Ok(());
};
if path.exists() {
return Ok(());
}
if let Some(parent) = path.parent()
&& !parent.as_os_str().is_empty()
{
std::fs::create_dir_all(parent).with_context(|| {
format!(
"default profile directory cannot be created: {}",
parent.display()
)
})?;
}
let mut config_file = std::fs::File::create(path).with_context(|| {
format!("default profile cannot be created: {}", path.display())
})?;
config_file
.write_all(DEFAULT_CONFIG_CONTENT.as_bytes())
.with_context(|| {
format!("default profile cannot be written: {}", path.display())
})?;
Comment on lines +255 to +279
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 | 🔴 Critical

Create the default config atomically.

The exists() check at Line 259 and File::create() at Line 272 form a TOCTOU window. If another process creates the file in between, this path will truncate it back to the template content.

🔒 Proposed fix
     fn ensure_file(&self) -> anyhow::Result<()> {
         let Self::File { path, .. } = self else {
             return Ok(());
         };
-        if path.exists() {
-            return Ok(());
-        }
         if let Some(parent) = path.parent()
             && !parent.as_os_str().is_empty()
         {
             std::fs::create_dir_all(parent).with_context(|| {
                 format!(
@@
-        let mut config_file = std::fs::File::create(path).with_context(|| {
-            format!("default profile cannot be created: {}", path.display())
-        })?;
+        let mut config_file = match std::fs::OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(path)
+        {
+            Ok(file) => file,
+            Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => {
+                return Ok(());
+            }
+            Err(err) => {
+                return Err(err).with_context(|| {
+                    format!("default profile cannot be created: {}", path.display())
+                });
+            }
+        };
         config_file
             .write_all(DEFAULT_CONFIG_CONTENT.as_bytes())
             .with_context(|| {
                 format!("default profile cannot be written: {}", path.display())
             })?;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@clash-bin/src/main.rs` around lines 255 - 279, The current ensure_file
implementation has a TOCTOU race: it checks path.exists() then calls
File::create(path) which can truncate a file created by another process; replace
the non-atomic logic with an atomic create_new attempt (e.g. use
std::fs::OpenOptions::new().write(true).create_new(true).open(path)) to create
the file only if it does not already exist and treat ErrorKind::AlreadyExists as
success, or alternatively write the default content to a temporary file in the
same directory and atomically rename it into place; update the code paths around
ensure_file, the block that creates the parent dir, the OpenOptions/File::create
usage and the subsequent write_all(DEFAULT_CONFIG_CONTENT.as_bytes()) to use the
chosen atomic method so we never truncate an existing file.

Ok(())
}

fn try_parse(&self) -> clash::Result<clash::ClashRuntimeConfig> {
match self {
Self::File { path, display_path } => {
if std::fs::metadata(path).map(|metadata| metadata.len() == 0)? {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
format!("configuration file {display_path} is empty"),
)
.into());
}
clash::Config::File(path.to_string_lossy().to_string()).try_parse()
}
Self::Bytes { content } => {
clash::Config::Str(content.clone()).try_parse()
}
}
}
}

fn preprocess_args(args: impl IntoIterator<Item = String>) -> Vec<String> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you add a test for this so that

$ clash-rs -d DIR -f test.yml
$ clash-rs -d DIR # no -f so just find config.yaml under DIR
$ clash-rs -f test.yaml # find test.yaml in current dir
$ clash-rs # find config.yaml under current dir

all work?

args.into_iter()
.map(|arg| match arg.as_str() {
"-ext-ctl-unix" => "--ext-ctl-unix".to_string(),
"-ext-ctl-pipe" => "--ext-ctl-pipe".to_string(),
"-config" => "--config-string".to_string(),
"-f" => "--config".to_string(),
_ if arg.starts_with("-config=") => {
arg.replacen("-config=", "--config-string=", 1)
}
_ if arg.starts_with("-f=") => arg.replacen("-f=", "--config=", 1),
_ if arg.starts_with("-f") && arg.len() > 2 => {
format!("--config={}", &arg[2..])
}
_ => arg,
})
.collect()
}

fn decode_config(config: &str) -> anyhow::Result<ConfigInput> {
let content = STANDARD
.decode(config)
.map_err(|err| anyhow!("decode config: {err}"))?;
let content = String::from_utf8_lossy(&content).into_owned();
Ok(ConfigInput::Bytes { content })
}

fn resolve_config_path(
config: &Path,
directory: &Path,
) -> anyhow::Result<ConfigInput> {
if config == Path::new("-") {
return resolve_config_file(config);
}
if config.is_absolute() {
Ok(ConfigInput::from_file_path(config.to_path_buf()))
} else {
Ok(ConfigInput::from_file_path(directory.join(config)))
}
}

fn resolve_config_file(file: &Path) -> anyhow::Result<ConfigInput> {
if file == Path::new("-") {
let mut content = String::new();
std::io::stdin()
.read_to_string(&mut content)
.context("read configuration from stdin")?;
if !content.is_empty() {
return Ok(ConfigInput::Bytes { content });
}
return Ok(ConfigInput::from_file_path(PathBuf::from(
DEFAULT_CONFIG_FILE,
)));
}
Ok(ConfigInput::from_file_path(file.to_path_buf()))
}

fn absolutize(path: PathBuf) -> anyhow::Result<PathBuf> {
if path.is_absolute() {
Ok(path)
} else {
Ok(std::env::current_dir()
.context("get current directory")?
.join(path))
}
}

fn print_test_success(input: &ConfigInput) {
match input {
ConfigInput::File { display_path, .. } => {
println!("configuration file {display_path} test is successful");
}
ConfigInput::Bytes { .. } => {
println!("configuration file {DEFAULT_CONFIG_FILE} test is successful");
}
}
}

fn print_test_failure(input: &ConfigInput) {
match input {
ConfigInput::File { display_path, .. } => {
println!("configuration file {display_path} test failed");
}
ConfigInput::Bytes { .. } => {
println!("configuration test failed");
}
}
}

fn print_cli_log(level: &str, msg: &str) {
let format = format_description!(
"[year]-[month]-[day]T[hour]:[minute]:[second].[subsecond \
digits:9][offset_hour sign:mandatory]:[offset_minute]"
);
let timestamp = OffsetDateTime::now_local()
.unwrap_or_else(|_| OffsetDateTime::now_utc())
.format(format)
.unwrap_or_else(|_| "1970-01-01T00:00:00.000000000+00:00".to_string());
println!(
"time=\"{}\" level={} msg=\"{}\"",
timestamp,
level,
escape_logrus_text(msg)
);
}

fn escape_logrus_text(msg: &str) -> String {
msg.chars().fold(String::new(), |mut escaped, ch| {
match ch {
'\\' => escaped.push_str("\\\\"),
'"' => escaped.push_str("\\\""),
'\n' => escaped.push_str("\\n"),
'\r' => escaped.push_str("\\r"),
'\t' => escaped.push_str("\\t"),
_ => escaped.push(ch),
}
escaped
})
}
Loading
Loading