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
6 changes: 4 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@ DTIM__DEFAULT__PORT=3030
DTIM__DEFAULT__LOG_LEVEL="info"

# Sensitive info (.env only)
DTIM__DEFAULT__STORAGE__DATABASE_URL="postgres://user:pass@localhost/db"
DTIM__DEFAULT__WATCHERS__VIRUSTOTAL_API_KEY="your_api_key"
DATABASE_URL="postgres://postgres:@localhost:5432/postgres" # For Diesel CLI

DTIM__DEFAULT__STORAGE__DATABASE_URL=${DATABASE_URL}
Comment on lines +7 to +9

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Review database connection string for security

The PostgreSQL connection string uses default credentials without a password. While acceptable for a local development example, this poses security risks:

  1. Uses default "postgres" superuser account
  2. Has no password protection
  3. Uses default "postgres" database

Add clarifying comments to guide users on securing their database:

-DATABASE_URL="postgres://postgres:@localhost:5432/postgres" # For Diesel CLI
+DATABASE_URL="postgres://postgres:@localhost:5432/postgres" # For Diesel CLI - Add password after 'postgres:' for production use
 
-DTIM__DEFAULT__STORAGE__DATABASE_URL=${DATABASE_URL}
+DTIM__DEFAULT__STORAGE__DATABASE_URL=${DATABASE_URL} # Uses same connection as Diesel CLI
📝 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
DATABASE_URL="postgres://postgres:@localhost:5432/postgres" # For Diesel CLI
DTIM__DEFAULT__STORAGE__DATABASE_URL=${DATABASE_URL}
DATABASE_URL="postgres://postgres:@localhost:5432/postgres" # For Diesel CLI - Add password after 'postgres:' for production use
DTIM__DEFAULT__STORAGE__DATABASE_URL=${DATABASE_URL} # Uses same connection as Diesel CLI
🤖 Prompt for AI Agents
In the .env.example file around lines 7 to 9, the PostgreSQL connection string
uses default credentials with no password and the superuser account, which is
insecure. Add comments above or beside the DATABASE_URL line to clarify that
this is for local development only, recommend changing the username, setting a
strong password, and using a dedicated database for production environments to
guide users on securing their database connection.

DTIM__DEFAULT__WATCHERS__VIRUSTOTAL_API_KEY="your_api_key"
107 changes: 107 additions & 0 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ default-run = "dtim"
axum = { version = "0.8", features = ["http2", "tokio"] }
axum-server = { version = "0.7", features = ["tls-rustls"] }
chrono = { version = "0.4", features = ["serde"] }
uuid = { version = "1.16", features = ["v7", "serde"] }
uuid = { version = "1.16", features = ["v5", "v7", "serde"] }
serde = { version = "1.0", features = ["derive"] }
serde_json = { version = "1.0", features = ["preserve_order"] }
serde_with = "3.12"
Expand All @@ -28,3 +28,4 @@ async-trait = "0.1"
http-body-util = "0.1"
dotenvy = "0.15"
once_cell = "1.21"
diesel = { version = "2.2.0", features = ["postgres", "r2d2", "chrono"] }
9 changes: 9 additions & 0 deletions diesel.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# For documentation on how to configure this file,
# see https://diesel.rs/guides/configuring-diesel-cli

[print_schema]
file = "src/db/schema.rs"
custom_type_derives = ["diesel::query_builder::QueryId", "Clone"]

[migrations_directory]
dir = "/Users/caden/Developer/dtim/migrations"
Comment on lines +8 to +9

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid absolute paths for migrations directory.

The migrations_directory.dir is hardcoded to a local filesystem path. This will break for other developers and CI environments.
Use a relative path, for example:

[migrations_directory]
-dir = "/Users/caden/Developer/dtim/migrations"
+dir = "migrations"
🤖 Prompt for AI Agents
In diesel.toml around lines 8 to 9, the migrations_directory.dir is set to an
absolute path which is not portable. Change the value from the absolute path
"/Users/caden/Developer/dtim/migrations" to a relative path "migrations" to
ensure it works across different environments and for other developers.

Empty file added migrations/.keep
Empty file.
6 changes: 6 additions & 0 deletions migrations/00000000000000_diesel_initial_setup/down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-- This file was automatically created by Diesel to setup helper functions
-- and other internal bookkeeping. This file is safe to edit, any future
-- changes will be added to existing projects as new migrations.

DROP FUNCTION IF EXISTS diesel_manage_updated_at(_tbl regclass);
DROP FUNCTION IF EXISTS diesel_set_updated_at();
36 changes: 36 additions & 0 deletions migrations/00000000000000_diesel_initial_setup/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
-- This file was automatically created by Diesel to setup helper functions
-- and other internal bookkeeping. This file is safe to edit, any future
-- changes will be added to existing projects as new migrations.




-- Sets up a trigger for the given table to automatically set a column called
-- `updated_at` whenever the row is modified (unless `updated_at` was included
-- in the modified columns)
--
-- # Example
--
-- ```sql
-- CREATE TABLE users (id SERIAL PRIMARY KEY, updated_at TIMESTAMP NOT NULL DEFAULT NOW());
--
-- SELECT diesel_manage_updated_at('users');
-- ```
CREATE OR REPLACE FUNCTION diesel_manage_updated_at(_tbl regclass) RETURNS VOID AS $$
BEGIN
EXECUTE format('CREATE TRIGGER set_updated_at BEFORE UPDATE ON %s
FOR EACH ROW EXECUTE PROCEDURE diesel_set_updated_at()', _tbl);
END;
$$ LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION diesel_set_updated_at() RETURNS trigger AS $$
BEGIN
IF (
NEW IS DISTINCT FROM OLD AND
NEW.updated_at IS NOT DISTINCT FROM OLD.updated_at
) THEN
NEW.updated_at := current_timestamp;
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE encrypted_indicators;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Ensure safe, idempotent rollback of the table.

Using a plain DROP TABLE will error if the table doesn’t exist.
Consider making this idempotent and handling dependent objects by switching to:

-DROP TABLE encrypted_indicators;
+DROP TABLE IF EXISTS encrypted_indicators CASCADE;
🤖 Prompt for AI Agents
In migrations/2025-05-21-234457_create_encrypted_indicators/down.sql at line 1,
the current DROP TABLE statement will cause an error if the table does not
exist. To make the rollback safe and idempotent, modify the statement to include
IF EXISTS so it only attempts to drop the table if it exists. Also, add CASCADE
to handle dependent objects automatically during the drop.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE encrypted_indicators (
id CHAR(64) PRIMARY KEY,
ciphertext BYTEA NOT NULL,
nonce BYTEA NOT NULL,
mac BYTEA NOT NULL,
tlp_level TEXT NOT NULL
);
36 changes: 23 additions & 13 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,17 @@ use ed25519_dalek::VerifyingKey;
use http_body_util::BodyExt as _;
use rustls::ServerConfig;
use serde::Serialize;
use std::{str::FromStr as _, sync::Arc};
use std::sync::Arc;
use tokio::sync::Mutex;

use crate::models::ThreatIndicator;
use crate::{
crypto::MeshIdentity,
db::models::EncryptedIndicator,
error::{ApiError, ApiErrorResponse},
node::{Node, NodePeer},
};
use crate::{crypto::SymmetricKeyManager, models::StixBundle};
use crate::{
models::{EncryptedThreatIndicator, ThreatIndicator},
uuid::Uuid,
};

#[derive(Clone)]
pub struct AppState {
Expand Down Expand Up @@ -200,13 +198,14 @@ struct GossipIndicatorsResponse {

async fn gossip_indicators_handler(
State(state): State<Arc<AppState>>,
Json(indicators): Json<Vec<EncryptedThreatIndicator>>,
Json(indicators): Json<Vec<EncryptedIndicator>>,
) -> ApiResponse<GossipIndicatorsResponse> {
let mut node = state.node.lock().await;
let mut count = 0;
for encrypted in indicators {
if let Ok(indicator) = ThreatIndicator::decrypt(&encrypted, &state.key_mgr) {
node.add_or_increment_indicator(indicator);
node.add_or_increment_indicator(indicator)
.map_err(|_| ApiError::INTERNAL_SERVER_ERROR)?;
count += 1;
}
}
Comment on lines 199 to 211

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Minimise lock contention by decrypting before acquiring the Node mutex

state.node.lock().await is taken before the loop that performs CPU-bound decryption and signature checks.
While the lock is held, every other request that needs the node (including reads) will block, lowering throughput and increasing latency under load.

-    let mut node = state.node.lock().await;
-    let mut count = 0;
-    for encrypted in indicators {
-        if let Ok(indicator) = ThreatIndicator::decrypt(&encrypted, &state.key_mgr) {
-            node.add_or_increment_indicator(indicator)
-                .map_err(|_| ApiError::INTERNAL_SERVER_ERROR)?;
-            count += 1;
-        }
-    }
+    // 1.  Decrypt outside the critical section
+    let mut decrypted: Vec<ThreatIndicator> = indicators
+        .into_iter()
+        .filter_map(|enc| ThreatIndicator::decrypt(&enc, &state.key_mgr).ok())
+        .collect();
+
+    // 2. Acquire the lock *once* for all DB mutations
+    let mut node = state.node.lock().await;
+    let mut count = 0;
+    for indicator in decrypted.drain(..) {
+        node.add_or_increment_indicator(indicator)
+            .map_err(|_| ApiError::INTERNAL_SERVER_ERROR)?;
+        count += 1;
+    }

Benefits
• Shorter critical section → better concurrency
• Avoids holding the lock while performing CPU work or I/O on the key manager
• Slightly cleaner code by separating “validation/decryption” from “state mutation”.

🤖 Prompt for AI Agents
In src/api.rs around lines 199 to 211, the Node mutex lock is acquired before
decrypting indicators, causing unnecessary lock contention. To fix this, move
the decryption and validation of each indicator outside the locked section by
first decrypting all indicators, collecting the valid ones, and then acquiring
the lock once to update the node state. This reduces the critical section
duration, improves concurrency, and separates CPU-bound decryption from state
mutation.

Expand All @@ -229,7 +228,9 @@ async fn get_public_indicators_handler(
State(state): State<Arc<AppState>>,
) -> ApiResponse<GetIndicatorsResponse> {
let node = state.node.lock().await;
let indicators = node.list_indicators_by_tlp(crate::models::TlpLevel::White);
let indicators = node
.list_indicators_by_tlp(crate::models::TlpLevel::White)
.map_err(|_| ApiError::INTERNAL_SERVER_ERROR)?;
// Return anonymized (open) view
Ok((
StatusCode::OK,
Expand All @@ -244,7 +245,9 @@ async fn get_private_indicators_handler(
State(state): State<Arc<AppState>>,
) -> ApiResponse<GetIndicatorsResponse> {
let node = state.node.lock().await;
let indicators = node.list_indicators_by_tlp(crate::models::TlpLevel::Red);
let indicators = node
.list_indicators_by_tlp(crate::models::TlpLevel::Red)
.map_err(|_| ApiError::INTERNAL_SERVER_ERROR)?;
// TODO: Ensure the node is an authenticated recipient for private indicators
// Return anonymized (moderate) view
Ok((
Expand All @@ -261,8 +264,9 @@ async fn get_indicator_by_id_handler(
Path(id): Path<String>,
) -> ApiResponse<serde_json::Value> {
let node = state.node.lock().await;
let id = Uuid::from_str(&id).map_err(|_| ApiError::INVALID_INDICATOR_ID)?;
let indicator = node.get_indicator_by_id(&id).ok_or(ApiError::NOT_FOUND)?;
let indicator = node
.get_indicator_by_id(&id)
.map_err(|_| ApiError::NOT_FOUND)?;
Ok((StatusCode::OK, Json(indicator.to_json(node.get_level()))))
}

Expand Down Expand Up @@ -319,7 +323,12 @@ async fn taxii_get_objects_handler(
Path(_collection_id): Path<String>,
) -> ApiResponse<serde_json::Value> {
let node = state.node.lock().await;
let stix_objects = node.list_objects_by_tlp(crate::models::TlpLevel::White);
let stix_objects = node
.list_objects_by_tlp(crate::models::TlpLevel::White)
.map_err(|error| {
println!("Error: {:?}", error);
ApiError::INTERNAL_SERVER_ERROR
})?;
let bundle = StixBundle::new(stix_objects);
Ok((StatusCode::OK, Json(bundle.to_stix())))
}
Expand All @@ -331,6 +340,7 @@ async fn taxii_post_objects_handler(
) -> ApiResponse<serde_json::Value> {
let indicator = ThreatIndicator::from_stix(stix).map_err(|_| ApiError::INVALID_STIX_OBJECT)?;
let mut node = state.node.lock().await;
node.add_indicator(indicator);
node.add_indicator(indicator)
.map_err(|_| ApiError::INTERNAL_SERVER_ERROR)?;
Ok((StatusCode::OK, Json(serde_json::json!({ "status": "ok" }))))
}
Loading
Loading