mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-27 08:00:17 +00:00
feat(workspace): layered memory with sensitivity-based privacy redirect (#1112)
* feat(workspace): layered memory with sensitivity-based privacy redirect Introduce MemoryLayer type for named memory layers with sensitivity levels and write permissions. Layers map to synthetic user_id values in workspace tables, enabling shared/private memory isolation. - Add MemoryLayer, LayerSensitivity types with default_for_user() - Add layer-aware write methods (write_to_layer, append_to_layer) - Add PatternPrivacyClassifier to guard shared layer writes - Add optional 'layer' parameter to memory_write tool and HTTP API - Add 'redirected' and 'actual_layer' fields to write response - Add MEMORY_LAYERS env var (JSON) for layer configuration - Workspace user_id now derived from GATEWAY_USER_ID (was hardcoded "default") - 10 integration tests for layered memory operations Addresses prerequisite for Issue #59 (multi-tenancy). Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add explicit default to memory_write layer schema Add "default": "private" to the layer parameter's JSON schema so LLM tool consumers can see the default without reading code. Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: extract resolve_layer_target to deduplicate layer writes Consolidate shared layer-lookup, writable check, and privacy classification logic from write_to_layer and append_to_layer into a single resolve_layer_target helper. Flagged on #349 review — the duplication originates in this PR. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address review feedback on layered memory PR - Fix email regex pipe bug in TLD character class (privacy.rs) - Add append support to web memory_write handler via `append` field - Validate MemoryLayer name/scope: reject empty, check duplicates - Remove hardcoded 'private' default from tool schema; omit layer fields from output when no layer specified - Document scope isolation risk for multi-tenant (Issue #59) Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address adversarial review findings - CRITICAL: fix identity file protection bypass via trailing slash (normalize target path before protection checks) - HIGH: check private layer is writable before privacy redirect - HIGH: map LayerNotFound/ReadOnly to proper 4xx HTTP status codes - HIGH: honor `append` field in non-layer HTTP write path - MEDIUM: remove redundant DB fetch in append_to_layer (narrower TOCTOU window) - MEDIUM: remove dead memory_write_handler from handlers/memory.rs Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: opt-in privacy classifier, force override, confidence scoring Address review feedback from @zmanian: - Privacy classifier is now opt-in via with_privacy_classifier() instead of always-on. Default hardcoded patterns (doctor, therapy, email, phone) had unacceptable false positive rates in household contexts. LLM chooses the correct layer via system prompt; regex can't improve on that. - Add ConfigurablePrivacyClassifier for operator-supplied patterns. - PatternPrivacyClassifier defaults narrowed to hard PII only (SSN, credit card, credentials). - Add force param to write_to_layer/append_to_layer to skip classifier. - PrivacyClassifier trait returns SensitivityResult { is_sensitive, confidence } instead of bool, ready for probabilistic classifiers. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: remove redundant heartbeat match arm in memory_write The heartbeat arm was identical to the catch-all — resolved_path already points to paths::HEARTBEAT when target is "heartbeat". Addresses review feedback from gemini-code-assist on #1112. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: return Result from PatternPrivacyClassifier::new() Replace .expect() with proper error propagation per project no-panics policy. Remove Default impl (unused in production). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * refactor: move memory_layers from GatewayConfig to WorkspaceConfig Resolve merge conflicts between HEAD (transcription, search, env helpers) and the workspace config branch. GatewayConfig no longer owns memory_layers; WorkspaceConfig::resolve() handles parsing, validation (name length >64, character set, empty scope, duplicates), and fallback defaults. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test: strengthen privacy classifier and layer isolation coverage Add 8 privacy classifier edge case tests (format variants, keywords, longer documents, empty/partial inputs) and 5 layer write isolation integration tests (cross-scope invisibility, overwrite, empty path, sensitive-to-private no-redirect). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: tautological test assertion and add WorkspaceConfig validation tests Replace always-true `is_ok() || is_err()` in write_empty_path_to_layer with actual behavior assertion (write succeeds with normalized empty path). Add 8 unit tests for WorkspaceConfig::resolve() covering valid JSON parsing, invalid JSON, empty/long/invalid-char layer names, empty scopes, duplicates, and default fallback behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: cargo fmt after staging merge Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]> Co-authored-by: [email protected] <[email protected]>
This commit is contained in:
standardtoaster
@@ -111,6 +111,10 @@ impl ChannelsConfig {
|
||||
|
||||
let gateway_enabled = parse_bool_env("GATEWAY_ENABLED", cs.gateway_enabled)?;
|
||||
let gateway = if gateway_enabled {
|
||||
let user_id = optional_env("GATEWAY_USER_ID")?
|
||||
.or_else(|| cs.gateway_user_id.clone())
|
||||
.unwrap_or_else(|| "default".to_string());
|
||||
|
||||
Some(GatewayConfig {
|
||||
host: optional_env("GATEWAY_HOST")?
|
||||
.or_else(|| cs.gateway_host.clone())
|
||||
@@ -121,7 +125,7 @@ impl ChannelsConfig {
|
||||
)?,
|
||||
auth_token: optional_env("GATEWAY_AUTH_TOKEN")?
|
||||
.or_else(|| cs.gateway_auth_token.clone()),
|
||||
user_id: owner_id.to_string(),
|
||||
user_id,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
|
||||
+17
-2
@@ -24,6 +24,7 @@ mod skills;
|
||||
mod transcription;
|
||||
mod tunnel;
|
||||
mod wasm;
|
||||
mod workspace;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::{LazyLock, Mutex, Once};
|
||||
@@ -53,6 +54,7 @@ pub use self::skills::SkillsConfig;
|
||||
pub use self::transcription::TranscriptionConfig;
|
||||
pub use self::tunnel::TunnelConfig;
|
||||
pub use self::wasm::WasmConfig;
|
||||
pub use self::workspace::WorkspaceConfig;
|
||||
pub use crate::llm::config::{
|
||||
BedrockConfig, CacheRetention, LlmConfig, NearAiConfig, OAUTH_PLACEHOLDER, OpenAiCodexConfig,
|
||||
RegistryProviderConfig,
|
||||
@@ -98,6 +100,7 @@ pub struct Config {
|
||||
pub skills: SkillsConfig,
|
||||
pub transcription: TranscriptionConfig,
|
||||
pub search: WorkspaceSearchConfig,
|
||||
pub workspace: WorkspaceConfig,
|
||||
pub observability: crate::observability::ObservabilityConfig,
|
||||
/// Channel-relay integration (Slack via external relay service).
|
||||
/// Present only when both `CHANNEL_RELAY_URL` and `CHANNEL_RELAY_API_KEY` are set.
|
||||
@@ -175,6 +178,9 @@ impl Config {
|
||||
},
|
||||
transcription: TranscriptionConfig::default(),
|
||||
search: WorkspaceSearchConfig::default(),
|
||||
workspace: WorkspaceConfig {
|
||||
memory_layers: vec![],
|
||||
},
|
||||
observability: crate::observability::ObservabilityConfig::default(),
|
||||
relay: None,
|
||||
}
|
||||
@@ -305,13 +311,21 @@ impl Config {
|
||||
async fn build(settings: &Settings) -> Result<Self, ConfigError> {
|
||||
let owner_id = resolve_owner_id(settings)?;
|
||||
|
||||
let tunnel = TunnelConfig::resolve(settings)?;
|
||||
let channels = ChannelsConfig::resolve(settings, &owner_id)?;
|
||||
let workspace_user_id = channels
|
||||
.gateway
|
||||
.as_ref()
|
||||
.map(|gw| gw.user_id.clone())
|
||||
.unwrap_or_else(|| "default".to_string());
|
||||
|
||||
Ok(Self {
|
||||
owner_id: owner_id.clone(),
|
||||
database: DatabaseConfig::resolve()?,
|
||||
llm: LlmConfig::resolve(settings)?,
|
||||
embeddings: EmbeddingsConfig::resolve(settings)?,
|
||||
tunnel: TunnelConfig::resolve(settings)?,
|
||||
channels: ChannelsConfig::resolve(settings, &owner_id)?,
|
||||
tunnel,
|
||||
channels,
|
||||
agent: AgentConfig::resolve(settings)?,
|
||||
safety: resolve_safety_config(settings)?,
|
||||
wasm: WasmConfig::resolve(settings)?,
|
||||
@@ -325,6 +339,7 @@ impl Config {
|
||||
skills: SkillsConfig::resolve()?,
|
||||
transcription: TranscriptionConfig::resolve(settings)?,
|
||||
search: WorkspaceSearchConfig::resolve()?,
|
||||
workspace: WorkspaceConfig::resolve(&workspace_user_id)?,
|
||||
observability: crate::observability::ObservabilityConfig {
|
||||
backend: std::env::var("OBSERVABILITY_BACKEND").unwrap_or_else(|_| "none".into()),
|
||||
},
|
||||
|
||||
@@ -0,0 +1,208 @@
|
||||
use crate::config::helpers::optional_env;
|
||||
use crate::error::ConfigError;
|
||||
use crate::workspace::layer::MemoryLayer;
|
||||
|
||||
/// Workspace memory configuration.
|
||||
///
|
||||
/// Controls memory layer definitions for privacy-aware writes.
|
||||
/// Layers are parsed from the `MEMORY_LAYERS` env var (JSON array)
|
||||
/// or default to a single private layer scoped to the gateway user.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct WorkspaceConfig {
|
||||
pub memory_layers: Vec<MemoryLayer>,
|
||||
}
|
||||
|
||||
impl WorkspaceConfig {
|
||||
pub(crate) fn resolve(user_id: &str) -> Result<Self, ConfigError> {
|
||||
let memory_layers: Vec<MemoryLayer> = match optional_env("MEMORY_LAYERS")? {
|
||||
Some(json_str) => {
|
||||
serde_json::from_str(&json_str).map_err(|e| ConfigError::InvalidValue {
|
||||
key: "MEMORY_LAYERS".to_string(),
|
||||
message: format!("must be valid JSON array of layer objects: {e}"),
|
||||
})?
|
||||
}
|
||||
None => MemoryLayer::default_for_user(user_id),
|
||||
};
|
||||
|
||||
// Validate layer names and scopes
|
||||
for layer in &memory_layers {
|
||||
if layer.name.trim().is_empty() {
|
||||
return Err(ConfigError::InvalidValue {
|
||||
key: "MEMORY_LAYERS".to_string(),
|
||||
message: "layer name must not be empty".to_string(),
|
||||
});
|
||||
}
|
||||
if layer.name.len() > 64 {
|
||||
return Err(ConfigError::InvalidValue {
|
||||
key: "MEMORY_LAYERS".to_string(),
|
||||
message: format!("layer name '{}' exceeds 64 characters", layer.name),
|
||||
});
|
||||
}
|
||||
if !layer
|
||||
.name
|
||||
.chars()
|
||||
.all(|c| c.is_alphanumeric() || c == '_' || c == '-')
|
||||
{
|
||||
return Err(ConfigError::InvalidValue {
|
||||
key: "MEMORY_LAYERS".to_string(),
|
||||
message: format!(
|
||||
"layer name '{}' contains invalid characters (only alphanumeric, _, - allowed)",
|
||||
layer.name
|
||||
),
|
||||
});
|
||||
}
|
||||
if layer.scope.trim().is_empty() {
|
||||
return Err(ConfigError::InvalidValue {
|
||||
key: "MEMORY_LAYERS".to_string(),
|
||||
message: format!("layer '{}' has an empty scope", layer.name),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Check for duplicate layer names
|
||||
{
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
for layer in &memory_layers {
|
||||
if !seen.insert(&layer.name) {
|
||||
return Err(ConfigError::InvalidValue {
|
||||
key: "MEMORY_LAYERS".to_string(),
|
||||
message: format!("duplicate layer name '{}'", layer.name),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(Self { memory_layers })
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::Mutex;
|
||||
|
||||
// Serialize env-var-dependent tests to avoid races.
|
||||
static ENV_LOCK: Mutex<()> = Mutex::new(());
|
||||
|
||||
fn with_env(key: &str, val: Option<&str>, f: impl FnOnce()) {
|
||||
let _guard = ENV_LOCK.lock().unwrap();
|
||||
let prev = std::env::var(key).ok();
|
||||
match val {
|
||||
Some(v) => unsafe { std::env::set_var(key, v) },
|
||||
None => unsafe { std::env::remove_var(key) },
|
||||
}
|
||||
f();
|
||||
match prev {
|
||||
Some(v) => unsafe { std::env::set_var(key, v) },
|
||||
None => unsafe { std::env::remove_var(key) },
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn valid_json_parses_correctly() {
|
||||
let json = r#"[{"name":"private","scope":"alice","writable":true,"sensitivity":"private"},{"name":"shared","scope":"shared","writable":true,"sensitivity":"shared"}]"#;
|
||||
with_env("MEMORY_LAYERS", Some(json), || {
|
||||
let config = WorkspaceConfig::resolve("alice").expect("should parse");
|
||||
assert_eq!(config.memory_layers.len(), 2);
|
||||
assert_eq!(config.memory_layers[0].name, "private");
|
||||
assert_eq!(config.memory_layers[1].name, "shared");
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn invalid_json_returns_error() {
|
||||
with_env("MEMORY_LAYERS", Some("not json"), || {
|
||||
let result = WorkspaceConfig::resolve("alice");
|
||||
assert!(result.is_err(), "invalid JSON should fail");
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("valid JSON"),
|
||||
"error should mention JSON: {err}"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_layer_name_returns_error() {
|
||||
let json = r#"[{"name":"","scope":"alice"}]"#;
|
||||
with_env("MEMORY_LAYERS", Some(json), || {
|
||||
let result = WorkspaceConfig::resolve("alice");
|
||||
assert!(result.is_err(), "empty layer name should fail");
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(err.contains("empty"), "error should mention empty: {err}");
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layer_name_exceeding_64_chars_returns_error() {
|
||||
let long_name = "a".repeat(65);
|
||||
let json = format!(r#"[{{"name":"{long_name}","scope":"alice"}}]"#);
|
||||
with_env("MEMORY_LAYERS", Some(&json), || {
|
||||
let result = WorkspaceConfig::resolve("alice");
|
||||
assert!(result.is_err(), "long layer name should fail");
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("exceeds 64"),
|
||||
"error should mention 64 chars: {err}"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn layer_name_with_invalid_chars_returns_error() {
|
||||
for bad_name in ["has space", "has@at", "has.dot", "has/slash"] {
|
||||
let json = format!(r#"[{{"name":"{bad_name}","scope":"alice"}}]"#);
|
||||
with_env("MEMORY_LAYERS", Some(&json), || {
|
||||
let result = WorkspaceConfig::resolve("alice");
|
||||
assert!(
|
||||
result.is_err(),
|
||||
"layer name '{bad_name}' should fail validation"
|
||||
);
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("invalid characters"),
|
||||
"error for '{bad_name}' should mention invalid characters: {err}"
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn empty_scope_returns_error() {
|
||||
let json = r#"[{"name":"private","scope":""}]"#;
|
||||
with_env("MEMORY_LAYERS", Some(json), || {
|
||||
let result = WorkspaceConfig::resolve("alice");
|
||||
assert!(result.is_err(), "empty scope should fail");
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("empty scope"),
|
||||
"error should mention empty scope: {err}"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn duplicate_layer_names_returns_error() {
|
||||
let json = r#"[{"name":"private","scope":"alice"},{"name":"private","scope":"bob"}]"#;
|
||||
with_env("MEMORY_LAYERS", Some(json), || {
|
||||
let result = WorkspaceConfig::resolve("alice");
|
||||
assert!(result.is_err(), "duplicate names should fail");
|
||||
let err = result.unwrap_err().to_string();
|
||||
assert!(
|
||||
err.contains("duplicate"),
|
||||
"error should mention duplicate: {err}"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_env_defaults_to_single_private_layer() {
|
||||
with_env("MEMORY_LAYERS", None, || {
|
||||
let config = WorkspaceConfig::resolve("alice").expect("should default");
|
||||
assert_eq!(config.memory_layers.len(), 1);
|
||||
assert_eq!(config.memory_layers[0].name, "private");
|
||||
assert_eq!(config.memory_layers[0].scope, "alice");
|
||||
assert!(config.memory_layers[0].writable);
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user