fix: address security and UX issues from review
1. Fix path traversal vulnerability in fs.rs - Canonicalize paths to resolve ".." sequences and symlinks - Validate that resolved paths remain within workspace boundaries - Return 403 Forbidden for path traversal attempts 2. Fix OpenCode hidden agents filter bypass in new-mission-dialog - Apply hidden_agents filter from OpenAgentConfig to OpenCode agents - Matches existing behavior for Claude Code agents
This commit is contained in:
@@ -123,13 +123,16 @@ export function NewMissionDialog({
|
||||
// Combine all agents from enabled backends
|
||||
const allAgents = useMemo((): CombinedAgent[] => {
|
||||
const result: CombinedAgent[] = [];
|
||||
const openCodeHiddenAgents = config?.hidden_agents || [];
|
||||
const claudeCodeHiddenAgents = claudeCodeLibConfig?.hidden_agents || [];
|
||||
|
||||
for (const backend of enabledBackends) {
|
||||
let agentNames: string[] = [];
|
||||
|
||||
if (backend.id === 'opencode') {
|
||||
agentNames = opencodeAgents?.map(a => a.name) || parseAgentNames(agentsPayload);
|
||||
// Filter out hidden OpenCode agents
|
||||
const allOpenCodeAgents = opencodeAgents?.map(a => a.name) || parseAgentNames(agentsPayload);
|
||||
agentNames = allOpenCodeAgents.filter(name => !openCodeHiddenAgents.includes(name));
|
||||
} else if (backend.id === 'claudecode') {
|
||||
// Filter out hidden Claude Code agents
|
||||
const allClaudeAgents = claudecodeAgents?.map(a => a.name) || [];
|
||||
@@ -147,7 +150,7 @@ export function NewMissionDialog({
|
||||
}
|
||||
|
||||
return result;
|
||||
}, [enabledBackends, opencodeAgents, claudecodeAgents, agentsPayload, claudeCodeLibConfig]);
|
||||
}, [enabledBackends, opencodeAgents, claudecodeAgents, agentsPayload, config, claudeCodeLibConfig]);
|
||||
|
||||
// Group agents by backend for display
|
||||
const agentsByBackend = useMemo(() => {
|
||||
|
||||
@@ -178,29 +178,85 @@ async fn resolve_path_for_workspace(
|
||||
)
|
||||
})?;
|
||||
|
||||
let workspace_root = workspace.path.canonicalize().map_err(|e| {
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Failed to canonicalize workspace path: {}", e),
|
||||
)
|
||||
})?;
|
||||
|
||||
let input = Path::new(path);
|
||||
|
||||
// If the path is absolute, use it directly (but validate it's within workspace)
|
||||
if input.is_absolute() {
|
||||
return Ok(input.to_path_buf());
|
||||
}
|
||||
|
||||
// Resolve relative path against workspace path
|
||||
// For "context" paths, use the workspace's context directory
|
||||
if path.starts_with("./context") || path.starts_with("context") {
|
||||
// Resolve the final path based on input type
|
||||
let resolved = if input.is_absolute() {
|
||||
input.to_path_buf()
|
||||
} else if path.starts_with("./context") || path.starts_with("context") {
|
||||
// For "context" paths, use the workspace's context directory
|
||||
let suffix = path
|
||||
.trim_start_matches("./")
|
||||
.trim_start_matches("context/")
|
||||
.trim_start_matches("context");
|
||||
let context_path = workspace.path.join("context");
|
||||
let context_path = workspace_root.join("context");
|
||||
if suffix.is_empty() {
|
||||
return Ok(context_path);
|
||||
context_path
|
||||
} else {
|
||||
context_path.join(suffix)
|
||||
}
|
||||
return Ok(context_path.join(suffix));
|
||||
} else {
|
||||
// Default: resolve relative to workspace path
|
||||
workspace_root.join(path)
|
||||
};
|
||||
|
||||
// Canonicalize to resolve ".." and symlinks, then validate within workspace
|
||||
// For non-existent paths, we validate the parent directory exists and is within workspace
|
||||
let canonical = if resolved.exists() {
|
||||
resolved.canonicalize().map_err(|e| {
|
||||
(
|
||||
StatusCode::BAD_REQUEST,
|
||||
format!("Failed to resolve path: {}", e),
|
||||
)
|
||||
})?
|
||||
} else {
|
||||
// For new files, check that the parent is within workspace
|
||||
let parent = resolved.parent().ok_or_else(|| {
|
||||
(
|
||||
StatusCode::BAD_REQUEST,
|
||||
"Invalid path: no parent directory".to_string(),
|
||||
)
|
||||
})?;
|
||||
if !parent.exists() {
|
||||
return Err((
|
||||
StatusCode::BAD_REQUEST,
|
||||
format!("Parent directory does not exist: {}", parent.display()),
|
||||
));
|
||||
}
|
||||
let canonical_parent = parent.canonicalize().map_err(|e| {
|
||||
(
|
||||
StatusCode::BAD_REQUEST,
|
||||
format!("Failed to resolve parent path: {}", e),
|
||||
)
|
||||
})?;
|
||||
// Reconstruct the path with canonical parent + filename
|
||||
if let Some(filename) = resolved.file_name() {
|
||||
canonical_parent.join(filename)
|
||||
} else {
|
||||
return Err((StatusCode::BAD_REQUEST, "Invalid path".to_string()));
|
||||
}
|
||||
};
|
||||
|
||||
// Validate that the resolved path is within the workspace
|
||||
if !canonical.starts_with(&workspace_root) {
|
||||
return Err((
|
||||
StatusCode::FORBIDDEN,
|
||||
format!(
|
||||
"Path traversal attempt: {} is outside workspace {}",
|
||||
canonical.display(),
|
||||
workspace_root.display()
|
||||
),
|
||||
));
|
||||
}
|
||||
|
||||
// Default: resolve relative to workspace path
|
||||
Ok(workspace.path.join(path))
|
||||
Ok(canonical)
|
||||
}
|
||||
|
||||
fn resolve_upload_base(path: &str) -> Result<PathBuf, (StatusCode, String)> {
|
||||
|
||||
@@ -760,13 +760,10 @@ pub async fn run_claudecode_turn(
|
||||
for block in evt.message.content {
|
||||
match block {
|
||||
ContentBlock::Text { text } => {
|
||||
// Text content is the final assistant response
|
||||
// Don't send as Thinking - it will be in the final AssistantMessage
|
||||
if !text.is_empty() {
|
||||
final_result = text.clone();
|
||||
let _ = events_tx.send(AgentEvent::Thinking {
|
||||
content: text,
|
||||
done: false,
|
||||
mission_id: Some(mission_id),
|
||||
});
|
||||
final_result = text;
|
||||
}
|
||||
}
|
||||
ContentBlock::ToolUse { id, name, input } => {
|
||||
@@ -779,10 +776,13 @@ pub async fn run_claudecode_turn(
|
||||
});
|
||||
}
|
||||
ContentBlock::Thinking { thinking } => {
|
||||
// Only send if this is new content not already streamed
|
||||
// The streaming deltas already accumulated this, so this is
|
||||
// typically the final complete thinking block
|
||||
if !thinking.is_empty() {
|
||||
let _ = events_tx.send(AgentEvent::Thinking {
|
||||
content: thinking,
|
||||
done: false,
|
||||
done: true, // Mark as done since this is the final block
|
||||
mission_id: Some(mission_id),
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user