Skip to content
Merged
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
7 changes: 2 additions & 5 deletions docker-compose.prod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,20 @@ services:
# dockerfile: docker/Dockerfile
# target: prod
image: portabase/agent:latest
# platform: linux/arm64
container_name: rust-prod
volumes:
- ./databases.json:/config/config.json
environment:
# APP_ENV: production
LOG: info
TZ: "Europe/Paris"
# DATABASES_CONFIG_FILE: "config.toml"
EDGE_KEY: "eyJzZXJ2ZXJVcmwiOiJodHRwOi8vbG9jYWxob3N0Ojg4ODciLCJhZ2VudElkIjoiNWU1OGU2MGEtODhiMy00YTBjLWI0NDktNTQ3OWZhOTQzZDBkIiwibWFzdGVyS2V5QjY0IjoiQlhWM1hvbEM2NTZTVjdkTmdjV1BHUWxrKytycExJNmxHRGk3Q1BCNWllbz0ifQ=="
#DATABASES_CONFIG_FILE: "config.toml"
EDGE_KEY: "<to complete>"
extra_hosts:
- "localhost:host-gateway"
networks:
- portabase



db-mongodb-auth:
container_name: db-mongodb-auth
image: mongo:latest
Expand Down
13 changes: 7 additions & 6 deletions src/domain/redis/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::services::config::DatabaseConfig;
use anyhow::{Context, Result};
use tokio::process::Command;
use tokio::time::{Duration, timeout};
use tracing::{debug, info};
use tracing::{debug, error, info};

pub async fn run(cfg: DatabaseConfig) -> Result<bool> {
let mut cmd = Command::new("redis-cli");
Expand All @@ -21,7 +21,7 @@ pub async fn run(cfg: DatabaseConfig) -> Result<bool> {

cmd.arg("PING");

debug!("Command Ping: {:?}", cmd);
debug!("Command Ping Redis: {:?}", cmd);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/domain/redis/ping.rs

Repository: Portabase/agent

Length of output: 2009


Redact credentials from the debug command log.

Line 24 logs the full tokio::process::Command after line 19 appends the password with the -a flag. Since tokio::process::Command's Debug output includes all arguments, this will expose Redis credentials in debug logs.

🔐 Safer logging
-    debug!("Command Ping Redis: {:?}", cmd);
+    debug!("Running Redis ping against {}:{}", cfg.host, cfg.port);
📝 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
debug!("Command Ping Redis: {:?}", cmd);
debug!("Running Redis ping against {}:{}", cfg.host, cfg.port);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/domain/redis/ping.rs` at line 24, The debug log prints the full
tokio::process::Command (variable cmd) which includes Redis credentials added
with "-a"; instead, sanitize before logging by constructing a redacted
representation (e.g., clone or extract cmd.get_args()/arg list, replace the
password value after "-a" with "<REDACTED>" or remove that flag+value) and log
that sanitized string or args rather than the raw cmd; update the debug call in
ping.rs (the debug!("Command Ping Redis: {:?}", cmd) site) to use the redacted
representation.


let result = timeout(Duration::from_secs(10), cmd.output()).await;

Expand All @@ -31,16 +31,17 @@ pub async fn run(cfg: DatabaseConfig) -> Result<bool> {
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);

info!("Redis stdout: {}", stdout);
info!("Redis stderr: {}", stderr);
if !stdout.is_empty() {
error!("Redis stderr: {}", stderr);
}
Comment on lines +34 to +36
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The stderr log is gated by the wrong stream.

Line 34 checks stdout, so a healthy PONG path will emit Redis stderr: on success, while stderr-only failures can be skipped. This condition should be keyed off stderr.

🩹 Suggested fix
-            if !stdout.is_empty() {
+            if !stderr.is_empty() {
                 error!("Redis stderr: {}", stderr);
             }
📝 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
if !stdout.is_empty() {
error!("Redis stderr: {}", stderr);
}
if !stderr.is_empty() {
error!("Redis stderr: {}", stderr);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/domain/redis/ping.rs` around lines 34 - 36, The log condition is checking
the wrong stream: change the gate from stdout to stderr so that error!("Redis
stderr: {}", stderr) only runs when stderr is non-empty; locate the block using
the stdout and stderr variables (in the Redis ping handling function) and
replace if !stdout.is_empty() { ... } with if !stderr.is_empty() { error!("Redis
stderr: {}", stderr); }.


if stderr.contains("NOAUTH") {
info!("Redis authentication failed (NOAUTH required)");
error!("Redis authentication failed (NOAUTH required)");
return Ok(false);
}

if !output.status.success() {
info!("Redis command failed with status: {:?}", output.status);
error!("Redis command failed with status: {:?}", output.status);
return Ok(false);
}

Expand Down
17 changes: 9 additions & 8 deletions src/domain/valkey/ping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::services::config::DatabaseConfig;
use anyhow::{Context, Result};
use tokio::process::Command;
use tokio::time::{Duration, timeout};
use tracing::{debug, info};
use tracing::{debug, info, error};

pub async fn run(cfg: DatabaseConfig) -> Result<bool> {
let mut cmd = Command::new("valkey-cli");
Expand All @@ -21,33 +21,34 @@ pub async fn run(cfg: DatabaseConfig) -> Result<bool> {

cmd.arg("PING");

debug!("Command Ping: {:?}", cmd);
debug!("Command Ping Valkey: {:?}", cmd);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "ping.rs" -path "*/valkey/*" | head -5

Repository: Portabase/agent

Length of output: 86


🏁 Script executed:

cat -n ./src/domain/valkey/ping.rs

Repository: Portabase/agent

Length of output: 2016


🏁 Script executed:

find . -name "Cargo.toml" -type f | head -1 | xargs cat | grep -A 20 "dependencies"

Repository: Portabase/agent

Length of output: 1125


🌐 Web query:

tokio::process::Command Debug implementation 1.49 arguments

💡 Result:

The tokio::process::Command struct implements the Debug trait. This has been present at least since early versions like 1.8.0 and continues in the latest versions (1.50.0 as of 2026-03-03). In Tokio 1.49.0, a fix was made to "remove unnecessary trait bounds on the Debug implementation" for tasks, but the Command Debug impl itself is standard and derives from its fields, including the underlying std::process::Command (which shows program and arguments) and the kill_on_drop flag. The Debug output typically displays the configured program name, arguments, environment variables, working directory, and other settings from the wrapped std::process::Command, plus Tokio-specific fields like kill_on_drop.

Citations:


Redact credentials from the debug command log.

Line 24 logs the full command using {:?} format after Line 19 appends the password with the -a flag. The Debug implementation of tokio::process::Command includes all configured arguments, which will expose Valkey credentials in debug logs.

🔐 Safer logging
-    debug!("Command Ping Valkey: {:?}", cmd);
+    debug!("Running Valkey ping against {}:{}", cfg.host, cfg.port);
📝 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
debug!("Command Ping Valkey: {:?}", cmd);
debug!("Running Valkey ping against {}:{}", cfg.host, cfg.port);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/domain/valkey/ping.rs` at line 24, The debug statement prints the full
Command (cmd) including arguments and thus exposes the Valkey password added
with "-a"; change the logging to avoid {:?} on cmd and instead log only the
executable and a sanitized args list: inspect cmd's arguments (the variable cmd
in ping.rs / the function building the Command), mask or omit the value that
follows "-a" (replace with "<redacted>" or drop that arg), and log the
filtered/masked args or just the program name so credentials are not emitted.


let result = timeout(Duration::from_secs(10), cmd.output()).await;

match result {
Ok(output) => {
let output = output.context("Failed to execute redis-cli")?;
let output = output.context("Failed to execute valkey-cli")?;
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);

info!("Redis stdout: {}", stdout);
info!("Redis stderr: {}", stderr);
if !stdout.is_empty() {
error!("Valkey stderr: {}", stderr);
}
Comment on lines +34 to +36
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The stderr log is gated by the wrong stream.

Line 34 checks stdout, so a healthy PONG path will emit Valkey stderr: on success, while stderr-only failures can be skipped. This condition should be keyed off stderr.

🩹 Suggested fix
-            if !stdout.is_empty() {
+            if !stderr.is_empty() {
                 error!("Valkey stderr: {}", stderr);
             }
📝 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
if !stdout.is_empty() {
error!("Valkey stderr: {}", stderr);
}
if !stderr.is_empty() {
error!("Valkey stderr: {}", stderr);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/domain/valkey/ping.rs` around lines 34 - 36, The log condition is
checking the wrong stream: instead of gating the stderr log on stdout, change
the condition to check stderr (i.e., if !stderr.is_empty()) so that
error!("Valkey stderr: {}", stderr) runs only when stderr contains data; locate
this in the ping logic where stdout and stderr variables are inspected
(variables stdout, stderr in src/domain/valkey/ping.rs) and update the
conditional accordingly.


if stderr.contains("NOAUTH") {
info!("Redis authentication failed (NOAUTH required)");
error!("Valkey authentication failed (NOAUTH required)");
return Ok(false);
}

if !output.status.success() {
info!("Redis command failed with status: {:?}", output.status);
error!("Valkey command failed with status: {:?}", output.status);
return Ok(false);
}

Ok(stdout.contains("PONG"))
}
Err(_) => {
info!("Timeout connecting to Redis at {}:{}", cfg.host, cfg.port);
info!("Timeout connecting to Valkey at {}:{}", cfg.host, cfg.port);
Ok(false)
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/services/api/endpoints/agent/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ pub struct DatabasePayload<'a> {
pub dbms: &'a str,
#[serde(rename = "generatedId")]
pub generated_id: &'a str,
#[serde(rename = "pingStatus")]
pub ping_status: bool
}

#[derive(Serialize)]
Expand Down
22 changes: 15 additions & 7 deletions src/services/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use crate::settings::CONFIG;
use reqwest::Client;
use std::error::Error;
use std::sync::Arc;
use futures_util::future::try_join_all;
use crate::domain::factory::DatabaseFactory;

pub struct StatusService {
ctx: Arc<Context>,
Expand All @@ -25,14 +27,20 @@ impl StatusService {
pub async fn ping(&self, databases: &[DatabaseConfig]) -> Result<PingResult, Box<dyn Error>> {
let edge_key = &self.ctx.edge_key;

let databases_payload: Vec<DatabasePayload> = databases
.iter()
.map(|db| DatabasePayload {
name: &db.name,
dbms: &db.db_type.as_str(),
generated_id: &db.generated_id,
let databases_payload: Vec<DatabasePayload> = try_join_all(
databases.into_iter().map(|db| async move {
let db_engine = DatabaseFactory::create_for_backup(db.clone()).await;

let reachable = db_engine.ping().await?;

Ok::<DatabasePayload, anyhow::Error>(DatabasePayload {
name: &db.name,
dbms: &db.db_type.as_str(),
generated_id: &db.generated_id,
ping_status: reachable,
})
})
.collect();
).await?;
Comment on lines +30 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the status.rs file
find . -name "status.rs" -path "*/services/*" | head -5

Repository: Portabase/agent

Length of output: 229


🏁 Script executed:

# Look for ping.rs files in domain subdirectories
find . -name "ping.rs" -path "*/domain/*" | head -10

Repository: Portabase/agent

Length of output: 227


🏁 Script executed:

# Check the repository structure
ls -la src/ 2>/dev/null | head -20

Repository: Portabase/agent

Length of output: 620


🏁 Script executed:

# Read the status.rs file to see lines 30-43 and the import at line 11
cat -n src/services/status.rs | head -50

Repository: Portabase/agent

Length of output: 1973


🏁 Script executed:

# Read redis/ping.rs around line 30
cat -n src/domain/redis/ping.rs | head -40

Repository: Portabase/agent

Length of output: 1528


🏁 Script executed:

# Read valkey/ping.rs around line 30
cat -n src/domain/valkey/ping.rs | head -40

Repository: Portabase/agent

Length of output: 1533


🏁 Script executed:

# Check a couple other ping implementations to see if pattern is consistent
cat -n src/domain/mysql/ping.rs | head -40

Repository: Portabase/agent

Length of output: 955


🏁 Script executed:

# Check how the ping result is used in the context
cat -n src/services/status.rs | tail -20

Repository: Portabase/agent

Length of output: 767


🏁 Script executed:

# Search for callers of the StatusService::ping method
rg "StatusService::ping|\.ping\(" src/services/ -A 3 -B 1

Repository: Portabase/agent

Length of output: 897


One ping error should not abort the entire status report.

Line 30–43 uses try_join_all(...).await?, so any Err from a single database aborts the entire payload. This happens because src/domain/redis/ping.rs Line 30 and src/domain/valkey/ping.rs Line 30 bubble CLI execution errors with ?. However, src/domain/mysql/ping.rs already handles errors gracefully by returning Ok(false). For consistency and better observability in a health endpoint, apply the same pattern: log ping failures per database and emit ping_status: false rather than failing the whole status report.

Other services in this codebase (backup/runner.rs, restore/runner.rs) already use graceful degradation with match or unwrap_or(false), so this change aligns with existing patterns.

🩹 Suggested approach
-        let databases_payload: Vec<DatabasePayload> = try_join_all(
+        let databases_payload: Vec<DatabasePayload> = join_all(
             databases.into_iter().map(|db| async move {
                 let db_engine = DatabaseFactory::create_for_backup(db.clone()).await;
-
-                let reachable = db_engine.ping().await?;
-
-                Ok::<DatabasePayload, anyhow::Error>(DatabasePayload {
+                let reachable = match db_engine.ping().await {
+                    Ok(reachable) => reachable,
+                    Err(err) => {
+                        tracing::error!(database = %db.name, error = ?err, "Database ping failed");
+                        false
+                    }
+                };
+
+                DatabasePayload {
                     name: &db.name,
                     dbms: &db.db_type.as_str(),
                     generated_id: &db.generated_id,
                     ping_status: reachable,
-                })
+                }
             })
-        ).await?;
+        ).await;

Update line 11 to import join_all instead of try_join_all.

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

In `@src/services/status.rs` around lines 30 - 43, Replace the fallible
try_join_all with join_all and make the per-database async closure swallow ping
errors: call DatabaseFactory::create_for_backup and db_engine.ping() inside the
closure, then on Err(log the error with your crate logger, e.g.
tracing::error!/log::error!) and return DatabasePayload with ping_status: false
(preserving name, dbms, generated_id), and on Ok set ping_status to
true/returned value; also update the import to use join_all instead of
try_join_all so a single ping failure does not abort the whole aggregation.


let version_str = CONFIG.app_version.as_str();
let result = self
Expand Down
10 changes: 10 additions & 0 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ impl Settings {
.parse::<usize>()
.expect("POOLING must be a valid positive integer");

if pooling_seconds < 1 || pooling_seconds > 600 {
panic!("POOLING must be between 1 second and 600 seconds (10 minutes)");
}

if pooling_seconds < 5 {
eprintln!(
"[WARNING] POOLING is set to {}s. Values under 5s are not recommended for production.",
pooling_seconds
);
}
let tz = env::var("TZ").unwrap_or_else(|_| "UTC".to_string());

Self {
Expand Down
Loading