mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
* 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]>
209 lines
7.7 KiB
Rust
209 lines
7.7 KiB
Rust
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);
|
|
});
|
|
}
|
|
}
|