Skip to content

Cors and token #5850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
michaelneale merged 1 commit into from
Nov 23, 2025
Merged

Cors and token #5850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 49 additions & 21 deletions crates/goose-cli/src/commands/web.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use axum::response::Redirect;
use axum::{
extract::{
ws::{Message, WebSocket, WebSocketUpgrade},
Request, State,
Query, Request, State,
},
http::StatusCode,
middleware::{self, Next},
Expand All @@ -21,7 +21,7 @@ use serde::{Deserialize, Serialize};
use serde_json::Value;
use std::{net::SocketAddr, sync::Arc};
use tokio::sync::{Mutex, RwLock};
use tower_http::cors::{Any, CorsLayer};
use tower_http::cors::{AllowOrigin, Any, CorsLayer};
use tracing::error;
use webbrowser;

Expand All @@ -32,6 +32,7 @@ struct AppState {
agent: Arc<Agent>,
cancellations: CancellationStore,
auth_token: Option<String>,
ws_token: String,
}

#[derive(Serialize, Deserialize)]
Expand Down Expand Up @@ -87,17 +88,14 @@ async fn auth_middleware(
req: Request,
next: Next,
) -> Result<Response, StatusCode> {
// Skip auth for health check
if req.uri().path() == "/api/health" {
return Ok(next.run(req).await);
}

// If no auth token is configured, skip authentication entirely
Copy link
Collaborator

@michaelneale michaelneale Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually a helpful and IMO necessary comment, I don't like just blindly deleting oneline coments.

Copy link
Collaborator Author

@DOsinga DOsinga Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interestingly enough, goose deleted this comment. which it probably did because I have general settings that it should delete useless comments.

I would still argue that the comment says the same thing as the code below though.

Copy link
Collaborator

@michaelneale michaelneale Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it never ever wants to with mine - I guess it tends to follow "house style" over what system prompt says! @DOsinga sometimes I wonder if we need to put some logic in the editor tool for it to to return an error if it detects single line comment (error can be "this is an inane comment, please either remove and try again or consider if it is really needed) or something?

let Some(ref expected_token) = state.auth_token else {
return Ok(next.run(req).await);
};

// Check for Bearer token first
if let Some(auth_header) = req.headers().get("authorization") {
if let Ok(auth_str) = auth_header.to_str() {
if let Some(token) = auth_str.strip_prefix("Bearer ") {
Expand All @@ -106,7 +104,6 @@ async fn auth_middleware(
}
}

// Check for Basic auth (password-only, ignore username)
if let Some(basic_token) = auth_str.strip_prefix("Basic ") {
if let Ok(decoded) = base64::engine::general_purpose::STANDARD.decode(basic_token) {
if let Ok(credentials) = String::from_utf8(decoded) {
Expand All @@ -119,7 +116,6 @@ async fn auth_middleware(
}
}

// Authentication failed - return 401 with WWW-Authenticate header
let mut response = Response::new("Authentication required".into());
*response.status_mut() = StatusCode::UNAUTHORIZED;
response.headers_mut().insert(
Expand All @@ -135,7 +131,6 @@ pub async fn handle_web(
open: bool,
auth_token: Option<String>,
) -> Result<()> {
// Setup logging
crate::logging::setup_logging(Some("goose-web"), None)?;

let config = goose::config::Config::global();
Expand Down Expand Up @@ -176,10 +171,34 @@ pub async fn handle_web(
}
}

let ws_token = if auth_token.is_none() {
uuid::Uuid::new_v4().to_string()
} else {
String::new()
};

let state = AppState {
agent: Arc::new(agent),
cancellations: Arc::new(RwLock::new(std::collections::HashMap::new())),
auth_token,
auth_token: auth_token.clone(),
ws_token,
};

let cors_layer = if auth_token.is_none() {
let allowed_origins = [
"http://localhost:3000".parse().unwrap(),
"http://127.0.0.1:3000".parse().unwrap(),
format!("http://{}:{}", host, port).parse().unwrap(),
];
CorsLayer::new()
.allow_origin(AllowOrigin::list(allowed_origins))
.allow_methods(Any)
.allow_headers(Any)
} else {
CorsLayer::new()
.allow_origin(Any)
.allow_methods(Any)
.allow_headers(Any)
};

let app = Router::new()
Expand All @@ -194,12 +213,7 @@ pub async fn handle_web(
state.clone(),
auth_middleware,
))
.layer(
CorsLayer::new()
.allow_origin(Any)
.allow_methods(Any)
.allow_headers(Any),
)
.layer(cors_layer)
.with_state(state);

let addr: SocketAddr = format!("{}:{}", host, port).parse()?;
Expand All @@ -214,7 +228,6 @@ pub async fn handle_web(
println!(" Press Ctrl+C to stop\n");

if open {
// Open browser
let url = format!("http://{}", addr);
if let Err(e) = webbrowser::open(&url) {
eprintln!("Failed to open browser: {}", e);
Expand All @@ -241,14 +254,15 @@ async fn serve_index() -> Result<Redirect, (http::StatusCode, String)> {

async fn serve_session(
axum::extract::Path(session_name): axum::extract::Path<String>,
State(state): State<AppState>,
) -> Html<String> {
let html = include_str!("../../static/index.html");
// Inject the session name into the HTML so JavaScript can use it
let html_with_session = html.replace(
"<script src=\"/static/script.js\"></script>",
&format!(
"<script>window.GOOSE_SESSION_NAME = '{}';</script>\n <script src=\"/static/script.js\"></script>",
session_name
"<script>window.GOOSE_SESSION_NAME = '{}'; window.GOOSE_WS_TOKEN = '{}';</script>\n <script src=\"/static/script.js\"></script>",
session_name,
state.ws_token
)
);
Html(html_with_session)
Expand Down Expand Up @@ -324,11 +338,25 @@ async fn get_session(
}
}

#[derive(Deserialize)]
struct WsQuery {
token: Option<String>,
}

async fn websocket_handler(
ws: WebSocketUpgrade,
State(state): State<AppState>,
) -> impl IntoResponse {
ws.on_upgrade(|socket| handle_socket(socket, state))
Query(query): Query<WsQuery>,
) -> Result<impl IntoResponse, StatusCode> {
if state.auth_token.is_none() {
let provided_token = query.token.as_deref().unwrap_or("");
if provided_token != state.ws_token {
tracing::warn!("WebSocket connection rejected: invalid token");
return Err(StatusCode::FORBIDDEN);
}
}
Comment on lines +351 to +357
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WebSocket connections will fail when auth_token is configured. The auth_middleware requires an Authorization header (line 99-117), but browsers cannot set custom headers in WebSocket upgrade requests. This means:

  1. When auth_token.is_some(), the auth_middleware will always return 401 for /ws requests
  2. The websocket_handler will never execute
  3. WebSocket connections are broken in authenticated mode

The auth_middleware needs to allow /ws to pass through and let websocket_handler validate the token from the query parameter:

async fn auth_middleware(
    State(state): State<AppState>,
    req: Request,
    next: Next,
) -> Result<Response, StatusCode> {
    // Skip auth for health check and WebSocket
    if req.uri().path() == "/api/health" || req.uri().path() == "/ws" {
        return Ok(next.run(req).await);
    }
    // ... rest of auth logic
}

Then in websocket_handler, validate auth_token when present:

if let Some(ref expected_token) = state.auth_token {
    let provided_token = query.token.as_deref().unwrap_or("");
    if provided_token != expected_token {
        tracing::warn!("WebSocket connection rejected: invalid auth token");
        return Err(StatusCode::FORBIDDEN);
    }
} else if state.auth_token.is_none() {
    // Validate ws_token
    let provided_token = query.token.as_deref().unwrap_or("");
    if provided_token != state.ws_token {
        tracing::warn!("WebSocket connection rejected: invalid token");
        return Err(StatusCode::FORBIDDEN);
    }
}

Copilot uses AI. Check for mistakes.

Ok(ws.on_upgrade(|socket| handle_socket(socket, state)))
}

async fn handle_socket(socket: WebSocket, state: AppState) {
Expand Down
5 changes: 3 additions & 2 deletions crates/goose-cli/static/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ function removeThinkingIndicator() {
// Connect to WebSocket
function connectWebSocket() {
const protocol = window.location.protocol === 'https:' ? 'wss:' : 'ws:';
const wsUrl = `${protocol}//${window.location.host}/ws`;
const token = window.GOOSE_WS_TOKEN || '';
const wsUrl = `${protocol}//${window.location.host}/ws?token=${encodeURIComponent(token)}`;

socket = new WebSocket(wsUrl);

Expand Down Expand Up @@ -520,4 +521,4 @@ function updateSessionTitle() {
}

// Update title on load
updateSessionTitle();
updateSessionTitle();
Loading