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]]
|
[[package]]
|
||||||
name = "secrets-mcp"
|
name = "secrets-mcp"
|
||||||
version = "0.2.0"
|
version = "0.2.1"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"askama",
|
"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")
|
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 ──────────────────────────────────────────
|
// ─── Client-supplied key extraction ──────────────────────────────────────────
|
||||||
|
|
||||||
/// Parse a 64-char hex string (from X-Encryption-Key header) into a 32-byte key.
|
/// 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)
|
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 ───────────────────────────────────────────────────────
|
// ─── Public hex helpers ───────────────────────────────────────────────────────
|
||||||
|
|
||||||
pub mod hex {
|
pub mod hex {
|
||||||
@@ -186,22 +130,4 @@ mod tests {
|
|||||||
let dec = decrypt_json(&key, &enc).unwrap();
|
let dec = decrypt_json(&key, &enc).unwrap();
|
||||||
assert_eq!(dec, value);
|
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 INDEX IF NOT EXISTS idx_oauth_accounts_user ON oauth_accounts(user_id);
|
||||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_oauth_accounts_user_provider
|
CREATE UNIQUE INDEX IF NOT EXISTS idx_oauth_accounts_user_provider
|
||||||
ON oauth_accounts(user_id, 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)
|
.execute(pool)
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
[package]
|
[package]
|
||||||
name = "secrets-mcp"
|
name = "secrets-mcp"
|
||||||
version = "0.2.0"
|
version = "0.2.1"
|
||||||
edition.workspace = true
|
edition.workspace = true
|
||||||
|
|
||||||
[[bin]]
|
[[bin]]
|
||||||
|
|||||||
@@ -5,11 +5,11 @@ use axum::{
|
|||||||
body::{Body, Bytes, to_bytes},
|
body::{Body, Bytes, to_bytes},
|
||||||
extract::{ConnectInfo, Request},
|
extract::{ConnectInfo, Request},
|
||||||
http::{
|
http::{
|
||||||
HeaderMap, Method,
|
HeaderMap, Method, StatusCode,
|
||||||
header::{CONTENT_LENGTH, CONTENT_TYPE, USER_AGENT},
|
header::{CONTENT_LENGTH, CONTENT_TYPE, USER_AGENT},
|
||||||
},
|
},
|
||||||
middleware::Next,
|
middleware::Next,
|
||||||
response::Response,
|
response::{IntoResponse, Response},
|
||||||
};
|
};
|
||||||
|
|
||||||
/// Axum middleware that logs structured info for every HTTP request.
|
/// 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) => {
|
Err(e) => {
|
||||||
tracing::warn!(path, error = %e, "failed to buffer MCP request body for logging");
|
tracing::warn!(path, error = %e, "failed to buffer MCP request body for logging");
|
||||||
// Reconstruct with empty body; request was consumed — return 500.
|
let elapsed = start.elapsed().as_millis();
|
||||||
// This branch is highly unlikely in practice.
|
tracing::info!(
|
||||||
let resp = next.run(Request::from_parts(parts, Body::empty())).await;
|
method = method.as_str(),
|
||||||
return resp;
|
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]
|
#[tool_router]
|
||||||
impl SecretsService {
|
impl SecretsService {
|
||||||
#[tool(
|
#[tool(
|
||||||
description = "Search entries in the secrets store. Returns entries with metadata and \
|
description = "Search entries in the secrets store. Requires Bearer API key. Returns \
|
||||||
secret field names (not values). Use secrets_get to decrypt secret values.",
|
entries with metadata and secret field names (not values). Use secrets_get to decrypt secret values.",
|
||||||
annotations(
|
annotations(
|
||||||
title = "Search Secrets",
|
title = "Search Secrets",
|
||||||
read_only_hint = true,
|
read_only_hint = true,
|
||||||
@@ -312,7 +312,7 @@ impl SecretsService {
|
|||||||
ctx: RequestContext<RoleServer>,
|
ctx: RequestContext<RoleServer>,
|
||||||
) -> Result<CallToolResult, rmcp::ErrorData> {
|
) -> Result<CallToolResult, rmcp::ErrorData> {
|
||||||
let t = Instant::now();
|
let t = Instant::now();
|
||||||
let user_id = Self::user_id_from_ctx(&ctx)?;
|
let user_id = Self::require_user_id(&ctx)?;
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
tool = "secrets_search",
|
tool = "secrets_search",
|
||||||
?user_id,
|
?user_id,
|
||||||
@@ -334,11 +334,11 @@ impl SecretsService {
|
|||||||
sort: input.sort.as_deref().unwrap_or("name"),
|
sort: input.sort.as_deref().unwrap_or("name"),
|
||||||
limit: input.limit.unwrap_or(20),
|
limit: input.limit.unwrap_or(20),
|
||||||
offset: input.offset.unwrap_or(0),
|
offset: input.offset.unwrap_or(0),
|
||||||
user_id,
|
user_id: Some(user_id),
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
.await
|
.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 summary = input.summary.unwrap_or(false);
|
||||||
let entries: Vec<serde_json::Value> = result
|
let entries: Vec<serde_json::Value> = result
|
||||||
@@ -849,7 +849,7 @@ impl ServerHandler for SecretsService {
|
|||||||
"Manage cross-device secrets and configuration securely. \
|
"Manage cross-device secrets and configuration securely. \
|
||||||
Data is encrypted with your passphrase-derived key. \
|
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. \
|
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, \
|
secrets_get to decrypt secret values, \
|
||||||
and secrets_add/secrets_update to write encrypted secrets."
|
and secrets_add/secrets_update to write encrypted secrets."
|
||||||
.to_string(),
|
.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