feat: add selective Workdir symlink policies
This commit is contained in:
@@ -699,6 +699,7 @@ impl WorkerController {
|
||||
target: bash_output_dir.clone(),
|
||||
permission: manifest::Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}])
|
||||
.map_err(std::io::Error::other)?;
|
||||
|
||||
|
||||
@@ -743,6 +743,7 @@ fn comm_info_from_spawned_child(child: &session_store::WorkerSpawnedChild) -> Co
|
||||
target: rule.target.clone(),
|
||||
permission,
|
||||
recursive: rule.recursive,
|
||||
symlink_policy: rule.symlink_policy,
|
||||
})
|
||||
})
|
||||
.collect();
|
||||
@@ -1324,6 +1325,7 @@ mod tests {
|
||||
target: root.path().to_path_buf(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
active_child_segment,
|
||||
)
|
||||
@@ -1795,6 +1797,7 @@ mod tests {
|
||||
target: PathBuf::from("/tmp"),
|
||||
permission: "read".into(),
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
callback_address: PathBuf::from("/tmp/parent.sock"),
|
||||
}
|
||||
|
||||
@@ -286,6 +286,7 @@ fn read_rule(target: PathBuf) -> ScopeRule {
|
||||
target,
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -294,6 +295,7 @@ fn write_rule(target: PathBuf) -> ScopeRule {
|
||||
target,
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -306,6 +308,7 @@ fn workspace_scope(
|
||||
target: workspace_root.to_path_buf(),
|
||||
permission,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
};
|
||||
let deny = deny_write
|
||||
.iter()
|
||||
@@ -711,6 +714,7 @@ permission = "write"
|
||||
target: target.to_path_buf(),
|
||||
permission,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -18,8 +18,9 @@ use workdir::workspace::WorkspaceWorkdirSessionOperationRequest;
|
||||
use workdir::{
|
||||
CommandHandle, CommandOutput, CommandOutputRequest, CommandRequest, CommandStatus, EditRequest,
|
||||
EditResult, GlobRequest, GlobResult, GrepRequest, GrepResult, ListRequest, ListResult,
|
||||
ReadRequest, ReadResult, StatRequest, StatResult, Workdir, WorkdirError, WorkdirSession,
|
||||
WorkdirSessionCapabilities, WorkdirSessionHandle, WriteRequest, WriteResult,
|
||||
ReadRequest, ReadResult, StatRequest, StatResult, Workdir, WorkdirError,
|
||||
WorkdirScopeAuthorizationRequest, WorkdirSession, WorkdirSessionCapabilities,
|
||||
WorkdirSessionHandle, WriteRequest, WriteResult,
|
||||
};
|
||||
|
||||
use workspace_api::{
|
||||
@@ -283,6 +284,16 @@ impl WorkdirSession for WorkspaceAttachedWorkdirSession {
|
||||
WorkdirSessionCapabilities::ALL
|
||||
}
|
||||
|
||||
async fn authorize_scope_path(
|
||||
&self,
|
||||
request: WorkdirScopeAuthorizationRequest,
|
||||
) -> Result<(), WorkdirError> {
|
||||
match self.operate(WorkdirSessionOperation::AuthorizeScope(request))? {
|
||||
WorkdirSessionOperationResult::AuthorizeScope => Ok(()),
|
||||
_ => Err(Self::mismatch("authorize_scope")),
|
||||
}
|
||||
}
|
||||
|
||||
async fn stat(&self, request: StatRequest) -> Result<StatResult, WorkdirError> {
|
||||
match self.operate(WorkdirSessionOperation::Stat(request))? {
|
||||
WorkdirSessionOperationResult::Stat(result) => Ok(result),
|
||||
@@ -1242,10 +1253,14 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn scoped_broker_operations_carry_no_child_context() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "visible.txt", "kind": "file", "size": 8}
|
||||
}))]));
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![
|
||||
response(json!({ "operation": "authorize_scope" })),
|
||||
response(json!({ "operation": "authorize_scope" })),
|
||||
response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "visible.txt", "kind": "file", "size": 8}
|
||||
})),
|
||||
]));
|
||||
let broker = workdir::WorkdirToolBroker::new(WorkspaceAttachedWorkdirSession::handle(
|
||||
client.clone(),
|
||||
));
|
||||
@@ -1255,6 +1270,7 @@ mod tests {
|
||||
target: workdir::WorkdirPath::new("").unwrap(),
|
||||
permission: workdir::WorkdirToolScopePermission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("").unwrap(),
|
||||
command: false,
|
||||
@@ -1269,7 +1285,7 @@ mod tests {
|
||||
.unwrap();
|
||||
|
||||
let requests = client.requests();
|
||||
assert_eq!(requests.len(), 1);
|
||||
assert_eq!(requests.len(), 3);
|
||||
for request in requests {
|
||||
assert_eq!(
|
||||
request.path,
|
||||
|
||||
@@ -411,11 +411,13 @@ mod tests {
|
||||
target: dir.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
deny: vec![ScopeRule {
|
||||
target: secret.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
};
|
||||
let scope = Scope::from_config(&cfg).unwrap();
|
||||
@@ -574,11 +576,13 @@ mod tests {
|
||||
target: dir.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
deny: vec![ScopeRule {
|
||||
target: secret.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
};
|
||||
let scope = Scope::from_config(&cfg).unwrap();
|
||||
|
||||
@@ -299,6 +299,7 @@ mod tests {
|
||||
target: "/tmp/work".into(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
callback_address: "/run/yoi/my-worker/sock".into(),
|
||||
}];
|
||||
|
||||
@@ -77,6 +77,7 @@ pub(crate) fn write_rule(path: &str, recursive: bool) -> ScopeRule {
|
||||
target: PathBuf::from(path),
|
||||
permission: Permission::Write,
|
||||
recursive,
|
||||
symlink_policy: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -85,6 +86,7 @@ pub(crate) fn read_rule(path: &str, recursive: bool) -> ScopeRule {
|
||||
target: PathBuf::from(path),
|
||||
permission: Permission::Read,
|
||||
recursive,
|
||||
symlink_policy: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1030,6 +1030,7 @@ fn record_from_worker_state(child: &WorkerSpawnedChild) -> io::Result<SpawnedWor
|
||||
target: rule.target.clone(),
|
||||
permission,
|
||||
recursive: rule.recursive,
|
||||
symlink_policy: rule.symlink_policy,
|
||||
})
|
||||
})
|
||||
.collect::<io::Result<Vec<_>>>()?;
|
||||
@@ -1072,6 +1073,7 @@ mod tests {
|
||||
target: std::path::PathBuf::from("/tmp"),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
deny: Vec::new(),
|
||||
})
|
||||
@@ -1090,6 +1092,7 @@ mod tests {
|
||||
target: root.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
deny: Vec::new(),
|
||||
})
|
||||
@@ -1109,6 +1112,7 @@ mod tests {
|
||||
target: workdir::WorkdirPath::new("").unwrap(),
|
||||
permission: workdir::WorkdirToolScopePermission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("").unwrap(),
|
||||
command: false,
|
||||
|
||||
@@ -16,8 +16,8 @@ use manifest::{
|
||||
CompactionConfigPartial, EngineManifestConfig, FileUploadLimitsPartial,
|
||||
PermissionConfigPartial, ProfileDiscovery, ProfileError, ProfileRegistry,
|
||||
ProfileRegistrySource, ProfileResolveOptions, ProfileResolver, ProfileSelector, ScopeConfig,
|
||||
ScopeRule, SessionConfigPartial, ToolOutputLimitsPartial, WorkerManifest, WorkerManifestConfig,
|
||||
WorkerMetaConfig,
|
||||
ScopeRule, SessionConfigPartial, SymlinkPolicy, ToolOutputLimitsPartial, WorkerManifest,
|
||||
WorkerManifestConfig, WorkerMetaConfig,
|
||||
};
|
||||
use serde::Deserialize;
|
||||
use tokio::sync::mpsc;
|
||||
@@ -61,7 +61,9 @@ struct SubWorkerSpawnInput {
|
||||
task: String,
|
||||
/// Allow rules delegated to the spawned SubWorker. Must be a subset of the
|
||||
/// spawner's explicit delegation authority; direct tool scope alone is not
|
||||
/// sufficient. Omit `recursive` for normal workspace/worktree delegation; it defaults to true.
|
||||
/// sufficient. Omit `recursive` for normal workspace/worktree delegation;
|
||||
/// it defaults to true. Omit `symlink_policy` for the least-authority
|
||||
/// `resolved` policy; `logical` requires matching parent authority.
|
||||
scope: Vec<ScopeRuleInput>,
|
||||
/// Explicitly grant command execution through the parent-owned Workdir tool broker.
|
||||
#[serde(default)]
|
||||
@@ -88,6 +90,27 @@ struct ScopeRuleInput {
|
||||
/// children only. Defaults to `true`.
|
||||
#[serde(default = "default_true")]
|
||||
recursive: bool,
|
||||
/// Symbolic-link identity used by this rule. `resolved` is the default
|
||||
/// and least authority; `logical` requires matching parent authority.
|
||||
#[serde(default)]
|
||||
symlink_policy: SymlinkPolicyInput,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Deserialize, schemars::JsonSchema, Clone, Copy)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
enum SymlinkPolicyInput {
|
||||
#[default]
|
||||
Resolved,
|
||||
Logical,
|
||||
}
|
||||
|
||||
impl From<SymlinkPolicyInput> for SymlinkPolicy {
|
||||
fn from(value: SymlinkPolicyInput) -> Self {
|
||||
match value {
|
||||
SymlinkPolicyInput::Resolved => Self::Resolved,
|
||||
SymlinkPolicyInput::Logical => Self::Logical,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, schemars::JsonSchema, Clone, Copy)]
|
||||
@@ -506,6 +529,7 @@ impl Tool for SubWorkerSpawnTool {
|
||||
target: child_bash_output_dir.clone(),
|
||||
permission: manifest::Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}])
|
||||
.map_err(|error| {
|
||||
ToolError::ExecutionFailed(format!(
|
||||
@@ -707,6 +731,7 @@ fn parse_workdir_scope(rules: &[ScopeRuleInput]) -> Result<Vec<WorkdirToolScopeR
|
||||
PermissionInput::Write => WorkdirToolScopePermission::Write,
|
||||
},
|
||||
recursive: rule.recursive,
|
||||
symlink_policy: rule.symlink_policy.into(),
|
||||
})
|
||||
})
|
||||
.collect()
|
||||
@@ -1074,21 +1099,26 @@ mod tests {
|
||||
target: ".".to_string(),
|
||||
permission: PermissionInput::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
},
|
||||
ScopeRuleInput {
|
||||
target: "src".to_string(),
|
||||
permission: PermissionInput::Write,
|
||||
recursive: false,
|
||||
symlink_policy: SymlinkPolicyInput::Logical,
|
||||
},
|
||||
])
|
||||
.unwrap();
|
||||
assert_eq!(rules[0].target.as_str(), "");
|
||||
assert_eq!(rules[1].target.as_str(), "src");
|
||||
assert_eq!(rules[0].symlink_policy, SymlinkPolicy::Resolved);
|
||||
assert_eq!(rules[1].symlink_policy, SymlinkPolicy::Logical);
|
||||
for target in ["/host/path", "../escape"] {
|
||||
let error = parse_workdir_scope(&[ScopeRuleInput {
|
||||
target: target.to_string(),
|
||||
permission: PermissionInput::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}])
|
||||
.unwrap_err();
|
||||
assert!(matches!(error, ToolError::InvalidArgument(_)));
|
||||
@@ -1126,6 +1156,7 @@ mod tests {
|
||||
target: path.to_path_buf(),
|
||||
permission,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1533,10 +1564,14 @@ enabled = false
|
||||
assert!(record.installed_tools.iter().any(|tool| tool == "Write"));
|
||||
assert!(!record.installed_tools.iter().any(|tool| tool == "Bash"));
|
||||
assert_eq!(calls.load(Ordering::SeqCst), 1);
|
||||
assert!(
|
||||
remote_client.requests().is_empty(),
|
||||
"spawning a child must not open or delegate a provider Workdir session"
|
||||
);
|
||||
let requests = remote_client.requests();
|
||||
assert!(!requests.is_empty());
|
||||
assert!(requests.iter().all(|request| {
|
||||
let body = request.body.as_deref().unwrap_or_default();
|
||||
body.contains("authorize_scope")
|
||||
&& !body.contains(&bash_output_dir.display().to_string())
|
||||
&& !body.contains(&workspace_root.display().to_string())
|
||||
}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1548,6 +1583,9 @@ enabled = false
|
||||
.expect("schema properties");
|
||||
assert!(properties.contains_key("cwd"), "schema: {schema}");
|
||||
assert!(properties.contains_key("command"), "schema: {schema}");
|
||||
let schema_text = serde_json::to_string(&schema).unwrap();
|
||||
assert!(schema_text.contains("symlink_policy"), "schema: {schema}");
|
||||
assert!(schema_text.contains("logical"), "schema: {schema}");
|
||||
let required = schema
|
||||
.get("required")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
@@ -1708,10 +1746,29 @@ enabled = false
|
||||
self.requests
|
||||
.lock()
|
||||
.expect("remote Workdir request lock")
|
||||
.push(request);
|
||||
Err(WorkspaceClientError::Request(
|
||||
"SubWorker spawn must not call the remote Workdir provider".into(),
|
||||
))
|
||||
.push(request.clone());
|
||||
let operation: workdir::workspace::WorkspaceWorkdirSessionOperationRequest =
|
||||
serde_json::from_str(request.body.as_deref().unwrap_or_default()).map_err(
|
||||
|error| {
|
||||
WorkspaceClientError::Request(format!(
|
||||
"invalid remote Workdir operation: {error}"
|
||||
))
|
||||
},
|
||||
)?;
|
||||
match operation.operation {
|
||||
workdir::http::WorkdirSessionOperation::AuthorizeScope(_) => {
|
||||
Ok(WorkspaceResponse {
|
||||
status: 200,
|
||||
body: serde_json::to_string(
|
||||
&workdir::http::WorkdirSessionOperationResult::AuthorizeScope,
|
||||
)
|
||||
.unwrap(),
|
||||
})
|
||||
}
|
||||
_ => Err(WorkspaceClientError::Request(
|
||||
"SubWorker spawn may only authorize its provider-side scope".into(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1905,6 +1962,7 @@ max_tokens = 3333
|
||||
target: PathBuf::from("/tmp/child"),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}];
|
||||
|
||||
let config_json =
|
||||
|
||||
@@ -7256,6 +7256,7 @@ fn delegated_scope_rule_to_scope_rule(rule: WorkerSpawnedScopeRule) -> Option<Sc
|
||||
target: rule.target,
|
||||
permission,
|
||||
recursive: rule.recursive,
|
||||
symlink_policy: rule.symlink_policy,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -7508,6 +7509,7 @@ mod spawned_context_tests {
|
||||
target: cwd.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
deny: Vec::new(),
|
||||
},
|
||||
@@ -7544,6 +7546,7 @@ mod spawned_context_tests {
|
||||
target: workspace_root.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
deny: Vec::new(),
|
||||
},
|
||||
|
||||
@@ -27,6 +27,7 @@ async fn restore_reclaims_and_clears_legacy_process_children() {
|
||||
target: scope_root.path().to_path_buf(),
|
||||
permission: "write".into(),
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
});
|
||||
store.write(&metadata).unwrap();
|
||||
@@ -35,6 +36,7 @@ async fn restore_reclaims_and_clears_legacy_process_children() {
|
||||
target: scope_root.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
};
|
||||
let parent_scope = SharedScope::new(
|
||||
Scope::from_config(&ScopeConfig {
|
||||
|
||||
@@ -36,6 +36,7 @@ async fn legacy_callback_cannot_register_process_subworker_authority() {
|
||||
target: scope_root.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
symlink_policy: Default::default(),
|
||||
}],
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user