fix: enforce workspace memory settings contract
This commit is contained in:
@@ -449,6 +449,17 @@ pub struct WebFetchConfig {
|
|||||||
pub allow_private_addresses: Option<bool>,
|
pub allow_private_addresses: Option<bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Maximum Unicode scalar values accepted in a normalized Workspace Memory language name.
|
||||||
|
pub const MAX_WORKSPACE_MEMORY_LANGUAGE_CHARS: usize = 64;
|
||||||
|
|
||||||
|
/// Return whether a Workspace Memory language is already normalized and safe to persist.
|
||||||
|
pub fn is_normalized_workspace_memory_language(language: &str) -> bool {
|
||||||
|
!language.is_empty()
|
||||||
|
&& language == language.trim()
|
||||||
|
&& language.chars().count() <= MAX_WORKSPACE_MEMORY_LANGUAGE_CHARS
|
||||||
|
&& !language.chars().any(char::is_control)
|
||||||
|
}
|
||||||
|
|
||||||
/// Immutable Workspace Memory settings bound into a Worker launch snapshot.
|
/// Immutable Workspace Memory settings bound into a Worker launch snapshot.
|
||||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
#[serde(deny_unknown_fields)]
|
#[serde(deny_unknown_fields)]
|
||||||
@@ -536,7 +547,7 @@ pub struct MemoryConfig {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl MemoryConfig {
|
impl MemoryConfig {
|
||||||
/// Replace any profile-authored language fields with a trusted Workspace snapshot.
|
/// Replace any untrusted manifest values with a trusted Workspace snapshot.
|
||||||
pub fn bind_workspace_settings(&mut self, snapshot: &WorkspaceMemorySettingsSnapshot) {
|
pub fn bind_workspace_settings(&mut self, snapshot: &WorkspaceMemorySettingsSnapshot) {
|
||||||
self.workspace_id = Some(snapshot.workspace_id.clone());
|
self.workspace_id = Some(snapshot.workspace_id.clone());
|
||||||
self.settings_revision = Some(snapshot.settings_revision);
|
self.settings_revision = Some(snapshot.settings_revision);
|
||||||
@@ -1256,6 +1267,18 @@ model_id = "claude-sonnet-4-20250514"
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn workspace_memory_language_validation_is_bounded_free_form_utf8() {
|
||||||
|
assert!(is_normalized_workspace_memory_language("Français"));
|
||||||
|
assert!(is_normalized_workspace_memory_language("日本語"));
|
||||||
|
assert!(!is_normalized_workspace_memory_language(""));
|
||||||
|
assert!(!is_normalized_workspace_memory_language(" English "));
|
||||||
|
assert!(!is_normalized_workspace_memory_language("English\n"));
|
||||||
|
assert!(!is_normalized_workspace_memory_language(
|
||||||
|
&"x".repeat(MAX_WORKSPACE_MEMORY_LANGUAGE_CHARS + 1)
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn memory_section_with_language() {
|
fn memory_section_with_language() {
|
||||||
let toml = format!("{MINIMAL_REQUIRED}\n[memory]\nlanguage = \"Japanese\"\n");
|
let toml = format!("{MINIMAL_REQUIRED}\n[memory]\nlanguage = \"Japanese\"\n");
|
||||||
|
|||||||
@@ -562,7 +562,7 @@ fn resolve_profile_value(
|
|||||||
mcp: profile.mcp,
|
mcp: profile.mcp,
|
||||||
compaction,
|
compaction,
|
||||||
web: profile.web,
|
web: profile.web,
|
||||||
memory: profile.memory,
|
memory: profile.memory.map(Into::into),
|
||||||
skills: profile.skills,
|
skills: profile.skills,
|
||||||
};
|
};
|
||||||
let config = WorkerManifestConfig::builtin_defaults().merge(config.resolve_paths(profile_dir));
|
let config = WorkerManifestConfig::builtin_defaults().merge(config.resolve_paths(profile_dir));
|
||||||
@@ -582,6 +582,51 @@ fn resolve_profile_value(
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Default, Deserialize)]
|
||||||
|
#[serde(deny_unknown_fields)]
|
||||||
|
struct ProfileMemoryConfig {
|
||||||
|
#[serde(default)]
|
||||||
|
workspace_root: Option<PathBuf>,
|
||||||
|
#[serde(default)]
|
||||||
|
query_result_limit: Option<usize>,
|
||||||
|
#[serde(default)]
|
||||||
|
query_excerpt_lines: Option<usize>,
|
||||||
|
#[serde(default)]
|
||||||
|
inject_summary: Option<bool>,
|
||||||
|
#[serde(default)]
|
||||||
|
extract_model: Option<ModelManifest>,
|
||||||
|
#[serde(default)]
|
||||||
|
extract_threshold: Option<u64>,
|
||||||
|
#[serde(default)]
|
||||||
|
extract_worker_max_turns: Option<u32>,
|
||||||
|
#[serde(default)]
|
||||||
|
consolidation_model: Option<ModelManifest>,
|
||||||
|
#[serde(default)]
|
||||||
|
consolidation_threshold_files: Option<usize>,
|
||||||
|
#[serde(default)]
|
||||||
|
consolidation_threshold_bytes: Option<u64>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl From<ProfileMemoryConfig> for MemoryConfig {
|
||||||
|
fn from(profile: ProfileMemoryConfig) -> Self {
|
||||||
|
Self {
|
||||||
|
workspace_root: profile.workspace_root,
|
||||||
|
query_result_limit: profile.query_result_limit,
|
||||||
|
query_excerpt_lines: profile.query_excerpt_lines,
|
||||||
|
inject_summary: profile.inject_summary,
|
||||||
|
workspace_id: None,
|
||||||
|
settings_revision: None,
|
||||||
|
language: None,
|
||||||
|
extract_model: profile.extract_model,
|
||||||
|
extract_threshold: profile.extract_threshold,
|
||||||
|
extract_worker_max_turns: profile.extract_worker_max_turns,
|
||||||
|
consolidation_model: profile.consolidation_model,
|
||||||
|
consolidation_threshold_files: profile.consolidation_threshold_files,
|
||||||
|
consolidation_threshold_bytes: profile.consolidation_threshold_bytes,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Default, Deserialize)]
|
#[derive(Debug, Default, Deserialize)]
|
||||||
#[serde(deny_unknown_fields)]
|
#[serde(deny_unknown_fields)]
|
||||||
struct ProfileConfig {
|
struct ProfileConfig {
|
||||||
@@ -612,7 +657,7 @@ struct ProfileConfig {
|
|||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
web: Option<WebConfig>,
|
web: Option<WebConfig>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
memory: Option<MemoryConfig>,
|
memory: Option<ProfileMemoryConfig>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
skills: Option<SkillsConfig>,
|
skills: Option<SkillsConfig>,
|
||||||
}
|
}
|
||||||
@@ -1334,6 +1379,34 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn profile_rejects_workspace_memory_snapshot_authority_fields() {
|
||||||
|
let tmp = TempDir::new().unwrap();
|
||||||
|
for (field, value) in [
|
||||||
|
("workspace_id", serde_json::json!("workspace-a")),
|
||||||
|
("settings_revision", serde_json::json!(2)),
|
||||||
|
("language", serde_json::json!("Japanese")),
|
||||||
|
] {
|
||||||
|
let artifact = serde_json::json!({ "memory": { (field): value } });
|
||||||
|
let error = resolve_profile_artifact_value(
|
||||||
|
artifact,
|
||||||
|
ProfileSource::Registry {
|
||||||
|
source: ProfileRegistrySource::Builtin,
|
||||||
|
name: "test".to_string(),
|
||||||
|
path: None,
|
||||||
|
provenance: None,
|
||||||
|
},
|
||||||
|
tmp.path(),
|
||||||
|
"test-worker",
|
||||||
|
)
|
||||||
|
.unwrap_err();
|
||||||
|
assert!(
|
||||||
|
error.to_string().contains("unknown field"),
|
||||||
|
"unexpected error for {field}: {error}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn builtin_companion_can_manage_workdirs() {
|
fn builtin_companion_can_manage_workdirs() {
|
||||||
let tmp = TempDir::new().unwrap();
|
let tmp = TempDir::new().unwrap();
|
||||||
|
|||||||
@@ -2810,9 +2810,9 @@ fn validate_create_workspace_scope(
|
|||||||
"Memory settings revision must be at least 1".to_string(),
|
"Memory settings revision must be at least 1".to_string(),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
if !matches!(snapshot.language.as_str(), "English" | "Japanese") {
|
if !manifest::is_normalized_workspace_memory_language(&snapshot.language) {
|
||||||
return Err(RuntimeError::InvalidRequest(
|
return Err(RuntimeError::InvalidRequest(
|
||||||
"Memory settings language must be a normalized supported value".to_string(),
|
"Memory settings language must be a normalized bounded UTF-8 value".to_string(),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
@@ -3133,6 +3133,8 @@ mod tests {
|
|||||||
fn workspace_create_requires_matching_normalized_memory_settings_snapshot() {
|
fn workspace_create_requires_matching_normalized_memory_settings_snapshot() {
|
||||||
let mut request = scoped_task_request("memory-snapshot", "workspace-a");
|
let mut request = scoped_task_request("memory-snapshot", "workspace-a");
|
||||||
assert!(validate_create_workspace_scope(&request, Some("workspace-a")).is_ok());
|
assert!(validate_create_workspace_scope(&request, Some("workspace-a")).is_ok());
|
||||||
|
request.memory_settings.as_mut().unwrap().language = "Français".to_string();
|
||||||
|
assert!(validate_create_workspace_scope(&request, Some("workspace-a")).is_ok());
|
||||||
|
|
||||||
request.memory_settings = None;
|
request.memory_settings = None;
|
||||||
assert!(validate_create_workspace_scope(&request, Some("workspace-a")).is_err());
|
assert!(validate_create_workspace_scope(&request, Some("workspace-a")).is_err());
|
||||||
|
|||||||
@@ -5232,7 +5232,7 @@ fn validate_workspace_memory_snapshot(
|
|||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
if snapshot.settings_revision == 0
|
if snapshot.settings_revision == 0
|
||||||
|| !matches!(snapshot.language.as_str(), "English" | "Japanese")
|
|| !manifest::is_normalized_workspace_memory_language(&snapshot.language)
|
||||||
{
|
{
|
||||||
return Err(WorkerError::InvalidState(format!(
|
return Err(WorkerError::InvalidState(format!(
|
||||||
"Workspace Worker {worker_name} has corrupt Memory settings snapshot metadata"
|
"Workspace Worker {worker_name} has corrupt Memory settings snapshot metadata"
|
||||||
|
|||||||
@@ -1198,22 +1198,36 @@ impl SqliteWorkspaceStore {
|
|||||||
let language = normalize_workspace_memory_language(language)?;
|
let language = normalize_workspace_memory_language(language)?;
|
||||||
self.with_conn_mut(|conn| {
|
self.with_conn_mut(|conn| {
|
||||||
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
|
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
|
||||||
let current_revision = tx
|
let current = tx
|
||||||
.query_row(
|
.query_row(
|
||||||
"SELECT settings_revision FROM workspace_memory_settings WHERE workspace_id = ?1",
|
"SELECT workspace_id, settings_revision, language, created_at, updated_at \
|
||||||
|
FROM workspace_memory_settings WHERE workspace_id = ?1",
|
||||||
params![workspace_id],
|
params![workspace_id],
|
||||||
|row| row.get::<_, i64>(0),
|
|row| {
|
||||||
|
let revision = row.get::<_, i64>(1)?;
|
||||||
|
Ok(WorkspaceMemorySettingsRecord {
|
||||||
|
workspace_id: row.get(0)?,
|
||||||
|
settings_revision: revision.try_into().map_err(|_| {
|
||||||
|
rusqlite::Error::IntegralValueOutOfRange(1, revision)
|
||||||
|
})?,
|
||||||
|
language: row.get(2)?,
|
||||||
|
created_at: row.get(3)?,
|
||||||
|
updated_at: row.get(4)?,
|
||||||
|
})
|
||||||
|
},
|
||||||
)
|
)
|
||||||
.optional()?
|
.optional()?
|
||||||
.ok_or_else(|| Error::Store("Workspace Memory settings are missing".to_string()))?;
|
.ok_or_else(|| Error::Store("Workspace Memory settings are missing".to_string()))?;
|
||||||
let current_revision: u64 = current_revision.try_into().map_err(|_| {
|
let current_revision = current.settings_revision;
|
||||||
rusqlite::Error::IntegralValueOutOfRange(0, current_revision)
|
|
||||||
})?;
|
|
||||||
if current_revision != expected_revision {
|
if current_revision != expected_revision {
|
||||||
return Err(Error::WorkspaceConfigConflict(format!(
|
return Err(Error::WorkspaceConfigConflict(format!(
|
||||||
"Workspace Memory settings revision changed: expected {expected_revision}, current {current_revision}"
|
"Workspace Memory settings revision changed: expected {expected_revision}, current {current_revision}"
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
|
if current.language == language {
|
||||||
|
tx.commit()?;
|
||||||
|
return Ok(current);
|
||||||
|
}
|
||||||
let next_revision = current_revision.checked_add(1).ok_or_else(|| {
|
let next_revision = current_revision.checked_add(1).ok_or_else(|| {
|
||||||
Error::InvalidInput("Workspace Memory settings revision overflow".to_string())
|
Error::InvalidInput("Workspace Memory settings revision overflow".to_string())
|
||||||
})?;
|
})?;
|
||||||
@@ -5916,13 +5930,14 @@ fn collect_reference_diagnostics(
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn normalize_workspace_memory_language(language: &str) -> Result<String> {
|
fn normalize_workspace_memory_language(language: &str) -> Result<String> {
|
||||||
match language.trim().to_ascii_lowercase().as_str() {
|
let language = language.trim();
|
||||||
"english" | "en" => Ok("English".to_string()),
|
if !manifest::is_normalized_workspace_memory_language(language) {
|
||||||
"japanese" | "ja" => Ok("Japanese".to_string()),
|
return Err(Error::InvalidInput(format!(
|
||||||
_ => Err(Error::InvalidInput(
|
"Workspace Memory language must be a non-empty UTF-8 string of at most {} characters without control characters",
|
||||||
"Workspace Memory language must be one of: English, Japanese".to_string(),
|
manifest::MAX_WORKSPACE_MEMORY_LANGUAGE_CHARS
|
||||||
)),
|
)));
|
||||||
}
|
}
|
||||||
|
Ok(language.to_string())
|
||||||
}
|
}
|
||||||
|
|
||||||
fn bound_worker_create_fingerprint(
|
fn bound_worker_create_fingerprint(
|
||||||
@@ -8682,11 +8697,15 @@ INSERT INTO workdir_registry (
|
|||||||
);
|
);
|
||||||
assert_eq!(reserved.memory_settings.settings_revision, 1);
|
assert_eq!(reserved.memory_settings.settings_revision, 1);
|
||||||
assert_eq!(reserved.memory_settings.language, "English");
|
assert_eq!(reserved.memory_settings.language, "English");
|
||||||
|
let unchanged_memory_settings = store
|
||||||
|
.update_workspace_memory_settings("workspace-a", 1, " English ")
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(unchanged_memory_settings, memory_settings);
|
||||||
let updated_memory_settings = store
|
let updated_memory_settings = store
|
||||||
.update_workspace_memory_settings("workspace-a", 1, "ja")
|
.update_workspace_memory_settings("workspace-a", 1, " Français ")
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(updated_memory_settings.settings_revision, 2);
|
assert_eq!(updated_memory_settings.settings_revision, 2);
|
||||||
assert_eq!(updated_memory_settings.language, "Japanese");
|
assert_eq!(updated_memory_settings.language, "Français");
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
store
|
store
|
||||||
.reserve_worker_create(
|
.reserve_worker_create(
|
||||||
|
|||||||
Reference in New Issue
Block a user