agent: Temporarily remove sandbox feature (#61484)
see title --- Release Notes: - N/A or Added/Fixed/Improved ... --------- Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
This commit is contained in:
1 parent
c4c55bade4
commit
39bea4e56d
6 files changed
+92
-20
No files matched your search
@@ -5,9 +5,10 @@
|
||||
//! caller see the same answer (and so the `target_os` gate lives in one
|
||||
//! place instead of scattered across the agent crate).
|
||||
//!
|
||||
//! The current policy is: enabled iff the project is local, the platform has an
|
||||
//! integration, and the user has not persistently allowed unsandboxed execution
|
||||
//! (the `allow_unsandboxed` sandbox setting). Setting `allow_unsandboxed`
|
||||
//! The current policy is: enabled iff the user has the `sandboxing` feature
|
||||
//! flag turned on, the project is local, the platform has an integration, and
|
||||
//! the user has not persistently allowed unsandboxed execution (the
|
||||
//! `allow_unsandboxed` sandbox setting). Setting `allow_unsandboxed`
|
||||
//! persistently turns sandboxing off for the model-facing surface entirely:
|
||||
//! the plain (non-sandboxed) `terminal` tool is exposed and the system prompt
|
||||
//! omits the sandbox section, since every command would run without a wrap
|
||||
@@ -18,12 +19,14 @@
|
||||
//!
|
||||
//! macOS (Seatbelt), Linux (Bubblewrap), and Windows (Bubblewrap via WSL)
|
||||
//! have real sandbox integrations; on platforms without one the per-command
|
||||
//! wrap is a no-op, so commands run with the agent's ambient permissions.
|
||||
//! wrap is a no-op, so commands run with the agent's ambient permissions even
|
||||
//! when the flag is on.
|
||||
//!
|
||||
//! Naming note: this module is about agent terminal sandboxing specifically.
|
||||
//! Other agent operations (e.g. file edits) are gated separately.
|
||||
|
||||
use agent_settings::{AgentSettings, SandboxPermissions};
|
||||
use feature_flags::{FeatureFlagAppExt as _, SandboxingFeatureFlag};
|
||||
use gpui::App;
|
||||
use http_proxy::HostPattern;
|
||||
use project::Project;
|
||||
@@ -184,19 +187,26 @@ pub fn settings_sandbox_policy(persistent: &SandboxPermissions) -> SandboxPolicy
|
||||
/// prompt in place, since the model is still operating in the sandbox model and
|
||||
/// only escaping individual commands (tracked in `ThreadSandboxGrants`).
|
||||
pub(crate) fn sandboxing_enabled_for_project(project: &Project, cx: &App) -> bool {
|
||||
sandboxing_available_for_project(project)
|
||||
sandboxing_available_for_project(project, cx)
|
||||
&& !AgentSettings::get_global(cx)
|
||||
.sandbox_permissions
|
||||
.allow_unsandboxed
|
||||
}
|
||||
|
||||
/// Whether sandboxing is *applicable* for this project at all — the project is
|
||||
/// local and the platform has a sandbox integration — independent of the
|
||||
/// persistent `allow_unsandboxed` setting. Used by the UI to distinguish
|
||||
/// "sandboxing isn't relevant here" (don't show the indicator) from "sandboxing
|
||||
/// is available but turned off in settings" (show it, struck out).
|
||||
pub(crate) fn sandboxing_available_for_project(project: &Project) -> bool {
|
||||
project.is_local()
|
||||
/// Whether agent-run terminal commands should be wrapped in an OS-level
|
||||
/// sandbox for this process. See module docs for the policy.
|
||||
pub(crate) fn sandboxing_enabled(cx: &App) -> bool {
|
||||
cx.has_flag::<SandboxingFeatureFlag>()
|
||||
}
|
||||
|
||||
/// Whether sandboxing is *applicable* for this project at all — the feature is
|
||||
/// enabled, the project is local, and the platform has a sandbox integration —
|
||||
/// independent of the persistent `allow_unsandboxed` setting. Used by the UI to
|
||||
/// distinguish "sandboxing isn't relevant here" (don't show the indicator) from
|
||||
/// "sandboxing is available but turned off in settings" (show it, struck out).
|
||||
pub(crate) fn sandboxing_available_for_project(project: &Project, cx: &App) -> bool {
|
||||
sandboxing_enabled(cx)
|
||||
&& project.is_local()
|
||||
&& cfg!(any(
|
||||
target_os = "macos",
|
||||
target_os = "linux",
|
||||
|
||||
@@ -7302,6 +7302,7 @@ async fn test_fetch_tool_allow_rule_skips_confirmation(cx: &mut TestAppContext)
|
||||
/// A fetch to a host that hasn't been granted network access prompts for the
|
||||
/// shared per-host sandbox grant, even when the tool itself is allowed.
|
||||
#[gpui::test]
|
||||
#[ignore]
|
||||
async fn test_fetch_tool_prompts_for_ungranted_host(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
@@ -7389,6 +7390,7 @@ async fn test_fetch_tool_granted_host_skips_prompt(cx: &mut TestAppContext) {
|
||||
|
||||
/// Loopback / IP-literal hosts can't be granted individually, so without
|
||||
/// unsandboxed access a fetch to them is refused with guidance to grant it.
|
||||
#[ignore]
|
||||
#[gpui::test]
|
||||
async fn test_fetch_tool_refuses_loopback_without_unsandboxed(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
@@ -7478,6 +7480,7 @@ async fn test_fetch_tool_unsandboxed_lifts_restrictions(cx: &mut TestAppContext)
|
||||
/// is refused just like a direct loopback fetch. This is the redirect variant of
|
||||
/// the SSRF protection — the approved domain can't be used to bounce the request
|
||||
/// onto the local machine.
|
||||
#[ignore]
|
||||
#[gpui::test]
|
||||
async fn test_fetch_tool_refuses_redirect_to_loopback(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
@@ -7536,6 +7539,7 @@ async fn test_fetch_tool_refuses_redirect_to_loopback(cx: &mut TestAppContext) {
|
||||
/// A granted host that redirects to a *different*, ungranted host triggers a
|
||||
/// fresh per-host authorization prompt for the redirect target — the redirect is
|
||||
/// not silently followed to a host the user never approved.
|
||||
#[ignore]
|
||||
#[gpui::test]
|
||||
async fn test_fetch_tool_reauthorizes_redirect_to_new_host(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
@@ -1846,12 +1846,12 @@ impl Thread {
|
||||
sandboxing_enabled_for_project(self.project.read(cx), cx)
|
||||
}
|
||||
|
||||
/// Whether sandboxing is *applicable* for this thread's project (local
|
||||
/// project, supported platform), regardless of whether it's been turned off
|
||||
/// in settings. The UI shows the sandbox indicator whenever this is true,
|
||||
/// drawing it struck-out when sandboxing is disabled.
|
||||
/// Whether sandboxing is *applicable* for this thread's project (feature on,
|
||||
/// local project, supported platform), regardless of whether it's been
|
||||
/// turned off in settings. The UI shows the sandbox indicator whenever this
|
||||
/// is true, drawing it struck-out when sandboxing is disabled.
|
||||
pub fn sandboxing_available(&self, cx: &App) -> bool {
|
||||
sandboxing_available_for_project(self.project.read(cx))
|
||||
sandboxing_available_for_project(self.project.read(cx), cx)
|
||||
}
|
||||
|
||||
/// The directory subtrees the sandbox always grants write access to for this
|
||||
@@ -3975,15 +3975,47 @@ impl Thread {
|
||||
let model = self
|
||||
.model()
|
||||
.ok_or_else(|| anyhow!(NoModelConfiguredError))?;
|
||||
let sandboxing_enabled = crate::sandboxing::sandboxing_enabled(cx);
|
||||
let tools = if let Some(turn) = self.running_turn.as_ref() {
|
||||
turn.tools
|
||||
.iter()
|
||||
.filter_map(|(tool_name, tool)| {
|
||||
log::trace!("Including tool: {}", tool_name);
|
||||
let mut description = tool.description().to_string();
|
||||
let mut schema = tool.input_schema(model.tool_input_format()).log_err()?;
|
||||
// TEMPORARY (sandboxing feature flag): with the flag off,
|
||||
// the fetch and create_directory descriptions/schemas must
|
||||
// not advertise sandbox-dependent behavior (host grants,
|
||||
// out-of-project creation via the `reason` field), since
|
||||
// the corresponding runtime paths are disabled. Restore
|
||||
// the pre-sandboxing model-facing surface here rather than
|
||||
// forking the tools; delete this when the flag is removed
|
||||
// again.
|
||||
if !sandboxing_enabled {
|
||||
if tool_name.as_ref() == FetchTool::NAME {
|
||||
description =
|
||||
"Fetches a URL and returns the content as Markdown.".to_string();
|
||||
} else if tool_name.as_ref() == CreateDirectoryTool::NAME {
|
||||
description = "Creates a new directory at the specified path within \
|
||||
the project. Returns confirmation that the directory was \
|
||||
created.\n\nThis tool creates a directory and all necessary \
|
||||
parent directories. It should be used whenever you need to \
|
||||
create new directories within the project.\nThe only supported \
|
||||
path outside the project is `~/.agents/skills` or a descendant, \
|
||||
for global agent skills."
|
||||
.to_string();
|
||||
if let Some(properties) = schema
|
||||
.get_mut("properties")
|
||||
.and_then(|value| value.as_object_mut())
|
||||
{
|
||||
properties.remove("reason");
|
||||
}
|
||||
}
|
||||
}
|
||||
Some(LanguageModelRequestTool::function(
|
||||
tool_name.to_string(),
|
||||
tool.description().to_string(),
|
||||
tool.input_schema(model.tool_input_format()).log_err()?,
|
||||
description,
|
||||
schema,
|
||||
tool.supports_input_streaming(),
|
||||
))
|
||||
})
|
||||
|
||||
@@ -695,6 +695,7 @@ mod tests {
|
||||
/// on approval, creates the *specific* new directory (not its broad parent).
|
||||
#[cfg(any(target_os = "linux", target_os = "macos"))]
|
||||
#[gpui::test]
|
||||
#[ignore]
|
||||
async fn test_create_directory_out_of_project_creates_and_grants(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
@@ -752,6 +753,7 @@ mod tests {
|
||||
/// trace on the filesystem.
|
||||
#[cfg(any(target_os = "linux", target_os = "macos"))]
|
||||
#[gpui::test]
|
||||
#[ignore]
|
||||
async fn test_create_directory_out_of_project_denied_cleans_up(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
|
||||
@@ -281,7 +281,14 @@ impl AgentTool for FetchTool {
|
||||
// fetch to a host the user never approved. We disable the HTTP
|
||||
// client's own redirect following and re-run the grant for each hop
|
||||
// before requesting it.
|
||||
let unsandboxed = cx.update(|cx| event_stream.unsandboxed_access_granted(cx));
|
||||
//
|
||||
// When the sandboxing feature flag is off the terminal isn't
|
||||
// sandboxed either, so gating fetch by host would provide no
|
||||
// isolation; skip it entirely (the pre-sandboxing behavior).
|
||||
let unsandboxed = cx.update(|cx| {
|
||||
!crate::sandboxing::sandboxing_enabled(cx)
|
||||
|| event_stream.unsandboxed_access_granted(cx)
|
||||
});
|
||||
|
||||
let mut current_url = input.url.clone();
|
||||
let mut redirects = 0;
|
||||
|
||||
@@ -118,3 +118,20 @@ impl FeatureFlag for AutoWatchFeatureFlag {
|
||||
type Value = PresenceFlag;
|
||||
}
|
||||
register_feature_flag!(AutoWatchFeatureFlag);
|
||||
|
||||
/// Wraps agent-run terminal commands in an OS-level sandbox where supported,
|
||||
/// and applies the shared per-host network grants to the `fetch` tool and the
|
||||
/// out-of-project write grants to the `create_directory` tool. When off,
|
||||
/// these tools run with the agent's full ambient permissions, as they always
|
||||
/// have.
|
||||
pub struct SandboxingFeatureFlag;
|
||||
|
||||
impl FeatureFlag for SandboxingFeatureFlag {
|
||||
const NAME: &'static str = "sandboxing";
|
||||
type Value = PresenceFlag;
|
||||
|
||||
fn enabled_for_staff() -> bool {
|
||||
false
|
||||
}
|
||||
}
|
||||
register_feature_flag!(SandboxingFeatureFlag);
|
||||
Reference in new issue
Block a user