diff --git a/README.md b/README.md index 2d1b107..08cb3f1 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ See [`./USAGE.md`](./USAGE.md) for usage examples and run `cargo run -p cli -- - - **21 crates** in workspace - **Binary name:** `frontal-code` - **Default model:** `claude-opus-5` -- **Default permissions:** `danger-full-access` +- **Default permissions:** `prompt` for the CLI; hosted workers use `workspace-write` ## Attribution diff --git a/crates/code-server/src/lib.rs b/crates/code-server/src/lib.rs index d709166..0d0e1c8 100644 --- a/crates/code-server/src/lib.rs +++ b/crates/code-server/src/lib.rs @@ -2,7 +2,7 @@ //! //! Hosted HTTP and WebSocket control-plane surface for `Frontal Code`. -use std::collections::{BTreeMap, HashMap, VecDeque}; +use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::env; use std::fmt; use std::fs; @@ -135,6 +135,8 @@ struct OrphanPolicyRule { #[derive(Debug, Clone)] pub struct ServerConfig { pub bind_addr: SocketAddr, + /// Whether tasks may clone and publish to repositories given as paths on this host. + pub allow_local_repos: bool, pub event_replay_limit: usize, pub lane_transport_kind: LaneTransportKind, pub api_key: Option, @@ -197,6 +199,7 @@ impl Default for ServerConfig { clickhouse_user: "frontal".to_string(), clickhouse_password: String::new(), worker_token_ttl_secs: 7200, + allow_local_repos: false, state_file: default_server_state_file(), orphan_approval_delay: DEFAULT_ORPHAN_APPROVAL_DELAY, orphan_auto_retry_after: None, @@ -279,6 +282,8 @@ impl ServerConfig { if let Ok(password) = env::var("FRONTAL_CODE_SERVER_CLICKHOUSE_PASSWORD") { config.clickhouse_password = password; } + config.allow_local_repos = env::var("FRONTAL_CODE_SERVER_ALLOW_LOCAL_REPOS") + .is_ok_and(|value| matches!(value.trim(), "1" | "true")); if let Ok(ttl) = env::var("FRONTAL_CODE_WORKER_TOKEN_TTL_SECS") { config.worker_token_ttl_secs = ttl.trim().parse()?; } @@ -368,6 +373,24 @@ pub struct ServerState { worker_access: Arc, model_proxy_client: reqwest::Client, publish_workspace_root: PathBuf, + // Tasks with a publish in flight. Held so two requests cannot push the same task twice. + publishing_tasks: Arc>>, + // Off unless configured. See `ServerConfig::allow_local_repos`. + allow_local_repos: bool, +} + +/// A task's publish slot. Dropping it frees the slot, including when a publish fails. +struct PublishSlot { + publishing_tasks: Arc>>, + task_id: String, +} + +impl Drop for PublishSlot { + fn drop(&mut self) { + if let Ok(mut tasks) = self.publishing_tasks.lock() { + tasks.remove(&self.task_id); + } + } } #[cfg(test)] @@ -513,6 +536,8 @@ impl ServerState { .build() .unwrap_or_default(), publish_workspace_root: default_server_workspace_root(), + publishing_tasks: Arc::new(Mutex::new(HashSet::new())), + allow_local_repos: false, }; state.recover_loaded_state(); state @@ -536,6 +561,11 @@ impl ServerState { self } + fn with_local_repos_allowed(mut self, allowed: bool) -> Self { + self.allow_local_repos = allowed; + self + } + #[cfg(test)] #[must_use] fn with_lane_transport_kind( @@ -1867,7 +1897,8 @@ pub async fn serve(config: ServerConfig) -> Result<(), Box>, Json(request): Json, ) -> Result, AppError> { + // Refuse a second completion, or one after cancellation. Checked before the lane is + // touched, so a stale worker cannot change a task that has already ended. + let task = state + .tasks + .get(&task_id) + .ok_or_else(|| AppError::not_found(format!("task not found: {task_id}")))?; + if matches!( + task.status, + TaskStatus::Completed | TaskStatus::Failed | TaskStatus::Stopped + ) { + return Err(AppError::bad_request(format!( + "task {task_id} is already in terminal state: {}", + task.status + ))); + } + let mut context = state .context_for(&task_id) .ok_or_else(|| AppError::not_found(format!("task not found: {task_id}")))?; @@ -4586,11 +4660,42 @@ fn write_local_docker_task_payload( Ok(payload_path) } +#[allow(clippy::too_many_lines)] +/// Whether the server may clone and publish to a repository that is a path on this host. +/// Off by default: a task could otherwise read or push to any repository the server can +/// reach. Local development and the e2e scripts turn it on. +/// Checks a repository location from a task request. A remote URL is always allowed. A +/// path on this host needs `FRONTAL_CODE_SERVER_ALLOW_LOCAL_REPOS=1`. +fn check_repo_source(location: &str, allow_local_repos: bool) -> Result<(), String> { + if location.starts_with('-') { + return Err("repository location must not start with `-`".to_string()); + } + if FsPath::new(location).exists() && !allow_local_repos { + return Err( + "local repository paths are disabled; set FRONTAL_CODE_SERVER_ALLOW_LOCAL_REPOS=1 to allow them" + .to_string(), + ); + } + Ok(()) +} + #[allow(clippy::too_many_lines)] fn create_task_internal(state: &ServerState, request: CreateTaskRequest) -> Result { if request.prompt.trim().is_empty() { return Err(AppError::bad_request("prompt must not be empty")); } + if let Some(repo_url) = request.repo_url.as_deref() { + check_repo_source(repo_url, state.allow_local_repos).map_err(AppError::bad_request)?; + } + if let Some(repository) = request.repository.as_deref() { + check_repo_source(repository, state.allow_local_repos).map_err(AppError::bad_request)?; + } + for reference in [request.base_ref.as_deref(), request.branch.as_deref()] + .into_iter() + .flatten() + { + publish::validate_ref_name(reference).map_err(AppError::bad_request)?; + } let description = request.repository.as_deref(); let task = state.tasks.create(&request.prompt, description); @@ -5075,6 +5180,13 @@ impl AppError { } } + fn conflict(message: impl Into) -> Self { + Self { + status: StatusCode::CONFLICT, + message: message.into(), + } + } + fn internal(message: impl Into) -> Self { Self { status: StatusCode::INTERNAL_SERVER_ERROR, @@ -5826,18 +5938,21 @@ mod tests { let workspace_root = dir.join("workspaces"); let docker = Arc::new(MockDockerRunner::default()); let access = Arc::new(WorkerAccess::new(Some(store.clone()), 3600)); - let state = Arc::new(ServerState::with_lane_transport_kind_and_access( - DEFAULT_EVENT_REPLAY_LIMIT, - LaneTransportKind::LocalDocker, - Arc::new(LocalDockerLaneWorkerTransport::with_runner( - workspace_root.clone(), - "frontal-code-worker:test".to_string(), - "http://host.docker.internal:8788", - access.clone(), - docker.clone(), - )), - access, - )); + let state = Arc::new( + ServerState::with_lane_transport_kind_and_access( + DEFAULT_EVENT_REPLAY_LIMIT, + LaneTransportKind::LocalDocker, + Arc::new(LocalDockerLaneWorkerTransport::with_runner( + workspace_root.clone(), + "frontal-code-worker:test".to_string(), + "http://host.docker.internal:8788", + access.clone(), + docker.clone(), + )), + access, + ) + .with_local_repos_allowed(true), + ); let router = app(state.clone()); let create_response = router @@ -6042,18 +6157,21 @@ mod tests { ..MockDockerRunner::default() }); let access = Arc::new(WorkerAccess::new(Some(store.clone()), 3600)); - let state = Arc::new(ServerState::with_lane_transport_kind_and_access( - DEFAULT_EVENT_REPLAY_LIMIT, - LaneTransportKind::LocalDocker, - Arc::new(LocalDockerLaneWorkerTransport::with_runner( - workspace_root, - "frontal-code-worker:test".to_string(), - "http://host.docker.internal:8788", - access.clone(), - docker, - )), - access, - )); + let state = Arc::new( + ServerState::with_lane_transport_kind_and_access( + DEFAULT_EVENT_REPLAY_LIMIT, + LaneTransportKind::LocalDocker, + Arc::new(LocalDockerLaneWorkerTransport::with_runner( + workspace_root, + "frontal-code-worker:test".to_string(), + "http://host.docker.internal:8788", + access.clone(), + docker, + )), + access, + ) + .with_local_repos_allowed(true), + ); let router = app(state.clone()); let create_response = router @@ -6737,7 +6855,8 @@ mod tests { Arc::new(FakeLaneWorkerTransport::new()), ) .with_control_plane_api_key(Some("admin-key".to_string())) - .with_publish_workspace_root(root.join("workspaces")); + .with_publish_workspace_root(root.join("workspaces")) + .with_local_repos_allowed(true); let state_arc = Arc::new(std::mem::take(&mut state)); state_arc.record_context( "task-publish-route", @@ -7175,6 +7294,58 @@ mod tests { ); } + #[tokio::test] + async fn complete_task_refuses_a_second_completion() { + let state = Arc::new(ServerState::default()); + let create_response = app(state.clone()) + .oneshot( + Request::builder() + .method("POST") + .uri("/v1/tasks") + .header("content-type", "application/json") + .body(Body::from( + serde_json::to_vec(&json!({ + "prompt": "Finish once", + "repository": "repo-twice", + "source": "api" + })) + .unwrap(), + )) + .unwrap(), + ) + .await + .unwrap(); + let created_body = to_bytes(create_response.into_body(), usize::MAX) + .await + .unwrap(); + let created: serde_json::Value = serde_json::from_slice(&created_body).unwrap(); + let task_id = created["task_id"].as_str().unwrap().to_string(); + + let complete = |finish_reason: &'static str| { + app(state.clone()).oneshot( + Request::builder() + .method("POST") + .uri(format!("/v1/tasks/{task_id}/complete")) + .header("content-type", "application/json") + .body(Body::from( + serde_json::to_vec(&json!({ + "finish_reason": finish_reason, + "tokens_output": 1, + "result": "first" + })) + .unwrap(), + )) + .unwrap(), + ) + }; + + assert_eq!(complete("stop").await.unwrap().status(), StatusCode::OK); + let second = complete("error").await.unwrap(); + assert_eq!(second.status(), StatusCode::BAD_REQUEST); + let task = state.tasks.get(&task_id).expect("task still tracked"); + assert_eq!(task.status, TaskStatus::Completed); + } + fn policy_request( provider: Option<&str>, model: Option<&str>, diff --git a/crates/code-server/src/publish.rs b/crates/code-server/src/publish.rs index 988c4cc..2483b9d 100644 --- a/crates/code-server/src/publish.rs +++ b/crates/code-server/src/publish.rs @@ -19,9 +19,14 @@ pub(crate) const MAX_PATCH_BYTES: usize = 16 * 1024 * 1024; pub(crate) const PUBLISH_BRANCH_PREFIX: &str = "frontal-code/"; /// Paths a worker may not change. `.github/workflows` changes CI behavior, and `.git` -/// holds repository configuration. +/// holds repository configuration. Matching is case-insensitive, since some file systems +/// fold case. const FORBIDDEN_PATH_PREFIXES: &[&str] = &[".git/", ".github/workflows/", ".github/actions/"]; +/// File names a worker may not change in any directory. `CODEOWNERS` decides who must +/// review a change, and `.gitmodules` points git at other repositories. +const FORBIDDEN_FILE_NAMES: &[&str] = &["codeowners", ".gitmodules"]; + /// Checks a patch before it is applied. The checks are about what the patch does to the /// repository, not about its content: it must not touch `.git`, CI definitions, or /// symbolic links and submodules, which can point outside the tree. @@ -73,12 +78,39 @@ fn check_path(path: &str) -> Result<(), String> { { return Err(format!("patch path escapes the repository: {path}")); } - let dotted = format!("{path}/"); + let lower = path.to_ascii_lowercase(); + let dotted = format!("{lower}/"); for forbidden in FORBIDDEN_PATH_PREFIXES { - if dotted.starts_with(forbidden) || path == forbidden.trim_end_matches('/') { + if dotted.starts_with(forbidden) || lower == forbidden.trim_end_matches('/') { return Err(format!("patch changes a protected path: {path}")); } } + // A nested `.git` directory is a repository of its own, wherever it sits. + let segments: Vec<&str> = lower.split('/').collect(); + let file_name = segments.last().copied().unwrap_or_default(); + if segments.contains(&".git") || FORBIDDEN_FILE_NAMES.contains(&file_name) { + return Err(format!("patch changes a protected path: {path}")); + } + Ok(()) +} + +/// Checks that a base or branch name is safe to pass to git. Option-like names, spaces, +/// control characters, and names git refuses are rejected. +pub(crate) fn validate_ref_name(name: &str) -> Result<(), String> { + let allowed_chars = name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '/' | '.' | '_' | '-')); + if name.is_empty() + || !allowed_chars + || name.starts_with('-') + || name.starts_with('/') + || name.ends_with('/') + || name.contains("..") + || name.contains("//") + || name.to_ascii_lowercase().ends_with(".lock") + { + return Err(format!("`{name}` is not a valid git ref name")); + } Ok(()) } @@ -128,6 +160,9 @@ pub(crate) fn publish_changes(request: &PublishRequest<'_>) -> Result Option { let store = self.store.as_ref()?; - match store.validate(token).await { + match store.validate(token, self.ttl_secs).await { Ok(task) => task, Err(error) => { eprintln!("worker token check failed: {error}"); @@ -373,6 +375,33 @@ pub(crate) fn upstream_for( }) } +/// Whether the proxy forwards `method` `rest` for `provider`. Only model calls are allowed: +/// a worker token must not be able to reach an arbitrary upstream path such as a file +/// upload, fine-tuning, or account endpoint. +pub(crate) fn proxy_path_allowed(provider: &str, method: &Method, rest: &str) -> bool { + if *method != Method::POST { + return false; + } + let path = rest.trim_start_matches('/'); + let openai_chat = matches!( + path, + "chat/completions" | "responses" | "v1/chat/completions" | "v1/responses" + ); + // Azure routes model calls under the deployment name. + let azure_chat = path + .strip_prefix("openai/deployments/") + .is_some_and(|tail| tail.ends_with("/chat/completions") && !tail.contains("..")); + let anthropic = matches!(path, "v1/messages" | "v1/messages/count_tokens"); + match provider { + "openai" | "xai" | "ollama" => openai_chat, + "azure" => openai_chat || azure_chat, + // `frontal` and `anthropic` share FRONTAL_BASE_URL, and the CLI picks the wire format. + "frontal" => openai_chat || anthropic, + "anthropic" => anthropic, + _ => false, + } +} + /// Forwards one model request. The caller must present a live worker token. pub(crate) async fn proxy_model_request( access: &WorkerAccess, @@ -389,6 +418,13 @@ pub(crate) async fn proxy_model_request( return error_response(StatusCode::UNAUTHORIZED, "invalid or revoked worker token"); } + if !proxy_path_allowed(provider, request.method(), rest) { + return error_response( + StatusCode::NOT_FOUND, + "that path is not a model endpoint the proxy forwards", + ); + } + let upstream = match upstream_for(provider, server_env) { Ok(upstream) => upstream, Err((status, message)) => return error_response(status, &message), @@ -684,6 +720,59 @@ mod proxy_tests { assert!(!head.contains(&token), "worker token leaked upstream"); } + #[test] + fn proxy_forwards_only_model_call_paths_per_provider() { + assert!(proxy_path_allowed( + "openai", + &Method::POST, + "chat/completions" + )); + assert!(proxy_path_allowed( + "ollama", + &Method::POST, + "v1/chat/completions" + )); + assert!(proxy_path_allowed( + "anthropic", + &Method::POST, + "v1/messages" + )); + assert!(proxy_path_allowed( + "anthropic", + &Method::POST, + "v1/messages/count_tokens" + )); + assert!(proxy_path_allowed( + "azure", + &Method::POST, + "openai/deployments/gpt/chat/completions" + )); + + // Other paths, other methods, and shape mismatches are refused. + assert!(!proxy_path_allowed("openai", &Method::POST, "files")); + assert!(!proxy_path_allowed( + "openai", + &Method::POST, + "fine_tuning/jobs" + )); + assert!(!proxy_path_allowed( + "openai", + &Method::GET, + "chat/completions" + )); + assert!(!proxy_path_allowed( + "anthropic", + &Method::POST, + "chat/completions" + )); + assert!(!proxy_path_allowed("openai", &Method::POST, "../admin")); + assert!(!proxy_path_allowed( + "azure", + &Method::POST, + "openai/deployments/../files/chat/completions" + )); + } + #[tokio::test] async fn proxy_refuses_missing_or_revoked_tokens_without_contacting_the_provider() { let Some(store) = crate::worker_tokens::tests::test_store() else { @@ -795,6 +884,23 @@ mod worker_policy_tests { } } + #[test] + fn a_worker_cannot_run_arbitrary_code_through_interpreters_or_runners() { + let policy = worker_policy(); + for command in [ + "python3 -c 'import os'", + "find . -exec sh -c 'id' \\;", + "find . -delete", + "npm run deploy", + ] { + assert_ne!( + outcome(&policy, command), + PermissionOutcome::Allow, + "expected `{command}` not to be allowed" + ); + } + } + #[test] fn a_worker_can_run_its_allowed_read_and_build_commands() { let policy = worker_policy(); diff --git a/crates/code-server/src/worker_tokens.rs b/crates/code-server/src/worker_tokens.rs index 32eb9c7..87886ce 100644 --- a/crates/code-server/src/worker_tokens.rs +++ b/crates/code-server/src/worker_tokens.rs @@ -216,6 +216,7 @@ enum Command { }, Validate { token: String, + ttl_secs: f64, reply: Responder, String>>, }, Revoke { @@ -259,8 +260,7 @@ impl WorkerTokenStore { /// synchronous lane transport. pub(crate) fn issue_blocking(&self, task_id: &str, ttl_secs: u64) -> Result { let (reply_tx, reply_rx) = std_mpsc::channel(); - // Lifetimes beyond u32::MAX seconds (136 years) are clamped, not wrapped. - let ttl_secs = f64::from(u32::try_from(ttl_secs).unwrap_or(u32::MAX)); + let ttl_secs = ttl_as_secs(ttl_secs); self.send(Command::Issue { task_id: task_id.to_string(), ttl_secs, @@ -273,10 +273,19 @@ impl WorkerTokenStore { /// Returns the task a live token belongs to. `Ok(None)` means the token is unknown, /// expired, or revoked. - pub(crate) async fn validate(&self, token: &str) -> Result, String> { + /// + /// A live token's expiry is pushed `ttl_secs` into the future. The TTL is an idle + /// timeout, so a task that keeps calling the model keeps its token. Revoking on + /// completion or cancellation still ends access at once. + pub(crate) async fn validate( + &self, + token: &str, + ttl_secs: u64, + ) -> Result, String> { let (reply_tx, reply_rx) = oneshot::channel(); self.send(Command::Validate { token: token.to_string(), + ttl_secs: ttl_as_secs(ttl_secs), reply: Responder::Async(reply_tx), })?; reply_rx @@ -357,7 +366,11 @@ fn run_store( ttl_secs, reply, } => reply.send(issue(db, &task_id, ttl_secs).await), - Command::Validate { token, reply } => reply.send(validate(db, &token).await), + Command::Validate { + token, + ttl_secs, + reply, + } => reply.send(validate(db, &token, ttl_secs).await), Command::Revoke { task_id, reply } => reply.send(revoke(db, &task_id).await), } } @@ -382,6 +395,11 @@ fn reply_unavailable(command: Command) { } } +/// Lifetimes beyond `u32::MAX` seconds (136 years) are clamped, not wrapped. +fn ttl_as_secs(ttl_secs: u64) -> f64 { + f64::from(u32::try_from(ttl_secs).unwrap_or(u32::MAX)) +} + async fn issue(db: &mut Client, task_id: &str, ttl_secs: f64) -> Result { let mut bytes = [0_u8; 32]; getrandom::getrandom(&mut bytes).map_err(|error| format!("random source: {error}"))?; @@ -415,13 +433,14 @@ async fn issue(db: &mut Client, task_id: &str, ttl_secs: f64) -> Result Result, String> { +async fn validate(db: &mut Client, token: &str, ttl_secs: f64) -> Result, String> { let hash = hash_token(token); let row = db .query_opt( - "SELECT task_id FROM hosted_worker_tokens - WHERE token_hash = $1 AND revoked_at IS NULL AND expires_at > now()", - &[&hash], + "UPDATE hosted_worker_tokens SET expires_at = now() + make_interval(secs => $2) + WHERE token_hash = $1 AND revoked_at IS NULL AND expires_at > now() + RETURNING task_id", + &[&hash, &ttl_secs], ) .await .map_err(|error| format!("token store: {error}"))?; @@ -556,23 +575,23 @@ pub(crate) mod tests { let task = unique_task("revoke"); let first = store.issue_blocking(&task, 3600).expect("first token"); assert_eq!( - futures_block(store.validate(&first)).expect("validate"), + futures_block(store.validate(&first, 3600)).expect("validate"), Some(task.clone()) ); let second = store.issue_blocking(&task, 3600).expect("second token"); assert_eq!( - futures_block(store.validate(&first)).expect("validate"), + futures_block(store.validate(&first, 3600)).expect("validate"), None ); assert_eq!( - futures_block(store.validate(&second)).expect("validate"), + futures_block(store.validate(&second, 3600)).expect("validate"), Some(task.clone()) ); store.revoke_blocking(&task).expect("revoke"); assert_eq!( - futures_block(store.validate(&second)).expect("validate"), + futures_block(store.validate(&second, 3600)).expect("validate"), None ); } @@ -583,11 +602,29 @@ pub(crate) mod tests { let task = unique_task("expire"); let token = store.issue_blocking(&task, 0).expect("token"); assert_eq!( - futures_block(store.validate(&token)).expect("validate"), + futures_block(store.validate(&token, 3600)).expect("validate"), None ); } + #[test] + fn a_token_that_is_used_stays_live_past_its_original_expiry() { + let Some(store) = test_store() else { return }; + let task = unique_task("slide"); + let token = store.issue_blocking(&task, 2).expect("token"); + std::thread::sleep(std::time::Duration::from_millis(1_200)); + assert_eq!( + futures_block(store.validate(&token, 2)).expect("validate"), + Some(task.clone()) + ); + // Without the refresh above, the token would have expired at the two-second mark. + std::thread::sleep(std::time::Duration::from_millis(1_200)); + assert_eq!( + futures_block(store.validate(&token, 2)).expect("validate"), + Some(task.clone()) + ); + } + #[test] fn tokens_are_stored_as_hashes_only() { let Some((url, tls)) = test_database() else { diff --git a/docs/CONTAINERS.md b/docs/CONTAINERS.md index 7f8aa7a..b138f8f 100644 --- a/docs/CONTAINERS.md +++ b/docs/CONTAINERS.md @@ -219,6 +219,13 @@ Each task gets its own internal Docker network. The worker is attached to it, an The worker does not push. At the end of a run it sends its changes as a patch to `/v1/tasks//publish`. The server applies the patch to a fresh clone, commits it, and pushes to a branch under `frontal-code/`. The server opens a pull request when a GitHub integration is configured. See [design/worker-architecture.md](design/worker-architecture.md#publication). +The server refuses a patch that changes `.git`, `.github/workflows`, `.github/actions`, `CODEOWNERS`, or `.gitmodules`, in any letter case and at any depth, and it refuses symbolic links and submodules. Only one publish runs per task at a time. A second request gets `409 Conflict` while the first is in progress. + +Repository locations and refs are checked when the task is created and again at publish time: + +- A remote URL is always accepted. A path on the server's host is refused unless `FRONTAL_CODE_SERVER_ALLOW_LOCAL_REPOS=1`. Local development and the e2e scripts turn it on. Otherwise a task could clone or push to any repository the server can reach. +- `base_ref` and `branch` must be valid git ref names. Names that start with `-`, contain spaces or `..`, or end in `.lock` are refused. + ### Container hardening Every worker runs with `--cap-drop ALL`, `--security-opt no-new-privileges:true`, a read-only root filesystem, `--pids-limit 512`, `--memory 4g`, and `--cpus 2`. Its only writable mounts are the task checkout and two tmpfs directories. It is never privileged, and it never gets the Docker socket. @@ -232,11 +239,11 @@ The worker sends each model request to `/v1/model-proxy/` on the serve 3. Adds the server's key for that provider and forwards the request to the provider's base URL from the server's environment. 4. Streams the response back to the worker. -A token that is stolen from the container is useful only for model calls and for its own task's endpoints. It stops working when the task completes or is cancelled, or when the server restarts (tokens are held in memory). +A token that is stolen from the container is useful only for model calls and for its own task's endpoints. The proxy forwards only model-call paths (for example `chat/completions`, `responses`, and `v1/messages`) and only `POST`, so a token cannot reach file, fine-tuning, or account endpoints. Tokens are stored as SHA-256 hashes in Postgres, so they survive a server restart. Each successful use extends the token's expiry by the configured TTL (`FRONTAL_CODE_WORKER_TOKEN_TTL_SECS`, default 7200), so the TTL is an idle timeout and a long-running task keeps access. The token is revoked as soon as the task completes or is cancelled. Bedrock is not proxied: its requests are signed per request and cannot use a static key. A task that asks for `bedrock` is refused. -The worker has network access and runs with `danger-full-access` by default, so a prompt-injected agent could still read its own environment and send data out. The token limits the damage to model calls and one task's endpoints. The default permission mode is a separate decision: see the README. +The worker runs with the `workspace-write` permission mode by default. File edits are confined to its checkout, and shell commands are refused unless an allow rule grants them. Its container runs with no Linux capabilities, `no-new-privileges`, a read-only root filesystem, and its own `--internal` Docker network. That network has no route to the internet and reaches only the relay to the server. A prompt-injected agent can still run the build and test tools it is allowed, and those run project code. The network, not the allow list, is the real boundary. The token limits what a compromised worker can do to model calls and to its own task's endpoints. ## Docker Compose diff --git a/docs/design/worker-architecture.md b/docs/design/worker-architecture.md index dfb7d92..5b30a7a 100644 --- a/docs/design/worker-architecture.md +++ b/docs/design/worker-architecture.md @@ -46,7 +46,8 @@ What a worker never holds: provider keys, `GITHUB_TOKEN`, the control-plane API - **Storage:** Postgres table `hosted_worker_tokens`. The table holds the SHA-256 of the token, never the token. Each row has a task id, an expiry, and an optional revocation time. - **Issue:** at bootstrap. A new token for a task revokes that task's previous token. - **Use:** on each request. The worker may call only: read the task, read its runtime, read and write its GitHub integration, publish, and complete. Those endpoints are listed in `worker_access.rs`, and everything else returns 401. -- **Revoke:** on completion, on cancel, and on expiry (`FRONTAL_CODE_WORKER_TOKEN_TTL_SECS`, default 7200). +- **Expiry:** an idle timeout. Each successful use pushes the expiry out by `FRONTAL_CODE_WORKER_TOKEN_TTL_SECS` (default 7200), so a task that keeps calling the model keeps its token. +- **Revoke:** on completion and on cancel, which ends access at once. A token that goes unused past its TTL also stops working. - **Restart:** tokens survive a restart because they are in Postgres. A running worker keeps working. The server does not hold worker tokens in memory. The Postgres client runs on a dedicated thread, so the synchronous lane transport and the async handlers share one store. diff --git a/infrastructure/README.md b/infrastructure/README.md index 1457fbf..0ac49d5 100644 --- a/infrastructure/README.md +++ b/infrastructure/README.md @@ -75,7 +75,7 @@ docker compose --env-file infrastructure/compose/.env \ In this mode: -- `code-server` uses `/var/run/docker.sock` to launch worker containers +- `code-server` launches worker containers through a Docker socket proxy (`docker-socket-proxy`, pinned by digest) that allows only the API calls the server makes. The raw Docker socket is mounted only into the proxy, not into `code-server` - workers default to `FRONTAL_CODE_SERVER_DOCKER_IMAGE=frontal-code-worker:local` - workers call back to the server through `FRONTAL_CODE_SERVER_CALLBACK_URL` - per-task repo checkouts live under `FRONTAL_CODE_SERVER_WORKSPACE_ROOT` diff --git a/infrastructure/compose/docker-compose.docker-worker.yml b/infrastructure/compose/docker-compose.docker-worker.yml index 61a9ef5..b874c5f 100644 --- a/infrastructure/compose/docker-compose.docker-worker.yml +++ b/infrastructure/compose/docker-compose.docker-worker.yml @@ -1,6 +1,36 @@ +# Local-docker lane. code-server starts worker containers through a Docker socket proxy +# that allows only the API calls the server makes (containers, images, networks, ping, +# version). The raw Docker socket is not mounted into code-server, which would be +# root-equivalent on the host. The proxy is pinned by digest and sits on its own internal +# network, so only code-server can reach it. services: + docker-socket-proxy: + image: tecnativa/docker-socket-proxy:0.3.0@sha256:9e4b9e7517a6b660f2cc903a19b257b1852d5b3344794e3ea334ff00ae677ac2 + restart: unless-stopped + environment: + CONTAINERS: "1" + IMAGES: "1" + NETWORKS: "1" + POST: "1" + PING: "1" + VERSION: "1" + volumes: + - /var/run/docker.sock:/var/run/docker.sock:ro + networks: + - docker-api + security_opt: + - no-new-privileges:true + code-server: environment: FRONTAL_CODE_SERVER_LANE_TRANSPORT: ${FRONTAL_CODE_SERVER_LANE_TRANSPORT:-local-docker} - volumes: - - /var/run/docker.sock:/var/run/docker.sock + DOCKER_HOST: tcp://docker-socket-proxy:2375 + depends_on: + - docker-socket-proxy + networks: + - default + - docker-api + +networks: + docker-api: + internal: true diff --git a/scripts/linux-amd64-worker-e2e.sh b/scripts/linux-amd64-worker-e2e.sh index e777b0a..6f291c5 100755 --- a/scripts/linux-amd64-worker-e2e.sh +++ b/scripts/linux-amd64-worker-e2e.sh @@ -57,7 +57,7 @@ docker run -d --name "$CONTAINER" --platform linux/amd64 -p "127.0.0.1:$PORT:$PO -v frontal-cargo-registry:/usr/local/cargo/registry -v frontal-target-amd64:/src/target-linux \ -v "$WORK":"$WORK" -v /var/run/docker.sock:/var/run/docker.sock -v "$TLS_DIR":/certs:ro \ -e FRONTAL_CODE_SERVER_HOST=0.0.0.0 -e FRONTAL_CODE_SERVER_PORT="$PORT" \ - -e FRONTAL_CODE_SERVER_API_KEY="$KEY" -e FRONTAL_CODE_SERVER_LANE_TRANSPORT=local-docker \ + -e FRONTAL_CODE_SERVER_API_KEY="$KEY" -e FRONTAL_CODE_SERVER_LANE_TRANSPORT=local-docker -e FRONTAL_CODE_SERVER_ALLOW_LOCAL_REPOS=1 \ -e FRONTAL_CODE_SERVER_DOCKER_IMAGE=frontal-code-worker:local \ -e FRONTAL_CODE_SERVER_CALLBACK_URL="http://host.docker.internal:$PORT" \ -e FRONTAL_CODE_SERVER_WORKSPACE_ROOT="$WORK/workspaces" \ diff --git a/scripts/worker-e2e.sh b/scripts/worker-e2e.sh index 136c2bb..af42ac6 100755 --- a/scripts/worker-e2e.sh +++ b/scripts/worker-e2e.sh @@ -66,6 +66,7 @@ FRONTAL_CODE_SERVER_HOST=0.0.0.0 \ FRONTAL_CODE_SERVER_PORT="$PORT" \ FRONTAL_CODE_SERVER_API_KEY="$API_KEY" \ FRONTAL_CODE_SERVER_LANE_TRANSPORT=local-docker \ +FRONTAL_CODE_SERVER_ALLOW_LOCAL_REPOS=1 \ FRONTAL_CODE_SERVER_DOCKER_IMAGE="$IMAGE" \ FRONTAL_CODE_SERVER_CALLBACK_URL="http://host.docker.internal:$PORT" \ FRONTAL_CODE_SERVER_WORKSPACE_ROOT="$WORK/workspaces" \