Skip to content

Commit 5db08c6

Browse files
vsilentCopilot
andcommitted
fix: address PR #85 review comments
- Extract ensure_config_file() helper to eliminate duplicate code - Use metadata().is_file() with permission error handling instead of Path::exists() - Fix TOCTOU: use OpenOptions::create_new(true) for atomic creates - Remove baked-in admin/admin credentials from .env template - Create .env with 0600 permissions on Unix (contains secrets) - Add missing env vars: COMPOSE_AGENT_ENABLED, VAULT_ADDRESS, VAULT_TOKEN, TRYDIRECT_IP, UPDATE_BINARY_URL, UPDATE_EXPECTED_SHA256 - Honor --config path in init (derive output dir from it) - 2 new tests: env var completeness, Unix permission check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 603d128 commit 5db08c6

2 files changed

Lines changed: 173 additions & 29 deletions

File tree

src/agent/init.rs

Lines changed: 102 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,30 @@ COMMAND_TIMEOUT_SECS=300
3737
METRICS_INTERVAL_SECS=15
3838
# METRICS_WEBHOOK=https://example.com/metrics
3939
40-
# ── UI / API credentials (defaults to admin/admin) ───────────────────
41-
STATUS_PANEL_USERNAME=admin
42-
STATUS_PANEL_PASSWORD=admin
40+
# ── UI / API credentials (must be set explicitly before use) ─────────
41+
STATUS_PANEL_USERNAME=
42+
STATUS_PANEL_PASSWORD=
4343
4444
# ── Docker ───────────────────────────────────────────────────────────
4545
# DOCKER_SOCK=unix:///var/run/docker.sock
4646
# NGINX_CONTAINER=nginx
4747
48+
# ── Compose Agent ────────────────────────────────────────────────────
49+
# COMPOSE_AGENT_ENABLED=false
50+
4851
# ── Backup / Security ───────────────────────────────────────────────
4952
# DEPLOYMENT_HASH=
5053
# BACKUP_PATH=/data/encrypted/backup.tar.gz.cpt
54+
# TRYDIRECT_IP=
55+
56+
# ── Vault (optional) ────────────────────────────────────────────────
57+
# VAULT_ADDRESS=http://127.0.0.1:8200
58+
# VAULT_TOKEN=
5159
5260
# ── Self-update (optional) ──────────────────────────────────────────
5361
# UPDATE_SERVER_URL=
62+
# UPDATE_BINARY_URL=
63+
# UPDATE_EXPECTED_SHA256=
5464
# UPDATE_STORAGE_PATH=/var/lib/status-panel
5565
"#;
5666

@@ -65,13 +75,15 @@ pub struct InitResult {
6575
/// Generate default config.json and .env files in the given directory.
6676
///
6777
/// Existing files are never overwritten unless `force` is true.
78+
/// The .env file is created with restricted permissions (0600) on Unix.
6879
pub fn generate_default_config(dir: &Path, force: bool) -> Result<InitResult> {
6980
let config_path = dir.join("config.json");
7081
let env_path = dir.join(".env");
7182

7283
let config_created =
7384
write_if_absent(&config_path, DEFAULT_CONFIG, force).context("writing config.json")?;
74-
let env_created = write_if_absent(&env_path, DEFAULT_ENV, force).context("writing .env")?;
85+
let env_created =
86+
write_if_absent_secure(&env_path, DEFAULT_ENV, force).context("writing .env")?;
7587

7688
if config_created {
7789
info!(path = %config_path.display(), "created default config.json");
@@ -88,14 +100,66 @@ pub fn generate_default_config(dir: &Path, force: bool) -> Result<InitResult> {
88100
})
89101
}
90102

91-
/// Write `content` to `path` if the file does not exist or `force` is true.
103+
/// Write `content` to `path` atomically if the file does not exist.
104+
/// When `force` is true, truncates and overwrites.
92105
/// Returns `true` when the file was actually written.
93106
fn write_if_absent(path: &Path, content: &str, force: bool) -> Result<bool> {
94-
if path.exists() && !force {
95-
return Ok(false);
107+
use std::fs::OpenOptions;
108+
use std::io::Write;
109+
110+
if force {
111+
std::fs::write(path, content)?;
112+
return Ok(true);
96113
}
114+
115+
// Atomic create: fails with AlreadyExists if file is present (no TOCTOU)
116+
match OpenOptions::new().write(true).create_new(true).open(path) {
117+
Ok(mut f) => {
118+
f.write_all(content.as_bytes())?;
119+
Ok(true)
120+
}
121+
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => Ok(false),
122+
Err(e) => Err(e.into()),
123+
}
124+
}
125+
126+
/// Like `write_if_absent` but sets 0600 permissions on Unix (for secret-bearing files).
127+
fn write_if_absent_secure(path: &Path, content: &str, force: bool) -> Result<bool> {
128+
use std::fs::OpenOptions;
129+
use std::io::Write;
130+
131+
if force {
132+
write_with_restricted_perms(path, content)?;
133+
return Ok(true);
134+
}
135+
136+
match OpenOptions::new().write(true).create_new(true).open(path) {
137+
Ok(mut f) => {
138+
f.write_all(content.as_bytes())?;
139+
drop(f);
140+
set_restricted_perms(path);
141+
Ok(true)
142+
}
143+
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => Ok(false),
144+
Err(e) => Err(e.into()),
145+
}
146+
}
147+
148+
fn write_with_restricted_perms(path: &Path, content: &str) -> Result<()> {
97149
std::fs::write(path, content)?;
98-
Ok(true)
150+
set_restricted_perms(path);
151+
Ok(())
152+
}
153+
154+
#[cfg(unix)]
155+
fn set_restricted_perms(path: &Path) {
156+
use std::os::unix::fs::PermissionsExt;
157+
let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600));
158+
}
159+
160+
#[cfg(not(unix))]
161+
fn set_restricted_perms(_path: &Path) {
162+
// No-op on non-Unix platforms
99163
}
100164

101165
#[cfg(test)]
@@ -160,4 +224,34 @@ mod tests {
160224
assert_eq!(cfg.reqdata.email, "admin@example.com");
161225
assert!(!cfg.compose_agent_enabled);
162226
}
227+
228+
#[test]
229+
fn test_env_contains_all_documented_vars() {
230+
assert!(DEFAULT_ENV.contains("AGENT_ID="));
231+
assert!(DEFAULT_ENV.contains("AGENT_TOKEN="));
232+
assert!(DEFAULT_ENV.contains("DASHBOARD_URL="));
233+
assert!(DEFAULT_ENV.contains("STATUS_PANEL_USERNAME="));
234+
assert!(DEFAULT_ENV.contains("STATUS_PANEL_PASSWORD="));
235+
assert!(DEFAULT_ENV.contains("COMPOSE_AGENT_ENABLED"));
236+
assert!(DEFAULT_ENV.contains("VAULT_ADDRESS"));
237+
assert!(DEFAULT_ENV.contains("VAULT_TOKEN"));
238+
assert!(DEFAULT_ENV.contains("DEPLOYMENT_HASH"));
239+
assert!(DEFAULT_ENV.contains("UPDATE_SERVER_URL"));
240+
assert!(DEFAULT_ENV.contains("UPDATE_STORAGE_PATH"));
241+
// Credentials must not have default values baked in
242+
assert!(DEFAULT_ENV.contains("STATUS_PANEL_USERNAME=\n"));
243+
assert!(DEFAULT_ENV.contains("STATUS_PANEL_PASSWORD=\n"));
244+
}
245+
246+
#[cfg(unix)]
247+
#[test]
248+
fn test_env_file_has_restricted_permissions() {
249+
use std::os::unix::fs::PermissionsExt;
250+
let dir = tempfile::tempdir().unwrap();
251+
generate_default_config(dir.path(), false).unwrap();
252+
253+
let meta = std::fs::metadata(dir.path().join(".env")).unwrap();
254+
let mode = meta.permissions().mode() & 0o777;
255+
assert_eq!(mode, 0o600, "expected 0600, got {:o}", mode);
256+
}
163257
}

src/main.rs

Lines changed: 71 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,72 @@
11
use dotenvy::dotenv;
22
use status_panel::{agent, commands, comms, monitoring, utils};
33

4-
use anyhow::{Context, Result};
4+
use anyhow::Result;
55
use clap::{Parser, Subcommand};
66
use tracing::info;
77

88
/// Application version from Cargo.toml
99
const VERSION: &str = env!("CARGO_PKG_VERSION");
1010
const PKG_NAME: &str = env!("CARGO_PKG_NAME");
1111

12+
/// Check that `path` points to a readable file. Prints a friendly error and
13+
/// exits if the file is missing, is not a regular file, or is not readable.
14+
fn ensure_config_file(path: &str) {
15+
match std::fs::metadata(path) {
16+
Ok(meta) if meta.is_file() => {
17+
// Verify readability
18+
if let Err(err) = std::fs::File::open(path) {
19+
if err.kind() == std::io::ErrorKind::PermissionDenied {
20+
eprintln!();
21+
eprintln!(" Config file is not readable: {path}");
22+
eprintln!();
23+
eprintln!(" Check the file permissions and try again.");
24+
eprintln!();
25+
std::process::exit(1);
26+
}
27+
eprintln!();
28+
eprintln!(" Cannot open config file: {path}");
29+
eprintln!(" Error: {err}");
30+
eprintln!();
31+
std::process::exit(1);
32+
}
33+
}
34+
Ok(_) => {
35+
eprintln!();
36+
eprintln!(" Config path is not a regular file: {path}");
37+
eprintln!();
38+
eprintln!(" Run 'status init' to generate a default configuration,");
39+
eprintln!(" or specify a valid file with --config <path>");
40+
eprintln!();
41+
std::process::exit(1);
42+
}
43+
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
44+
eprintln!();
45+
eprintln!(" Config file not found: {path}");
46+
eprintln!();
47+
eprintln!(" Run 'status init' to generate a default configuration,");
48+
eprintln!(" or specify a custom path with --config <path>");
49+
eprintln!();
50+
std::process::exit(1);
51+
}
52+
Err(err) if err.kind() == std::io::ErrorKind::PermissionDenied => {
53+
eprintln!();
54+
eprintln!(" Config file is not readable: {path}");
55+
eprintln!();
56+
eprintln!(" Check the file permissions and try again.");
57+
eprintln!();
58+
std::process::exit(1);
59+
}
60+
Err(err) => {
61+
eprintln!();
62+
eprintln!(" Cannot access config file: {path}");
63+
eprintln!(" Error: {err}");
64+
eprintln!();
65+
std::process::exit(1);
66+
}
67+
}
68+
}
69+
1270
/// Print startup banner with version and system info
1371
fn print_banner() {
1472
let rust_version = rustc_version_runtime::version();
@@ -237,15 +295,7 @@ async fn main() -> Result<()> {
237295

238296
match args.command {
239297
Some(Commands::Serve { port, with_ui }) => {
240-
if !std::path::Path::new(&args.config).exists() {
241-
eprintln!();
242-
eprintln!(" Config file not found: {}", args.config);
243-
eprintln!();
244-
eprintln!(" Run 'status init' to generate a default configuration,");
245-
eprintln!(" or specify a custom path with --config <path>");
246-
eprintln!();
247-
std::process::exit(1);
248-
}
298+
ensure_config_file(&args.config);
249299
if with_ui {
250300
info!("Starting local API server with UI on port {port}");
251301
} else {
@@ -416,8 +466,16 @@ async fn main() -> Result<()> {
416466
}
417467
}
418468
Some(Commands::Init { force }) => {
419-
let cwd = std::env::current_dir().context("could not determine current directory")?;
420-
let result = agent::init::generate_default_config(&cwd, force)?;
469+
// Derive output directory from --config path (defaults to current dir)
470+
let config_path = std::path::Path::new(&args.config);
471+
let dir = config_path
472+
.parent()
473+
.filter(|p| !p.as_os_str().is_empty())
474+
.map(|p| p.to_path_buf())
475+
.unwrap_or_else(|| {
476+
std::env::current_dir().expect("could not determine current directory")
477+
});
478+
let result = agent::init::generate_default_config(&dir, force)?;
421479

422480
println!();
423481
if result.config_created {
@@ -450,15 +508,7 @@ async fn main() -> Result<()> {
450508
}
451509
None => {
452510
// Default: run the agent daemon
453-
if !std::path::Path::new(&args.config).exists() {
454-
eprintln!();
455-
eprintln!(" Config file not found: {}", args.config);
456-
eprintln!();
457-
eprintln!(" Run 'status init' to generate a default configuration,");
458-
eprintln!(" or specify a custom path with --config <path>");
459-
eprintln!();
460-
std::process::exit(1);
461-
}
511+
ensure_config_file(&args.config);
462512
if args.compose_mode {
463513
info!("Starting compose-agent daemon mode");
464514
// Set CONTROL_PLANE environment variable for identification

0 commit comments

Comments
 (0)