feat(core): FK for user_id columns; MCP search requires user
- Add fk_entries_user_id, fk_entries_history_user_id, fk_audit_log_user_id (ON DELETE SET NULL) - Add scripts/cleanup-orphan-user-ids.sql for pre-deploy orphan user_id cleanup - Remove deprecated SERVER_MASTER_KEY / per-user key wrap helpers from secrets-core - secrets-mcp: require authenticated user for secrets_search; improve body-read failure response - Bump secrets-mcp to 0.2.1 Made-with: Cursor
This commit is contained in:
2
Cargo.lock
generated
2
Cargo.lock
generated
@@ -1968,7 +1968,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "secrets-mcp"
|
||||
version = "0.2.0"
|
||||
version = "0.2.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"askama",
|
||||
|
||||
@@ -55,35 +55,6 @@ pub fn decrypt_json(master_key: &[u8; 32], data: &[u8]) -> Result<Value> {
|
||||
serde_json::from_slice(&bytes).context("deserialize decrypted JSON")
|
||||
}
|
||||
|
||||
// ─── Per-user key management (DEPRECATED — kept only for migration) ───────────
|
||||
|
||||
/// Generate a new random 32-byte per-user encryption key.
|
||||
#[allow(dead_code)]
|
||||
pub fn generate_user_key() -> [u8; 32] {
|
||||
use aes_gcm::aead::rand_core::RngCore;
|
||||
let mut key = [0u8; 32];
|
||||
OsRng.fill_bytes(&mut key);
|
||||
key
|
||||
}
|
||||
|
||||
/// Wrap a per-user key with the server master key using AES-256-GCM.
|
||||
#[allow(dead_code)]
|
||||
pub fn wrap_user_key(server_master_key: &[u8; 32], user_key: &[u8; 32]) -> Result<Vec<u8>> {
|
||||
encrypt(server_master_key, user_key.as_ref())
|
||||
}
|
||||
|
||||
/// Unwrap a per-user key using the server master key.
|
||||
#[allow(dead_code)]
|
||||
pub fn unwrap_user_key(server_master_key: &[u8; 32], wrapped: &[u8]) -> Result<[u8; 32]> {
|
||||
let bytes = decrypt(server_master_key, wrapped)?;
|
||||
if bytes.len() != 32 {
|
||||
bail!("unwrapped user key has unexpected length {}", bytes.len());
|
||||
}
|
||||
let mut key = [0u8; 32];
|
||||
key.copy_from_slice(&bytes);
|
||||
Ok(key)
|
||||
}
|
||||
|
||||
// ─── Client-supplied key extraction ──────────────────────────────────────────
|
||||
|
||||
/// Parse a 64-char hex string (from X-Encryption-Key header) into a 32-byte key.
|
||||
@@ -100,33 +71,6 @@ pub fn extract_key_from_hex(hex_str: &str) -> Result<[u8; 32]> {
|
||||
Ok(key)
|
||||
}
|
||||
|
||||
// ─── Server master key ────────────────────────────────────────────────────────
|
||||
|
||||
/// Load the server master key from `SERVER_MASTER_KEY` environment variable (64 hex chars).
|
||||
pub fn load_master_key_auto() -> Result<[u8; 32]> {
|
||||
let hex_str = std::env::var("SERVER_MASTER_KEY").map_err(|_| {
|
||||
anyhow::anyhow!(
|
||||
"SERVER_MASTER_KEY is not set. \
|
||||
Generate one with: openssl rand -hex 32"
|
||||
)
|
||||
})?;
|
||||
|
||||
if hex_str.is_empty() {
|
||||
bail!("SERVER_MASTER_KEY is set but empty");
|
||||
}
|
||||
|
||||
let bytes = hex::decode_hex(hex_str.trim())?;
|
||||
if bytes.len() != 32 {
|
||||
bail!(
|
||||
"SERVER_MASTER_KEY must be 64 hex chars (32 bytes), got {} bytes",
|
||||
bytes.len()
|
||||
);
|
||||
}
|
||||
let mut key = [0u8; 32];
|
||||
key.copy_from_slice(&bytes);
|
||||
Ok(key)
|
||||
}
|
||||
|
||||
// ─── Public hex helpers ───────────────────────────────────────────────────────
|
||||
|
||||
pub mod hex {
|
||||
@@ -186,22 +130,4 @@ mod tests {
|
||||
let dec = decrypt_json(&key, &enc).unwrap();
|
||||
assert_eq!(dec, value);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_key_wrap_unwrap_roundtrip() {
|
||||
let server_key = [0xABu8; 32];
|
||||
let user_key = [0xCDu8; 32];
|
||||
let wrapped = wrap_user_key(&server_key, &user_key).unwrap();
|
||||
let unwrapped = unwrap_user_key(&server_key, &wrapped).unwrap();
|
||||
assert_eq!(unwrapped, user_key);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_key_wrap_wrong_server_key_fails() {
|
||||
let server_key1 = [0xABu8; 32];
|
||||
let server_key2 = [0xEFu8; 32];
|
||||
let user_key = [0xCDu8; 32];
|
||||
let wrapped = wrap_user_key(&server_key1, &user_key).unwrap();
|
||||
assert!(unwrap_user_key(&server_key2, &wrapped).is_err());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -156,6 +156,37 @@ pub async fn migrate(pool: &PgPool) -> Result<()> {
|
||||
CREATE INDEX IF NOT EXISTS idx_oauth_accounts_user ON oauth_accounts(user_id);
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_oauth_accounts_user_provider
|
||||
ON oauth_accounts(user_id, provider);
|
||||
|
||||
-- FK: user_id columns -> users(id) (nullable = legacy rows; ON DELETE SET NULL)
|
||||
DO $$ BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM pg_constraint WHERE conname = 'fk_entries_user_id'
|
||||
) THEN
|
||||
ALTER TABLE entries
|
||||
ADD CONSTRAINT fk_entries_user_id
|
||||
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
DO $$ BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM pg_constraint WHERE conname = 'fk_entries_history_user_id'
|
||||
) THEN
|
||||
ALTER TABLE entries_history
|
||||
ADD CONSTRAINT fk_entries_history_user_id
|
||||
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
DO $$ BEGIN
|
||||
IF NOT EXISTS (
|
||||
SELECT 1 FROM pg_constraint WHERE conname = 'fk_audit_log_user_id'
|
||||
) THEN
|
||||
ALTER TABLE audit_log
|
||||
ADD CONSTRAINT fk_audit_log_user_id
|
||||
FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL;
|
||||
END IF;
|
||||
END $$;
|
||||
"#,
|
||||
)
|
||||
.execute(pool)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "secrets-mcp"
|
||||
version = "0.2.0"
|
||||
version = "0.2.1"
|
||||
edition.workspace = true
|
||||
|
||||
[[bin]]
|
||||
|
||||
@@ -5,11 +5,11 @@ use axum::{
|
||||
body::{Body, Bytes, to_bytes},
|
||||
extract::{ConnectInfo, Request},
|
||||
http::{
|
||||
HeaderMap, Method,
|
||||
HeaderMap, Method, StatusCode,
|
||||
header::{CONTENT_LENGTH, CONTENT_TYPE, USER_AGENT},
|
||||
},
|
||||
middleware::Next,
|
||||
response::Response,
|
||||
response::{IntoResponse, Response},
|
||||
};
|
||||
|
||||
/// Axum middleware that logs structured info for every HTTP request.
|
||||
@@ -68,10 +68,23 @@ pub async fn request_logging_middleware(req: Request, next: Next) -> Response {
|
||||
}
|
||||
Err(e) => {
|
||||
tracing::warn!(path, error = %e, "failed to buffer MCP request body for logging");
|
||||
// Reconstruct with empty body; request was consumed — return 500.
|
||||
// This branch is highly unlikely in practice.
|
||||
let resp = next.run(Request::from_parts(parts, Body::empty())).await;
|
||||
return resp;
|
||||
let elapsed = start.elapsed().as_millis();
|
||||
tracing::info!(
|
||||
method = method.as_str(),
|
||||
path,
|
||||
status = StatusCode::INTERNAL_SERVER_ERROR.as_u16(),
|
||||
elapsed_ms = elapsed,
|
||||
client_ip = ip.as_deref(),
|
||||
ua = ua.as_deref(),
|
||||
content_length = content_len,
|
||||
mcp_session = mcp_session.as_deref(),
|
||||
"mcp request",
|
||||
);
|
||||
return (
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"failed to read request body",
|
||||
)
|
||||
.into_response();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -298,8 +298,8 @@ struct EnvMapInput {
|
||||
#[tool_router]
|
||||
impl SecretsService {
|
||||
#[tool(
|
||||
description = "Search entries in the secrets store. Returns entries with metadata and \
|
||||
secret field names (not values). Use secrets_get to decrypt secret values.",
|
||||
description = "Search entries in the secrets store. Requires Bearer API key. Returns \
|
||||
entries with metadata and secret field names (not values). Use secrets_get to decrypt secret values.",
|
||||
annotations(
|
||||
title = "Search Secrets",
|
||||
read_only_hint = true,
|
||||
@@ -312,7 +312,7 @@ impl SecretsService {
|
||||
ctx: RequestContext<RoleServer>,
|
||||
) -> Result<CallToolResult, rmcp::ErrorData> {
|
||||
let t = Instant::now();
|
||||
let user_id = Self::user_id_from_ctx(&ctx)?;
|
||||
let user_id = Self::require_user_id(&ctx)?;
|
||||
tracing::info!(
|
||||
tool = "secrets_search",
|
||||
?user_id,
|
||||
@@ -334,11 +334,11 @@ impl SecretsService {
|
||||
sort: input.sort.as_deref().unwrap_or("name"),
|
||||
limit: input.limit.unwrap_or(20),
|
||||
offset: input.offset.unwrap_or(0),
|
||||
user_id,
|
||||
user_id: Some(user_id),
|
||||
},
|
||||
)
|
||||
.await
|
||||
.map_err(|e| mcp_err_internal_logged("secrets_search", user_id, e))?;
|
||||
.map_err(|e| mcp_err_internal_logged("secrets_search", Some(user_id), e))?;
|
||||
|
||||
let summary = input.summary.unwrap_or(false);
|
||||
let entries: Vec<serde_json::Value> = result
|
||||
@@ -849,7 +849,7 @@ impl ServerHandler for SecretsService {
|
||||
"Manage cross-device secrets and configuration securely. \
|
||||
Data is encrypted with your passphrase-derived key. \
|
||||
Include your 64-char hex key in the X-Encryption-Key header for all read/write operations. \
|
||||
Use secrets_search to discover entries (no key needed), \
|
||||
Use secrets_search to discover entries (Bearer token required; encryption key not needed), \
|
||||
secrets_get to decrypt secret values, \
|
||||
and secrets_add/secrets_update to write encrypted secrets."
|
||||
.to_string(),
|
||||
|
||||
22
scripts/cleanup-orphan-user-ids.sql
Normal file
22
scripts/cleanup-orphan-user-ids.sql
Normal file
@@ -0,0 +1,22 @@
|
||||
-- Run against prod BEFORE deploying secrets-mcp with FK migration.
|
||||
-- Requires: write access to SECRETS_DATABASE_URL.
|
||||
-- Example: psql "$SECRETS_DATABASE_URL" -v ON_ERROR_STOP=1 -f scripts/cleanup-orphan-user-ids.sql
|
||||
|
||||
BEGIN;
|
||||
|
||||
UPDATE entries
|
||||
SET user_id = NULL
|
||||
WHERE user_id IS NOT NULL
|
||||
AND NOT EXISTS (SELECT 1 FROM users u WHERE u.id = entries.user_id);
|
||||
|
||||
UPDATE entries_history
|
||||
SET user_id = NULL
|
||||
WHERE user_id IS NOT NULL
|
||||
AND NOT EXISTS (SELECT 1 FROM users u WHERE u.id = entries_history.user_id);
|
||||
|
||||
UPDATE audit_log
|
||||
SET user_id = NULL
|
||||
WHERE user_id IS NOT NULL
|
||||
AND NOT EXISTS (SELECT 1 FROM users u WHERE u.id = audit_log.user_id);
|
||||
|
||||
COMMIT;
|
||||
Reference in New Issue
Block a user