asp/keep
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: harden security, eliminate panics, remove dead code, add Dockerfile

Browse Source 
Security:
- Use constant-time password comparison (subtle crate) to prevent timing attacks
- Replace permissive CORS with configurable origin-restricted CORS
- Add TLS warning when password auth is used without HTTPS

Bug fixes:
- Convert MetaPlugin panics to anyhow::Result (get_meta_plugin, outputs_mut, options_mut)
- Replace item.id.unwrap() with proper error handling across 15 call sites
- Fix panic on unknown column type in list mode
- Fix conflicting PIPESIZE constant (was 8192 vs 65536, now unified to 8192)
- Add 256MB filter chain buffer limit to prevent OOM
- Gracefully skip unregistered plugins instead of panicking

Dead code removal:
- Delete unused filter parser files (filter_parser.rs, filter.pest, parser/ module)
- ~260 lines of dead PEG parser code removed

Code consolidation:
- Add is_content_binary_from_metadata() helper (was duplicated in 4 places)
- Simplify save_item_raw() to delegate to save_item_raw_streaming() (~90 lines removed)

Incomplete features:
- Populate filter_plugins in status output from global registry
- Add FallbackMagicFileMetaPlugin (was referenced but never implemented)
- Document init_plugins() as intentional no-op

Infrastructure:
- Add Dockerfile (static musl binary on scratch, 4.8MB)
- Add .dockerignore
- Add cors_origin to ServerConfig and config.rs
This commit is contained in:
Andrew Phillips
2026-03-13 07:57:36 -03:00
parent bee980605f
commit b166477202
43 changed files with 561 additions and 687 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/modes/common.rs
View File

@@ -206,12 +206,14 @@ pub fn settings_meta_plugin_types(
// Try to find the MetaPluginType by meta name
let mut found = false;
for meta_plugin_type in MetaPluginType::iter() {
let meta_plugin =
crate::meta_plugin::get_meta_plugin(meta_plugin_type.clone(), None, None);
if meta_plugin.meta_type().to_string() == trimmed_name {
meta_plugin_types.push(meta_plugin_type);
found = true;
break;
if let Ok(meta_plugin) =
crate::meta_plugin::get_meta_plugin(meta_plugin_type.clone(), None, None)
{
if meta_plugin.meta_type().to_string() == trimmed_name {
meta_plugin_types.push(meta_plugin_type);
found = true;
break;
}
}
}

2
src/modes/generate_config.rs
View File

@@ -53,6 +53,7 @@ struct ServerConfig {
password_file: Option<String>,
password: Option<String>,
password_hash: Option<String>,
cors_origin: Option<String>,
}
#[derive(Debug, Serialize, Deserialize)]
@@ -145,6 +146,7 @@ pub fn mode_generate_config(_cmd: &mut Command, _settings: &crate::config::Setti
password_file: None,
password: None,
password_hash: None,
cors_origin: None,
}),
compression_plugin: None,
meta_plugins: Some(vec![

4
src/modes/get.rs
View File

@@ -1,4 +1,4 @@
use anyhow::{Result, anyhow};
use anyhow::{Context, Result, anyhow};
use std::io::Write;
use crate::common::PIPESIZE;
@@ -55,7 +55,7 @@ pub fn mode_get(
.find_item(conn, ids, tags, &std::collections::HashMap::new())
.map_err(|e| anyhow!("Unable to find matching item in database: {}", e))?;
let item_id = item_with_meta.item.id.unwrap();
let item_id = item_with_meta.item.id.context("Item missing ID")?;
// Determine if we should detect binary data
let mut detect_binary = !settings.force && std::io::stdout().is_terminal();

6
src/modes/info.rs
View File

@@ -1,7 +1,7 @@
use crate::config;
use crate::modes::common::{OutputFormat, format_size};
use crate::services::types::ItemWithMeta;
use anyhow::{Result, anyhow};
use anyhow::{Context, Result, anyhow};
use clap::Command;
use clap::error::ErrorKind;
use serde::{Deserialize, Serialize};
@@ -141,7 +141,7 @@ fn show_item(
}
let item = item_with_meta.item;
let item_id = item.id.unwrap();
let item_id = item.id.context("Item missing ID")?;
let item_tags: Vec<String> = item_with_meta.tags.iter().map(|t| t.name.clone()).collect();
let mut table = crate::modes::common::create_table(false);
@@ -249,7 +249,7 @@ fn show_item_structured(
let item_tags: Vec<String> = item_with_meta.tags.iter().map(|t| t.name.clone()).collect();
let meta_map = item_with_meta.meta_as_map();
let item = item_with_meta.item;
let item_id = item.id.unwrap();
let item_id = item.id.context("Item missing ID")?;
let mut item_path_buf = data_path.clone();
item_path_buf.push(item_id.to_string());

8
src/modes/list.rs
View File

@@ -8,7 +8,7 @@ use crate::modes::common::ColumnType;
use crate::modes::common::{OutputFormat, format_size};
use crate::services::item_service::ItemService;
use crate::services::types::ItemWithMeta;
use anyhow::Result;
use anyhow::{Context, Result};
use comfy_table::CellAlignment;
use comfy_table::{Attribute, Cell, Color, Row};
use serde::{Deserialize, Serialize};
@@ -202,7 +202,7 @@ pub fn mode_list(
let item = item_with_meta.item;
let mut item_path = data_path.clone();
item_path.push(item.id.unwrap().to_string());
item_path.push(item.id.context("Item missing ID")?.to_string());
let mut table_row = Row::new();
@@ -210,7 +210,7 @@ pub fn mode_list(
let column_type = column
.name
.parse::<ColumnType>()
.unwrap_or_else(|_| panic!("Unknown column {:?}", column.name));
.with_context(|| format!("Unknown column type {:?} in list format", column.name))?;
let mut meta_name: Option<&str> = None;
@@ -343,7 +343,7 @@ fn show_list_structured(
let tags: Vec<String> = item_with_meta.tags.iter().map(|t| t.name.clone()).collect();
let meta = item_with_meta.meta_as_map();
let item = item_with_meta.item;
let item_id = item.id.unwrap();
let item_id = item.id.context("Item missing ID")?;
let mut item_path = data_path.clone();
item_path.push(item_id.to_string());

20
src/modes/server/api/item.rs
View File

@@ -481,7 +481,10 @@ pub async fn handle_post_item(
let metadata = item_with_meta.meta_as_map();
let item_info = ItemInfo {
id: item_with_meta.item.id.unwrap(),
id: item_with_meta.item.id.ok_or_else(|| {
warn!("Item missing ID");
StatusCode::INTERNAL_SERVER_ERROR
})?,
ts: item_with_meta.item.ts.to_rfc3339(),
size: item_with_meta.item.size,
compression,
@@ -542,7 +545,10 @@ pub async fn handle_get_item_latest_content(
match item_with_meta {
Ok(item) => {
let item_id = item.item.id.unwrap();
let item_id = item.item.id.ok_or_else(|| {
warn!("Item missing ID");
StatusCode::INTERNAL_SERVER_ERROR
})?;
let metadata = item.meta_as_map();
// Handle as_meta parameter
if params.as_meta {
@@ -948,7 +954,10 @@ pub async fn handle_delete_item(
.map_err(handle_item_error)?;
let item_info = ItemInfo {
id: deleted_item.id.unwrap(),
id: deleted_item.id.ok_or_else(|| {
warn!("Item missing ID");
StatusCode::INTERNAL_SERVER_ERROR
})?,
ts: deleted_item.ts.to_rfc3339(),
size: deleted_item.size,
compression: deleted_item.compression,
@@ -1001,7 +1010,10 @@ pub async fn handle_get_item_info(
let metadata = item_with_meta.meta_as_map();
let item_info = ItemInfo {
id: item_with_meta.item.id.unwrap(),
id: item_with_meta.item.id.ok_or_else(|| {
warn!("Item missing ID");
StatusCode::INTERNAL_SERVER_ERROR
})?,
ts: item_with_meta.item.ts.to_rfc3339(),
size: item_with_meta.item.size,
compression: item_with_meta.item.compression.clone(),

21
src/modes/server/common.rs
View File

@@ -27,6 +27,7 @@ use std::net::SocketAddr;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Instant;
use subtle::ConstantTimeEq;
use tokio::sync::Mutex;
use utoipa::ToSchema;
@@ -75,6 +76,12 @@ pub struct ServerConfig {
///
/// When both cert_file and key_file are set, the server uses HTTPS.
pub key_file: Option<PathBuf>,
/// Optional CORS allowed origin.
///
/// When set, cross-origin requests are restricted to this origin.
/// Defaults to "http://localhost" if not specified. Use "*" to allow
/// all origins (not recommended for production).
pub cors_origin: Option<String>,
}
/// Application state shared across all routes.
@@ -661,8 +668,11 @@ fn check_bearer_auth(
return pwhash::unix::verify(provided_password, hash);
}
// Otherwise, do direct comparison
provided_password == expected_password
// Otherwise, do constant-time comparison to prevent timing attacks
provided_password
.as_bytes()
.ct_eq(expected_password.as_bytes())
.into()
}
/// Validates basic authentication credentials.
@@ -704,9 +714,12 @@ fn check_basic_auth(
return pwhash::unix::verify(provided_password, hash);
}
// Otherwise, do direct comparison
// Otherwise, do constant-time comparison to prevent timing attacks
let expected_credentials = format!("keep:{expected_password}");
return decoded_str == expected_credentials;
return decoded_str
.as_bytes()
.ct_eq(expected_credentials.as_bytes())
.into();
}
}
}

42
src/modes/server/mod.rs
View File

@@ -54,6 +54,7 @@ pub fn mode_server(
password_hash: settings.server_password_hash(),
cert_file: settings.server_cert_file(),
key_file: settings.server_key_file(),
cors_origin: settings.server_cors_origin(),
};
// Create ItemService once
@@ -122,6 +123,31 @@ async fn run_server(
create_auth_middleware(config.password.clone(), config.password_hash.clone()),
));
// Build CORS layer - restricted by default, configurable via cors_origin setting
let cors_origin = config.cors_origin.as_deref().unwrap_or("http://localhost");
let cors_layer = if cors_origin == "*" {
CorsLayer::permissive()
} else {
CorsLayer::new()
.allow_origin(
cors_origin
.parse::<axum::http::HeaderValue>()
.unwrap_or_else(|_| {
log::warn!(
"Invalid CORS origin '{cors_origin}', defaulting to http://localhost"
);
"http://localhost".parse().unwrap()
}),
)
.allow_methods([
axum::http::Method::GET,
axum::http::Method::POST,
axum::http::Method::PUT,
axum::http::Method::DELETE,
])
.allow_headers(tower_http::cors::Any)
};
// Create the app with documentation routes open and others protected
let app = Router::new()
// Add documentation routes without authentication
@@ -135,11 +161,25 @@ async fn run_server(
.layer(
ServiceBuilder::new()
.layer(TraceLayer::new_for_http())
.layer(CorsLayer::permissive()),
.layer(cors_layer),
);
let addr: SocketAddr = bind_address.parse()?;
// Warn if password auth is enabled without TLS
if config.password.is_some() || config.password_hash.is_some() {
#[cfg(not(feature = "tls"))]
log::warn!(
"SECURITY: Password authentication enabled but TLS support is not compiled in. Password will be transmitted in plain text!"
);
#[cfg(feature = "tls")]
if config.cert_file.is_none() || config.key_file.is_none() {
log::warn!(
"SECURITY: Password authentication enabled but TLS is not configured. Password will be transmitted in plain text!"
);
}
}
// Build the app into a service
let service = app.into_make_service_with_connect_info::<SocketAddr>();

14
src/modes/status.rs
View File

@@ -78,7 +78,9 @@ fn build_meta_plugins_configured_table(status_info: &StatusInfo) -> Option<Table
};
// First, create a default plugin to get its default options
let default_plugin = get_meta_plugin(meta_plugin_type.clone(), None, None);
let Ok(default_plugin) = get_meta_plugin(meta_plugin_type.clone(), None, None) else {
continue;
};
// Start with the default options
let mut effective_options = default_plugin.options().clone();
@@ -96,14 +98,18 @@ fn build_meta_plugins_configured_table(status_info: &StatusInfo) -> Option<Table
.collect();
// Create the actual plugin with merged options - the constructor will handle setting up outputs
let actual_plugin = get_meta_plugin(
let Ok(actual_plugin) = get_meta_plugin(
meta_plugin_type.clone(),
Some(effective_options.clone()),
Some(outputs_converted),
);
) else {
continue;
};
// Get the default plugin to see its default options
let default_plugin = get_meta_plugin(meta_plugin_type.clone(), None, None);
let Ok(default_plugin) = get_meta_plugin(meta_plugin_type.clone(), None, None) else {
continue;
};
// Start with the default options
let mut all_options = default_plugin.options().clone();

4
src/modes/status_plugins.rs
View File

@@ -92,7 +92,9 @@ fn build_meta_plugin_table(
};
// Create a default plugin to get its default options
let default_plugin = get_meta_plugin(meta_plugin_type.clone(), None, None);
let Ok(default_plugin) = get_meta_plugin(meta_plugin_type.clone(), None, None) else {
continue;
};
// Get and sort options
let mut options: Vec<_> = default_plugin.options().iter().collect();